Complete initial implementation of new clearsigned message verification

diff --git a/gatekeeper.pl b/gatekeeper.pl
index cad4a523fcd70563d245fe641d10a5db88644e34..1616677acc8416b7bda7f4894dfdfa7ad2d1c2bb 100755 (executable)
--- a/gatekeeper.pl
+++ b/gatekeeper.pl
@@ -180,7 +180,7 @@ use Date::Manip;
 use Sys::Syslog qw(:DEFAULT setlogsock);
 use Getopt::Long;
 use Text::Wrap;
-use POSIX qw(strftime);
+use POSIX qw(strftime WIFSIGNALED WTERMSIG);
 use Cwd;
 use Email::MessageID;
 
@@ -1309,12 +1309,27 @@ sub guess_uploader_email {
 
 =item $results = verify_clearsigned_message ( $text, @keyrings )
 
-Verify the PGP-clearsigned message in TEXT, using a key from KEYRINGS.
+Verify the PGP-clearsigned message in TEXT, using a key from KEYRINGS.  The
+TEXT may be tainted, but the list of KEYRINGS must be untainted.
+
+The message signature should be considered verified iff C<exitcode> is zero
+and C<TILT> is not defined in the returned hashref.
 
 The return value is a hashref containing:
 
 =over
 
+=item TILT
+
+An arrayref of reasons the results should be considered invalid.  This key
+will not exist if the verification was successful and trustworthy.
+
+The presense of this key in the returned hashref indicates that we saw
+something very wrong from gpgv.  Note that our handling is fairly paranoid,
+for example, multiple signatures on the input will result in this being
+set, as we assume that gpgv has been somehow subverted if more than one
+verification result is returned.
+
 =item exitcode
 
 The exit status from gpgv.  This will be zero if gpgv considers the
@@ -1341,6 +1356,9 @@ The fingerprint of the PGP key that signed TEXT, if available.
 
 =back
 
+The C<raw_*> fields in the returned hashref are tainted; the extracted
+values are untainted.  The C<TILT> field, if present, is untainted.
+
 =cut
 
 sub verify_clearsigned_message {
@@ -1359,7 +1377,7 @@ sub verify_clearsigned_message {
   # The three output streams from gpgv must be kept separate, or
   # CVE-2018-12020 "SigSpoof" issues can occur.  Worse, the gpgv status
   # output must be examined with care, as there has been at least one bug
-  # (CVE-2022-34903) whereby gpgv could be tricked to emit arbitrary output
+  # (CVE-2022-34903) whereby GPG could be tricked to emit arbitrary output
   # on the status pipe.
   pipe my $gpgv_stdin, my $gpgv_stdin_source
     or ftp_abort('failed to create pipe for gpgv stdin');
@@ -1493,7 +1511,87 @@ sub verify_clearsigned_message {
   my %ret = (exitcode => $?, raw_output => $raw_output,
             raw_log => $raw_log, raw_status => $raw_status);
 
-  # TODO:  analyze results...
+  # Analyze the results
+
+  # CVE-2022-34903 caused GPG to dump a chunk of its heap to the status fd,
+  # and, eventually, segfault upon reaching unallocated address space.
+  # This had two recognizable consequences:
+  #  - The GPG process dies with SIGSEGV.
+  #  - The status output very likely contains multiple NUL bytes.
+  push @{$ret{TILT}}, 'gpgv died on signal '.WTERMSIG($ret{exitcode})
+    if WIFSIGNALED($ret{exitcode});
+  for (qw(output log status))
+    { push @{$ret{TILT}}, "gpgv $_ contained NUL byte"
+       if $ret{'raw_'.$_} =~ m/\0/ }
+
+  local *_;
+  # counters
+  my $intro_status = 0; my $check_status = 0; my $verdict_status = 0;
+
+  open my $status, '<', \$ret{raw_status}
+    or ftp_abort('open in-memory file for gpgv status');
+  while (<$status>) {
+    chomp;
+    unless (m/^\[GNUPG:\] /g) {
+      push @{$ret{TILT}}, "gpgv status line lacks required prefix";
+      last;    # stop parsing if an invalid line is found
+    }
+
+    if (m/\GNEWSIG/gc) {
+      $intro_status++;         # Note that NEWSIG is optional
+    } elsif (m/\G(GOOD|EXP|EXPKEY|REVKEY|BAD|ERR)SIG ([[:xdigit:]]+) /gc) {
+      #  $1 -- result tag               $2 -- long ID or fingerprint
+      # The next field is the primary username, except ERRSIG, but there is
+      # no guarantee that the primary UID will contain an email address.
+      if (length($2) > 16) {   # We have a key fingerprint
+       $ret{key_fingerprint} = $2;
+       $ret{key_longid} = substr $2,-16;
+      } else {                 # We only have a long key ID
+       $ret{key_longid} = $2;
+      }
+
+      if ($1 eq 'BAD') {
+       $verdict_status++;
+       push @{$ret{TILT}}, 'gpgv reported a bad signature, but exited zero'
+         if 0 == $ret{exitcode};
+      } elsif ($1 eq 'ERR') {          # an ERRSIG line
+       $verdict_status++;
+       if (m/\G(\d+)\s(\d+)\s([[:xdigit:]]{2})\s([-:T[:digit:]]+)\s(\d+)/gc) {
+       #  $1 -- pubkey algorithm        $2 -- digest algorithm
+       #  $3 -- timestamp               $4 -- result code
+       } else
+         { push @{$ret{TILT}}, 'gpgv ERRSIG line failed parsing' }
+
+       push @{$ret{TILT}}, 'gpgv reported an error, but exited zero'
+         if 0 == $ret{exitcode};
+      } else {                         # GOODSIG/EXPSIG/EXPKEYSIG/REVKEYSIG
+       $check_status++;
+      }
+    } elsif (m/\G(VALID)SIG\s([[:xdigit:]]+)\s(\d{4}-\d{2}-\d{2})\s
+              ([-:T[:digit:]]+)\s([-:T[:digit:]]+)\s(\d+)\s(\S+)\s
+              (\d+)\s(\d+)\s([[:xdigit:]]{2})\s([[:xdigit:]]+)
+             /gcx) {
+      $verdict_status++;
+      #  $1 -- valid tag                $2 -- key fingerprint
+      #  $3 -- signature date           $4 -- signature timestamp
+      #  $5 -- expiration timestamp     $6 -- signature version
+      #  $7 -- reserved                         $8 -- pubkey algorithm
+      #  $9 -- digest algorithm                $10 -- signature class
+      # $11 -- primary key fingerprint
+      $ret{key_fingerprint} = $2;
+      $ret{key_longid} = substr $2,-16;
+    }
+  }
+  close $status or ftp_abort('close in-memory file for gpgv status');
+
+  push @{$ret{TILT}}, 'gpgv reported more than one signature'
+    if $intro_status > 1;
+  push @{$ret{TILT}}, 'gpgv reported more than one signature check'
+    if $check_status > 1;
+  push @{$ret{TILT}}, 'gpgv reported more than one signature verdict'
+    if $verdict_status > 1;
+  push @{$ret{TILT}}, 'gpgv reported no signature verdict at all'
+    if $verdict_status < 1;
 
   return \%ret;
 }