File 3501-ssh-Check-the-user-name-when-doing-pubkey-auth.patch of Package erlang
From ec7c68692dbe451bf92655005f069204fcc93b5c Mon Sep 17 00:00:00 2001
From: Hans Nilsson <hans@erlang.org>
Date: Wed, 23 Sep 2020 11:01:01 +0200
Subject: [PATCH] ssh: Check the user name when doing pubkey auth
This name (option {user,UserName}) has nothing to do with the
user name in the OS, but Codenomicum Defensics complains about
not testing it, so therfor thte test is added.
It is off by default (for compatibility), but could be enabled
with the option pk_check_user set to true.
---
lib/ssh/doc/src/hardening.xml | 37 ++++++++++++++++++++++++
lib/ssh/doc/src/ssh.xml | 30 ++++++++++++++++++--
lib/ssh/src/ssh.hrl | 5 ++--
lib/ssh/src/ssh_auth.erl | 53 ++++++++++++++++++++++++++---------
lib/ssh/src/ssh_options.erl | 6 ++++
5 files changed, 113 insertions(+), 18 deletions(-)
diff --git a/lib/ssh/doc/src/hardening.xml b/lib/ssh/doc/src/hardening.xml
index 5d65f5da3f..c1d3f7669c 100644
--- a/lib/ssh/doc/src/hardening.xml
+++ b/lib/ssh/doc/src/hardening.xml
@@ -143,6 +143,43 @@
</taglist>
</section>
+ <section>
+ <title>Verifying the remote client in a daemon (server)</title>
+ <taglist>
+ <tag>Password checking</tag>
+ <item>
+ <p>The default password checking is with the list in the
+ <seealso marker="ssh#option-user_passwords">user_passwords</seealso> option in the SSH daemon.
+ It could be replaced with a <seealso marker="ssh#option-pwdfun">pwdfun</seealso> plugin. The
+ arity four variant (<seealso marker="ssh#type-pwdfun_4"><c>pwdfun_4()</c></seealso>)
+ can also be used for introducing delays after failed password checking attempts. Here is a simple
+ example of such a pwdfun:
+ </p>
+ <code>
+fun(User, Password, _PeerAddress, State) ->
+ case lists:member({User,Password}, my_user_pwds()) of
+ true ->
+ {true, undefined}; % Reset delay time
+ false when State == undefined ->
+ timer:sleep(1000),
+ {false, 2000}; % Next delay is 2000 ms
+ false when is_integer(State) ->
+ timer:sleep(State),
+ {false, 2*State} % Double the delay for each failure
+ end
+end.
+</code>
+ <p>If a public key is used for logging in, there is normally no checking of the user name. It
+ could be enabled by setting the option
+ <seealso marker="ssh#option-pk_check_user"><c>pk_check_user</c></seealso>
+ to <c>true</c>.
+ In that case the pwdfun will get the atom <c>pubkey</c> in the password argument.
+ </p>
+ </item>
+
+ </taglist>
+ </section>
+
<section>
<title>Hardening in the cryptographic area</title>
<section>
diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml
index a0355219a1..b88bdc1667 100644
--- a/lib/ssh/doc/src/ssh.xml
+++ b/lib/ssh/doc/src/ssh.xml
@@ -565,6 +565,26 @@
</p>
</item>
+ <tag><marker id="option-pk_check_user"/><c>pk_check_user</c></tag>
+ <item>
+ <p>Enables checking of the
+ <seealso marker="#type-authentication_client_options">client's user name</seealso>
+ in the server when doing public key authentication. It is disabled by default.
+ </p>
+ <p>The term "user" is used differently in OpenSSH and SSH in Erlang/OTP:
+ see more in the <seealso marker="terminology#the-term--user-">User's Guide</seealso>.
+ </p>
+ <p>If the option is enabled, and no
+ <seealso marker="#option-pwdfun"><c>pwdfun</c></seealso>
+ is present, the user name must present in the
+ <seealso marker="#option-user_passwords">user_passwords</seealso>
+ for the check to succeed but the value of the password is not checked.
+ </p>
+ <p>In case of a <seealso marker="#option-pwdfun"><c>pwdfun</c></seealso>
+ checking the user, the atom <c>pubkey</c> is put in the password argument.
+ </p>
+ </item>
+
<tag><marker id="option-password"/><c>password</c></tag>
<item>
<p>Provides a global password that authenticates any user.</p>
@@ -587,7 +607,6 @@
the <c>State</c> variable could be used. This state is per connection only. The first time the pwdfun
is called for a connection, the <c>State</c> variable has the value <c>undefined</c>.
</p>
-
<p>The fun should return:
</p>
<list type="bulleted">
@@ -598,9 +617,12 @@
<item><c>{true, NewState:any()}</c> if the user and password is valid</item>
<item><c>{false, NewState:any()}</c> if the user or password is invalid</item>
</list>
-
<p>A third usage is to block login attempts from a missbehaving peer. The <c>State</c> described above
can be used for this. The return value <c>disconnect</c> is useful for this.</p>
+ <p>In case of the <seealso marker="#option-pk_check_user"><c>pk_check_user</c></seealso> is set,
+ the atom <c>pubkey</c> is put in the password argument when validating a public key login. The
+ pwdfun is then responsible to check that the user name is valid.
+ </p>
</item>
<tag><c>pwdfun</c> with
@@ -613,6 +635,10 @@
<item><c>true</c> if the user and password is valid</item>
<item><c>false</c> if the user or password is invalid</item>
</list>
+ <p>In case of the <seealso marker="#option-pk_check_user"><c>pk_check_user</c></seealso> is set,
+ the atom <c>pubkey</c> is put in the password argument when validating a public key login. The
+ pwdfun is then responsible to check that the user name is valid.
+ </p>
<p>This variant is kept for compatibility.</p>
</item>
</taglist>
diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl
index 08aef95f72..10b1d4aec7 100644
--- a/lib/ssh/src/ssh.hrl
+++ b/lib/ssh/src/ssh.hrl
@@ -356,6 +356,7 @@
ssh_file:system_dir_daemon_option()
| {auth_method_kb_interactive_data, prompt_texts() }
| {user_passwords, [{UserName::string(),Pwd::string()}]}
+ | {pk_check_user, boolean()}
| {password, string()}
| {pwdfun, pwdfun_2() | pwdfun_4()} .
@@ -369,9 +370,9 @@
-type kb_int_fun_4() :: fun((Peer::ip_port(), User::string(), Service::string(), State::any()) -> kb_int_tuple()).
-type kb_int_tuple() :: {Name::string(), Instruction::string(), Prompt::string(), Echo::boolean()}.
--type pwdfun_2() :: fun((User::string(), Password::string()) -> boolean()) .
+-type pwdfun_2() :: fun((User::string(), Password::string()|pubkey) -> boolean()) .
-type pwdfun_4() :: fun((User::string(),
- Password::string(),
+ Password::string()|pubkey,
PeerAddress::ip_port(),
State::any()) ->
boolean() | disconnect | {boolean(),NewState::any()}
diff --git a/lib/ssh/src/ssh_auth.erl b/lib/ssh/src/ssh_auth.erl
index 943e824de8..5bc2859b4e 100644
--- a/lib/ssh/src/ssh_auth.erl
+++ b/lib/ssh/src/ssh_auth.erl
@@ -232,10 +232,9 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
service = "ssh-connection",
method = "password",
data = <<?FALSE, ?UINT32(Sz), BinPwd:Sz/binary>>}, _,
- #ssh{opts = Opts,
- userauth_supported_methods = Methods} = Ssh) ->
+ #ssh{userauth_supported_methods = Methods} = Ssh) ->
Password = unicode:characters_to_list(BinPwd),
- case check_password(User, Password, Opts, Ssh) of
+ case check_password(User, Password, Ssh) of
{true,Ssh1} ->
{authorized, User,
{#ssh_msg_userauth_success{}, Ssh1}
@@ -287,10 +286,16 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
>>
},
_SessionId,
- #ssh{opts = Opts,
- userauth_supported_methods = Methods} = Ssh) ->
-
- case pre_verify_sig(User, KeyBlob, Opts) of
+ #ssh{userauth_supported_methods = Methods} = Ssh0) ->
+ Ssh =
+ case check_user(User, Ssh0) of
+ {true,Ssh01} -> Ssh01#ssh{user=User};
+ {false,Ssh01} -> Ssh01#ssh{user=false}
+ end,
+
+ case
+ pre_verify_sig(User, KeyBlob, Ssh)
+ of
true ->
{not_authorized, {User, undefined},
{#ssh_msg_userauth_pk_ok{algorithm_name = binary_to_list(BAlg),
@@ -312,10 +317,15 @@ handle_userauth_request(#ssh_msg_userauth_request{user = User,
SigWLen/binary>>
},
SessionId,
- #ssh{userauth_supported_methods = Methods} = Ssh) ->
+ #ssh{user = PreVerifyUser,
+ userauth_supported_methods = Methods} = Ssh0) ->
- case verify_sig(SessionId, User, "ssh-connection",
- BAlg, KeyBlob, SigWLen, Ssh) of
+ {UserOk,Ssh} = check_user(User, Ssh0),
+ case
+ ((PreVerifyUser == User) orelse (PreVerifyUser == undefined)) andalso
+ UserOk andalso
+ verify_sig(SessionId, User, "ssh-connection", BAlg, KeyBlob, SigWLen, Ssh)
+ of
true ->
{authorized, User,
{#ssh_msg_userauth_success{}, Ssh}
@@ -429,7 +439,7 @@ handle_userauth_info_response(#ssh_msg_userauth_info_response{num_responses = 1,
orelse
proplists:get_value(one_empty, ?GET_OPT(tstflg,Opts), false),
- case check_password(User, unicode:characters_to_list(Password), Opts, Ssh) of
+ case check_password(User, unicode:characters_to_list(Password), Ssh) of
{true,Ssh1} when SendOneEmpty==true ->
{authorized_but_one_more, User,
{#ssh_msg_userauth_info_request{name = "",
@@ -473,8 +483,23 @@ method_preference(SigKeyAlgs) ->
],
PubKeyDefs ++ NonPKmethods.
-check_password(User, Password, Opts, Ssh) ->
+check_user(User, Ssh) ->
+ case ?GET_OPT(pk_check_user, Ssh#ssh.opts) of
+ true ->
+ check_password(User, pubkey, Ssh);
+ _ ->
+ {true, Ssh} % i.e, skip the test
+ end.
+
+check_password(User, Password, #ssh{opts=Opts} = Ssh) ->
case ?GET_OPT(pwdfun, Opts) of
+ undefined when Password==pubkey ->
+ %% Just check the User name
+ case lists:keysearch(User, 1, ?GET_OPT(user_passwords,Opts)) of
+ {value, {User, _}} -> {true, Ssh};
+ false -> {false, Ssh}
+ end;
+
undefined ->
Static = get_password_option(Opts, User),
{crypto:equal_const_time(Password,Static), Ssh};
@@ -508,7 +533,7 @@ get_password_option(Opts, User) ->
false -> ?GET_OPT(password, Opts)
end.
-pre_verify_sig(User, KeyBlob, Opts) ->
+pre_verify_sig(User, KeyBlob, #ssh{opts=Opts}) ->
try
Key = ssh_message:ssh2_pubkey_decode(KeyBlob), % or exception
ssh_transport:call_KeyCb(is_auth_key, [Key, User], Opts)
@@ -517,7 +542,7 @@ pre_verify_sig(User, KeyBlob, Opts) ->
false
end.
-verify_sig(SessionId, User, Service, AlgBin, KeyBlob, SigWLen, #ssh{opts = Opts} = Ssh) ->
+verify_sig(SessionId, User, Service, AlgBin, KeyBlob, SigWLen, #ssh{opts=Opts} = Ssh) ->
try
Alg = binary_to_list(AlgBin),
Key = ssh_message:ssh2_pubkey_decode(KeyBlob), % or exception
diff --git a/lib/ssh/src/ssh_options.erl b/lib/ssh/src/ssh_options.erl
index b27777093b..4733f4e775 100644
--- a/lib/ssh/src/ssh_options.erl
+++ b/lib/ssh/src/ssh_options.erl
@@ -508,6 +508,12 @@ default(server) ->
class => user_options
},
+ {pk_check_user, def} =>
+ #{default => false,
+ chk => fun erlang:is_boolean/1,
+ class => user_options
+ },
+
{password, def} =>
#{default => undefined,
chk => fun check_string/1,
--
2.26.2
