You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
326 lines
10 KiB
326 lines
10 KiB
From 41ecb7dc3932dd57bac52980982c76bf036ccfd8 Mon Sep 17 00:00:00 2001 |
|
From: TJ Saunders <tj@castaglia.org> |
|
Date: Wed, 12 Jul 2017 23:14:59 -0700 |
|
Subject: [PATCH] Bug#4309: Allow SFTP/SCP logins to succeed properly when |
|
"AllowEmptyPasswords off" in effect. |
|
|
|
Also ensure that a truly empty SFTP/SCP password IS properly rejected in such |
|
a configuration. |
|
--- |
|
contrib/mod_sftp/auth-password.c | 41 +++++++- |
|
modules/mod_auth.c | 55 +++++++---- |
|
tests/t/lib/ProFTPD/Tests/Modules/mod_sftp.pm | 132 ++++++++++++++++++++++++++ |
|
3 files changed, 205 insertions(+), 23 deletions(-) |
|
|
|
diff --git a/contrib/mod_sftp/auth-password.c b/contrib/mod_sftp/auth-password.c |
|
index 2605af7f6..8fb9804bd 100644 |
|
--- a/contrib/mod_sftp/auth-password.c |
|
+++ b/contrib/mod_sftp/auth-password.c |
|
@@ -1,6 +1,6 @@ |
|
/* |
|
* ProFTPD - mod_sftp 'password' user authentication |
|
- * Copyright (c) 2008-2015 TJ Saunders |
|
+ * Copyright (c) 2008-2017 TJ Saunders |
|
* |
|
* This program is free software; you can redistribute it and/or modify |
|
* it under the terms of the GNU General Public License as published by |
|
@@ -37,6 +37,7 @@ int sftp_auth_password(struct ssh2_packet *pkt, cmd_rec *pass_cmd, |
|
char *passwd; |
|
int have_new_passwd, res; |
|
struct passwd *pw; |
|
+ size_t passwd_len; |
|
|
|
cipher_algo = sftp_cipher_get_read_algo(); |
|
mac_algo = sftp_mac_get_read_algo(); |
|
@@ -77,6 +78,7 @@ int sftp_auth_password(struct ssh2_packet *pkt, cmd_rec *pass_cmd, |
|
|
|
passwd = sftp_msg_read_string(pkt->pool, buf, buflen); |
|
passwd = sftp_utf8_decode_str(pkt->pool, passwd); |
|
+ passwd_len = strlen(passwd); |
|
|
|
pass_cmd->arg = passwd; |
|
|
|
@@ -92,7 +94,7 @@ int sftp_auth_password(struct ssh2_packet *pkt, cmd_rec *pass_cmd, |
|
pr_cmd_dispatch_phase(pass_cmd, POST_CMD_ERR, 0); |
|
pr_cmd_dispatch_phase(pass_cmd, LOG_CMD_ERR, 0); |
|
|
|
- pr_memscrub(passwd, strlen(passwd)); |
|
+ pr_memscrub(passwd, passwd_len); |
|
|
|
*send_userauth_fail = TRUE; |
|
errno = EPERM; |
|
@@ -109,15 +111,46 @@ int sftp_auth_password(struct ssh2_packet *pkt, cmd_rec *pass_cmd, |
|
session.c->remote_name, pr_netaddr_get_ipstr(session.c->remote_addr), |
|
pr_netaddr_get_ipstr(session.c->local_addr), session.c->local_port); |
|
|
|
- pr_memscrub(passwd, strlen(passwd)); |
|
+ pr_memscrub(passwd, passwd_len); |
|
|
|
*send_userauth_fail = TRUE; |
|
errno = ENOENT; |
|
return 0; |
|
} |
|
|
|
+ if (passwd_len == 0) { |
|
+ config_rec *c; |
|
+ int allow_empty_passwords = TRUE; |
|
+ |
|
+ c = find_config(main_server->conf, CONF_PARAM, "AllowEmptyPasswords", |
|
+ FALSE); |
|
+ if (c != NULL) { |
|
+ allow_empty_passwords = *((int *) c->argv[0]); |
|
+ } |
|
+ |
|
+ if (allow_empty_passwords == FALSE) { |
|
+ pr_log_debug(DEBUG5, |
|
+ "Refusing empty password from user '%s' (AllowEmptyPasswords false)", |
|
+ user); |
|
+ pr_log_auth(PR_LOG_NOTICE, |
|
+ "Refusing empty password from user '%s'", user); |
|
+ |
|
+ pr_event_generate("mod_auth.empty-password", user); |
|
+ pr_response_add_err(R_501, "Login incorrect."); |
|
+ |
|
+ pr_cmd_dispatch_phase(pass_cmd, POST_CMD_ERR, 0); |
|
+ pr_cmd_dispatch_phase(pass_cmd, LOG_CMD_ERR, 0); |
|
+ |
|
+ pr_memscrub(passwd, passwd_len); |
|
+ |
|
+ *send_userauth_fail = TRUE; |
|
+ errno = EPERM; |
|
+ return 0; |
|
+ } |
|
+ } |
|
+ |
|
res = pr_auth_authenticate(pkt->pool, user, passwd); |
|
- pr_memscrub(passwd, strlen(passwd)); |
|
+ pr_memscrub(passwd, passwd_len); |
|
|
|
switch (res) { |
|
case PR_AUTH_OK: |
|
diff --git a/modules/mod_auth.c b/modules/mod_auth.c |
|
index 2b76070f7..b60cea5a9 100644 |
|
--- a/modules/mod_auth.c |
|
+++ b/modules/mod_auth.c |
|
@@ -2636,35 +2636,52 @@ MODRET auth_pre_pass(cmd_rec *cmd) { |
|
|
|
allow_empty_passwords = *((int *) c->argv[0]); |
|
if (allow_empty_passwords == FALSE) { |
|
+ const char *proto; |
|
+ int reject_empty_passwd = FALSE, using_ssh2 = FALSE; |
|
size_t passwd_len = 0; |
|
|
|
+ proto = pr_session_get_protocol(0); |
|
+ if (strcmp(proto, "ssh2") == 0) { |
|
+ using_ssh2 = TRUE; |
|
+ } |
|
+ |
|
if (cmd->argc > 1) { |
|
if (cmd->arg != NULL) { |
|
passwd_len = strlen(cmd->arg); |
|
} |
|
} |
|
|
|
- /* Make sure to NOT enforce 'AllowEmptyPasswords off' if e.g. |
|
- * the AllowDotLogin TLSOption is in effect. |
|
- */ |
|
- if (cmd->argc == 1 || |
|
- passwd_len == 0) { |
|
- |
|
- if (session.auth_mech == NULL || |
|
- strcmp(session.auth_mech, "mod_tls.c") != 0) { |
|
- pr_log_debug(DEBUG5, |
|
- "Refusing empty password from user '%s' (AllowEmptyPasswords " |
|
- "false)", user); |
|
- pr_log_auth(PR_LOG_NOTICE, |
|
- "Refusing empty password from user '%s'", user); |
|
- |
|
- pr_event_generate("mod_auth.empty-password", user); |
|
- pr_response_add_err(R_501, _("Login incorrect.")); |
|
- return PR_ERROR(cmd); |
|
+ if (passwd_len == 0) { |
|
+ reject_empty_passwd = TRUE; |
|
+ |
|
+ /* Make sure to NOT enforce 'AllowEmptyPasswords off' if e.g. |
|
+ * the AllowDotLogin TLSOption is in effect, or if the protocol is |
|
+ * SSH2 (for mod_sftp uses "fake" PASS commands for the SSH login |
|
+ * protocol). |
|
+ */ |
|
+ |
|
+ if (session.auth_mech != NULL && |
|
+ strcmp(session.auth_mech, "mod_tls.c") == 0) { |
|
+ pr_log_debug(DEBUG9, "%s", "'AllowEmptyPasswords off' in effect, " |
|
+ "BUT client authenticated via the AllowDotLogin TLSOption"); |
|
+ reject_empty_passwd = FALSE; |
|
} |
|
|
|
- pr_log_debug(DEBUG9, "%s", "'AllowEmptyPasswords off' in effect, " |
|
- "BUT client authenticated via the AllowDotLogin TLSOption"); |
|
+ if (using_ssh2 == TRUE) { |
|
+ reject_empty_passwd = FALSE; |
|
+ } |
|
+ } |
|
+ |
|
+ if (reject_empty_passwd == TRUE) { |
|
+ pr_log_debug(DEBUG5, |
|
+ "Refusing empty password from user '%s' (AllowEmptyPasswords " |
|
+ "false)", user); |
|
+ pr_log_auth(PR_LOG_NOTICE, |
|
+ "Refusing empty password from user '%s'", user); |
|
+ |
|
+ pr_event_generate("mod_auth.empty-password", user); |
|
+ pr_response_add_err(R_501, _("Login incorrect.")); |
|
+ return PR_ERROR(cmd); |
|
} |
|
} |
|
} |
|
diff --git a/tests/t/lib/ProFTPD/Tests/Modules/mod_sftp.pm b/tests/t/lib/ProFTPD/Tests/Modules/mod_sftp.pm |
|
index c919844ea..c608e76fc 100644 |
|
--- a/tests/t/lib/ProFTPD/Tests/Modules/mod_sftp.pm |
|
+++ b/tests/t/lib/ProFTPD/Tests/Modules/mod_sftp.pm |
|
@@ -1279,6 +1279,11 @@ my $TESTS = { |
|
test_class => [qw(bug forking sftp ssh2)], |
|
}, |
|
|
|
+ sftp_config_allow_empty_passwords_off_bug4309 => { |
|
+ order => ++$order, |
|
+ test_class => [qw(bug forking sftp ssh2)], |
|
+ }, |
|
+ |
|
sftp_multi_channels => { |
|
order => ++$order, |
|
test_class => [qw(forking sftp ssh2)], |
|
@@ -41885,6 +41890,133 @@ sub sftp_config_insecure_hostkey_perms_bug4098 { |
|
test_cleanup($setup->{log_file}, $ex); |
|
} |
|
|
|
+sub sftp_config_allow_empty_passwords_off_bug4309 { |
|
+ my $self = shift; |
|
+ my $tmpdir = $self->{tmpdir}; |
|
+ my $setup = test_setup($tmpdir, 'sftp'); |
|
+ |
|
+ my $other_user = 'nopassword'; |
|
+ my $other_passwd = ''; |
|
+ my $other_uid = 1000; |
|
+ my $other_gid = 1000; |
|
+ |
|
+ auth_user_write($setup->{auth_user_file}, $other_user, $other_passwd, |
|
+ $other_uid, $other_gid, $setup->{home_dir}, '/bin/bash'); |
|
+ auth_group_write($setup->{auth_group_file}, $setup->{group}, $setup->{gid}, |
|
+ $other_user); |
|
+ |
|
+ my $rsa_host_key = File::Spec->rel2abs('t/etc/modules/mod_sftp/ssh_host_rsa_key'); |
|
+ my $dsa_host_key = File::Spec->rel2abs('t/etc/modules/mod_sftp/ssh_host_dsa_key'); |
|
+ |
|
+ my $config = { |
|
+ PidFile => $setup->{pid_file}, |
|
+ ScoreboardFile => $setup->{scoreboard_file}, |
|
+ SystemLog => $setup->{log_file}, |
|
+ TraceLog => $setup->{log_file}, |
|
+ Trace => 'DEFAULT:10 ssh2:20 sftp:20', |
|
+ |
|
+ AuthUserFile => $setup->{auth_user_file}, |
|
+ AuthGroupFile => $setup->{auth_group_file}, |
|
+ |
|
+ IfModules => { |
|
+ 'mod_delay.c' => { |
|
+ DelayEngine => 'off', |
|
+ }, |
|
+ |
|
+ 'mod_sftp.c' => [ |
|
+ "SFTPEngine on", |
|
+ "SFTPLog $setup->{log_file}", |
|
+ "SFTPHostKey $rsa_host_key", |
|
+ "SFTPHostKey $dsa_host_key", |
|
+ "AllowEmptyPasswords off", |
|
+ ], |
|
+ }, |
|
+ }; |
|
+ |
|
+ my ($port, $config_user, $config_group) = config_write($setup->{config_file}, |
|
+ $config); |
|
+ |
|
+ # Open pipes, for use between the parent and child processes. Specifically, |
|
+ # the child will indicate when it's done with its test by writing a message |
|
+ # to the parent. |
|
+ my ($rfh, $wfh); |
|
+ unless (pipe($rfh, $wfh)) { |
|
+ die("Can't open pipe: $!"); |
|
+ } |
|
+ |
|
+ require Net::SSH2; |
|
+ |
|
+ my $ex; |
|
+ |
|
+ # Fork child |
|
+ $self->handle_sigchld(); |
|
+ defined(my $pid = fork()) or die("Can't fork: $!"); |
|
+ if ($pid) { |
|
+ eval { |
|
+ my $ssh2 = Net::SSH2->new(); |
|
+ |
|
+ sleep(1); |
|
+ |
|
+ # First, we'll try to login with normal user/password; this should |
|
+ # succeed. |
|
+ unless ($ssh2->connect('127.0.0.1', $port)) { |
|
+ my ($err_code, $err_name, $err_str) = $ssh2->error(); |
|
+ die("Can't connect to SSH2 server: [$err_name] ($err_code) $err_str"); |
|
+ } |
|
+ |
|
+ unless ($ssh2->auth_password($setup->{user}, $setup->{passwd})) { |
|
+ my ($err_code, $err_name, $err_str) = $ssh2->error(); |
|
+ die("Can't login to SSH2 server: [$err_name] ($err_code) $err_str"); |
|
+ } |
|
+ |
|
+ my $sftp = $ssh2->sftp(); |
|
+ unless ($sftp) { |
|
+ my ($err_code, $err_name, $err_str) = $ssh2->error(); |
|
+ die("Can't use SFTP on SSH2 server: [$err_name] ($err_code) $err_str"); |
|
+ } |
|
+ |
|
+ $sftp = undef; |
|
+ $ssh2->disconnect(); |
|
+ $ssh2 = undef; |
|
+ |
|
+ # Then, we'll try to login with an empty password; this should fail. |
|
+ |
|
+ $ssh2 = Net::SSH2->new(); |
|
+ unless ($ssh2->connect('127.0.0.1', $port)) { |
|
+ my ($err_code, $err_name, $err_str) = $ssh2->error(); |
|
+ die("Can't connect to SSH2 server: [$err_name] ($err_code) $err_str"); |
|
+ } |
|
+ |
|
+ if ($ssh2->auth_password($other_user, $other_passwd)) { |
|
+ die("Login with empty password succeeded unexpectedly"); |
|
+ } |
|
+ |
|
+ $ssh2->disconnect(); |
|
+ }; |
|
+ if ($@) { |
|
+ $ex = $@; |
|
+ } |
|
+ |
|
+ $wfh->print("done\n"); |
|
+ $wfh->flush(); |
|
+ |
|
+ } else { |
|
+ eval { server_wait($setup->{config_file}, $rfh) }; |
|
+ if ($@) { |
|
+ warn($@); |
|
+ exit 1; |
|
+ } |
|
+ |
|
+ exit 0; |
|
+ } |
|
+ |
|
+ # Stop server |
|
+ server_stop($setup->{pid_file}); |
|
+ $self->assert_child_ok($pid); |
|
+ |
|
+ test_cleanup($setup->{log_file}, $ex); |
|
+} |
|
+ |
|
sub sftp_multi_channel_downloads { |
|
my $self = shift; |
|
my $tmpdir = $self->{tmpdir};
|
|
|