Kevin J. McCarthy wrote: > However, taking another look at close() in the perl book, I do see that > $! may also be non-zero "if a syscall fails". I will add a check for > that to the close() call (along with a comment that the callers are > checking $?) and resend the patch later today. Does that sound good?
I'm attaching a revised patch. In addition to the above change, it also adds a word boundary "\b" to the CRL "grep", to prevent a substring match on a serial number. -Kevin
# HG changeset patch # User Kevin McCarthy <[email protected]> # Date 1431882100 25200 # Sun May 17 10:01:40 2015 -0700 # Node ID 429c830a188963be44035bd3e11691b08c9397ba # Parent c66a6bd5d0d57eca9fba0a90b34c9adab0c0ce27 smime_keys: 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,136 @@ 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>; + if (! close($fh)) { + # NOTE: Callers should check the value of $? for the exit status. + if ($!) { + die "Syserr closing '$opensslbin @args' pipe: $!"; + } + } + + 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 +451,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\b/) { + $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 +548,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 +594,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 +628,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 +766,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 +934,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 +997,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); } - }
signature.asc
Description: PGP signature
