[Rt-devel] problem with signed mail submission while gpg is indisposed

S.P.Zeidler spz at serpens.de
Mon Feb 1 14:02:57 EST 2010


Hi,

for RT 3.8.7 I got the following error messages:

[Mon Feb 01 07:42:17 2010] [error] [client A.B.C.D] FastCGI: server "/usr/pkg/bin/mason_handler.fcgi" stderr: readline() on closed filehandle GEN57 at /usr/pkg/lib/rt3/RT/Crypt/GnuPG.pm line 1372.
[Mon Feb 01 07:42:17 2010] [error] [client A.B.C.D] FastCGI: server "/usr/pkg/bin/mason_handler.fcgi" stderr: readline() on closed filehandle GEN58 at /usr/pkg/lib/rt3/RT/Crypt/GnuPG.pm line 1372.
[Mon Feb 01 07:42:17 2010] [error] [client A.B.C.D] FastCGI: server "/usr/pkg/bin/mason_handler.fcgi" stderr: readline() on closed filehandle GEN59 at /usr/pkg/lib/rt3/RT/Crypt/GnuPG.pm line 1372.
[Mon Feb 01 07:42:17 2010] [error] [client A.B.C.D] FastCGI: server "/usr/pkg/bin/mason_handler.fcgi" stderr: Use of uninitialized value $res{"status"} in pattern match (m//) at /usr/pkg/lib/rt3/RT/Crypt/GnuPG.pm line 1383.

Due to a problem with gpg and:
    foreach ( qw(stderr logger status) ) {
		$res{$_} = do { local $/; readline $handle{$_} };
		delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
		close $handle{$_};
	}
hitting a closed handle, and
    if ( $res{'status'} !~ /DECRYPTION_OKAY/ ) {
(the lines above properly checked if $res{'status'} etc were defined)

Suggested fix:
	$res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };

and
    if ( !defined $res{'status'} ) {
		$res{'message'} = $@? $@: "gpg failed ". ($? >> 8);
		seek $tmp_fh, 0, 0;
		return ($tmp_fh, $tmp_fn, %res);
	}
before the
	if ( $res{'status'} !~ /DECRYPTION_OKAY/ ) {

This will likely lose the message txt, but at least rt doesn't stop working,
and the original message gets preserved in the 'original message' field.

gpg bailing rather unceremoniously is likely a rare occurrence.
That doesn't mean that one oughtn't deal with a failure more gracefully. :)

An additional change that I'd appreciate would be to add a flag 'Call'
to the %GnuPG hash so that by adding
$gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
after each occurrence of
my $gnupg = new GnuPG::Interface;
one could be reasonably sure to pick up the right gpg (and find it in the
first place if not in a standard location).

A patch that garnishes most readline calls in lib/RT/Crypt/GnuPG.pm
with an if opened and adds the other points I'm asking for is attached.

best regards,
	spz
-- 
spz at serpens.de (S.P.Zeidler)
-------------- next part --------------

--- lib/RT/Crypt/GnuPG.pm.orig	2009-12-11 17:27:20.000000000 +0000
+++ lib/RT/Crypt/GnuPG.pm
@@ -434,6 +434,7 @@ sub SignEncryptRFC3156 {
     );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnuPGOptions
@@ -484,12 +485,12 @@ sub SignEncryptRFC3156 {
             waitpid $pid, 0;
         };
         my $err = $@;
-        my @signature = readline $handle{'stdout'};
+        my @signature = readline $handle{'stdout'} if $handle{'stdout'}->opened ;
         close $handle{'stdout'};
 
         $res{'exit_code'} = $?;
         foreach ( qw(stderr logger status) ) {
-            $res{$_} = do { local $/; readline $handle{$_} };
+            $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
             delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
             close $handle{$_};
         }
@@ -545,7 +546,7 @@ sub SignEncryptRFC3156 {
 
         $res{'exit_code'} = $?;
         foreach ( qw(stderr logger status) ) {
-            $res{$_} = do { local $/; readline $handle{$_} };
+            $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
             delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
             close $handle{$_};
         }
@@ -616,6 +617,7 @@ sub _SignEncryptTextInline {
     return unless $args{'Sign'} || $args{'Encrypt'};
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnupGOptions
@@ -670,7 +672,7 @@ sub _SignEncryptTextInline {
     my $err = $@;
 
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -704,6 +706,7 @@ sub _SignEncryptAttachmentInline {
     return unless $args{'Sign'} || $args{'Encrypt'};
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnupGOptions
@@ -757,7 +760,7 @@ sub _SignEncryptAttachmentInline {
     my $err = $@;
 
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -806,6 +809,7 @@ sub SignEncryptContent {
     return unless $args{'Sign'} || $args{'Encrypt'};
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnupGOptions
@@ -858,7 +862,7 @@ sub SignEncryptContent {
     my $err = $@;
 
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1077,6 +1081,7 @@ sub VerifyAttachment {
     my %args = ( Data => undef, Signature => undef, Top => undef, @_ );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $opt{'digest-algo'} ||= 'SHA1';
     $gnupg->options->hash_init(
@@ -1114,7 +1119,7 @@ sub VerifyAttachment {
     };
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1131,6 +1136,7 @@ sub VerifyRFC3156 {
     my %args = ( Data => undef, Signature => undef, Top => undef, @_ );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $opt{'digest-algo'} ||= 'SHA1';
     $gnupg->options->hash_init(
@@ -1161,7 +1167,7 @@ sub VerifyRFC3156 {
     };
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1184,6 +1190,7 @@ sub DecryptRFC3156 {
     );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnupGOptions
@@ -1226,7 +1233,7 @@ sub DecryptRFC3156 {
     };
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1237,6 +1244,11 @@ sub DecryptRFC3156 {
     # if the decryption is fine but the signature is bad, then without this
     # status check we lose the decrypted text
     # XXX: add argument to the function to control this check
+    if ( !defined $res{'status'} ) {
+	$res{'message'} = $@? $@: "gpg failed ". ($? >> 8);
+        seek $tmp_fh, 0, 0;
+	return ($tmp_fh, $tmp_fn, %res);
+    }
     if ( $res{'status'} !~ /DECRYPTION_OKAY/ ) {
         if ( $@ || $? ) {
             $res{'message'} = $@? $@: "gpg exitted with error code ". ($? >> 8);
@@ -1262,6 +1274,7 @@ sub DecryptInline {
     );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnuPGOptions
@@ -1369,7 +1382,7 @@ sub _DecryptInlineBlock {
     };
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1380,6 +1393,11 @@ sub _DecryptInlineBlock {
     # if the decryption is fine but the signature is bad, then without this
     # status check we lose the decrypted text
     # XXX: add argument to the function to control this check
+    if ( !defined $res{'status'} ) {
+	$res{'message'} = $@? $@: "gpg failed ". ($? >> 8);
+        seek $tmp_fh, 0, 0;
+	return ($tmp_fh, $tmp_fn, %res);
+    }
     if ( $res{'status'} !~ /DECRYPTION_OKAY/ ) {
         if ( $@ || $? ) {
             $res{'message'} = $@? $@: "gpg exitted with error code ". ($? >> 8);
@@ -1400,6 +1418,7 @@ sub DecryptAttachment {
     );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnuPGOptions
@@ -1451,6 +1470,7 @@ sub DecryptContent {
     );
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
 
     # handling passphrase in GnupGOptions
@@ -1489,7 +1509,7 @@ sub DecryptContent {
     };
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -1500,6 +1520,11 @@ sub DecryptContent {
     # if the decryption is fine but the signature is bad, then without this
     # status check we lose the decrypted text
     # XXX: add argument to the function to control this check
+    if ( !defined $res{'status'} ) {
+	$res{'message'} = $@? $@: "gpg failed ". ($? >> 8);
+        seek $tmp_fh, 0, 0;
+	return ($tmp_fh, $tmp_fn, %res);
+    }
     if ( $res{'status'} !~ /DECRYPTION_OKAY/ ) {
         if ( $@ || $? ) {
             $res{'message'} = $@? $@: "gpg exitted with error code ". ($? >> 8);
@@ -2040,6 +2065,7 @@ sub GetKeysInfo {
     }
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $opt{'digest-algo'} ||= 'SHA1';
     $opt{'with-colons'} = undef; # parseable format
@@ -2064,12 +2090,12 @@ sub GetKeysInfo {
         waitpid $pid, 0;
     };
 
-    my @info = readline $handle{'stdout'};
+    my @info = readline $handle{'stdout'} if $handle{'stdout'}->opened ;
     close $handle{'stdout'};
 
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -2239,6 +2265,7 @@ sub DeleteKey {
     my $key = shift;
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $gnupg->options->hash_init(
         _PrepareGnuPGOptions( %opt ),
@@ -2270,7 +2297,7 @@ sub DeleteKey {
     my %res;
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -2287,6 +2314,7 @@ sub ImportKey {
     my $key = shift;
 
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $gnupg->options->hash_init(
         _PrepareGnuPGOptions( %opt ),
@@ -2313,7 +2341,7 @@ sub ImportKey {
     my %res;
     $res{'exit_code'} = $?;
     foreach ( qw(stderr logger status) ) {
-        $res{$_} = do { local $/; readline $handle{$_} };
+        $res{$_} = do { local $/; readline $handle{$_} if $handle{$_}->opened };
         delete $res{$_} unless $res{$_} && $res{$_} =~ /\S/s;
         close $handle{$_};
     }
@@ -2370,6 +2398,7 @@ properly (and false otherwise).
 
 sub Probe {
     my $gnupg = new GnuPG::Interface;
+    $gnupg->call(RT->Config->Get('GnuPG')->{'Call'}) if RT->Config->Get('GnuPG')->{'Call'};
     my %opt = RT->Config->Get('GnuPGOptions');
     $gnupg->options->hash_init(
         _PrepareGnuPGOptions( %opt ),


More information about the Rt-devel mailing list