Improve handling of c=y sasl flag when plus variants are disabled

Previously we aborted connection with that flag unless client used
SASL2 and allow_unencrypted_sasl2 was enabled.

Now in addition to this option, we will also accept connection
if admin disabled all plus mechanisms, or in SASL2 case,
if filtering out unavailable methods based on stored user passwords
we removed all -PLUS mechanisms.

Author: prefiks

src/xmpp_sasl.erl

@@ -18,7 +18,7 @@
 -module(xmpp_sasl).
 -author('alexey@process-one.net').
 
--export([server_new/5, server_start/6, server_step/2,
+-export([server_new/5, server_start/7, server_step/2,
 	 listmech/0, format_error/2]).
 
 %% TODO: write correct types for these callbacks
@@ -60,7 +60,7 @@
 -export_type([mechanism/0, error_reason/0,
 	      sasl_state/0, sasl_return/0, sasl_property/0]).
 
--callback mech_new(binary(), channel_bindings(), list(binary()),
+-callback mech_new(binary(), channel_bindings(), boolean(), list(binary()),
 		   binary(), binary(), map()) -> mech_state().
 -callback mech_step(mech_state(), binary()) -> sasl_return().
 
@@ -105,14 +105,14 @@ server_new(ServerHost, GetPassword, CheckPassword, CheckPasswordDigest, GetFastT
 		    check_password => CheckPassword,
 		    check_password_digest => CheckPasswordDigest}}.
 
--spec server_start(sasl_state(), mechanism(), binary(), channel_bindings(),
+-spec server_start(sasl_state(), mechanism(), binary(), channel_bindings(), boolean(),
   list(binary()) | undefined | not_available, binary() | undefined) -> sasl_return().
-server_start(State, Mech, ClientIn, ChannelBindings, Mechs, UAId) ->
+server_start(State, Mech, ClientIn, ChannelBindings, PlusDisabled, Mechs, UAId) ->
     case get_mod(Mech) of
 	undefined ->
 	    {error, unsupported_mechanism, <<"">>};
 	Module ->
-	    MechState = Module:mech_new(Mech, ChannelBindings, Mechs, UAId,
+	    MechState = Module:mech_new(Mech, ChannelBindings, PlusDisabled, Mechs, UAId,
 					State#sasl_state.server_host,
 					State#sasl_state.callbacks),
 	    State1 = State#sasl_state{mech_name = Mech,

src/xmpp_sasl_anonymous.erl

@@ -20,11 +20,11 @@
 -behaviour(xmpp_sasl).
 -protocol({xep, 175, '1.2'}).
 
--export([mech_new/6, mech_step/2]).
+-export([mech_new/7, mech_step/2]).
 
 -record(state, {server = <<"">> :: binary()}).
 
-mech_new(_Mech, _CB, _Mechs, _UAId, Host, _Callbacks) ->
+mech_new(_Mech, _CB, _PD, _Mechs, _UAId, Host, _Callbacks) ->
     #state{server = Host}.
 
 mech_step(#state{}, _ClientIn) ->

src/xmpp_sasl_digest.erl

@@ -20,7 +20,7 @@
 -author('alexey@sevcom.net').
 -dialyzer({no_match, [get_local_fqdn/1]}).
 
--export([mech_new/6, mech_step/2, format_error/1]).
+-export([mech_new/7, mech_step/2, format_error/1]).
 %% For tests
 -export([parse/1]).
 
@@ -58,7 +58,7 @@ format_error(not_authorized) ->
 format_error(unexpected_response) ->
     {'not-authorized', <<"Unexpected response">>}.
 
-mech_new(_Mech, _CB, _Mechs, _UAId, Host, #{get_password := GetPassword,
+mech_new(_Mech, _CB, _PD, _Mechs, _UAId, Host, #{get_password := GetPassword,
 				     check_password_digest := CheckPasswordDigest}) ->
     #state{step = 1, nonce = p1_rand:get_string(),
 	   host = Host, hostfqdn = get_local_fqdn(Host),

src/xmpp_sasl_fast.erl

@@ -19,7 +19,7 @@
 -behaviour(xmpp_sasl).
 -author('alexey@process-one.net').
 
--export([mech_new/6, mech_step/2, format_error/1]).
+-export([mech_new/7, mech_step/2, format_error/1]).
 %% For tests
 -export([parse/1]).
 
@@ -34,15 +34,15 @@ format_error({Condition, Text}) ->
 format_error(incompatible_mechs) ->
     {'not-authorized', <<"Incompatible SCRAM methods">>};
 format_error(missing_ua) ->
-    {'malformed-request', <<"Missing user-agent">>};
+    {'malformed', <<"Missing user-agent">>};
 format_error(bad_channel_binding) ->
     {'not-authorized', <<"Invalid channel binding">>};
 format_error(parser_failed) ->
     {'not-authorized', <<"Response decoding failed">>};
 format_error(not_authorized) ->
     {'not-authorized', <<"Invalid username or password">>}.
 
-mech_new(Mech, CB, _Mechs, UAId, _Host, #{get_fast_tokens := GetFastTokens}) ->
+mech_new(Mech, CB, _PD, _Mechs, UAId, _Host, #{get_fast_tokens := GetFastTokens}) ->
     #state{mech = Mech, cb = CB, ua = UAId, get_fast_tokens = GetFastTokens}.
 
 

src/xmpp_sasl_oauth.erl

@@ -19,7 +19,7 @@
 -behaviour(xmpp_sasl).
 -author('alexey@process-one.net').
 
--export([mech_new/6, mech_step/2, format_error/1]).
+-export([mech_new/7, mech_step/2, format_error/1]).
 %% For tests
 -export([parse/1]).
 
@@ -34,7 +34,7 @@ format_error(parser_failed) ->
 format_error(not_authorized) ->
     {'not-authorized', <<"Invalid token">>}.
 
-mech_new(_Mech, _CB, _Mechs, _UAId, Host, #{check_password := CheckPassword}) ->
+mech_new(_Mech, _CB, _PD, _Mechs, _UAId, Host, #{check_password := CheckPassword}) ->
     #state{host = Host, check_password = CheckPassword}.
 
 mech_step(State, ClientIn) ->

src/xmpp_sasl_plain.erl

@@ -19,7 +19,7 @@
 -behaviour(xmpp_sasl).
 -author('alexey@process-one.net').
 
--export([mech_new/6, mech_step/2, format_error/1]).
+-export([mech_new/7, mech_step/2, format_error/1]).
 %% For tests
 -export([parse/1]).
 
@@ -35,7 +35,7 @@ format_error(parser_failed) ->
 format_error(not_authorized) ->
     {'not-authorized', <<"Invalid username or password">>}.
 
-mech_new(_Mech, _CB, _Mechs, _UAId, _Host, #{check_password := CheckPassword}) ->
+mech_new(_Mech, _CB, _PD, _Mechs, _UAId, _Host, #{check_password := CheckPassword}) ->
     #state{check_password = CheckPassword}.
 
 mech_step(State, ClientIn) ->

src/xmpp_sasl_scram.erl

@@ -22,7 +22,7 @@
 -protocol({rfc, 5802}).
 -protocol({xep, 474, '0.4.0'}).
 
--export([mech_new/6, mech_step/2, format_error/1]).
+-export([mech_new/7, mech_step/2, format_error/1]).
 
 -include("scram.hrl").
 
@@ -33,6 +33,7 @@
 	{step = 2                :: 2 | 4,
 	 algo = sha              :: sha | sha256 | sha512,
 	 channel_bindings = none :: none | not_available | #{atom() => binary()},
+	 plus_disabled = false   :: boolean(),
 	 ssdp                    :: undefined | binary(),
 	 stored_key = <<"">>     :: binary(),
 	 server_key = <<"">>     :: binary(),
@@ -76,7 +77,7 @@ format_error(bad_channel_binding) ->
 format_error(incompatible_mechs) ->
     {'not-authorized', <<"Incompatible SCRAM methods">>}.
 
-mech_new(Mech, ChannelBindings, Mechs, _UAId, _Host, #{get_password := GetPassword}) ->
+mech_new(Mech, ChannelBindings, PlusDisabled, Mechs, _UAId, _Host, #{get_password := GetPassword}) ->
     NCB = case ChannelBindings of
 	      #{} -> none;
 	      _ -> ChannelBindings
@@ -103,7 +104,7 @@ mech_new(Mech, ChannelBindings, Mechs, _UAId, _Host, #{get_password := GetPasswo
 		       end]))
 	   end,
     #state{step = 2, get_password = GetPassword, algo = Algo,
-	channel_bindings = CB, ssdp = Ssdp}.
+	channel_bindings = CB, plus_disabled = PlusDisabled, ssdp = Ssdp}.
 
 mech_step(_State, ClientIn) when not is_binary(ClientIn) ->
     {error, parser_failed};
@@ -281,6 +282,8 @@ cbind_check(#state{channel_bindings = #{}}, _) ->
     false;
 cbind_check(#state{channel_bindings = not_available}, <<"y", _/binary>>) ->
     true;
+cbind_check(#state{plus_disabled = true}, <<"y", _/binary>>) ->
+    true;
 cbind_check(_, <<"y", _/binary>>) ->
     false;
 cbind_check(_, <<"n", _/binary>>) ->

src/xmpp_stream_in.erl

@@ -965,7 +965,7 @@ init_channel_bindings(#{socket := Socket} = State) ->
 process_sasl_request(#sasl_auth{mechanism = Mech, text = ClientIn},
 		     #{lserver := LServer} = State) ->
     State1 = State#{sasl_mech => Mech},
-    Mechs = get_sasl_mechanisms(State1, sasl),
+    {Mechs, PlusDisabled} = get_sasl_mechanisms(State1, sasl),
     case lists:member(Mech, Mechs) of
 	true when Mech == <<"EXTERNAL">> ->
 	    Res = case xmpp_stream_pkix:authenticate(State1, ClientIn) of
@@ -989,7 +989,7 @@ process_sasl_request(#sasl_auth{mechanism = Mech, text = ClientIn},
 		     end,
 	    SASLState = xmpp_sasl:server_new(LServer, GetPW, CheckPW, CheckPWDigest, undefined),
 	    CB = maps:get(sasl_channel_bindings, State1, none),
-	    Res = xmpp_sasl:server_start(SASLState, Mech, ClientIn, CB, Mechs2, undefined),
+	    Res = xmpp_sasl:server_start(SASLState, Mech, ClientIn, CB, PlusDisabled, Mechs2, undefined),
 	    process_sasl_result(Res, disable_sasl2(State1#{sasl_state => SASLState}));
 	false ->
 	    process_sasl_result({error, unsupported_mechanism, <<"">>}, disable_sasl2(State1))
@@ -1078,7 +1078,7 @@ process_sasl2_request(#sasl2_authenticate{mechanism = Mech, initial_response = C
     FastMechs = try callback(fast_mechanisms, State)
 		catch _:{?MODULE, undef} -> []
 		end,
-    Mechs = get_sasl_mechanisms(State1, sasl2),
+    {Mechs, PlusDisabled} = get_sasl_mechanisms(State1, sasl2),
     MechsAll = Mechs ++ FastMechs,
     UAId = case UA of
 	       #sasl2_user_agent{id = ID} when ID /= <<>> ->
@@ -1116,7 +1116,7 @@ process_sasl2_request(#sasl2_authenticate{mechanism = Mech, initial_response = C
 	    SASLState = xmpp_sasl:server_new(LServer, GetPW, CheckPW,
 					     CheckPWDigest, GetFastTokens),
 	    CB = maps:get(sasl_channel_bindings, State1, none),
-	    Res = xmpp_sasl:server_start(SASLState, Mech, ClientIn, CB, Mechs2, UAId),
+	    Res = xmpp_sasl:server_start(SASLState, Mech, ClientIn, CB, PlusDisabled, Mechs2, UAId),
 	    process_sasl2_result(Res, State1#{sasl_state => SASLState,
 					      sasl2_inline_els => SaslInline,
 					      sasl2_ua_id => UAId});
@@ -1414,7 +1414,7 @@ get_fast_tokens_fun(Mech, State) ->
     catch _:{?MODULE, undef} -> fun(_, _) -> [] end
     end.
 
--spec get_sasl_mechanisms(state(), sasl | sasl2) -> [xmpp_sasl:mechanism()].
+-spec get_sasl_mechanisms(state(), sasl | sasl2) -> {[xmpp_sasl:mechanism()], boolean()}.
 get_sasl_mechanisms(#{stream_encrypted := Encrypted,
 		      xmlns := NS} = State, Type) ->
     Mechs = if NS == ?NS_CLIENT -> xmpp_sasl:listmech();
@@ -1423,15 +1423,18 @@ get_sasl_mechanisms(#{stream_encrypted := Encrypted,
     Mechs1 = if Encrypted -> [<<"EXTERNAL">> | Mechs];
 		 true -> Mechs
 	     end,
-    Mechs2 = try callback(sasl_mechanisms, Mechs1, State) of
-		 {Sasl1, _} when Type == sasl -> Sasl1;
-		 {_, Sasl2} when Type == sasl2 -> Sasl2;
-		 Common -> Common
-	     catch _:{?MODULE, undef} -> Mechs1
-	     end,
+    {Mechs2, PlusDisabled} =
+	try callback(sasl_mechanisms, Mechs1, State) of
+	    {Sasl1, _} when Type == sasl -> {Sasl1, false};
+	    {_, Sasl2} when Type == sasl2 -> {Sasl2, false};
+	    {Sasl1, _, Disabled} when Type == sasl -> {Sasl1, Disabled};
+	    {_, Sasl2, Disabled} when Type == sasl2 -> {Sasl2, Disabled};
+	    Common -> {Common, false}
+	catch _:{?MODULE, undef} -> {Mechs1, false}
+	end,
     if Type == sasl2 ->
-	filter_sasl2_user_mechs(Mechs2, State);
-	true -> Mechs2
+	filter_sasl2_user_mechs(Mechs2, PlusDisabled, State);
+	true -> {Mechs2, PlusDisabled}
     end.
 
 pass_to_mech(_, all)                     -> all;
@@ -1444,16 +1447,24 @@ pass_to_mech(#scram{hash = sha512}, Acc) -> [<<"SCRAM-SHA-512">>, <<"SCRAM-SHA-5
 pass_to_mech(List, Acc) when is_list(List) ->
     lists:foldl(fun pass_to_mech/2, Acc, List).
 
-filter_sasl2_user_mechs(Mechs, State) ->
+filter_sasl2_user_mechs(Mechs, PlusDisabled, State) ->
     {Pass, _} = (get_password_fun_sasl2(<<>>, State))(<<>>),
     case pass_to_mech(Pass, []) of
-	all -> Mechs;
+	all -> {Mechs, PlusDisabled};
 	M ->
 	    OtherMechs = Mechs -- [<<"SCRAM-SHA-1">>, <<"SCRAM-SHA-1-PLUS">>,
 				   <<"SCRAM-SHA-256">>, <<"SCRAM-SHA-256-PLUS">>,
 				   <<"SCRAM-SHA-512">>, <<"SCRAM-SHA-512-PLUS">>],
 	    ScramMechs = Mechs -- OtherMechs,
-	    OtherMechs ++ (ScramMechs -- (ScramMechs -- M))
+	    Mechs2 = OtherMechs ++ (ScramMechs -- (ScramMechs -- M)),
+	    case PlusDisabled of
+		true -> {Mechs2, true};
+		_ ->
+		    PlusMechs = [<<"SCRAM-SHA-1-PLUS">>, <<"SCRAM-SHA-256-PLUS">>, <<"SCRAM-SHA-512-PLUS">>],
+		    HadPlusMechs = ScramMechs -- PlusMechs /= ScramMechs,
+		    HavePlusMechs = PlusMechs -- Mechs2 == PlusMechs,
+		    {Mechs2, HadPlusMechs andalso not HavePlusMechs}
+	    end
     end.
 
 
@@ -1463,7 +1474,7 @@ get_sasl_feature(#{stream_authenticated := false,
     TLSRequired = is_starttls_required(State),
     if
 	Encrypted or not TLSRequired ->
-	    Mechs = get_sasl_mechanisms(State, sasl),
+	    {Mechs, _} = get_sasl_mechanisms(State, sasl),
 	    [#sasl_mechanisms{list = Mechs}] ++
 	    case maps:get(sasl_channel_bindings, State, none) of
 		none -> [];
@@ -1478,7 +1489,7 @@ get_sasl_feature(_) ->
 
 -spec get_sasl2_feature(state()) -> [sasl2_authenticaton() | sasl_channel_binding()].
 get_sasl2_feature(#{stream_authenticated := false} = State) ->
-    Mechs = get_sasl_mechanisms(State, sasl2),
+    {Mechs, _} = get_sasl_mechanisms(State, sasl2),
 
     {SASL2Features, Bind2Features, ExtraFeatures} =
     try callback(inline_stream_features, State)