From 67f760ed7a1c0370a3f543482686fc5211f1c15d Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Tue, 25 Oct 2022 20:33:40 -0500 Subject: [PATCH] Complete initial implementation of new clearsigned message verification --- gatekeeper.pl | 106 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 4 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index cad4a52..1616677 100755 --- 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 is zero +and C 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 fields in the returned hashref are tainted; the extracted +values are untainted. The C 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; } -- 2.25.1