From c505918a86fb9ac8c19e47cd751a9db4e2d9efb2 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 29 Oct 2015 19:51:04 +0100 Subject: ssh: pwdfun/4 and simple tests Also solves OTP-13053 --- lib/ssh/src/ssh.erl | 4 +- lib/ssh/src/ssh.hrl | 1 + lib/ssh/src/ssh_auth.erl | 49 +++++++---- lib/ssh/test/ssh_options_SUITE.erl | 161 ++++++++++++++++++++++++++++++++++++- 4 files changed, 199 insertions(+), 16 deletions(-) (limited to 'lib') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 049018b21c..6f79b48091 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -462,7 +462,9 @@ handle_ssh_option({password, Value} = Opt) when is_list(Value) -> Opt; handle_ssh_option({user_passwords, Value} = Opt) when is_list(Value)-> Opt; -handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value) -> +handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value,2) -> + Opt; +handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value,4) -> Opt; handle_ssh_option({key_cb, Value} = Opt) when is_atom(Value) -> Opt; diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl index fc9d60c500..4ad936f742 100644 --- a/lib/ssh/src/ssh.hrl +++ b/lib/ssh/src/ssh.hrl @@ -138,6 +138,7 @@ kb_tries_left = 0, % integer(), num tries left for "keyboard-interactive" userauth_preference, available_host_keys, + pwdfun_user_state, authenticated = false }). diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 04749fcf8e..4272eb3c52 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -174,15 +174,15 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User, #ssh{opts = Opts, userauth_supported_methods = Methods} = Ssh) -> Password = unicode:characters_to_list(BinPwd), - case check_password(User, Password, Opts) of - true -> + case check_password(User, Password, Opts, Ssh) of + {true,Ssh1} -> {authorized, User, - ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh)}; - false -> + ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh1)}; + {false,Ssh1} -> {not_authorized, {User, {error,"Bad user or password"}}, ssh_transport:ssh_packet(#ssh_msg_userauth_failure{ authentications = Methods, - partial_success = false}, Ssh)} + partial_success = false}, Ssh1)} end; handle_userauth_request(#ssh_msg_userauth_request{user = User, @@ -335,16 +335,16 @@ handle_userauth_info_response(#ssh_msg_userauth_info_response{num_responses = 1, kb_tries_left = KbTriesLeft, user = User, userauth_supported_methods = Methods} = Ssh) -> - case check_password(User, unicode:characters_to_list(Password), Opts) of - true -> + case check_password(User, unicode:characters_to_list(Password), Opts, Ssh) of + {true,Ssh1} -> {authorized, User, - ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh)}; - false -> + ssh_transport:ssh_packet(#ssh_msg_userauth_success{}, Ssh1)}; + {false,Ssh1} -> {not_authorized, {User, {error,"Bad user or password"}}, ssh_transport:ssh_packet(#ssh_msg_userauth_failure{ authentications = Methods, partial_success = false}, - Ssh#ssh{kb_tries_left = max(KbTriesLeft-1, 0)} + Ssh1#ssh{kb_tries_left = max(KbTriesLeft-1, 0)} )} end; @@ -387,13 +387,34 @@ user_name(Opts) -> {ok, User} end. -check_password(User, Password, Opts) -> +check_password(User, Password, Opts, Ssh) -> case proplists:get_value(pwdfun, Opts) of undefined -> Static = get_password_option(Opts, User), - Password == Static; - Cheker -> - Cheker(User, Password) + {Password == Static, Ssh}; + + Checker when is_function(Checker,2) -> + {Checker(User, Password), Ssh}; + + Checker when is_function(Checker,4) -> + #ssh{pwdfun_user_state = PrivateState, + peer = {_,PeerAddr={_,_}} + } = Ssh, + case Checker(User, Password, PeerAddr, PrivateState) of + true -> + {true,Ssh}; + false -> + {false,Ssh}; + {true,NewState} -> + {true, Ssh#ssh{pwdfun_user_state=NewState}}; + {false,NewState} -> + {false, Ssh#ssh{pwdfun_user_state=NewState}}; + disconnect -> + throw(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, + description = + "Unable to connect using the available authentication methods", + language = ""}) + end end. get_password_option(Opts, User) -> diff --git a/lib/ssh/test/ssh_options_SUITE.erl b/lib/ssh/test/ssh_options_SUITE.erl index cf15ca4253..6a201d401f 100644 --- a/lib/ssh/test/ssh_options_SUITE.erl +++ b/lib/ssh/test/ssh_options_SUITE.erl @@ -45,6 +45,9 @@ max_sessions_ssh_connect_sequential/1, server_password_option/1, server_userpassword_option/1, + server_pwdfun_option/1, + server_pwdfun_4_option/1, + server_pwdfun_4_option_repeat/1, ssh_connect_arg4_timeout/1, ssh_connect_negtimeout_parallel/1, ssh_connect_negtimeout_sequential/1, @@ -83,6 +86,9 @@ all() -> connectfun_disconnectfun_client, server_password_option, server_userpassword_option, + server_pwdfun_option, + server_pwdfun_4_option, + server_pwdfun_4_option_repeat, {group, dir_options}, ssh_connect_timeout, ssh_connect_arg4_timeout, @@ -188,7 +194,9 @@ init_per_testcase(_TestCase, Config) -> Config. end_per_testcase(TestCase, Config) when TestCase == server_password_option; - TestCase == server_userpassword_option -> + TestCase == server_userpassword_option; + TestCase == server_pwdfun_option; + TestCase == server_pwdfun_4_option -> UserDir = filename:join(?config(priv_dir, Config), nopubkey), ssh_test_lib:del_dirs(UserDir), end_per_testcase(Config); @@ -271,6 +279,157 @@ server_userpassword_option(Config) when is_list(Config) -> {user_dir, UserDir}]), ssh:stop_daemon(Pid). +%%-------------------------------------------------------------------- +%%% validate to server that uses the 'pwdfun' option +server_pwdfun_option(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + CHKPWD = fun("foo",Pwd) -> Pwd=="bar"; + (_,_) -> false + end, + {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, PrivDir}, + {pwdfun,CHKPWD}]), + ConnectionRef = + ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "bar"}, + {user_interaction, false}, + {user_dir, UserDir}]), + ssh:close(ConnectionRef), + + Reason = "Unable to connect using the available authentication methods", + + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "morot"}, + {user_interaction, false}, + {user_dir, UserDir}]), + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "vego"}, + {password, "foo"}, + {user_interaction, false}, + {user_dir, UserDir}]), + ssh:stop_daemon(Pid). + + +%%-------------------------------------------------------------------- +%%% validate to server that uses the 'pwdfun/4' option +server_pwdfun_4_option(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + PWDFUN = fun("foo",Pwd,{_,_},undefined) -> Pwd=="bar"; + ("fie",Pwd,{_,_},undefined) -> {Pwd=="bar",new_state}; + ("bandit",_,_,_) -> disconnect; + (_,_,_,_) -> false + end, + {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, PrivDir}, + {pwdfun,PWDFUN}]), + ConnectionRef1 = + ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "bar"}, + {user_interaction, false}, + {user_dir, UserDir}]), + ssh:close(ConnectionRef1), + + ConnectionRef2 = + ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "fie"}, + {password, "bar"}, + {user_interaction, false}, + {user_dir, UserDir}]), + ssh:close(ConnectionRef2), + + Reason = "Unable to connect using the available authentication methods", + + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {password, "morot"}, + {user_interaction, false}, + {user_dir, UserDir}]), + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "fie"}, + {password, "morot"}, + {user_interaction, false}, + {user_dir, UserDir}]), + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "vego"}, + {password, "foo"}, + {user_interaction, false}, + {user_dir, UserDir}]), + + {error, Reason} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "bandit"}, + {password, "pwd breaking"}, + {user_interaction, false}, + {user_dir, UserDir}]), + ssh:stop_daemon(Pid). + + +%%-------------------------------------------------------------------- +server_pwdfun_4_option_repeat(Config) -> + PrivDir = ?config(priv_dir, Config), + UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth + file:make_dir(UserDir), + SysDir = ?config(data_dir, Config), + %% Test that the state works + Parent = self(), + PWDFUN = fun("foo",P="bar",_,S) -> Parent!{P,S},true; + (_,P,_,S=undefined) -> Parent!{P,S},{false,1}; + (_,P,_,S) -> Parent!{P,S}, {false,S+1} + end, + {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir}, + {user_dir, PrivDir}, + {auth_methods,"keyboard-interactive"}, + {pwdfun,PWDFUN}]), + + %% Try with passwords "incorrect", "Bad again" and finally "bar" + KIFFUN = fun(_,_,_) -> + K={k,self()}, + case get(K) of + undefined -> + put(K,1), + ["incorrect"]; + 2 -> + put(K,3), + ["bar"]; + S-> + put(K,S+1), + ["Bad again"] + end + end, + + ConnectionRef2 = + ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true}, + {user, "foo"}, + {keyboard_interact_fun, KIFFUN}, + {user_dir, UserDir}]), + ssh:close(ConnectionRef2), + ssh:stop_daemon(Pid), + + lists:foreach(fun(Expect) -> + receive + Expect -> ok; + Other -> ct:fail("Expect: ~p~nReceived ~p",[Expect,Other]) + after + 2000 -> ct:fail("Timeout expecting ~p",[Expect]) + end + end, [{"incorrect",undefined}, + {"Bad again",1}, + {"bar",2}]). + %%-------------------------------------------------------------------- system_dir_option(Config) -> DirUnread = proplists:get_value(unreadable_dir,Config), -- cgit v1.2.3 From 193ccf4009eb346ca5dd43679b219e395016b03d Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 30 Oct 2015 11:33:40 +0100 Subject: ssh: enable users to give option keyboard_interact_fun It is on purpose not documented. It needs more thinking before being finalized. --- lib/ssh/src/ssh.erl | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'lib') diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 6f79b48091..693b55a09e 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -337,6 +337,8 @@ handle_option([{pwdfun, _} = Opt | Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); handle_option([{key_cb, _} = Opt | Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); +handle_option([{keyboard_interact_fun, _} = Opt | Rest], SocketOptions, SshOptions) -> + handle_option(Rest, SocketOptions, [handle_ssh_option(Opt) | SshOptions]); %%Backwards compatibility handle_option([{allow_user_interaction, Value} | Rest], SocketOptions, SshOptions) -> handle_option(Rest, SocketOptions, [handle_ssh_option({user_interaction, Value}) | SshOptions]); @@ -468,6 +470,8 @@ handle_ssh_option({pwdfun, Value} = Opt) when is_function(Value,4) -> Opt; handle_ssh_option({key_cb, Value} = Opt) when is_atom(Value) -> Opt; +handle_ssh_option({keyboard_interact_fun, Value} = Opt) when is_function(Value,3) -> + Opt; handle_ssh_option({compression, Value} = Opt) when is_atom(Value) -> Opt; handle_ssh_option({exec, {Module, Function, _}} = Opt) when is_atom(Module), -- cgit v1.2.3 From 17517fb5ef4e9e7e6913a6eb4527f862ede29271 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Fri, 30 Oct 2015 12:25:16 +0100 Subject: ssh: make corrections of keyboard-interactive client * Newlines should be added after Name and Instructions field according to rfc4256. * There was an error in the argument list of the last clause of ssh_auth:keyboard_interact_get_responses/9 * Correct client kb-interactive behaviour at auth failure --- lib/ssh/src/ssh_auth.erl | 11 ++++++++--- lib/ssh/src/ssh_connection_handler.erl | 18 ++++++++++++++---- 2 files changed, 22 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 4272eb3c52..4967a2e4cd 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -364,6 +364,11 @@ 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). @@ -472,14 +477,14 @@ keyboard_interact_get_responses(false, undefined, undefined, _, _, _, [Prompt|_] ssh_no_io:read_line(Prompt, Opts); %% Throws error as keyboard interaction is not allowed keyboard_interact_get_responses(true, undefined, _,IoCb, Name, Instr, PromptInfos, Opts, _) -> keyboard_interact(IoCb, Name, Instr, PromptInfos, Opts); -keyboard_interact_get_responses(true, Fun, _, Name, Instr, PromptInfos, _, _, NumPrompts) -> +keyboard_interact_get_responses(true, Fun, _Pwd, _IoCb, Name, Instr, PromptInfos, _Opts, NumPrompts) -> keyboard_interact_fun(Fun, Name, Instr, PromptInfos, NumPrompts). keyboard_interact(IoCb, Name, Instr, Prompts, Opts) -> - if Name /= "" -> IoCb:format("~s", [Name]); + if Name /= "" -> IoCb:format("~s~n", [Name]); true -> ok end, - if Instr /= "" -> IoCb:format("~s", [Instr]); + if Instr /= "" -> IoCb:format("~s~n", [Instr]); true -> ok end, lists:map(fun({Prompt, true}) -> IoCb:read_line(Prompt, Opts); diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 7fb86c1108..a2d1b5b810 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -627,14 +627,24 @@ userauth_keyboard_interactive(#ssh_msg_userauth_info_response{} = Msg, retry_fun(User, Address, Reason, Opts), send_msg(Reply, State), {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} - end. - + end; +userauth_keyboard_interactive(Msg = #ssh_msg_userauth_failure{}, + #state{ssh_params = Ssh0 = + #ssh{role = client, + userauth_preference = Prefs0}} + = State) -> + Prefs = [{Method,M,F,A} || {Method,M,F,A} <- Prefs0, + Method =/= "keyboard-interactive"], + userauth(Msg, State#state{ssh_params = Ssh0#ssh{userauth_preference=Prefs}}). + -userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_failure{}, State) -> +userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_failure{}, + #state{ssh_params = #ssh{role = client}} = State) -> userauth(Msg, State); -userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_success{}, State) -> +userauth_keyboard_interactive_info_response(Msg=#ssh_msg_userauth_success{}, + #state{ssh_params = #ssh{role = client}} = State) -> userauth(Msg, State). %%-------------------------------------------------------------------- -- cgit v1.2.3 From 53bfbb61333af35cde29bb786817856925dcedf0 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Mon, 2 Nov 2015 13:08:13 +0100 Subject: ssh: Document pwdfun --- lib/ssh/doc/src/ssh.xml | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 2b190c98b6..31a5e71401 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -471,12 +471,43 @@ kex is implicit but public_key is set explicitly.

- boolean()}]]> + boolean()}]]> + +

Provides a function for password validation. This could used for calling an external system or if + passwords should be stored as a hash. The fun returns: + + true if the user and password is valid and + false otherwise. + +

+

This fun can also be used to make delays in authentication tries for example by calling + timer:sleep/1. To facilitate counting of failed tries + the State variable could be used. This state is per connection only. The first time the pwdfun + is called for a connection, the State variable has the value undefined. + The pwdfun can return - in addition to the values above - a new state + as: + + {true, NewState:any()} if the user and password is valid or + {false, NewState:any()} if the user or password is invalid + +

+

A third usage is to block login attempts from a missbehaving peer. The State described above + can be used for this. In addition to the responses above, the following return value is introduced: + + disconnect if the connection should be closed immediately after sending a SSH_MSG_DISCONNECT + message. + +

+ + + boolean()}]]>

Provides a function for password validation. This function is called with user and password as strings, and returns if the password is valid and otherwise.

+

This option ({pwdfun,fun/2}) is the same as a subset of the previous + ({pwdfun,fun/4}). It is kept for compatibility.

 -- cgit v1.2.3 From 19f3eafbb237af7b6a9d81ebbddae19c41418f8b Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Tue, 3 Nov 2015 14:53:30 +0100 Subject: ssh: changes after doc review --- lib/ssh/doc/src/ssh.xml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 31a5e71401..f530a68dd9 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -60,6 +60,29 @@ +
+ OPTIONS +

The exact behaviour of some functions can be adjusted with the use of options which are documented together + with the functions. Generally could each option be used at most one time in each function call. If given two or more + times, the effect is not predictable unless explicitly documented.

+

The options are of different kinds:

+ + Limits +

which alters limits in the system, for example number of simultaneous login attempts.

 + + Timeouts +

which give some defined behaviour if too long time elapses before a given event or action, + for example time to wait for an answer.

 + + Callbacks +

which gives the caller of the function the possibility to execute own code on some events, + for example calling an own logging function or to perform an own login function

 + + Behaviour +

which changes the systems behaviour.

 + +
+
DATA TYPES

Type definitions that are used more than once in @@ -471,7 +494,7 @@ kex is implicit but public_key is set explicitly.

- boolean()}]]> + boolean() | disconnect | {boolean(),any()} }]]>

Provides a function for password validation. This could used for calling an external system or if passwords should be stored as a hash. The fun returns: -- cgit v1.2.3