From 45508c30ed213268f80ddff4cc1754810d5ac560 Mon Sep 17 00:00:00 2001 From: Jacob Bachmeyer Date: Fri, 11 Nov 2022 23:06:27 -0600 Subject: [PATCH] Factor out similar code for spawning gpgv subprocess --- gatekeeper.pl | 299 ++++++++++++++++++++------------------------------ 1 file changed, 119 insertions(+), 180 deletions(-) diff --git a/gatekeeper.pl b/gatekeeper.pl index fef8189..f61f011 100755 --- a/gatekeeper.pl +++ b/gatekeeper.pl @@ -805,7 +805,115 @@ values are untainted. The C field, if present, is untainted. =cut -# helper for verify_clearsigned_message and verify_detached_signature +# helpers for verify_clearsigned_message and verify_detached_signature +sub _spawn_gpgv { + my $keyrings = shift; + my @file_args = @_; + + # We need a few pipes: + # - clearsigned message to gpgv stdin + # - output from gpgv stdout/stderr + # - log from gpgv --logger-fd + # - status from gpgv --status-fd + # - a flag pipe to indicate successful exec or carry an error + + # 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 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'); + pipe my $gpgv_output, my $gpgv_output_sink + or ftp_abort('failed to create pipe for gpgv output'); + pipe my $gpgv_log, my $gpgv_log_sink + or ftp_abort('failed to create pipe for gpgv log'); + pipe my $gpgv_status, my $gpgv_status_sink + or ftp_abort('failed to create pipe for gpgv status'); + pipe my $gpgv_flag, my $gpgv_flag_sink + or ftp_abort('failed to create pipe for gpgv flag'); + + # ensure autoflush on writes to gpgv + { my $outhandle = select $gpgv_stdin_source; $| = 1; select $outhandle } + + my @gpgv_args = ( GPGV_BIN, + '--logger-fd', fileno $gpgv_log_sink, + '--status-fd', fileno $gpgv_status_sink ); + push @gpgv_args, '--keyring', $_ for @$keyrings; + push @gpgv_args, @file_args; + + ftp_syslog('debug', 'DEBUG: gpgv command line: '.join(' ', @gpgv_args)) + if DEBUG; + + my $pid = fork; + ftp_abort('failed to fork child for gpgv') + unless defined $pid; + + unless ($pid) { + # We are in the child process... + close $gpgv_stdin_source; + close $gpgv_output; close $gpgv_log; + close $gpgv_status; close $gpgv_flag; + + our $AbortPipe = $gpgv_flag_sink; # pipe to parent + our $AbortExitCode = 120; # arbitrary 7-bit exit code + # no need to use local here; this process will either exec or abort + + # Adjust close-on-exec flags: + my $flags; + # - clear on status and log sinks + $flags = fcntl $gpgv_status_sink, F_GETFD, 0 + or ftp_abort("ERR: fcntl F_GETFD on status: $!"); + fcntl $gpgv_status_sink, F_SETFD, $flags & ~FD_CLOEXEC + or ftp_abort("ERR: fcntl F_SETFD on status: $!"); + $flags = fcntl $gpgv_log_sink, F_GETFD, 0 + or ftp_abort("ERR: fcntl F_GETFD on log: $!"); + fcntl $gpgv_log_sink, F_SETFD, $flags & ~FD_CLOEXEC + or ftp_abort("ERR: fcntl F_SETFD on log: $!"); + # - set on flag pipe sink + $flags = fcntl $gpgv_flag_sink, F_GETFD, 0 + or ftp_abort("ERR: fcntl F_GETFD on flag: $!"); + fcntl $gpgv_flag_sink, F_SETFD, $flags | FD_CLOEXEC + or ftp_abort("ERR: fcntl F_SETFD on flag: $!"); + + # Prepare STDIN/STDOUT/STDERR + open STDIN, '<&', $gpgv_stdin or ftp_abort("ERR: set stdin: $!"); + open STDOUT, '>&', $gpgv_output_sink or ftp_abort("ERR: set stdout: $!"); + open STDERR, '>&', $gpgv_output_sink or ftp_abort("ERR: set stderr: $!"); + + # Exec gpgv + exec { GPGV_BIN } @gpgv_args or ftp_abort("ERR: $!"); + } + + # The parent continues here... + close $gpgv_stdin; + close $gpgv_output_sink; close $gpgv_log_sink; + close $gpgv_status_sink; close $gpgv_flag_sink; + + # This is a bit tricky: we need to know if gpgv could not be run, so we + # have an extra pipe that will either report an error or be closed if the + # exec succeeds in the child process. + while (defined(my $err = <$gpgv_flag>)) { + chomp $err; + if ($err =~ m/^ERR: (.*)$/) { + # This is bad - we couldn't even execute the gpgv command properly + ftp_abort + ("gpg verify of directive file failed (error executing gpgv): $1"); + } + } + close $gpgv_flag; # child has closed its end one way or another + + foreach my $cell ([$gpgv_stdin_source, 'message'], [$gpgv_output, 'output'], + [$gpgv_log, 'log'], [$gpgv_status, 'status']) { + my $flags = fcntl $cell->[0], F_GETFL, 0 + or ftp_abort("gpgv: fcntl F_GETFL $cell->[1]: $!"); + fcntl $cell->[0], F_SETFL, $flags | O_NONBLOCK + or ftp_abort("gpgv: fcntl F_SETFL $cell->[1]: $!"); + } + + return $pid, $gpgv_stdin_source, $gpgv_output, $gpgv_log, $gpgv_status; +} + sub _analyze_gpgv_output { my $ret = shift; # hashref @@ -907,106 +1015,11 @@ sub verify_clearsigned_message { ftp_syslog('debug', 'DEBUG: message size is '.length($text)) if DEBUG; - # We need a few pipes: - # - clearsigned message to gpgv stdin - # - output from gpgv stdout/stderr - # - log from gpgv --logger-fd - # - status from gpgv --status-fd - # - a flag pipe to indicate successful exec or carry an error - - # 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 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'); - pipe my $gpgv_output, my $gpgv_output_sink - or ftp_abort('failed to create pipe for gpgv output'); - pipe my $gpgv_log, my $gpgv_log_sink - or ftp_abort('failed to create pipe for gpgv log'); - pipe my $gpgv_status, my $gpgv_status_sink - or ftp_abort('failed to create pipe for gpgv status'); - pipe my $gpgv_flag, my $gpgv_flag_sink - or ftp_abort('failed to create pipe for gpgv flag'); - - # ensure autoflush on writes to gpgv - { my $outhandle = select $gpgv_stdin_source; $| = 1; select $outhandle } - - my @gpgv_args = ( GPGV_BIN, - '--logger-fd', fileno $gpgv_log_sink, - '--status-fd', fileno $gpgv_status_sink ); - push @gpgv_args, '--keyring', $_ for @keyrings; - push @gpgv_args, '-'; - - ftp_syslog('debug', 'DEBUG: gpgv command line: '.join(' ', @gpgv_args)) - if DEBUG; - - my $pid = fork; - ftp_abort('failed to fork child for gpgv') - unless defined $pid; - - unless ($pid) { - # We are in the child process... - close $gpgv_stdin_source; - close $gpgv_output; close $gpgv_log; - close $gpgv_status; close $gpgv_flag; - - our $AbortPipe = $gpgv_flag_sink; # pipe to parent - our $AbortExitCode = 120; # arbitrary 7-bit exit code - # no need to use local here; this process will either exec or abort - - # Adjust close-on-exec flags: - my $flags; - # - clear on status and log sinks - $flags = fcntl $gpgv_status_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on status: $!"); - fcntl $gpgv_status_sink, F_SETFD, $flags & ~FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on status: $!"); - $flags = fcntl $gpgv_log_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on log: $!"); - fcntl $gpgv_log_sink, F_SETFD, $flags & ~FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on log: $!"); - # - set on flag pipe sink - $flags = fcntl $gpgv_flag_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on flag: $!"); - fcntl $gpgv_flag_sink, F_SETFD, $flags | FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on flag: $!"); - - # Prepare STDIN/STDOUT/STDERR - open STDIN, '<&', $gpgv_stdin or ftp_abort("ERR: set stdin: $!"); - open STDOUT, '>&', $gpgv_output_sink or ftp_abort("ERR: set stdout: $!"); - open STDERR, '>&', $gpgv_output_sink or ftp_abort("ERR: set stderr: $!"); - - # Exec gpgv - exec { GPGV_BIN } @gpgv_args or ftp_abort("ERR: $!"); - } - - # The parent continues here... - close $gpgv_stdin; - close $gpgv_output_sink; close $gpgv_log_sink; - close $gpgv_status_sink; close $gpgv_flag_sink; - - # This is a bit tricky: we need to know if gpgv could not be run, so we - # have an extra pipe that will either report an error or be closed if the - # exec succeeds in the child process. - while (defined(my $err = <$gpgv_flag>)) { - chomp $err; - if ($err =~ m/^ERR: (.*)$/) { - # This is bad - we couldn't even execute the gpgv command properly - ftp_abort - ("gpg verify of directive file failed (error executing gpgv): $1"); - } - } - close $gpgv_flag; # child has closed its end one way or another - - foreach my $cell ([$gpgv_stdin_source, 'message'], [$gpgv_output, 'output'], - [$gpgv_log, 'log'], [$gpgv_status, 'status']) { - my $flags = fcntl $cell->[0], F_GETFL, 0 - or ftp_abort("gpgv: fcntl F_GETFL $cell->[1]: $!"); - fcntl $cell->[0], F_SETFL, $flags | O_NONBLOCK - or ftp_abort("gpgv: fcntl F_SETFL $cell->[1]: $!"); - } + # "my (LIST) = ..." causes problems with CPerl mode here -- jcb + my $pid; my $gpgv_stdin_source; + my $gpgv_output; my $gpgv_log; my $gpgv_status; + ($pid, $gpgv_stdin_source, $gpgv_output, $gpgv_log, $gpgv_status) = + _spawn_gpgv(\@keyrings, '-'); local $SIG{PIPE} = sub { ftp_abort('gpgv exited unexpectedly') }; my $Rchk = ''; my $Wchk = ''; @@ -1075,85 +1088,11 @@ sub verify_detached_signature { if DEBUG; } - pipe my $gpgv_output, my $gpgv_output_sink - or ftp_abort('failed to create pipe for gpgv output'); - pipe my $gpgv_log, my $gpgv_log_sink - or ftp_abort('failed to create pipe for gpgv log'); - pipe my $gpgv_status, my $gpgv_status_sink - or ftp_abort('failed to create pipe for gpgv status'); - pipe my $gpgv_flag, my $gpgv_flag_sink - or ftp_abort('failed to create pipe for gpgv flag'); - - my @gpgv_args = ( GPGV_BIN, - '--logger-fd', fileno $gpgv_log_sink, - '--status-fd', fileno $gpgv_status_sink ); - push @gpgv_args, '--keyring', $_ for @keyrings; - push @gpgv_args, $sigfilename, $filename; - - ftp_syslog('debug', 'DEBUG: gpgv command line: '.join(' ', @gpgv_args)) - if DEBUG; - - my $pid = fork; - ftp_abort('failed to fork child for gpgv') - unless defined $pid; - - unless ($pid) { - # We are in the child process... - close $gpgv_output; close $gpgv_log; - close $gpgv_status; close $gpgv_flag; - - our $AbortPipe = $gpgv_flag_sink; # pipe to parent - our $AbortExitCode = 120; # arbitrary 7-bit exit code - # no need to use local here; this process will either exec or abort - - # Adjust close-on-exec flags: - my $flags; - # - clear on status and log sinks - $flags = fcntl $gpgv_status_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on status: $!"); - fcntl $gpgv_status_sink, F_SETFD, $flags & ~FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on status: $!"); - $flags = fcntl $gpgv_log_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on log: $!"); - fcntl $gpgv_log_sink, F_SETFD, $flags & ~FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on log: $!"); - # - set on flag pipe sink - $flags = fcntl $gpgv_flag_sink, F_GETFD, 0 - or ftp_abort("ERR: fcntl F_GETFD on flag: $!"); - fcntl $gpgv_flag_sink, F_SETFD, $flags | FD_CLOEXEC - or ftp_abort("ERR: fcntl F_SETFD on flag: $!"); - - # Prepare STDOUT/STDERR - open STDOUT, '>&', $gpgv_output_sink or ftp_abort("ERR: set stdout: $!"); - open STDERR, '>&', $gpgv_output_sink or ftp_abort("ERR: set stderr: $!"); - - # Exec gpgv - exec { GPGV_BIN } @gpgv_args or ftp_abort("ERR: $!"); - } - - # The parent continues here... - close $gpgv_output_sink; close $gpgv_log_sink; - close $gpgv_status_sink; close $gpgv_flag_sink; - - # This is a bit tricky: we need to know if gpgv could not be run, so we - # have an extra pipe that will either report an error or be closed if the - # exec succeeds in the child process. - while (defined(my $err = <$gpgv_flag>)) { - chomp $err; - if ($err =~ m/^ERR: (.*)$/) { - # This is bad - we couldn't even execute the gpgv command properly - ftp_abort - ("gpg verify of directive file failed (error executing gpgv): $1"); - } - } - close $gpgv_flag; # child has closed its end one way or another - - foreach my $cell ([$gpgv_output, 'output'], [$gpgv_log, 'log'], - [$gpgv_status, 'status']) { - my $flags = fcntl $cell->[0], F_GETFL, 0 - or ftp_abort("gpgv: fcntl F_GETFL $cell->[1]: $!"); - fcntl $cell->[0], F_SETFL, $flags | O_NONBLOCK - or ftp_abort("gpgv: fcntl F_SETFL $cell->[1]: $!"); + my $pid; my $gpgv_output; my $gpgv_log; my $gpgv_status; + { my $extra; # pipe to gpgv stdin; not used here + ($pid, $extra, $gpgv_output, $gpgv_log, $gpgv_status) = + _spawn_gpgv(\@keyrings, $sigfilename, $filename); + close $extra; } my $Rchk = ''; -- 2.25.1