From 41ecb7dc3932dd57bac52980982c76bf036ccfd8 Mon Sep 17 00:00:00 2001 From: TJ Saunders 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};