From 2e3d97a27bbfa86260dd248b793a1d358a836a1b Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 15 Jun 2016 13:03:40 +0200 Subject: ssh: Make client send a faulty pwd only once, ssh_auth part Conflicts: lib/ssh/src/ssh_connection_handler.erl --- lib/ssh/src/ssh_auth.erl | 214 ++++++++++++++++++++++++++++------------------- 1 file changed, 126 insertions(+), 88 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index b71bed033a..86a91c6d22 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -31,12 +31,107 @@ -export([publickey_msg/1, password_msg/1, keyboard_interactive_msg/1, service_request_msg/1, init_userauth_request_msg/1, userauth_request_msg/1, handle_userauth_request/3, - handle_userauth_info_request/3, handle_userauth_info_response/2 + handle_userauth_info_request/2, handle_userauth_info_response/2 ]). %%-------------------------------------------------------------------- %%% Internal application API %%-------------------------------------------------------------------- +%%%---------------------------------------------------------------- +userauth_request_msg(#ssh{userauth_methods = ServerMethods, + userauth_supported_methods = UserPrefMethods, % Note: this is not documented as supported for clients + userauth_preference = ClientMethods0 + } = Ssh0) -> + case sort_select_mthds(ClientMethods0, UserPrefMethods, ServerMethods) of + [] -> + Msg = #ssh_msg_disconnect{code = ?SSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE, + description = "Unable to connect using the available authentication methods", + language = "en"}, + {disconnect, Msg, ssh_transport:ssh_packet(Msg, Ssh0)}; + + [{Pref,Module,Function,Args} | Prefs] -> + Ssh = case Pref of + "keyboard-interactive" -> Ssh0; + _ -> Ssh0#ssh{userauth_preference = Prefs} + end, + case Module:Function(Args ++ [Ssh]) of + {not_ok, Ssh1} -> + userauth_request_msg(Ssh1#ssh{userauth_preference = Prefs}); + Result -> + {Pref,Result} + end + end. + + + +sort_select_mthds(Clients, undefined, Servers) -> + %% User has not expressed an opinion via option "auth_methods", use the server's prefs + sort_select_mthds(Clients, Servers, Servers); + +sort_select_mthds(Clients, Users0, Servers0) -> + %% The User has an opinion, use the intersection of that and the Servers whishes but + %% in the Users order + Servers = unique(Servers0), + Users = unique(Users0), + [C || Key <- Users, + lists:member(Key, Servers), + C <- Clients, + element(1,C) == Key]. + +unique(L) -> + lists:reverse( + lists:foldl(fun(E,Acc) -> + case lists:member(E,Acc) of + true -> Acc; + false -> [E|Acc] + end + end, [], L)). + + +%%%---- userauth_request_msg "callbacks" +password_msg([#ssh{opts = Opts, io_cb = IoCb, + user = User, service = Service} = Ssh0]) -> + {Password,Ssh} = + case proplists:get_value(password, Opts) of + undefined when IoCb == ssh_no_io -> + {not_ok, Ssh0}; + undefined -> + {IoCb:read_password("ssh password: ",Ssh0), Ssh0}; + PW -> + %% If "password" option is given it should not be tried again + {PW, Ssh0#ssh{opts = lists:keyreplace(password,1,Opts,{password,not_ok})}} + end, + case Password of + not_ok -> + {not_ok, Ssh}; + _ -> + ssh_transport:ssh_packet( + #ssh_msg_userauth_request{user = User, + service = Service, + method = "password", + data = + <>}, + Ssh) + end. + +%% See RFC 4256 for info on keyboard-interactive +keyboard_interactive_msg([#ssh{user = User, + opts = Opts, + service = Service} = Ssh]) -> + case proplists:get_value(password, Opts) of + not_ok -> + {not_ok,Ssh}; % No need to use a failed pwd once more + _ -> + ssh_transport:ssh_packet( + #ssh_msg_userauth_request{user = User, + service = Service, + method = "keyboard-interactive", + data = << ?STRING(<<"">>), + ?STRING(<<>>) >> }, + Ssh) + end. + publickey_msg([Alg, #ssh{user = User, session_id = SessionId, service = Service, @@ -48,7 +143,7 @@ publickey_msg([Alg, #ssh{user = User, StrAlgo = atom_to_list(Alg), case encode_public_key(StrAlgo, ssh_transport:extract_public_key(PrivKey)) of not_ok -> - not_ok; + {not_ok, Ssh}; PubKeyBlob -> SigData = build_sig_data(SessionId, User, Service, PubKeyBlob, StrAlgo), @@ -65,52 +160,15 @@ publickey_msg([Alg, #ssh{user = User, Ssh) end; _Error -> - not_ok + {not_ok, Ssh} end. -password_msg([#ssh{opts = Opts, io_cb = IoCb, - user = User, service = Service} = Ssh]) -> - Password = case proplists:get_value(password, Opts) of - undefined -> - user_interaction(IoCb, Ssh); - PW -> - PW - end, - case Password of - not_ok -> - not_ok; - _ -> - ssh_transport:ssh_packet( - #ssh_msg_userauth_request{user = User, - service = Service, - method = "password", - data = - <>}, - Ssh) - end. - -user_interaction(ssh_no_io, _) -> - not_ok; -user_interaction(IoCb, Ssh) -> - IoCb:read_password("ssh password: ", Ssh). - - -%% See RFC 4256 for info on keyboard-interactive -keyboard_interactive_msg([#ssh{user = User, - service = Service} = Ssh]) -> - ssh_transport:ssh_packet( - #ssh_msg_userauth_request{user = User, - service = Service, - method = "keyboard-interactive", - data = << ?STRING(<<"">>), - ?STRING(<<>>) >> }, - Ssh). - +%%%---------------------------------------------------------------- service_request_msg(Ssh) -> ssh_transport:ssh_packet(#ssh_msg_service_request{name = "ssh-userauth"}, Ssh#ssh{service = "ssh-userauth"}). +%%%---------------------------------------------------------------- init_userauth_request_msg(#ssh{opts = Opts} = Ssh) -> case user_name(Opts) of {ok, User} -> @@ -140,34 +198,9 @@ init_userauth_request_msg(#ssh{opts = Opts} = Ssh) -> language = "en"}) end. -userauth_request_msg(#ssh{userauth_preference = []} = Ssh) -> - Msg = #ssh_msg_disconnect{code = - ?SSH_DISCONNECT_NO_MORE_AUTH_METHODS_AVAILABLE, - description = "Unable to connect using the available" - " authentication methods", - language = "en"}, - {disconnect, Msg, ssh_transport:ssh_packet(Msg, Ssh)}; - -userauth_request_msg(#ssh{userauth_methods = Methods, - userauth_preference = [{Pref, Module, - Function, Args} | Prefs]} - = Ssh0) -> - Ssh = Ssh0#ssh{userauth_preference = Prefs}, - case lists:member(Pref, Methods) of - true -> - case Module:Function(Args ++ [Ssh]) of - not_ok -> - userauth_request_msg(Ssh); - Result -> - {Pref,Result} - end; - false -> - userauth_request_msg(Ssh) - end. - - -handle_userauth_request(#ssh_msg_service_request{name = - Name = "ssh-userauth"}, +%%%---------------------------------------------------------------- +%%% called by server +handle_userauth_request(#ssh_msg_service_request{name = Name = "ssh-userauth"}, _, Ssh) -> {ok, ssh_transport:ssh_packet(#ssh_msg_service_accept{name = Name}, Ssh#ssh{service = "ssh-connection"})}; @@ -319,21 +352,28 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User, partial_success = false}, Ssh)}. - -handle_userauth_info_request( - #ssh_msg_userauth_info_request{name = Name, - instruction = Instr, - num_prompts = NumPrompts, - data = Data}, IoCb, - #ssh{opts = Opts} = Ssh) -> +%%%---------------------------------------------------------------- +%%% keyboard-interactive client +handle_userauth_info_request(#ssh_msg_userauth_info_request{name = Name, + instruction = Instr, + num_prompts = NumPrompts, + data = Data}, + #ssh{opts = Opts, + io_cb = IoCb + } = Ssh) -> PromptInfos = decode_keyboard_interactive_prompts(NumPrompts,Data), - Responses = keyboard_interact_get_responses(IoCb, Opts, - Name, Instr, PromptInfos), - {ok, - ssh_transport:ssh_packet( - #ssh_msg_userauth_info_response{num_responses = NumPrompts, - data = Responses}, Ssh)}. + case keyboard_interact_get_responses(IoCb, Opts, Name, Instr, PromptInfos) of + not_ok -> + not_ok; + Responses -> + {ok, + ssh_transport:ssh_packet( + #ssh_msg_userauth_info_response{num_responses = NumPrompts, + data = Responses}, Ssh)} + end. +%%%---------------------------------------------------------------- +%%% keyboard-interactive server handle_userauth_info_response(#ssh_msg_userauth_info_response{num_responses = 1, data = <>}, #ssh{opts = Opts, @@ -369,11 +409,6 @@ method_preference(Algs) -> [{"publickey", ?MODULE, publickey_msg, [A]} | Acc] end, [{"password", ?MODULE, password_msg, []}, - {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}, - {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}, - {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}, - {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}, - {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []}, {"keyboard-interactive", ?MODULE, keyboard_interactive_msg, []} ], Algs). @@ -473,6 +508,9 @@ keyboard_interact_get_responses(IoCb, Opts, Name, Instr, PromptInfos) -> proplists:get_value(password, Opts, undefined), IoCb, Name, Instr, PromptInfos, Opts, NumPrompts). + +keyboard_interact_get_responses(_, _, not_ok, _, _, _, _, _, _) -> + not_ok; keyboard_interact_get_responses(_, undefined, Password, _, _, _, _, _, 1) when Password =/= undefined -> [Password]; %% Password auth implemented with keyboard-interaction and passwd is known -- cgit v1.2.3 From 3ad4e41e4653a910d592ac9912ec380b1cb1b25b Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 16 Jun 2016 18:26:50 +0200 Subject: ssh: Make client send a faulty pwd only once, ssh_connection_handler part --- lib/ssh/src/ssh_connection_handler.erl | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index ce1931e4f4..b73f8b23d2 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -612,11 +612,14 @@ userauth(#ssh_msg_userauth_banner{message = Msg}, userauth_keyboard_interactive(#ssh_msg_userauth_info_request{} = Msg, - #state{ssh_params = #ssh{role = client, - io_cb = IoCb} = Ssh0} = State) -> - {ok, {Reply, Ssh}} = ssh_auth:handle_userauth_info_request(Msg, IoCb, Ssh0), - send_msg(Reply, State), - {next_state, userauth_keyboard_interactive_info_response, next_packet(State#state{ssh_params = Ssh})}; + #state{ssh_params = #ssh{role = client} = Ssh0} = State) -> + case ssh_auth:handle_userauth_info_request(Msg, Ssh0) of + {ok, {Reply, Ssh}} -> + send_msg(Reply, State), + {next_state, userauth_keyboard_interactive_info_response, next_packet(State#state{ssh_params = Ssh})}; + not_ok -> + userauth(Msg, State) + end; userauth_keyboard_interactive(#ssh_msg_userauth_info_response{} = Msg, #state{ssh_params = #ssh{role = server, @@ -646,7 +649,18 @@ userauth_keyboard_interactive(Msg = #ssh_msg_userauth_failure{}, userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_failure{}, - #state{ssh_params = #ssh{role = client}} = State) -> + #state{ssh_params = #ssh{role = client, + opts = Opts} = Ssh0} = State0) -> + + State = case proplists:get_value(password, Opts) of + undefined -> + State0; + _ -> + State0#state{ssh_params = + Ssh0#ssh{opts = + lists:keyreplace(password,1,Opts, + {password,not_ok})}} + end, userauth(Msg, State); userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_success{}, #state{ssh_params = #ssh{role = client}} = State) -> @@ -1247,7 +1261,7 @@ init_ssh(client = Role, Vsn, Version, Options, Socket) -> end, AuthMethods = proplists:get_value(auth_methods, Options, - ?SUPPORTED_AUTH_METHODS), + undefined), {ok, PeerAddr} = inet:peername(Socket), PeerName = proplists:get_value(host, Options), -- cgit v1.2.3 From 688a278d0050ed088df17e351d14e3f9ba193501 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 17 Jun 2016 13:34:47 +0200 Subject: ssh: Fix type error in args of ssh_auth:sort_selected_mthds --- lib/ssh/src/ssh_auth.erl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 86a91c6d22..07585dbacd 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -66,11 +66,15 @@ userauth_request_msg(#ssh{userauth_methods = ServerMethods, sort_select_mthds(Clients, undefined, Servers) -> %% User has not expressed an opinion via option "auth_methods", use the server's prefs - sort_select_mthds(Clients, Servers, Servers); + sort_select_mthds1(Clients, Servers, string:tokens(?SUPPORTED_AUTH_METHODS,",")); sort_select_mthds(Clients, Users0, Servers0) -> %% The User has an opinion, use the intersection of that and the Servers whishes but %% in the Users order + sort_select_mthds1(Clients, string:tokens(Users0,","), Servers0). + + +sort_select_mthds1(Clients, Users0, Servers0) -> Servers = unique(Servers0), Users = unique(Users0), [C || Key <- Users, -- cgit v1.2.3 From c66f6df0678362ce09558af1981473b0b8d82baf Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 17 Jun 2016 14:41:22 +0200 Subject: ssh: Some code cuddling in ssh_io --- lib/ssh/src/ssh_io.erl | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_io.erl b/lib/ssh/src/ssh_io.erl index a5e627fdb3..6be5ea25bb 100644 --- a/lib/ssh/src/ssh_io.erl +++ b/lib/ssh/src/ssh_io.erl @@ -36,47 +36,42 @@ read_line(Prompt, Ssh) -> end. yes_no(Prompt, Ssh) -> - io:format("~s [y/n]?", [Prompt]), + format("~s [y/n]?", [Prompt]), proplists:get_value(user_pid, Ssh#ssh.opts) ! {self(), question}, receive + %% I can't see that the atoms y and n are ever received, but it must + %% be investigated before removing + y -> yes; + n -> no; + Answer -> case trim(Answer) of "y" -> yes; "n" -> no; "Y" -> yes; "N" -> no; - y -> yes; - n -> no; _ -> - io:format("please answer y or n\n"), + format("please answer y or n\n",[]), yes_no(Prompt, Ssh) end end. -read_password(Prompt, Ssh) -> +read_password(Prompt, #ssh{opts=Opts}) -> read_password(Prompt, Opts); +read_password(Prompt, Opts) when is_list(Opts) -> format("~s", [listify(Prompt)]), - case is_list(Ssh) of - false -> - proplists:get_value(user_pid, Ssh#ssh.opts) ! {self(), user_password}; - _ -> - proplists:get_value(user_pid, Ssh) ! {self(), user_password} - end, + proplists:get_value(user_pid, Opts) ! {self(), user_password}, receive + "" -> + read_password(Prompt, Opts); Answer -> - case Answer of - "" -> - read_password(Prompt, Ssh); - Pass -> Pass - end + Answer end. -listify(A) when is_atom(A) -> - atom_to_list(A); -listify(L) when is_list(L) -> - L; -listify(B) when is_binary(B) -> - binary_to_list(B). + +listify(A) when is_atom(A) -> atom_to_list(A); +listify(L) when is_list(L) -> L; +listify(B) when is_binary(B) -> binary_to_list(B). format(Fmt, Args) -> io:format(Fmt, Args). @@ -93,6 +88,3 @@ trim1([$\r|Cs]) -> trim(Cs); trim1([$\n|Cs]) -> trim(Cs); trim1([$\t|Cs]) -> trim(Cs); trim1(Cs) -> Cs. - - - -- cgit v1.2.3 From d68b279981496c5293746524e00ff77fd8a8b84c Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 17 Jun 2016 14:49:04 +0200 Subject: ssh: Fix a hazard bug in ssh_auth --- lib/ssh/src/ssh_io.erl | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_io.erl b/lib/ssh/src/ssh_io.erl index 6be5ea25bb..5e335c2063 100644 --- a/lib/ssh/src/ssh_io.erl +++ b/lib/ssh/src/ssh_io.erl @@ -31,7 +31,7 @@ read_line(Prompt, Ssh) -> format("~s", [listify(Prompt)]), proplists:get_value(user_pid, Ssh) ! {self(), question}, receive - Answer -> + Answer when is_list(Answer) -> Answer end. @@ -44,7 +44,7 @@ yes_no(Prompt, Ssh) -> y -> yes; n -> no; - Answer -> + Answer when is_list(Answer) -> case trim(Answer) of "y" -> yes; "n" -> no; @@ -62,20 +62,24 @@ read_password(Prompt, Opts) when is_list(Opts) -> format("~s", [listify(Prompt)]), proplists:get_value(user_pid, Opts) ! {self(), user_password}, receive - "" -> - read_password(Prompt, Opts); - Answer -> - Answer + Answer when is_list(Answer) -> + case trim(Answer) of + "" -> + read_password(Prompt, Opts); + Pwd -> + Pwd + end end. +format(Fmt, Args) -> + io:format(Fmt, Args). + +%%%================================================================ listify(A) when is_atom(A) -> atom_to_list(A); listify(L) when is_list(L) -> L; listify(B) when is_binary(B) -> binary_to_list(B). -format(Fmt, Args) -> - io:format(Fmt, Args). - trim(Line) when is_list(Line) -> lists:reverse(trim1(lists:reverse(trim1(Line)))); -- cgit v1.2.3 From c74cb8aaec77452f7c91ea5345c1b6120fe15224 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 17 Jun 2016 16:01:38 +0200 Subject: ssh: polishing of password prompt's linefeed --- lib/ssh/src/ssh_auth.erl | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 07585dbacd..0c378d084b 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -528,17 +528,18 @@ keyboard_interact_get_responses(true, Fun, _Pwd, _IoCb, Name, Instr, PromptInfos keyboard_interact_fun(Fun, Name, Instr, PromptInfos, NumPrompts). keyboard_interact(IoCb, Name, Instr, Prompts, Opts) -> - if Name /= "" -> IoCb:format("~s~n", [Name]); - true -> ok - end, - if Instr /= "" -> IoCb:format("~s~n", [Instr]); - true -> ok - end, + write_if_nonempty(IoCb, Name), + write_if_nonempty(IoCb, Instr), lists:map(fun({Prompt, true}) -> IoCb:read_line(Prompt, Opts); ({Prompt, false}) -> IoCb:read_password(Prompt, Opts) end, Prompts). +write_if_nonempty(_, "") -> ok; +write_if_nonempty(_, <<>>) -> ok; +write_if_nonempty(IoCb, Text) -> IoCb:format("~s~n",[Text]). + + keyboard_interact_fun(KbdInteractFun, Name, Instr, PromptInfos, NumPrompts) -> Prompts = lists:map(fun({Prompt, _Echo}) -> Prompt end, PromptInfos), -- cgit v1.2.3