diff options
-rw-r--r-- | lib/ssh/doc/src/ssh.xml | 8 | ||||
-rw-r--r-- | lib/ssh/src/ssh_connection_handler.erl | 17 | ||||
-rw-r--r-- | lib/ssh/src/ssh_options.erl | 55 | ||||
-rw-r--r-- | lib/ssh/src/ssh_transport.erl | 14 | ||||
-rw-r--r-- | lib/ssh/test/ssh_algorithms_SUITE.erl | 163 | ||||
-rw-r--r-- | lib/ssh/test/ssh_basic_SUITE.erl | 2 | ||||
-rw-r--r-- | lib/ssh/test/ssh_test_lib.erl | 6 |
7 files changed, 173 insertions, 92 deletions
diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index c659e093b9..5c9ce3d5fb 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -246,10 +246,12 @@ <tag><c><![CDATA[{pref_public_key_algs, list()}]]></c></tag> <item> <p>List of user (client) public key algorithms to try to use.</p> - <p>The default value is - <c><![CDATA[['ssh-rsa','ssh-dss','ecdsa-sha2-nistp256','ecdsa-sha2-nistp384','ecdsa-sha2-nistp521'] ]]></c> + <p>The default value is the <c>public_key</c> entry in + <seealso marker="#default_algorithms/0">ssh:default_algorithms/0</seealso>. + </p> + <p>If there is no public key of a specified type available, the corresponding entry is ignored. + Note that the available set is dependent on the underlying cryptolib and current user's public keys. </p> - <p>If there is no public key of a specified type available, the corresponding entry is ignored.</p> </item> <tag><c><![CDATA[{preferred_algorithms, algs_list()}]]></c></tag> diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 39bd54869f..6a6b9896cb 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -1701,15 +1701,18 @@ handle_ssh_msg_ext_info(#ssh_msg_ext_info{data=Data}, D0) -> lists:foldl(fun ext_info/2, D0, Data). -ext_info({"server-sig-algs",SigAlgs}, D0 = #data{ssh_params=#ssh{role=client}=Ssh0}) -> +ext_info({"server-sig-algs",SigAlgs}, D0 = #data{ssh_params=#ssh{role=client, + userauth_pubkeys=ClientSigAlgs}=Ssh0}) -> %% Make strings to eliminate risk of beeing bombed with odd strings that fills the atom table: SupportedAlgs = lists:map(fun erlang:atom_to_list/1, ssh_transport:supported_algorithms(public_key)), - Ssh = Ssh0#ssh{userauth_pubkeys = - [list_to_atom(SigAlg) || SigAlg <- string:tokens(SigAlgs,","), - %% length of SigAlg is implicitly checked by member: - lists:member(SigAlg, SupportedAlgs) - ]}, - D0#data{ssh_params = Ssh}; + ServerSigAlgs = [list_to_atom(SigAlg) || SigAlg <- string:tokens(SigAlgs,","), + %% length of SigAlg is implicitly checked by the comparison + %% in member/2: + lists:member(SigAlg, SupportedAlgs) + ], + CommonAlgs = [Alg || Alg <- ServerSigAlgs, + lists:member(Alg, ClientSigAlgs)], + D0#data{ssh_params = Ssh0#ssh{userauth_pubkeys = CommonAlgs} }; ext_info(_, D0) -> %% Not implemented diff --git a/lib/ssh/src/ssh_options.erl b/lib/ssh/src/ssh_options.erl index 78f68dbcb1..12c0190082 100644 --- a/lib/ssh/src/ssh_options.erl +++ b/lib/ssh/src/ssh_options.erl @@ -392,6 +392,12 @@ default(server) -> class => user_options }, + {preferred_algorithms, def} => + #{default => ssh:default_algorithms(), + chk => fun check_preferred_algorithms/1, + class => user_options + }, + %%%%% Undocumented {infofun, def} => #{default => fun(_,_,_) -> void end, @@ -430,12 +436,24 @@ default(client) -> }, {pref_public_key_algs, def} => - #{default => - ssh_transport:supported_algorithms(public_key), - chk => - fun check_pref_public_key_algs/1, - class => - ssh + #{default => ssh_transport:default_algorithms(public_key) -- ['rsa-sha2-256', + 'rsa-sha2-512'], + chk => fun check_pref_public_key_algs/1, + class => user_options + }, + + {preferred_algorithms, def} => + #{default => [{K,Vs} || {K,Vs0} <- ssh:default_algorithms(), + Vs <- [case K of + public_key -> + Vs0 -- ['rsa-sha2-256', + 'rsa-sha2-512']; + _ -> + Vs0 + end] + ], + chk => fun check_preferred_algorithms/1, + class => user_options }, {dh_gex_limits, def} => @@ -503,12 +521,6 @@ default(common) -> class => user_options }, - {preferred_algorithms, def} => - #{default => ssh:default_algorithms(), - chk => fun check_preferred_algorithms/1, - class => user_options - }, - {id_string, def} => #{default => undefined, % FIXME: see ssh_transport:ssh_vsn/0 chk => fun(random) -> @@ -817,16 +829,23 @@ valid_hash(X, _) -> error_in_check(X, "Expect atom or list in fingerprint spec" %%%---------------------------------------------------------------- check_preferred_algorithms(Algs) -> + [error_in_check(K,"Bad preferred_algorithms key") + || {K,_} <- Algs, + not lists:keymember(K,1,ssh:default_algorithms())], + try alg_duplicates(Algs, [], []) of [] -> {true, - [try ssh_transport:supported_algorithms(Key) - of - DefAlgs -> handle_pref_alg(Key,Vals,DefAlgs) - catch - _:_ -> error_in_check(Key,"Bad preferred_algorithms key") - end || {Key,Vals} <- Algs] + [case proplists:get_value(Key, Algs) of + undefined -> + {Key,DefAlgs}; + Vals -> + handle_pref_alg(Key,Vals,SupAlgs) + end + || {{Key,DefAlgs}, {Key,SupAlgs}} <- lists:zip(ssh:default_algorithms(), + ssh_transport:supported_algorithms()) + ] }; Dups -> diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl index bd1cb4bd22..1a15798080 100644 --- a/lib/ssh/src/ssh_transport.erl +++ b/lib/ssh/src/ssh_transport.erl @@ -92,10 +92,7 @@ default_algorithms(cipher) -> default_algorithms(mac) -> supported_algorithms(mac, same(['AEAD_AES_128_GCM', 'AEAD_AES_256_GCM'])); -default_algorithms(public_key) -> - supported_algorithms(public_key, ['rsa-sha2-256', - 'rsa-sha2-384', - 'rsa-sha2-512']); + default_algorithms(Alg) -> supported_algorithms(Alg, []). @@ -122,10 +119,9 @@ supported_algorithms(public_key) -> {'ecdsa-sha2-nistp384', [{public_keys,ecdsa}, {hashs,sha384}, {ec_curve,secp384r1}]}, {'ecdsa-sha2-nistp521', [{public_keys,ecdsa}, {hashs,sha512}, {ec_curve,secp521r1}]}, {'ecdsa-sha2-nistp256', [{public_keys,ecdsa}, {hashs,sha256}, {ec_curve,secp256r1}]}, + {'ssh-rsa', [{public_keys,rsa}, {hashs,sha} ]}, {'rsa-sha2-256', [{public_keys,rsa}, {hashs,sha256} ]}, - {'rsa-sha2-384', [{public_keys,rsa}, {hashs,sha384} ]}, {'rsa-sha2-512', [{public_keys,rsa}, {hashs,sha512} ]}, - {'ssh-rsa', [{public_keys,rsa}, {hashs,sha} ]}, {'ssh-dss', [{public_keys,dss}, {hashs,sha} ]} % Gone in OpenSSH 7.3.p1 ]); @@ -741,9 +737,11 @@ ext_info_message(#ssh{role=client, end; ext_info_message(#ssh{role=server, - send_ext_info=true} = Ssh0) -> + send_ext_info=true, + opts = Opts} = Ssh0) -> AlgsList = lists:map(fun erlang:atom_to_list/1, - ssh_transport:default_algorithms(public_key)), + proplists:get_value(public_key, + ?GET_OPT(preferred_algorithms, Opts))), Msg = #ssh_msg_ext_info{nr_extensions = 1, data = [{"server-sig-algs", string:join(AlgsList,",")}] }, diff --git a/lib/ssh/test/ssh_algorithms_SUITE.erl b/lib/ssh/test/ssh_algorithms_SUITE.erl index c94309bb3e..736461624d 100644 --- a/lib/ssh/test/ssh_algorithms_SUITE.erl +++ b/lib/ssh/test/ssh_algorithms_SUITE.erl @@ -68,7 +68,7 @@ groups() -> TagGroupSet ++ AlgoTcSet. -tags() -> [kex,cipher,mac,compression]. +tags() -> [kex,cipher,mac,compression,public_key]. two_way_tags() -> [cipher,mac,compression]. %%-------------------------------------------------------------------- @@ -123,20 +123,35 @@ init_per_group(Group, Config) -> Tag = proplists:get_value(name, hd(proplists:get_value(tc_group_path, Config))), Alg = Group, - PA = - case split(Alg) of - [_] -> - [Alg]; - [A1,A2] -> - [{client2server,[A1]}, - {server2client,[A2]}] - end, - ct:log("Init tests for tag=~p alg=~p",[Tag,PA]), - PrefAlgs = {preferred_algorithms,[{Tag,PA}]}, - start_std_daemon([PrefAlgs], - [{pref_algs,PrefAlgs} | Config]) + init_per_group(Tag, Alg, Config) end. + +init_per_group(public_key=Tag, Alg, Config) -> + ct:log("Init tests for public_key ~p",[Alg]), + PrefAlgs = {preferred_algorithms,[{Tag,[Alg]}]}, + %% Daemon started later in init_per_testcase + [{pref_algs,PrefAlgs}, + {tag_alg,{Tag,Alg}} + | Config]; + +init_per_group(Tag, Alg, Config) -> + PA = + case split(Alg) of + [_] -> + [Alg]; + [A1,A2] -> + [{client2server,[A1]}, + {server2client,[A2]}] + end, + ct:log("Init tests for tag=~p alg=~p",[Tag,PA]), + PrefAlgs = {preferred_algorithms,[{Tag,PA}]}, + start_std_daemon([PrefAlgs], + [{pref_algs,PrefAlgs}, + {tag_alg,{Tag,Alg}} + | Config]). + + end_per_group(_Alg, Config) -> case proplists:get_value(srvr_pid,Config) of Pid when is_pid(Pid) -> @@ -148,23 +163,49 @@ end_per_group(_Alg, Config) -> -init_per_testcase(sshc_simple_exec_os_cmd, Config) -> - start_pubkey_daemon([proplists:get_value(pref_algs,Config)], Config); -init_per_testcase(_TC, Config) -> - Config. +init_per_testcase(TC, Config) -> + init_per_testcase(TC, proplists:get_value(tag_alg,Config), Config). -end_per_testcase(sshc_simple_exec_os_cmd, Config) -> - case proplists:get_value(srvr_pid,Config) of - Pid when is_pid(Pid) -> - ssh:stop_daemon(Pid), - ct:log("stopped ~p",[proplists:get_value(srvr_addr,Config)]); - _ -> - ok +init_per_testcase(_, {public_key,Alg}, Config) -> + Opts = pubkey_opts(Config), + case {ssh_file:user_key(Alg,Opts), ssh_file:host_key(Alg,Opts)} of + {{ok,_}, {ok,_}} -> + start_pubkey_daemon([proplists:get_value(pref_algs,Config)], + [{extra_daemon,true}|Config]); + {{ok,_}, _} -> + {skip, "No host key"}; + + {_, {ok,_}} -> + {skip, "No user key"}; + + _ -> + {skip, "Neither host nor user key"} end; -end_per_testcase(_TC, Config) -> + +init_per_testcase(sshc_simple_exec_os_cmd, _, Config) -> + start_pubkey_daemon([proplists:get_value(pref_algs,Config)], + [{extra_daemon,true}|Config]); + +init_per_testcase(_, _, Config) -> Config. + +end_per_testcase(_TC, Config) -> + case proplists:get_value(extra_daemon, Config, false) of + true -> + case proplists:get_value(srvr_pid,Config) of + Pid when is_pid(Pid) -> + ssh:stop_daemon(Pid), + ct:log("stopped ~p",[proplists:get_value(srvr_addr,Config)]), + Config; + _ -> + Config + end; + _ -> + Config + end. + %%-------------------------------------------------------------------- %% Test Cases -------------------------------------------------------- %%-------------------------------------------------------------------- @@ -319,29 +360,32 @@ concat(A1, A2) -> list_to_atom(lists:concat([A1," + ",A2])). split(Alg) -> ssh_test_lib:to_atoms(string:tokens(atom_to_list(Alg), " + ")). specific_test_cases(Tag, Alg, SshcAlgos, SshdAlgos, TypeSSH) -> - [simple_exec, simple_sftp] ++ - case supports(Tag, Alg, SshcAlgos) of - true when TypeSSH == openSSH -> - [sshc_simple_exec_os_cmd]; - _ -> - [] - end ++ - case supports(Tag, Alg, SshdAlgos) of - true -> - [sshd_simple_exec]; - _ -> - [] - end ++ - case {Tag,Alg} of - {kex,_} when Alg == 'diffie-hellman-group-exchange-sha1' ; - Alg == 'diffie-hellman-group-exchange-sha256' -> - [simple_exec_groups, - simple_exec_groups_no_match_too_large, - simple_exec_groups_no_match_too_small - ]; - _ -> - [] - end. + case Tag of + public_key -> []; + _ -> [simple_exec, simple_sftp] + end + ++ case supports(Tag, Alg, SshcAlgos) of + true when TypeSSH == openSSH -> + [sshc_simple_exec_os_cmd]; + _ -> + [] + end ++ + case supports(Tag, Alg, SshdAlgos) of + true -> + [sshd_simple_exec]; + _ -> + [] + end ++ + case {Tag,Alg} of + {kex,_} when Alg == 'diffie-hellman-group-exchange-sha1' ; + Alg == 'diffie-hellman-group-exchange-sha256' -> + [simple_exec_groups, + simple_exec_groups_no_match_too_large, + simple_exec_groups_no_match_too_small + ]; + _ -> + [] + end. supports(Tag, Alg, Algos) -> lists:all(fun(A) -> @@ -371,19 +415,30 @@ start_std_daemon(Opts, Config) -> ct:log("started ~p:~p ~p",[Host,Port,Opts]), [{srvr_pid,Pid},{srvr_addr,{Host,Port}} | Config]. + start_pubkey_daemon(Opts0, Config) -> - Opts = [{auth_methods,"publickey"}|Opts0], - {Pid, Host, Port} = ssh_test_lib:std_daemon1(Config, Opts), - ct:log("started pubkey_daemon ~p:~p ~p",[Host,Port,Opts]), + ct:log("starting pubkey_daemon",[]), + Opts = pubkey_opts(Config) ++ Opts0, + {Pid, Host, Port} = ssh_test_lib:daemon([{failfun, fun ssh_test_lib:failfun/2} + | Opts]), + ct:log("started ~p:~p ~p",[Host,Port,Opts]), [{srvr_pid,Pid},{srvr_addr,{Host,Port}} | Config]. +pubkey_opts(Config) -> + SystemDir = filename:join(proplists:get_value(priv_dir,Config), "system"), + [{auth_methods,"publickey"}, + {system_dir, SystemDir}]. + + setup_pubkey(Config) -> DataDir = proplists:get_value(data_dir, Config), UserDir = proplists:get_value(priv_dir, Config), - ssh_test_lib:setup_dsa(DataDir, UserDir), - ssh_test_lib:setup_rsa(DataDir, UserDir), - ssh_test_lib:setup_ecdsa("256", DataDir, UserDir), + Keys = + [ssh_test_lib:setup_dsa(DataDir, UserDir), + ssh_test_lib:setup_rsa(DataDir, UserDir), + ssh_test_lib:setup_ecdsa("256", DataDir, UserDir)], + ssh_test_lib:write_auth_keys(Keys, UserDir), % 'authorized_keys' shall contain ALL pub keys Config. diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index 1e591bc295..62e2a585e4 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -612,7 +612,7 @@ exec_key_differs(Config, UserPKAlgs) -> {_Pid, _Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir}, {user_dir, SystemUserDir}, {preferred_algorithms, - [{public_key,['ssh-rsa']}]}]), + [{public_key,['ssh-rsa'|UserPKAlgs]}]}]), ct:sleep(500), IO = ssh_test_lib:start_io_server(), diff --git a/lib/ssh/test/ssh_test_lib.erl b/lib/ssh/test/ssh_test_lib.erl index 36ae2525da..7b273fecef 100644 --- a/lib/ssh/test/ssh_test_lib.erl +++ b/lib/ssh/test/ssh_test_lib.erl @@ -500,8 +500,12 @@ setup_ecdsa_auth_keys(_Size, Dir, UserDir) -> setup_auth_keys(Keys, Dir) -> AuthKeys = public_key:ssh_encode(Keys, auth_keys), AuthKeysFile = filename:join(Dir, "authorized_keys"), - file:write_file(AuthKeysFile, AuthKeys). + ok = file:write_file(AuthKeysFile, AuthKeys), + AuthKeys. +write_auth_keys(Keys, Dir) -> + AuthKeysFile = filename:join(Dir, "authorized_keys"), + file:write_file(AuthKeysFile, Keys). del_dirs(Dir) -> case file:list_dir(Dir) of |