Skip to content

Commit

Permalink
Set the "mod_sftp.file-status" note in more cases when the OPEN reque…
Browse files Browse the repository at this point in the history 
…st is

rejected/failed for any reason.
  • Loading branch information
@Castaglia
Castaglia committed Feb 27, 2017
1 parent 31b6a22 commit b3e50cb
Show file tree
Hide file tree
Showing 5 changed files with 438 additions and 15 deletions.
46 changes: 33 additions & 13 deletions contrib/mod_sftp/fxp.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* ProFTPD - mod_sftp sftp
* Copyright (c) 2008-2016 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
Expand Down Expand Up @@ -1043,6 +1043,16 @@ static void fxp_cmd_dispatch_err(cmd_rec *cmd) {
pr_response_clear(&resp_err_list);
}

static void fxp_cmd_note_file_status(cmd_rec *cmd, const char *status) {
if (pr_table_add(cmd->notes, "mod_sftp.file-status",
pstrdup(cmd->pool, status), 0) < 0) {
if (errno != EEXIST) {
pr_trace_msg(trace_channel, 3,
"error stashing file status in command notes: %s", strerror(errno));
}
}
}

static const char *fxp_get_request_type_desc(unsigned char request_type) {
switch (request_type) {
case SFTP_SSH2_FXP_INIT:
Expand Down Expand Up @@ -2982,13 +2992,7 @@ static int fxp_handle_abort(const void *key_data, size_t key_datasz,

if (cmd != NULL) {
/* Add a note indicating that this is a failed transfer. */
if (pr_table_add(cmd->notes, "mod_sftp.file-status",
pstrdup(fxh->pool, "failed"), 0) < 0) {
if (errno != EEXIST) {
pr_trace_msg(trace_channel, 3,
"error stashing file status in command notes: %s", strerror(errno));
}
}
fxp_cmd_note_file_status(cmd, "failed");
}

xferlog_write(0, pr_netaddr_get_sess_remote_name(), fxh->fh_bytes_xferred,
Expand Down Expand Up @@ -8687,6 +8691,7 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
fxp_strerror(status_code), NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -8741,6 +8746,7 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
fxp_strerror(status_code), NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -8768,6 +8774,7 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
fxp_strerror(status_code), NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand All @@ -8790,6 +8797,7 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
attrs = fxp_attrs_read(fxp, &fxp->payload, &fxp->payload_sz, &attr_flags,
&xattrs);
if (attrs == NULL) {
fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

/* XXX TODO: Provide a response to the client here */
Expand Down Expand Up @@ -8905,8 +8913,10 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
xerrno);

pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
Expand Down Expand Up @@ -9043,14 +9053,16 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
"('%s' [%d])", (unsigned long) status_code, reason,
xerrno != EOF ? strerror(xerrno) : "End of file", xerrno);

if (cmd2) {
if (cmd2 != NULL) {
pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);
}

fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
reason, NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -9088,14 +9100,16 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
"('%s' [%d])", (unsigned long) status_code, reason, strerror(xerrno),
xerrno);

if (cmd2) {
if (cmd2 != NULL) {
pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);
}

fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
reason, NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -9162,11 +9176,13 @@ static int fxp_handle_open(struct fxp_packet *fxp) {

pr_fsio_close(fh);

if (cmd2) {
if (cmd2 != NULL) {
pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);
}

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -9203,14 +9219,16 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
"('%s' [%d])", (unsigned long) status_code, reason,
xerrno != EOF ? strerror(xerrno) : "End of file", xerrno);

if (cmd2) {
if (cmd2 != NULL) {
pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);
}

fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
reason, NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down Expand Up @@ -9246,14 +9264,16 @@ static int fxp_handle_open(struct fxp_packet *fxp) {
pr_fsio_close(fh);
destroy_pool(fxh->pool);

if (cmd2) {
if (cmd2 != NULL) {
pr_response_add_err(R_451, "%s: %s", cmd2->arg, strerror(xerrno));
fxp_cmd_note_file_status(cmd2, "failed");
fxp_cmd_dispatch_err(cmd2);
}

fxp_status_write(fxp->pool, &buf, &buflen, fxp->request_id, status_code,
reason, NULL);

fxp_cmd_note_file_status(cmd, "failed");
fxp_cmd_dispatch_err(cmd);

resp = fxp_packet_create(fxp->pool, fxp->channel_id);
Expand Down
2 changes: 1 addition & 1 deletion contrib/mod_sql.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2869,7 +2869,7 @@ static const char *resolve_long_tag(cmd_rec *cmd, char *tag) {
long_tag = pstrdup(cmd->tmp_pool, "success");

} else {
long_tag = pstrdup(cmd->tmp_pool, "failed");
long_tag = pstrdup(cmd->tmp_pool, status);
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/mod_log.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1847,7 +1847,7 @@ static char *get_next_meta(pool *p, cmd_rec *cmd, unsigned char **f,
len = sstrncpy(argp, "success", sizeof(arg));

} else {
len = sstrncpy(argp, "failed", sizeof(arg));
len = sstrncpy(argp, status, sizeof(arg));
}
}

Expand Down
190 changes: 190 additions & 0 deletions tests/t/lib/ProFTPD/Tests/Logging/ExtendedLog.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,11 @@ my $TESTS = {
test_class => [qw(bug forking mod_sftp)],
},

extlog_sftp_xfer_status_filtered => {
order => ++$order,
test_class => [qw(forking mod_sftp)],
},

extlog_var_file_offset => {
order => ++$order,
test_class => [qw(forking)],
Expand Down Expand Up @@ -13297,6 +13302,191 @@ sub extlog_sftp_read_write_bug4067 {
unlink($log_file);
}

sub extlog_sftp_xfer_status_filtered {
my $self = shift;
my $tmpdir = $self->{tmpdir};

my $config_file = "$tmpdir/extlog.conf";
my $pid_file = File::Spec->rel2abs("$tmpdir/extlog.pid");
my $scoreboard_file = File::Spec->rel2abs("$tmpdir/extlog.scoreboard");

my $log_file = test_get_logfile();

my $auth_user_file = File::Spec->rel2abs("$tmpdir/extlog.passwd");
my $auth_group_file = File::Spec->rel2abs("$tmpdir/extlog.group");

my $user = 'proftpd';
my $passwd = 'test';
my $group = 'ftpd';
my $home_dir = File::Spec->rel2abs($tmpdir);
my $uid = 500;
my $gid = 500;

# Make sure that, if we're running as root, that the home directory has
# permissions/privs set for the account we create
if ($< == 0) {
unless (chmod(0755, $home_dir)) {
die("Can't set perms on $home_dir to 0755: $!");
}

unless (chown($uid, $gid, $home_dir)) {
die("Can't set owner of $home_dir to $uid/$gid: $!");
}
}

auth_user_write($auth_user_file, $user, $passwd, $uid, $gid, $home_dir,
'/bin/bash');
auth_group_write($auth_group_file, $group, $gid, $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 $ext_log = File::Spec->rel2abs("$tmpdir/custom.log");

my $config = {
PidFile => $pid_file,
ScoreboardFile => $scoreboard_file,
SystemLog => $log_file,
TraceLog => $log_file,
Trace => 'DEFAULT:10 ssh2:20 sftp:20',

AuthUserFile => $auth_user_file,
AuthGroupFile => $auth_group_file,

PathDenyFilter => '^.*\.csv$',
LogFormat => 'custom "%m %{transfer-status}"',
ExtendedLog => "$ext_log READ,WRITE custom",

IfModules => {
'mod_delay.c' => {
DelayEngine => 'off',
},

'mod_sftp.c' => [
"SFTPEngine on",
"SFTPLog $log_file",
"SFTPHostKey $rsa_host_key",
"SFTPHostKey $dsa_host_key",
],
},
};

my ($port, $config_user, $config_group) = config_write($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;

# Ignore SIGPIPE
local $SIG{PIPE} = sub { };

# Fork child
$self->handle_sigchld();
defined(my $pid = fork()) or die("Can't fork: $!");
if ($pid) {
eval {
my $ssh2 = Net::SSH2->new();
sleep(1);

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($user, $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");
}

my $fh = $sftp->open('test.csv', O_WRONLY|O_CREAT|O_TRUNC, 0644);
if ($fh) {
die("Open of test.csv succeeded unexpectedly");
}

my ($err_code, $err_name) = $sftp->error();
my $expected = 'SSH_FX_PERMISSION_DENIED';
$self->assert($expected eq $err_name,
"Expected error $expected, got $err_name");

# To close the SFTP channel, we have to explicitly destroy the object
$sftp = undef;

$ssh2->disconnect();
};

if ($@) {
$ex = $@;
}

$wfh->print("done\n");
$wfh->flush();

} else {
eval { server_wait($config_file, $rfh) };
if ($@) {
warn($@);
exit 1;
}

exit 0;
}

# Stop server
server_stop($pid_file);
$self->assert_child_ok($pid);

eval {
if (open(my $fh, "< $ext_log")) {
my $saw_failed = 0;

while (my $line = <$fh>) {
chomp($line);

if ($ENV{TEST_VERBOSE}) {
print STDERR "$line\n";
}

if ($line eq 'STOR failed') {
$saw_failed = 1;
last;
}
}

close($fh);
$self->assert($saw_failed, "Expected ExtendedLog line not seen");

} else {
die("Can't read $ext_log: $!");
}
};
if ($@) {
$ex = $@;
}

if ($ex) {
test_append_logfile($log_file, $ex);
unlink($log_file);

die($ex);
}

unlink($log_file);
}

sub extlog_var_file_offset {
my $self = shift;
my $tmpdir = $self->{tmpdir};
Expand Down
Loading

0 comments on commit b3e50cb

Please sign in to comment.