This patch changes smime_keys.pl to use an open/pipe instead of
backquotes for openssl execution.  This avoids the shell and thus
spaces, quote, and those kinds of issues.

This also fixes the problem in #2456 with crl validation: grep has been
removed from the command and is done in Perl.

Feedback welcome.

My next step is fixing the add_cert problem with certificate chains.
I'd appreciate if anyone is familiar with the "correct" way to deal
with that problem: whether it's just re-ordering the certs so the leaf
comes first, or splitting out the leaf and issuer certs.

I have a commented out section in the program that implements the
method from http://kb.wisc.edu/middleware/page.php?id=4091 but I'm not
convinced that's the correct approach.

Thanks,

-Kevin
# HG changeset patch
# User Kevin McCarthy <[email protected]>
# Date 1431814831 25200
#      Sat May 16 15:20:31 2015 -0700
# Node ID 47ff6be85b7d4e3b168d12676ec0b604fac34f3a
# Parent  c66a6bd5d0d57eca9fba0a90b34c9adab0c0ce27
Convert openssl execution to use open("-|",...). (see #3575) (see #2456)

This does a fork/exec, bypassing the shell, and so handles
spaces, quotes, and other shell-characters problems better than the
simple fix in changeset:c66a6bd5d0d5

This also fixes the "verify with crl" bug in #2456: the grep is now done
in perl.

diff --git a/smime_keys.pl b/smime_keys.pl
--- a/smime_keys.pl
+++ b/smime_keys.pl
@@ -33,22 +33,28 @@
 sub mycopy ($$);
 sub query_label ();
 sub mkdir_recursive ($);
 sub verify_files_exist (@);
 sub create_tempfile (;$);
 sub new_cert_structure ();
 
 # openssl helpers
+sub openssl_exec (@);
 sub openssl_format ($);
+sub openssl_x509_query ($@);
 sub openssl_hash ($);
 sub openssl_fingerprint ($);
 sub openssl_emails ($);
-sub openssl_do_verify($$$);
+sub openssl_p12_to_pem ($$);
+sub openssl_verify ($$);
+sub openssl_crl_text($);
+sub openssl_cert_flag ($$$);
 sub openssl_parse_pem ($$);
+sub openssl_dump_cert ($);
 
 # key/certificate management methods
 sub cm_list_certs ();
 sub cm_add_entry ($$$$$);
 sub cm_add_certificate ($$$$;$);
 sub cm_add_key ($$$$);
 sub cm_modify_entry ($$$;$);
 
@@ -288,90 +294,131 @@
   return $cert_data;
 }
 
 
 ##################
 # openssl helpers
 ##################
 
+sub openssl_exec (@) {
+  my (@args) = @_;
+
+  my $fh;
+
+  open($fh, "-|", $opensslbin, @args)
+    or die "Failed to run '$opensslbin @args': $!";
+  my @output = <$fh>;
+  close($fh);
+
+  return @output;
+}
+
 sub openssl_format ($) {
   my ($filename) = @_;
 
   return -B $filename ? 'DER' : 'PEM';
 }
 
+sub openssl_x509_query ($@) {
+  my ($filename, @query) = @_;
+
+  my $format = openssl_format($filename);
+  my @args = ("x509", "-in", $filename, "-inform", $format, "-noout", @query);
+  return openssl_exec(@args);
+}
+
 sub openssl_hash ($) {
   my ($filename) = @_;
 
-  my $format = openssl_format($filename);
-  my $cmd = "$opensslbin x509 -noout -hash -in '$filename' -inform $format";
-  my $cert_hash = `$cmd`;
-  $? and die "'$cmd' returned $?";
+  my $cert_hash = join("", openssl_x509_query($filename, "-hash"));
+  $? and die "openssl -hash '$filename' returned $?";
 
   chomp($cert_hash);
   return $cert_hash;
 }
 
 sub openssl_fingerprint ($) {
   my ($filename) = @_;
 
-  my $format = openssl_format($filename);
-  my $cmd = "$opensslbin x509 -in '$filename' -inform $format -fingerprint 
-noout";
-  my $fingerprint = `$cmd`;
-  $? and die "'$cmd' returned $?";
+  my $fingerprint = join("", openssl_x509_query($filename, "-fingerprint"));
+  $? and die "openssl -fingerprint '$filename' returned $?";
 
   chomp($fingerprint);
   return $fingerprint;
 }
 
 sub openssl_emails ($) {
   my ($filename) = @_;
 
-  my $format = openssl_format($filename);
-  my $cmd = "$opensslbin x509 -in '$filename' -inform $format -email -noout";
-  my @mailboxes = `$cmd`;
-  $? and die "'$cmd' returned $?";
+  my @mailboxes = openssl_x509_query($filename, "-email");
+  $? and die "openssl -email '$filename' returned $?";
 
   chomp(@mailboxes);
   return @mailboxes;
 }
 
-sub openssl_do_verify ($$$) {
+sub openssl_p12_to_pem ($$) {
+  my ($p12_file, $pem_file) = @_;
+
+  my @args = ("pkcs12", "-in", $p12_file, "-out", $pem_file);
+  openssl_exec(@args);
+  $? and die "openssl pkcs12 conversion returned $?";
+}
+
+sub openssl_verify ($$) {
+  my ($issuer_path, $cert_path) = @_;
+
+  my @args = ("verify", $root_certs_switch, $root_certs_path,
+              "-purpose", "smimesign", "-purpose", "smimeencrypt", 
"-untrusted",
+              $issuer_path, $cert_path);
+  my $output = join("", openssl_exec(@args));
+
+  chomp($output);
+  return $output;
+}
+
+sub openssl_crl_text($) {
+  my ($crl) = @_;
+
+  my @args = ("crl", "-text", "-noout", "-in", $crl);
+  my @output = openssl_exec(@args);
+  $? and die "openssl crl -text '$crl' returned $?";
+
+  return @output;
+}
+
+sub openssl_cert_flag ($$$) {
   my ($cert, $issuerid, $crl) = @_;
 
   my $result = 't';
   my $issuer_path;
   my $cert_path = "$certificates_path/$cert";
 
   if($issuerid eq '?') {
     $issuer_path = "$certificates_path/$cert";
   } else {
     $issuer_path = "$certificates_path/$issuerid";
   }
 
-  my $cmd = "$opensslbin verify $root_certs_switch '$root_certs_path' -purpose 
smimesign -purpose smimeencrypt -untrusted '$issuer_path' '$cert_path'";
-  my $output = `$cmd`;
-  chomp $output;
+  my $output = openssl_verify($issuer_path, $cert_path);
   if ($?) {
-    print "'$cmd' returned exit code " . ($? >> 8) . " with output:\n";
+    print "openssl verify returned exit code " . ($? >> 8) . " with output:\n";
     print "$output\n\n";
-    print "Marking as invalid\n";
+    print "Marking certificate as invalid\n";
     return 'i';
   }
   print "\n$output\n";
 
   if ($output !~ /OK/) {
     return 'i';
   }
 
-  my $format = openssl_format($cert_path);
-  $cmd = "$opensslbin x509 -dates -serial -noout -in '$cert_path' -inform 
$format";
-  (my $not_before, my $not_after, my $serial_in) = `$cmd`;
-  $? and die "'$cmd' returned $?";
+  my ($not_before, $not_after, $serial_in) = openssl_x509_query($cert_path, 
"-dates", "-serial");
+  $? and die "openssl -dates -serial '$cert_path' returned $?";
 
   if ( defined $not_before and defined $not_after ) {
     my %months = ('Jan', '00', 'Feb', '01', 'Mar', '02', 'Apr', '03',
                   'May', '04', 'Jun', '05', 'Jul', '06', 'Aug', '07',
                   'Sep', '08', 'Oct', '09', 'Nov', '10', 'Dec', '11');
 
     my @tmp = split (/\=/, $not_before);
     my $not_before_date = $tmp[1];
@@ -399,22 +446,27 @@
       }
     } else {
       print "Expiration Date: Parse Error :  $not_after_date\n\n";
     }
   }
 
   if ( defined $crl ) {
     my @serial = split (/\=/, $serial_in);
-    my $cmd = "$opensslbin crl -text -noout -in '$crl' | grep -A1 $serial[1]";
-    (my $l1, my $l2) = `$cmd`;
-    $? and die "'$cmd' returned $?";
+    my $match_line = undef;
+    my @crl_lines = openssl_crl_text($crl);
+    for (my $index = 0; $index <= $#crl_lines; $index++) {
+      if ($crl_lines[$index] =~ /Serial Number:\s*\Q$serial[1]\E/) {
+        $match_line = $crl_lines[$index + 1];
+        last;
+      }
+    }
 
-    if ( defined $l2 ) {
-      my @revoke_date = split (/:\s/, $l2);
+    if ( defined $match_line ) {
+      my @revoke_date = split (/:\s/, $match_line);
       print "FAILURE: Certificate $cert has been revoked on $revoke_date[1]\n";
       $result = 'r';
     }
   }
   print "\n";
 
   return $result;
 }
@@ -491,16 +543,27 @@
 
   if ($attrs_required && ($bag_count != $cert_count)) {
     die("Not all contents were bagged. can't continue.");
   }
 
   return @certs;
 }
 
+sub openssl_dump_cert ($) {
+  my ($filename) = @_;
+
+  my $format = openssl_format($filename);
+  my @args = ("x509", "-in", $filename, "-inform", $format);
+  my $output = join("", openssl_exec(@args));
+  $? and die "openssl x509 certicate dump returned $?";
+
+  return $output;
+}
+
 
 #################################
 # certificate management methods
 #################################
 
 sub cm_list_certs () {
   my %keyflags = ( 'i', '(Invalid)',  'r', '(Revoked)', 'e', '(Expired)',
                    'u', '(Unverified)', 'v', '(Valid)', 't', '(Trusted)');
@@ -526,25 +589,19 @@
     {
         open F, $certfile or
             die "Couldn't open $certfile: $!";
         local $/;
         $cert = <F>;
         close F;
     }
 
-    my $subject_in;
-    my $issuer_in;
-    my $date1_in;
-    my $date2_in;
-
-    my $format = openssl_format($certfile);
-    my $cmd = "$opensslbin x509 -subject -issuer -dates -noout -in '$certfile' 
-inform $format";
-    ($subject_in, $issuer_in, $date1_in, $date2_in) = `$cmd`;
-    $? and print "ERROR: '$cmd' returned $?\n\n" and next;
+    my ($subject_in, $issuer_in, $date1_in, $date2_in) =
+      openssl_x509_query($certfile, "-subject", "-issuer", "-dates");
+    $? and print "ERROR: openssl -subject -issuer -dates '$certfile' returned 
$?\n\n" and next;
 
 
     my @subject = split(/\//, $subject_in);
     while(@subject) {
       $tmp = shift @subject;
       ($tmp =~ /^CN\=/) and last;
       undef $tmp;
     }
@@ -566,22 +623,20 @@
       @tmp = split (/\=/, $date2_in);
       print $tab."Certificate is not valid before $tmp".
         $tab."                      or after  ".$tmp[1];
     }
 
     -e "$private_keys_path/$fields[1]" and
       print "$tab - Matching private key installed -\n";
 
-    $format = openssl_format("$certificates_path/$fields[1]");
-    $cmd = "$opensslbin x509 -purpose -noout -in '$certfile' -inform $format";
-    my $purpose_in = `$cmd`;
-    $? and die "'$cmd' returned $?";
+    my @purpose = openssl_x509_query($certfile, "-purpose");
+    $? and die "openssl -purpose '$certfile' returned $?";
+    chomp(@purpose);
 
-    my @purpose = split (/\n/, $purpose_in);
     print "$tab$purpose[0] (displays S/MIME options only)\n";
     while(@purpose) {
       $tmp = shift @purpose;
       ($tmp =~ /^S\/MIME/ and $tmp =~ /Yes/) or next;
       my @tmptmp = split (/:/, $tmp);
       print "$tab  $tmptmp[0]\n";
     }
 
@@ -706,17 +761,17 @@
         }
         else {
           print $newindex_fh " $label";
         }
       }
 
       if ($op eq 'V') {
         print "\n==> about to verify certificate of $fields[0]\n";
-        my $flag = openssl_do_verify($fields[1], $fields[3], $crl);
+        my $flag = openssl_cert_flag($fields[1], $fields[3], $crl);
         print $newindex_fh " $fields[2] $fields[3] $flag";
       }
 
       print $newindex_fh "\n";
       next;
     }
     print $newindex_fh $_;
   }
@@ -874,20 +929,19 @@
 
   print "\nNOTE: This will ask you for two passphrases:\n";
   print "       1. The passphrase you used for exporting\n";
   print "       2. The passphrase you wish to secure your private key 
with.\n\n";
 
   my ($pem_fh, $pem_file) = create_tempfile();
   close($pem_fh);
 
-  my $cmd = "$opensslbin pkcs12 -in '$filename' -out '$pem_file'";
-  system $cmd and die "'$cmd' returned $?";
+  openssl_p12_to_pem($filename, $pem_file);
+  -e $pem_file and -s $pem_file or die("Conversion of $filename failed.");
 
-  -e $pem_file and -s $pem_file or die("Conversion of $filename failed.");
   handle_add_pem($pem_file);
 }
 
 sub handle_add_chain ($$$) {
   my ($key_file, $cert_file, $issuer_file) = @_;
 
   my $cert_hash = openssl_hash($cert_file);
   my $issuer_hash = openssl_hash($issuer_file);
@@ -938,29 +992,24 @@
     -e "$root_certs_path/$root_hash" or
         mycopy $root_cert, "$root_certs_path/$root_hash";
   }
   else {
     open(ROOT_CERTS, ">>$root_certs_path") or
       die ("Couldn't open $root_certs_path for writing");
 
     my $md5fp = openssl_fingerprint($root_cert);
-    my $format = openssl_format($root_cert);
 
-    my $cmd = "$opensslbin x509 -in '$root_cert' -inform $format -text -noout";
-    $? and die "'$cmd' returned $?";
-    my @cert_text = `$cmd`;
+    my @cert_text = openssl_x509_query($root_cert, "-text");
+    $? and die "openssl -text '$root_cert' returned $?";
 
     print "Enter a label, name or description for this certificate: ";
     my $input = <STDIN>;
 
     my $line = "=======================================\n";
     print ROOT_CERTS "\n$input$line$md5fp\nPEM-Data:\n";
 
-    $cmd = "$opensslbin x509 -in '$root_cert' -inform $format";
-    my $cert = `$cmd`;
-    $? and die "'$cmd' returned $?";
+    my $cert = openssl_dump_cert($root_cert);
     print ROOT_CERTS $cert;
     print ROOT_CERTS @cert_text;
     close (ROOT_CERTS);
   }
-
 }

Attachment: signature.asc
Description: PGP signature

Reply via email to