Beautiful Code [164]
Code View: Scroll / Show All
my $expect = Expect->spawn ($self->gpgbin, @opts, '-o-', '--sign',
@extras, @secretkey, $tmpnam);
$expect->log_stdout($self->debug);
$expect->expect (undef, '-re',
'-----BEGIN', 'passphrase:', 'signing failed');
if ($expect->exp_match_number == 2) {
$self->doze; print $expect ($self->passphrase . "\r");
$expect->expect (undef, '-re', '-----BEGIN', 'passphrase:');
if ($expect->exp_match_number == 2) { # Passphrase incorrect
$self->doze;
print $expect ($self->passphrase . "\r");
$expect->expect (undef, 'passphrase:'); $self->doze;
print $expect ($self->passphrase . "\r");
$expect->expect (undef);
unlink $tmpnam;
return;
}
}
elsif ($expect->exp_match_number == 3) {
unlink $tmpnam; $expect->close;
return;
}
$expect->expect (undef);
my $info = $expect->exp_match . $expect->exp_before;
Using the Expect-based module also caused Heisenbugs—failures that weren't easily reproducible, and that I discovered were the result of sending input to gpg too fast. The calls to doze in the previous code are a workaround for this: they introduce a few milliseconds of delay before sending the next bit of input to gpg. This generally worked, but failures would still occur on heavily loaded systems.
All these issues pointed to moving away from Expect and using another mechanism to interact with the GnuPG binary. I considered the idea of writing a pure Perl implementation of OpenPGP, but decided against it for basically the same reasons that I had decided to use GnuPG in the first place: Cryptonite is primarily an email client, with integrated Open-PGP support. A full OpenPGP implementation would at least double the complexity of the code I would have to maintain.[**]
[**] A pure-Perl OpenPGP implementation, Crypt::OpenPGP, was written by Ben Trott in 2001–2002, and is available from CPAN. I'm looking forward to using it in future versions of Cryptonite that will support multiple cryptographic backends.
After a little experimenting, it looked like IPC::Run by Barrie Slaymaker might do the trick for communication with GnuPG. With IPC::Run, the previous code became:
my ($in, $out, $err, $in_q, $out_q, $err_q);
my $h = start ([$self->gpgbin, @opts, @secretkey, '-o-',
'--sign', @extras, $tmpnam],
\$in, \$out, \$err, timeout( 30 ));
my $i = 0;
while (1) {
pump $h until ($out =~ /NEED_PASSPHRASE (.{16}) (.{16}).*\n/g or
$out =~ /GOOD_PASSPHRASE/g);
if ($2) {
$in .= $self->passphrase . "\n";
pump $h until $out =~ /(GOOD|BAD)_PASSPHRASE/g;
last if $1 eq 'GOOD' or $i++ == 2;
}
}
finish $h;
my $d = $detach ? 'SIGNATURE' : 'MESSAGE';
$out =~ /(-----BEGIN PGP $d-----.*-----END PGP $d-----)/s;
my $info = $1;
IPC::Run works reliably without the mini-delays needed with Expect, is much clearer to read, and works perfectly on most platforms.
Some operations with gpg didn't require any interaction, and earlier versions of the module had used Perl's backtick operator for such cases. Because the backtick operator invokes a shell, it's a security risk. With IPC::Run, it was easy to replace the use of the backtick operator with a tiny secure backtick function, thereby bypassing the shell. This made it possible to eliminate all shell invocations in Crypt::GPG.
sub backtick {
my ($in, $out, $err, $in_q, $out_q, $err_q);
my $h = start ([@_], \$in, \$out, \$err, timeout( 10 ));
local $SIG{CHLD} = 'IGNORE';
local $SIG{PIPE} = 'IGNORE';
finish $h;
return ($out, $err);
}
Some users had also pointed out that using temporary files to store plaintext could be insecure. This problem could be easily overcome without touching the code, simply by using temporary files on a RAM disk with encrypted swap (such as OpenBSD provides) or an encrypted RAM disk, so plaintext would never be written to disk unencrypted.
Of course, it would be nice to modify the code to avoid writing plaintext to temporary files at all, but as there already existed a practical workaround, eliminating temporary files went