From b4ad3a9eb7a1b375d2dbbf93069ea9ae038d121f Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Wed, 14 Nov 2018 12:57:46 +0100 Subject: ssh: Make host and user key pre-checking better This will prevent crashes in ssh_file for public key types which have no passphrase option although being supported. Also centralize host key checking to avoid code duplication. This was already done for user keys. --- lib/ssh/src/ssh_connection_handler.erl | 50 +++++++++++++++++----------------- lib/ssh/src/ssh_file.erl | 3 +- lib/ssh/src/ssh_transport.erl | 18 +++++++----- 3 files changed, 38 insertions(+), 33 deletions(-) diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index e23df6ceca..7c87591cf2 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -1685,18 +1685,19 @@ peer_role(client) -> server; peer_role(server) -> client. %%-------------------------------------------------------------------- -available_hkey_algorithms(Role, Options) -> - KeyCb = ?GET_OPT(key_cb, Options), - case [A || A <- available_hkey_algos(Options), - (Role==client) orelse available_host_key(KeyCb, A, Options) - ] of - - [] when Role==client -> - error({shutdown, "No public key algs"}); - - [] when Role==server -> - error({shutdown, "No host key available"}); +available_hkey_algorithms(client, Options) -> + case available_hkey_algos(Options) of + [] -> + error({shutdown, "No public key algs"}); + Algs -> + [atom_to_list(A) || A<-Algs] + end; +available_hkey_algorithms(server, Options) -> + case [A || A <- available_hkey_algos(Options), + is_usable_host_key(A, Options)] of + [] -> + error({shutdown, "No host key available"}); Algs -> [atom_to_list(A) || A<-Algs] end. @@ -1712,18 +1713,6 @@ available_hkey_algos(Options) -> AvailableAndSupported. -%% Alg :: atom() -available_host_key({KeyCb,KeyCbOpts}, Alg, Opts) -> - UserOpts = ?GET_OPT(user_options, Opts), - case KeyCb:host_key(Alg, [{key_cb_private,KeyCbOpts}|UserOpts]) of - {ok,Key} -> - %% Check the key - the KeyCb may be a buggy plugin - ssh_transport:valid_key_sha_alg(Key, Alg); - _ -> - false - end. - - send_msg(Msg, State=#data{ssh_params=Ssh0}) when is_tuple(Msg) -> {Bytes, Ssh} = ssh_transport:ssh_packet(Msg, Ssh0), send_bytes(Bytes, State), @@ -1843,10 +1832,21 @@ ext_info(_, D0) -> D0. %%%---------------------------------------------------------------- -is_usable_user_pubkey(A, Ssh) -> - case ssh_auth:get_public_key(A, Ssh) of +is_usable_user_pubkey(Alg, Ssh) -> + try ssh_auth:get_public_key(Alg, Ssh) of {ok,_} -> true; _ -> false + catch + _:_ -> false + end. + +%%%---------------------------------------------------------------- +is_usable_host_key(Alg, Opts) -> + try ssh_transport:get_host_key(Alg, Opts) + of + _PrivHostKey -> true + catch + _:_ -> false end. %%%---------------------------------------------------------------- diff --git a/lib/ssh/src/ssh_file.erl b/lib/ssh/src/ssh_file.erl index 669b0f9be2..c5824b2519 100644 --- a/lib/ssh/src/ssh_file.erl +++ b/lib/ssh/src/ssh_file.erl @@ -268,7 +268,8 @@ identity_pass_phrase("rsa-sha2-384" ) -> rsa_pass_phrase; identity_pass_phrase("rsa-sha2-512" ) -> rsa_pass_phrase; identity_pass_phrase("ecdsa-sha2-"++_) -> ecdsa_pass_phrase; identity_pass_phrase(P) when is_atom(P) -> - identity_pass_phrase(atom_to_list(P)). + identity_pass_phrase(atom_to_list(P)); +identity_pass_phrase(_) -> undefined. lookup_host_key_fd(Fd, KeyToMatch, Host, KeyType) -> case io:get_line(Fd, '') of diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl index f49b49b2df..cc3ef46fc2 100644 --- a/lib/ssh/src/ssh_transport.erl +++ b/lib/ssh/src/ssh_transport.erl @@ -52,6 +52,7 @@ ssh_packet/2, pack/2, valid_key_sha_alg/2, sha/1, sign/3, verify/5, + get_host_key/2, call_KeyCb/3]). -export([dbg_trace/3]). @@ -432,7 +433,8 @@ key_exchange_first_msg(Kex, Ssh0) when Kex == 'ecdh-sha2-nistp256' ; %%% handle_kexdh_init(#ssh_msg_kexdh_init{e = E}, Ssh0 = #ssh{algorithms = #alg{kex=Kex, - hkey=SignAlg} = Algs}) -> + hkey=SignAlg} = Algs, + opts = Opts}) -> %% server {G, P} = dh_group(Kex), if @@ -440,7 +442,7 @@ handle_kexdh_init(#ssh_msg_kexdh_init{e = E}, Sz = dh_bits(Algs), {Public, Private} = generate_key(dh, [P,G,2*Sz]), K = compute_key(dh, E, Private, [P,G]), - MyPrivHostKey = get_host_key(Ssh0, SignAlg), + MyPrivHostKey = get_host_key(SignAlg, Opts), MyPubHostKey = extract_public_key(MyPrivHostKey), H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {E,Public,K}), H_SIG = sign(H, sha(SignAlg), MyPrivHostKey), @@ -579,14 +581,15 @@ handle_kex_dh_gex_init(#ssh_msg_kex_dh_gex_init{e = E}, #ssh{keyex_key = {{Private, Public}, {G, P}}, keyex_info = {Min, Max, NBits}, algorithms = #alg{kex=Kex, - hkey=SignAlg}} = Ssh0) -> + hkey=SignAlg}, + opts = Opts} = Ssh0) -> %% server if 1= K = compute_key(dh, E, Private, [P,G]), if 1 - MyPrivHostKey = get_host_key(Ssh0, SignAlg), + MyPrivHostKey = get_host_key(SignAlg, Opts), MyPubHostKey = extract_public_key(MyPrivHostKey), H = kex_hash(Ssh0, MyPubHostKey, sha(Kex), {Min,NBits,Max,P,G,E,Public,K}), H_SIG = sign(H, sha(SignAlg), MyPrivHostKey), @@ -654,7 +657,8 @@ handle_kex_dh_gex_reply(#ssh_msg_kex_dh_gex_reply{public_host_key = PeerPubHostK %%% handle_kex_ecdh_init(#ssh_msg_kex_ecdh_init{q_c = PeerPublic}, Ssh0 = #ssh{algorithms = #alg{kex=Kex, - hkey=SignAlg}}) -> + hkey=SignAlg}, + opts = Opts}) -> %% at server Curve = ecdh_curve(Kex), {MyPublic, MyPrivate} = generate_key(ecdh, Curve), @@ -662,7 +666,7 @@ handle_kex_ecdh_init(#ssh_msg_kex_ecdh_init{q_c = PeerPublic}, compute_key(ecdh, PeerPublic, MyPrivate, Curve) of K -> - MyPrivHostKey = get_host_key(Ssh0, SignAlg), + MyPrivHostKey = get_host_key(SignAlg, Opts), MyPubHostKey = extract_public_key(MyPrivHostKey), H = kex_hash(Ssh0, MyPubHostKey, sha(Curve), {PeerPublic, MyPublic, K}), H_SIG = sign(H, sha(SignAlg), MyPrivHostKey), @@ -778,7 +782,7 @@ sid(#ssh{session_id = Id}, _) -> Id. %% %% The host key should be read from storage %% -get_host_key(#ssh{opts=Opts}, SignAlg) -> +get_host_key(SignAlg, Opts) -> case call_KeyCb(host_key, [SignAlg], Opts) of {ok, PrivHostKey} -> %% Check the key - the KeyCb may be a buggy plugin -- cgit v1.2.3