From 8ed32fd895ad0735c404731b8b9aa2227595daae Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Fri, 4 Nov 2022 23:20:08 -0500 Subject: [PATCH] Revise archive procedure in gatekeeper - a structured exception for general processing errors is added - the archive sub is renamed to archive_filepair - a file and its detached signature are now archived together - the archived filename now contains an extra number beyond the timestamp only if actually needed for uniqueness - the extra number, if used, matches between a file and its signature - the archived filename is claimed by creating an "archive stamp" file - the option to archive and overwrite a file using "replace" now handles the file and its signature as a pair - the system mkdir(1) and mv(1) commands are no longer invoked here - the testsuite is adjusted accordingly --- gatekeeper.pl | 101 ++++++++++++++---------- testsuite/gatekeeper.all/03_triplet.exp | 5 +- testsuite/lib/gatekeeper.exp | 16 +--- 3 files changed, 66 insertions(+), 56 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index b9feffe..b1981c8 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -169,7 +169,8 @@ use constant (); # load this now for use in later BEGIN blocks use Carp qw(cluck); # for stack trace on error in ftp_syslog use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC - F_GETFL F_SETFL O_NONBLOCK); + F_GETFL F_SETFL O_NONBLOCK + O_WRONLY O_CREAT O_EXCL); use FindBin; use File::Spec; @@ -609,6 +610,13 @@ END } } +{ + package Local::Exception::processing_error; + {our @ISA = qw(Local::Exception)} + + sub command { (shift)->{command} } +} + { package Local::Exception::signature_error; {our @ISA = qw(Local::Exception)} @@ -2335,32 +2343,55 @@ sub mkdir_p { mkdir $_ or die "mkdir($_): $!" for @dir_steps; } -sub archive { - my $dir = shift; - my $subdir = shift; - my $file = shift; +=item archive_filepair ( $directory, $filename ) + +Move FILENAME (and its detached signature) from DIRECTORY in the managed +public file tree to the archive file tree. The DIRECTORY parameter is an +array reference containing a split directory name. + +=cut + +sub archive_filepair { + my $directory_list = shift; + my $filename = shift; our $Public_dir; our $Archive_dir; - # Abort if file to archive doesn't exist - fatal("$subdir/$file does not exist - can not archive",1) - if (!-e "$Public_dir/$subdir/$file"); - my $timestamp = strftime "%Y-%m-%d_%H-%M-%S", localtime; - # Add a large random number for good measure - $timestamp .= sprintf("_%09d",rand(1000000000)); - # Abort if a file with same name exists in the archive - fatal("$subdir/$file exists in archive - can not overwrite",1) - if (-e "$Archive_dir/$subdir/$timestamp" . "_$file"); - - my @mkdir_args = ("/bin/mkdir","-p","$Archive_dir/$subdir"); - fatal("@mkdir_args failed",0) if system (@mkdir_args) != 0; - my @mv_args = ("/bin/mv", "$dir/$file", - "$Archive_dir/$subdir/$timestamp"."_$file"); - fatal("@mv_args failed",0) if system (@mv_args) != 0; - ftp_syslog('info', - "archived $dir/$file to $Archive_dir/$subdir/$timestamp" - ."_$file"); + my $pubfilename = File::Spec::Unix->catfile(@$directory_list, $filename); + my $abspubfilename = + File::Spec->catfile($Public_dir, @$directory_list, $filename); + my $abspubsigname = + File::Spec->catfile($Public_dir, @$directory_list, $filename.'.sig'); + + throw processing_error => command => [archive => $filename], + summary => $pubfilename.' does not exist - can not archive' + unless -e $abspubfilename; + + mkdir_p $Archive_dir, @$directory_list; + my $timestamp = strftime '%Y-%m-%d_%H-%M-%S', localtime; + my $unique = -1; my $absarcfilename; my $absarcsigname; my $arcfilename; + + do { + $arcfilename = $timestamp.(++$unique ? "_$unique" : '').'_'.$filename; + $absarcfilename = + File::Spec->catfile($Archive_dir, @$directory_list, $arcfilename); + $absarcsigname = + File::Spec->catfile($Archive_dir, @$directory_list, $arcfilename.'.sig'); + } while (!sysopen(ARCSTAMP, + File::Spec->catfile($Archive_dir, @$directory_list, + $arcfilename.'.arcstamp'), + O_WRONLY|O_CREAT|O_EXCL)); + + rename $abspubsigname, $absarcsigname + or die "rename($abspubsigname, $absarcsigname): $!" + if -e $abspubsigname && ! -d $abspubfilename; + rename $abspubfilename, $absarcfilename + or die "rename($abspubfilename, $absarcfilename): $!"; + + ftp_syslog('info', "archived $pubfilename to $absarcfilename"); + + close ARCSTAMP; } # Install both SIG_FILE and UPLOAD_FILE in $Public_dir/$info{directory}. @@ -2399,21 +2430,10 @@ sub install_files { my $notification_str = ''; # We now allow overwriting of files - without warning!! - if (-e $final_signature) { - if ($header->{options}{replace}) { - archive($destdir, $header->{directory}, $sig_file); - ftp_syslog('info', "archived and overwrote " - ."$final_signature with uploaded version"); - $notification_str .= - "Archived and overwrote $final_signature with uploaded version\n"; - } else { - fatal("This signature file exists: $final_signature, if you want to " - ."replace the pair please use the 'replace' directive",1); - } - } - if (-e $final_upload) { + if (-e $final_signature || -e $final_upload) { if ($header->{options}{replace}) { - archive($destdir, $header->{directory}, $upload_file); + archive_filepair([File::Spec::Unix->splitdir($header->{directory})], + $upload_file); ftp_syslog('info', "overwrote " ."$final_upload with uploaded version"); $notification_str .= @@ -2494,9 +2514,7 @@ sub execute_commands { ftp_syslog('info', "removed symlink $step->[1] in $header->{directory}"); } elsif ($step->[0] eq 'archive') { # We now also allow archiving entire directories - archive($destdir, $header->{directory}, $step->[1].'.sig') - unless -d "$destdir/$step->[1]"; - archive($destdir, $header->{directory}, $step->[1]); + archive_filepair(\@directory, $step->[1]); } elsif (IN_TEST_MODE && $step->[0] eq 'no-op') { # do nothing } else { @@ -2694,7 +2712,8 @@ foreach my $packet (@packets) { # each list element is an array reference mail(join("\n",$E->summary,'',$E->trace_msg),1); } elsif ($E->type_p('package_configuration')) { mail($E->summary,0); - } elsif ($E->type_p('signature_error')) { + } elsif ($E->type_p('signature_error') + || $E->type_p('processing_error')) { mail($E->summary,1); } elsif (UNIVERSAL::can($E, 'message')) { # catch-all for exceptions carrying long-form messages diff --git a/testsuite/gatekeeper.all/03_triplet.exp b/testsuite/gatekeeper.all/03_triplet.exp index fa07ea5..953141b 100644 --- a/testsuite/gatekeeper.all/03_triplet.exp +++ b/testsuite/gatekeeper.all/03_triplet.exp @@ -1355,8 +1355,8 @@ foreach FVER $DIRECTIVE_FORMAT_VERSIONS { found,foo.bin.directive.asc "found directive in triplet" found-packet,foo.bin.directive.asc:foo.bin.sig:foo.bin \ "found triplet" - install,target-signature-exists \ - "existing signature not replaced" + install,target-file-exists \ + "existing file not replaced" } email-to { ftp-upload-script@gnu.org foo@example.gnu.org ftp-upload-report@gnu.org foo@example.org foo@example.net @@ -1482,7 +1482,6 @@ check_triplet "v1.2 format directive to replace file" setup { found,foo.bin.directive.asc "found directive in triplet" found-packet,foo.bin.directive.asc:foo.bin.sig:foo.bin \ "found triplet" - install,target-signature-replaced "signature replaced using v1.2" install,target-file-replaced "file replaced using v1.2" } email-to { ftp-upload-script@gnu.org foo@example.gnu.org diff --git a/testsuite/lib/gatekeeper.exp b/testsuite/lib/gatekeeper.exp index 8a9b1f6..94ed7f3 100644 --- a/testsuite/lib/gatekeeper.exp +++ b/testsuite/lib/gatekeeper.exp @@ -367,6 +367,10 @@ proc analyze_file_tree { base_dir name zones mode {itemlist {}} } { -re {^[\r\n]+} { exp_continue } -re "^$zone_base" { exp_continue } -re {^/([^\r\n]+)[\r\n]+} { + if { [regexp -- {[.]arcstamp$} $expect_out(1,string)] } { + # ignore archive stamp files + exp_continue + } regsub -- {/[-0-9_]+} $expect_out(1,string) / file if { [info exists filemap_want($file)] } { set filemap_have($file) 1 @@ -829,24 +833,12 @@ proc analyze_log { base_dir name assess } { } # TODO: move CVE checks to VL phase - -re {^gatekeeper\[[0-9]+\]: \(Test\) \[EX\]\ - This signature file exists: [^\r\n]+} { - # from install_files, if target exists and replace not set - set A(install,target-signature-exists) 1 - exp_continue - } -re {^gatekeeper\[[0-9]+\]: \(Test\) \[EX\]\ This file exists: [^\r\n]+} { # from install_files, if target exists and replace not set set A(install,target-file-exists) 1 exp_continue } - -re {^gatekeeper\[[0-9]+\]: \(Test\) \[EX\]\ - archived and overwrote [^\r\n]+} { - # from install_files, if target signature replaced - set A(install,target-signature-replaced) 1 - exp_continue - } -re {^gatekeeper\[[0-9]+\]: \(Test\) \[EX\]\ overwrote [^\r\n]+} { # from install_files, if target replaced -- 2.25.1