Revise and rename keyring_file to directory_keyrings
authorJacob Bachmeyer <jcb@gnu.org>
Fri, 28 Oct 2022 02:09:42 +0000 (21:09 -0500)
committerJacob Bachmeyer <jcb@gnu.org>
Fri, 28 Oct 2022 02:09:42 +0000 (21:09 -0500)
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
testsuite/gatekeeper.all/03_triplet.exp

index e0c95dba9ecb6237b28f564f44ea602152fe461b..f5007f8bfcd779829bfe2f095fb417776fe763ca 100755 (executable)
@@ -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;
index 80f6bd41b510afa096dbf4f6abb013322bf88542..5f40654ff20db53cad8399b1f15325f6256466b9 100644 (file)
@@ -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 {