From 8a55d1109a6cd41ea7cad4fa6f5004df5068f4b8 Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Thu, 27 Oct 2022 21:09:42 -0500 Subject: [PATCH] Revise and rename keyring_file to directory_keyrings This fixes a long-standing bug, due to keyring_file never actually returning an empty list, even if the "root" keyring does not actually exist in the filesystem. The testsuite is adjusted accordingly. This also introduces another minor issue, in that processing for a misconfigured package is now abandoned earlier, before email addresses are gathered. This will be corrected in later improvements. This also eliminates the special handling for a "root" keyring, although the feature remains available as part of the general case: "pubring.gpg" at the root of the package configuration tree applies to all directories in all packages. Be very careful with the keys on such a ring, as they would be of immense value to an attacker. --- gatekeeper.pl | 56 ++++++++++++------------- testsuite/gatekeeper.all/03_triplet.exp | 11 ++--- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index e0c95db..f5007f8 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -301,8 +301,6 @@ my $olddestfinal = "/home/gatekpr/$m_style-archived"; # private dir on SAME FILESYSTEM as $destfinal: my $desttmp = "/var/tmp/$m_style-out"; -my $master_keyring = "/home/gatekpr/etc/master_pubring.gpg"; - # We sometimes want to exclude e-mail addresses from being emailed. # Specifically, e-mail addresses we import from gpg keys - keys are still # valid but associated e-mail addresses are not. Ward, 2011-02-08. @@ -344,7 +342,6 @@ if (IN_TEST_MODE) { # override the above for testing $destfinal = File::Spec->catdir($base, 'pub'); $olddestfinal = File::Spec->catdir($base, 'archive'); - $master_keyring = File::Spec->catfile($base, 'rootring.gpg'); $email_blacklist = File::Spec->catfile($base, 'email.blacklist'); $maintainers_bypkg = File::Spec->catfile($base, 'm.bypkg'); $serials_path = File::Spec->catfile($base, 'serial.txt'); @@ -876,35 +873,34 @@ sub verify_clearsigned_message { # - Package configuration access # -# Return array of public key files for PACKAGE_NAME. -# -sub keyring_file { - my $package_name = shift; +=item @keyrings = directory_keyrings ( $directory ) + +Return list of keyrings present in package configuration and applicable to +DIRECTORY, which is a relative name beginning with the appropriate package. + +=cut + +sub directory_keyrings { my $directory = shift; - my @directory = split(/\//,$directory); - my @pubrings = (); - - # First of all, add our 'master' keyring, for people with root to the ftp - # upload mechanism - push(@pubrings,$master_keyring); - - # We go through each subdirectory, starting at the lowest subdirectory, - # and add each to an array of public key files - my $tmp = $directory; - while (1) { - if (-e "$package_config_base/$tmp/pubring.gpg") { - ftp_syslog('debug', "DEBUG: " - . "found keyring $package_config_base/$tmp/pubring.gpg") - if DEBUG; - push(@pubrings,"$package_config_base/$tmp/pubring.gpg"); + my @keyrings; + my @candidates; + + for (my @directory = File::Spec::Unix->splitdir($directory); + @directory; + pop @directory) + { push @candidates, File::Spec->catfile + ($package_config_base, @directory, 'pubring.gpg') } + push @candidates, File::Spec->catfile($package_config_base, 'pubring.gpg'); + + foreach my $keyring (@candidates) { + if (-f $keyring) { + ftp_syslog('debug', "DEBUG: found keyring $keyring") if DEBUG; + push @keyrings, $keyring; } - my $tmp2 = $tmp; - $tmp2 =~ s/\/[^\/]*$//; - last if ($tmp eq $tmp2); - $tmp = $tmp2; } - return @pubrings; + + return @keyrings; } sub email_addresses { @@ -1884,7 +1880,7 @@ sub read_directive_file { or fatal("no configuration directory for package $op_header->{package}",0); # Check that we have a keyring for this package: - my @keyrings = keyring_file ($op_header->{package},$op_header->{directory}); + my @keyrings = directory_keyrings($op_header->{directory}); fatal("no keyring for package $op_header->{package}",0) if ($#keyrings < 0); # Check that we actually have at least one command in the directive @@ -2068,7 +2064,7 @@ sub check_files { ftp_syslog('debug', "DEBUG: " ."$upload_file size is $upload_file_size") if DEBUG; - my @keyrings = keyring_file ($header->{package},$header->{directory}); + my @keyrings = directory_keyrings($header->{directory}); fatal("no keyring for package $header->{package}",0) if ($#keyrings < 0); my $valid = 0; diff --git a/testsuite/gatekeeper.all/03_triplet.exp b/testsuite/gatekeeper.all/03_triplet.exp index 80f6bd4..5f40654 100644 --- a/testsuite/gatekeeper.all/03_triplet.exp +++ b/testsuite/gatekeeper.all/03_triplet.exp @@ -369,15 +369,16 @@ check_triplet "bogus: signed but package has no keys" setup { found,bar.tar.gz.directive.asc "found directive in triplet" found-packet,bar.tar.gz.directive.asc:bar.tar.gz.sig:bar.tar.gz \ "found triplet" - gpgv,directive-verify-failed "reject upload for misconfigured package" + validate,package-no-keys "reject upload for misconfigured package" } email-to { ftp-upload-script@gnu.org - foo@example.gnu.org - ftp-upload-report@gnu.org bar@example.net bar@example.org + ftp-upload-report@gnu.org } } -# TODO: should be validate,package-no-keys instead of -# gpgv,directive-verify-failed but script currently cannot reach that error +# TODO: probably should send mail to foo@example.gnu.org, since that key +# signed the upload; perhaps also to the bar package maintainers; +# currently, the packet is discarded immediately upon finding that no +# keys are configured for package bar check_triplet "bogus: signed but package has no email addresses" setup { packages { -- 2.25.1