From c4e9732c040966366e0719a62550f30e45fc01a3 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 1 Sep 2016 22:38:13 +0200 Subject: ssh: separate clauses for first and second pk auth msg SSH sends the public key and user name twice. If we do not check the validity of that pair at the first time, Codenomicon Defensics will complain. --- lib/ssh/src/ssh_auth.erl | 66 +++++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 31 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 1dcf5d0708..7793d77f36 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -260,43 +260,45 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User, handle_userauth_request(#ssh_msg_userauth_request{user = User, service = "ssh-connection", method = "publickey", - data = Data}, + data = <> + }, SessionId, #ssh{opts = Opts, userauth_supported_methods = Methods} = Ssh) -> - <> = Data, - - {KeyBlob, SigWLen} = - case Rest of - <> -> - {KeyBlob0, SigWLen0}; - <<>> -> - {<<>>, <<>>} - end, + {not_authorized, {User, undefined}, + ssh_transport:ssh_packet( + #ssh_msg_userauth_pk_ok{algorithm_name = binary_to_list(BAlg), + key_blob = KeyBlob}, Ssh)}; - case HaveSig of - ?TRUE -> - case verify_sig(SessionId, User, "ssh-connection", - binary_to_list(BAlg), - KeyBlob, SigWLen, Opts) of - true -> - {authorized, User, - ssh_transport:ssh_packet( - #ssh_msg_userauth_success{}, Ssh)}; - false -> - {not_authorized, {User, undefined}, - ssh_transport:ssh_packet(#ssh_msg_userauth_failure{ - authentications = Methods, - partial_success = false}, Ssh)} - end; - ?FALSE -> - {not_authorized, {User, undefined}, +handle_userauth_request(#ssh_msg_userauth_request{user = User, + service = "ssh-connection", + method = "publickey", + data = <> + }, + SessionId, + #ssh{opts = Opts, + userauth_supported_methods = Methods} = Ssh) -> + + case verify_sig(SessionId, User, "ssh-connection", + binary_to_list(BAlg), + KeyBlob, SigWLen, Opts) of + true -> + {authorized, User, ssh_transport:ssh_packet( - #ssh_msg_userauth_pk_ok{algorithm_name = binary_to_list(BAlg), - key_blob = KeyBlob}, Ssh)} + #ssh_msg_userauth_success{}, Ssh)}; + false -> + {not_authorized, {User, undefined}, + ssh_transport:ssh_packet(#ssh_msg_userauth_failure{ + authentications = Methods, + partial_success = false}, Ssh)} end; handle_userauth_request(#ssh_msg_userauth_request{user = User, @@ -484,6 +486,8 @@ get_password_option(Opts, User) -> false -> proplists:get_value(password, Opts, false) end. +%%pre_verify_sig(SessionId, User, Service, Alg, KeyBlob, Opts) -> + verify_sig(SessionId, User, Service, Alg, KeyBlob, SigWLen, Opts) -> {ok, Key} = decode_public_key_v2(KeyBlob, Alg), KeyCb = proplists:get_value(key_cb, Opts, ssh_file), -- cgit v1.2.3 From c0dfb5487a5ca79c35506905090b14e4abe06e3a Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 1 Sep 2016 22:52:04 +0200 Subject: ssh: pre-verify key Conflicts: lib/ssh/src/ssh_auth.erl --- lib/ssh/src/ssh_auth.erl | 58 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 18 deletions(-) (limited to 'lib/ssh/src') diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl index 7793d77f36..0c1aaa2ede 100644 --- a/lib/ssh/src/ssh_auth.erl +++ b/lib/ssh/src/ssh_auth.erl @@ -266,14 +266,23 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User, _/binary >> }, - SessionId, + _SessionId, #ssh{opts = Opts, userauth_supported_methods = Methods} = Ssh) -> - {not_authorized, {User, undefined}, - ssh_transport:ssh_packet( - #ssh_msg_userauth_pk_ok{algorithm_name = binary_to_list(BAlg), - key_blob = KeyBlob}, Ssh)}; + case pre_verify_sig(User, binary_to_list(BAlg), + KeyBlob, Opts) of + true -> + {not_authorized, {User, undefined}, + ssh_transport:ssh_packet( + #ssh_msg_userauth_pk_ok{algorithm_name = binary_to_list(BAlg), + key_blob = KeyBlob}, Ssh)}; + false -> + {not_authorized, {User, undefined}, + ssh_transport:ssh_packet(#ssh_msg_userauth_failure{ + authentications = Methods, + partial_success = false}, Ssh)} + end; handle_userauth_request(#ssh_msg_userauth_request{user = User, service = "ssh-connection", @@ -486,21 +495,34 @@ get_password_option(Opts, User) -> false -> proplists:get_value(password, Opts, false) end. -%%pre_verify_sig(SessionId, User, Service, Alg, KeyBlob, Opts) -> +pre_verify_sig(User, Alg, KeyBlob, Opts) -> + try + {ok, Key} = decode_public_key_v2(KeyBlob, Alg), + KeyCb = proplists:get_value(key_cb, Opts, ssh_file), + KeyCb:is_auth_key(Key, User, Opts) + catch + _:_ -> + false + end. verify_sig(SessionId, User, Service, Alg, KeyBlob, SigWLen, Opts) -> - {ok, Key} = decode_public_key_v2(KeyBlob, Alg), - KeyCb = proplists:get_value(key_cb, Opts, ssh_file), - - case KeyCb:is_auth_key(Key, User, Opts) of - true -> - PlainText = build_sig_data(SessionId, User, - Service, KeyBlob, Alg), - <> = SigWLen, - <> = AlgSig, - ssh_transport:verify(PlainText, sha, Sig, Key); - false -> + try + {ok, Key} = decode_public_key_v2(KeyBlob, Alg), + KeyCb = proplists:get_value(key_cb, Opts, ssh_file), + + case KeyCb:is_auth_key(Key, User, Opts) of + true -> + PlainText = build_sig_data(SessionId, User, + Service, KeyBlob, Alg), + <> = SigWLen, + <> = AlgSig, + ssh_transport:verify(PlainText, ssh_transport:sha(list_to_atom(Alg)), Sig, Key); + false -> + false + end + catch + _:_ -> false end. -- cgit v1.2.3