From 71bfa173540b7b16a860b7e34cfa7cf93840b3da Mon Sep 17 00:00:00 2001 From: Badlop Date: Wed, 16 Mar 2011 18:38:44 +0100 Subject: [PATCH] Fix crash when SASL PLAIN denies auth (EJAB-1425) --- src/cyrsasl.erl | 9 +++++---- src/cyrsasl_digest.erl | 10 +++++----- src/cyrsasl_gssapi.erl | 8 ++++---- src/cyrsasl_plain.erl | 7 +++---- src/ejabberd_auth.erl | 3 ++- src/ejabberd_c2s.erl | 16 ++++++++-------- 6 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/cyrsasl.erl b/src/cyrsasl.erl index fefcb8426..94df55fe0 100644 --- a/src/cyrsasl.erl +++ b/src/cyrsasl.erl @@ -204,7 +204,7 @@ server_new(Service, ServerFQDN, UserRealm, _SecFlags, %% Continue = {continue, ServerOut, New_State} %% ServerOut = string() %% New_State = saslstate() -%% Error = {error, Reason} | {error, Username, Reason} +%% Error = {error, Reason} | {error, Reason, Username} %% Reason = term() %% Username = string() @@ -236,8 +236,9 @@ server_start(State, Mech, ClientIn) -> %% Continue = {continue, ServerOut, New_State} %% ServerOut = string() %% New_State = saslstate() -%% Error = {error, Reason} | {error, Username, Reason} +%% Error = {error, Reason} | {error, Reason, Text, Username} %% Reason = term() +%% Text = string() %% Username = string() server_step(State, ClientIn) -> @@ -254,8 +255,8 @@ server_step(State, ClientIn) -> {continue, ServerOut, NewMechState} -> {continue, ServerOut, State#sasl_state{mech_state = NewMechState}}; - {error, Error, Username} -> - {error, Error, Username}; + {error, Error, Text, Username} -> + {error, Error, Text, Username}; {error, Error} -> {error, Error} end. diff --git a/src/cyrsasl_digest.erl b/src/cyrsasl_digest.erl index 5545aadb6..92658f554 100644 --- a/src/cyrsasl_digest.erl +++ b/src/cyrsasl_digest.erl @@ -80,7 +80,7 @@ mech_new(#sasl_params{host=Host, get_password=GetPassword, %% Continue = {continue, ServerOut, New_State} %% ServerOut = string() %% New_State = mechstate() -%% Error = {error, Reason} | {error, Reason, Username} +%% Error = {error, Reason} | {error, Reason, Text, Username} %% Reason = term() mech_step(#state{step = 1, nonce = Nonce} = State, _) -> @@ -99,12 +99,12 @@ mech_step(#state{step = 3, nonce = Nonce} = State, ClientIn) -> false -> ?DEBUG("User login not authorized because digest-uri " "seems invalid: ~p", [DigestURI]), - {error, 'not-authorized', UserName}; + {error, 'not-authorized', "", UserName}; true -> AuthzId = proplists:get_value("authzid", KeyVals, ""), case (State#state.get_password)(UserName) of {false, _} -> - {error, 'not-authorized', UserName}; + {error, 'not-authorized', "", UserName}; {Passwd, AuthModule} -> case (State#state.check_password)(UserName, "", proplists:get_value("response", KeyVals, ""), @@ -121,9 +121,9 @@ mech_step(#state{step = 3, nonce = Nonce} = State, ClientIn) -> username = UserName, authzid = AuthzId}}; false -> - {error, 'not-authorized', UserName}; + {error, 'not-authorized', "", UserName}; {false, _} -> - {error, 'not-authorized', UserName} + {error, 'not-authorized', "", UserName} end end end diff --git a/src/cyrsasl_gssapi.erl b/src/cyrsasl_gssapi.erl index 4075d284f..bc76dcfb6 100644 --- a/src/cyrsasl_gssapi.erl +++ b/src/cyrsasl_gssapi.erl @@ -128,13 +128,13 @@ do_step(#state{needsmore=true,sasl=Sasl,step=Step}=State, ClientIn) -> {needsmore, RspAuth} -> ?DEBUG("needsmore~n", []), if (Step > 0) and (ClientIn =:= []) and (RspAuth =:= <<>>) -> - {error, "not-authorized"}; + {error, 'not-authorized'}; true -> {continue, binary_to_list(RspAuth), State#state{step=Step+1}} end; {error, _} -> - {error, "not-authorized"} + {error, 'not-authorized'} end. handle_step_ok(State, []) -> @@ -147,7 +147,7 @@ check_user(#state{authid=Authid,authzid=Authzid, authrealm=Auth_realm,host=Host,realm=Realm}) -> if Realm =/= Auth_realm -> ?DEBUG("bad realm ~p (expected ~p)~n",[Auth_realm, Realm]), - throw({error, "not-authorized"}); + throw({error, 'not-authorized'}); true -> ok end, @@ -155,7 +155,7 @@ check_user(#state{authid=Authid,authzid=Authzid, case ejabberd_auth:is_user_exists(Authid, Host) of false -> ?DEBUG("bad user ~p~n",[Authid]), - throw({error, "not-authorized"}); + throw({error, 'not-authorized'}); true -> ok end, diff --git a/src/cyrsasl_plain.erl b/src/cyrsasl_plain.erl index 32dff9136..7b529d210 100644 --- a/src/cyrsasl_plain.erl +++ b/src/cyrsasl_plain.erl @@ -62,8 +62,7 @@ mech_new(#sasl_params{check_password = CheckPassword}) -> %% Username = string() %% AuthzId = string() %% AuthModule = atom() -%% Error = {error, Reason} | {error, Reason, Username} -%% Reason = term() +%% Error = {error, Reason} | {error, Reason, Text, Username} mech_step(State, ClientIn) -> case prepare(ClientIn) of @@ -73,9 +72,9 @@ mech_step(State, ClientIn) -> {ok, [{username, User}, {authzid, AuthzId}, {auth_module, AuthModule}]}; {false, ReasonAuthFail} when is_list(ReasonAuthFail) -> - {error, ReasonAuthFail, User}; + {error, 'not-authorized', ReasonAuthFail, User}; _ -> - {error, 'not-authorized', User} + {error, 'not-authorized', "", User} end; _ -> {error, 'bad-protocol'} diff --git a/src/ejabberd_auth.erl b/src/ejabberd_auth.erl index b6b19f514..738976c77 100644 --- a/src/ejabberd_auth.erl +++ b/src/ejabberd_auth.erl @@ -141,7 +141,8 @@ check_password(User, Server, Password, Digest, DigestGen) %% {true, AuthModule} | {false, Reason::string()} %% where %% AuthModule = ejabberd_auth_anonymous | ejabberd_auth_external -%% | ejabberd_auth_ldap | ejabberd_auth_pam | ejabberd_auth_storage +%% | ejabberd_auth_internal | ejabberd_auth_ldap +%% | ejabberd_auth_odbc | ejabberd_auth_pam %% @doc Check if the user and password can login in server. %% The user can login if at least an authentication method accepts the user %% and the password. diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl index 64c7fa128..5b432199f 100644 --- a/src/ejabberd_c2s.erl +++ b/src/ejabberd_c2s.erl @@ -716,13 +716,13 @@ wait_for_feature_request({xmlstreamelement, #xmlel{ns = NS, name = Name} = El}, fsm_next_state(wait_for_sasl_response, StateData#state{ sasl_state = NewSASLState}); - {error, Error, Username} when is_list(Error) -> + {error, Error, Text, Username} -> ?INFO_MSG( - "(~w) Failed authentication for ~s@~s due to ~s", + "(~w) Failed authentication for ~s@~s due to ~p ~s", [StateData#state.socket, - Username, StateData#state.server, Error]), + Username, StateData#state.server, Error, Text]), send_element(StateData, - exmpp_server_sasl:failure(Error)), + exmpp_server_sasl:failure(Error, Text)), {next_state, wait_for_feature_request, StateData, ?C2S_OPEN_TIMEOUT}; {error, Error} -> @@ -834,13 +834,13 @@ wait_for_sasl_response({xmlstreamelement, #xmlel{ns = NS, name = Name} = El}, exmpp_server_sasl:challenge(ServerOut)), fsm_next_state(wait_for_sasl_response, StateData#state{sasl_state = NewSASLState}); - {error, Error, Username} -> + {error, Error, Text, Username} -> ?INFO_MSG( - "(~w) Failed authentication for ~s@~s", + "(~w) Failed authentication for ~s@~s due to ~p ~s", [StateData#state.socket, - Username, StateData#state.server]), + Username, StateData#state.server, Error, Text]), send_element(StateData, - exmpp_server_sasl:failure(Error)), + exmpp_server_sasl:failure(Error, Text)), fsm_next_state(wait_for_feature_request, StateData); {error, Error} -> send_element(StateData,