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.

327 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};