From 2c7b0514a4a89f9992c6d339700138943316a9fa Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Sat, 12 Nov 2022 23:28:08 -0600 Subject: [PATCH] Factor signature timestamp validation out of check_replay The main code now also checks the signature timestamp on an uploaded file; previously this was not checked at all. --- gatekeeper.pl | 45 ++++++++++++++++++++++++------------ testsuite/lib/gatekeeper.exp | 2 +- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index 0a66d5d..09cadee 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -2247,6 +2247,28 @@ sub validate_commands { } } +=item check_signature_timestamp ( $what , $timestamp ) + +Report the WHAT signature TIMESTAMP to the log and raise an exception if +TIMESTAMP is in the future. A "fuzz factor" avoids timezone issues. + +=cut + +sub check_signature_timestamp { + my $what = ucfirst shift; + my $timestamp = shift; + + ftp_syslog('debug', "DEBUG: $what signature made " + .strftime('%a %b %d %H:%M:%S %Y %Z', + localtime $timestamp)) if DEBUG; + + # Verify that this timestamp is not too far in the future. We allow a + # discrepancy of 1 day so we don't have to worry about timezones + if ($timestamp > (time() + 24*3600)) { + throw signature_from_future => timestamp => $timestamp + } +} + =item check_replay ( $oplist, $timestamp ) Check that OPLIST has not been seen before. This is accomplished by @@ -2268,18 +2290,6 @@ sub check_replay { # If a file is to be installed, ensure that this directive is newer than # any previous directive installing a file under the same full name. if (grep $_->[0] eq 'install', @$ops) { - - ftp_syslog('debug', "DEBUG: Signature made " - .strftime('%a %b %d %H:%M:%S %Y %Z', - localtime $timestamp)) if DEBUG; - - # Verify that this timestamp is not too far in the future. We allow a - # discrepancy of 1 day so we don't have to worry about timezones - if ($timestamp > (time() + 24*3600)) { - throw signature_from_future => timestamp => $timestamp - } - - foreach my $installed (map $_->[1], grep $_->[0] eq 'install', @$ops) { my $full_filename = File::Spec::Unix->catfile($op_header->{directory}, $installed); @@ -2741,8 +2751,7 @@ foreach my $packet (@packets) { # each list element is an array reference throw signature_error => sig_info => $dsig_info, summary => "gpg verification problem: could not extract timestamp" unless defined $dsig_info->{sig_creation}; - - check_replay($oplist, $dsig_info->{sig_creation}); + check_signature_timestamp(directive => $dsig_info->{sig_creation}); if (find_directive_elements($directive, 'filename')) { # There is a file associated with this upload; verify its signature now. @@ -2752,11 +2761,17 @@ foreach my $packet (@packets) { # each list element is an array reference File::Spec->catfile($Scratch_dir, $op_header->{filename}.'.sig'), @keyrings); - throw signature_error => sig_info => undef, + throw signature_error => sig_info => $fsig_info, summary => "gpg verify of upload file ($op_header->{filename}) failed" if $fsig_info->{exitcode} != 0 || defined $fsig_info->{TILT}; + throw signature_error => sig_info => $fsig_info, + summary => "gpg verification problem: could not extract timestamp" + unless defined $fsig_info->{sig_creation}; + check_signature_timestamp(file => $fsig_info->{sig_creation}); } + check_replay($oplist, $dsig_info->{sig_creation}); + $Phase = 'VL'; # If the upload carries a file, check it for known Automake CVE issues. diff --git a/testsuite/lib/gatekeeper.exp b/testsuite/lib/gatekeeper.exp index 2432841..de93459 100644 --- a/testsuite/lib/gatekeeper.exp +++ b/testsuite/lib/gatekeeper.exp @@ -753,7 +753,7 @@ proc analyze_log { base_dir name assess } { exp_continue } -re {^gatekeeper\[[0-9]+\]: \(Test\) \[AA\]\ - DEBUG: Signature made [^\r\n]+} { + DEBUG: (?:Directive|File) signature made [^\r\n]+} { # from read_directive_file, reporting signature timestamp set A(validate,signature-timestamp) 1 exp_continue -- 2.25.1