gr (grumpy_sysadmin) wrote in lj_dev,
gr
grumpy_sysadmin
lj_dev

email+PGP problems

[I started out with support on this, but I suspect it's actually a bug, rather than a support issue, exactly.]

I've had some problems with the email PGP posting which, as near as I can tell, hasn't been touched in months. There's some UTF-8 stuff and some image stuff that went in around it in livejournal/cgi-bin/ljemailgateway.pl, but none of that seems to be mangling the MIME::Entity in the case of my test messages.

I hacked ljemailgateway.pl (in ugly ways) so that I could play with this without installing all of lj locally. Unless there's a wrinkle hidden in the ljconfig stuff that I'm missing somehow (and I'm pretty sure there isn't) the code should validate the messages that post.livejournal.com has been rejecting.

Is it possible that someone doing maintenance on the lj servers updated MIME::Parser (less likely; if that weren't behaving many more people would be whining) or Mail::GnuPG without regression testing? Perhaps even gpg itself (there may have been some newly-paranoic changes in it that means my 1.2.4 behaves fine but whatever's on the server doesn't; like say refusing to have a homedir under /tmp).

If the callout to gpg returns undefined you get the not_signed output (at line 411 of ljemailgateway.pl)... but that'll happen for reasons other than the lack of a signature (the easiest example is that the permissions on gpg's homedir are more permissive than 0600). So there should maybe be some better error checking here (like, say, actually doing something with $err, defined on line 393; incidentally, closing it on line 398 would protect against a potential memory leak, though I'm pretty sure Perl will garbage collect anyway here).

I was hoping I'd be able to replicate the problem... but I can't. Nor can I use test for this, since PGP-signed emails are a paid-user-only feature. (Unless I'm confused and test is hacked so that one can create paid user accounts without actually paying, nor having to pass through SSL for CC information?) If someone wants to create me a "paid" account on test, I'd be glad to, erm, test this there.

Update: It's quite possible this is related to the stupid-git attacks and an overworked mail infrastructure.

Update 2.1: test.livejournal.com is now configured for PGP, but it lacks email post support,
so I still can't test this there, unfortunately. I'm reasonably sure that all that would happen is that the posting works, though, so I don't think this would help much.

Update 3: Here's a patch that will report the real error state in the case that Mail::GnuPG fails to fill $ret.
--- ljemailgateway.pl.orig  Wed Jun 30 17:02:41 2004
+++ ljemailgateway.pl Wed Jun 30 17:11:23 2004
@@ -147,9 +147,21 @@
                 'no_key' => "You don't have a PGP key uploaded.",
                 'bad_tmpdir' => "Problem generating tempdir: Please try again.",
                 'invalid_key' => "Your PGP key is invalid.  Please upload a proper key.",
-                'not_signed' => "You specified PGP verification, but your message isn't PGP signed!");
-        my $gpgcode = LJ::Emailpost::check_sig($u, $entity);
-        return $err->($gpg_errcodes{$gpgcode}, { sendmail => 1 }) unless $gpgcode eq 'good';
+                'not_signed' => "You specified PGP verification, but your message isn't PGP signed!",
+                'gpg_error' => "Something went wrong inside GnuPG, please try again.");
+        my $gpg_errmsg;
+        my $gpgcode = LJ::Emailpost::check_sig($u, $entity, \$gpg_errmsg);
+
+        # XXX need this to figure out what's broke, but gpg_error should
+        # probably be treated normally in the long run. (That is, lose the
+        # if again, and just use the unless.)
+        if ($gpgcode eq 'gpg_error') {
+          return $err->($gpg_errcodes{$gpgcode} . "\n"
+            . "The error message from Mail::GnuPG was:\n" . $gpg_errmsg,
+            { sendmail => 1 });
+        } else {
+            return $err->($gpg_errcodes{$gpgcode}, { sendmail => 1 }) unless $gpgcode eq 'good';
+        }
     }
 
     $body =~ s/^(?:\- )?[\-_]{2,}\s*\r?\n.*//ms; # trim sigs
@@ -377,7 +389,7 @@
 # Returns codes so we can use the pre-existing err subref,
 # without passing everything all over the place.
 sub check_sig {
-    my ($u, $entity) = @_;
+    my ($u, $entity, $gpg_errmsg_ref) = @_;
 
     LJ::load_user_props($u, 'public_key');
     my $key = $u->{public_key};
@@ -405,6 +417,14 @@
     my ($gpg_email, $ret);
     $gpg_email = new Mail::GnuPG( keydir=>$tmpdir );
     eval { $ret = $gpg_email->verify($entity); };
+    # Actually need to check the value of $@--the point of using
+    # eval()--in this case, since Mail::GnuPG::verify() won't return
+    # anything in the event that GnuPG dies internally (which can
+    # happen for reasons other than the lack of a signature).
+    if ($@) {
+        $$gpg_errmsg_ref = $@;
+        return 'gpg_error';
+    }
     if (defined($ret)) {
         $ret == 0 ? return 'good' : return 'bad';
     } else {
I can't know what errors to check for unless this is run on the production lj servers, but it's also not code that should be run on production servers for long (because it's just spewing what would have been die messages from inside Mail::GnuPG to the world). Should I email mahlon at this point?
Subscribe
  • Post a new comment

    Error

    Anonymous comments are disabled in this journal

    default userpic

    Your reply will be screened

    Your IP address will be recorded 

  • 36 comments