From: Jacob Bachmeyer <jcb@gnu.org>
Date: Sat, 12 Nov 2022 05:06:27 +0000 (-0600)
Subject: Factor out similar code for spawning gpgv subprocess
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=45508c30ed213268f80ddff4cc1754810d5ac560;p=gatekeeper.git

Factor out similar code for spawning gpgv subprocess
---

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<TILT> 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 = '';