Bug#433091: [patch] possible fix

[Michael Vogt <mvo@debian.org>]

Hi,

I did some investigation and it seems like gpgv is sending the right
message, but the gpgv method in apt is not reading them
properly. There is also the problem that VALIDSIG is send even for
expired/revoked signatures and apt is checking for that (instead of
using GOODSIG). However the docs in doc/DETAILS in gpg make this
difference not clear. 

Attached is a possible fix. It does the following:

 * recognize KEYEXPIRED and KEYREVOKED messages from gpgv and put them
   into a new "WorthlessSignatures " vector
 * only listen for GOODSIG message from gpgv and ignore VALIDSIG (as
   GOODSIG is only send when the signature is not done with a expired or
   revoked key)
 * if there is no good signature, show a message that displays the
   worthless signatures to the user (including the KEYEXPIRED or
   KEYREVSIG bits to ensure there is a way to know what is going on)
 * if there is one (or more) good signature and worthless signatures,
   just ignore the worthless ones

That should hopefully cover the problem without breaking strings and
compatibility. Feedback/review/testing is very welcome. I tested it in
a etch chroot with various expired settings and it works as it should,
but I need to make a test-suit for it too.

Cheers,
 Michael

=== modified file 'methods/gpgv.cc'
--- methods/gpgv.cc	2008-12-09 22:38:10 +0000
+++ methods/gpgv.cc	2009-04-07 12:20:08 +0000
@@ -17,13 +17,18 @@
 #define GNUPGBADSIG "[GNUPG:] BADSIG"
 #define GNUPGNOPUBKEY "[GNUPG:] NO_PUBKEY"
 #define GNUPGVALIDSIG "[GNUPG:] VALIDSIG"
+#define GNUPGGOODSIG "[GNUPG:] GOODSIG"
+#define GNUPGKEYEXPIRED "[GNUPG:] KEYEXPIRED"
+#define GNUPGREVKEYSIG "[GNUPG:] REVKEYSIG"
 #define GNUPGNODATA "[GNUPG:] NODATA"
 
 class GPGVMethod : public pkgAcqMethod
 {
    private:
    string VerifyGetSigners(const char *file, const char *outfile,
-				vector<string> &GoodSigners, vector<string> &BadSigners,
+				vector<string> &GoodSigners, 
+                                vector<string> &BadSigners,
+                                vector<string> &WorthlessSigners,
 				vector<string> &NoPubKeySigners);
    
    protected:
@@ -37,6 +42,7 @@
 string GPGVMethod::VerifyGetSigners(const char *file, const char *outfile,
 					 vector<string> &GoodSigners,
 					 vector<string> &BadSigners,
+					 vector<string> &WorthlessSigners,
 					 vector<string> &NoPubKeySigners)
 {
    // setup a (empty) stringstream for formating the return value
@@ -179,15 +185,27 @@
             std::cerr << "Got NODATA! " << std::endl;
          BadSigners.push_back(string(buffer+sizeof(GNUPGPREFIX)));
       }
-      if (strncmp(buffer, GNUPGVALIDSIG, sizeof(GNUPGVALIDSIG)-1) == 0)
+      if (strncmp(buffer, GNUPGKEYEXPIRED, sizeof(GNUPGKEYEXPIRED)-1) == 0)
+      {
+         if (_config->FindB("Debug::Acquire::gpgv", false))
+            std::cerr << "Got KEYEXPIRED! " << std::endl;
+         WorthlessSigners.push_back(string(buffer+sizeof(GNUPGPREFIX)));
+      }
+      if (strncmp(buffer, GNUPGREVKEYSIG, sizeof(GNUPGREVKEYSIG)-1) == 0)
+      {
+         if (_config->FindB("Debug::Acquire::gpgv", false))
+            std::cerr << "Got REVKEYSIG! " << std::endl;
+         WorthlessSigners.push_back(string(buffer+sizeof(GNUPGPREFIX)));
+      }
+      if (strncmp(buffer, GNUPGGOODSIG, sizeof(GNUPGGOODSIG)-1) == 0)
       {
          char *sig = buffer + sizeof(GNUPGPREFIX);
-         char *p = sig + sizeof("VALIDSIG");
+         char *p = sig + sizeof("GOODSIG");
          while (*p && isxdigit(*p)) 
             p++;
          *p = 0;
          if (_config->FindB("Debug::Acquire::gpgv", false))
-            std::cerr << "Got VALIDSIG, key ID:" << sig << std::endl;
+            std::cerr << "Got GOODSIG, key ID:" << sig << std::endl;
          GoodSigners.push_back(string(sig));
       }
    }
@@ -227,6 +245,8 @@
    string keyID;
    vector<string> GoodSigners;
    vector<string> BadSigners;
+   // a worthless signature is a expired or revoked one
+   vector<string> WorthlessSigners;
    vector<string> NoPubKeySigners;
    
    FetchResult Res;
@@ -235,13 +255,14 @@
 
    // Run gpgv on file, extract contents and get the key ID of the signer
    string msg = VerifyGetSigners(Path.c_str(), Itm->DestFile.c_str(),
-			      GoodSigners, BadSigners, NoPubKeySigners);
+                                 GoodSigners, BadSigners, WorthlessSigners,
+                                 NoPubKeySigners);
    if (GoodSigners.empty() || !BadSigners.empty() || !NoPubKeySigners.empty())
    {
       string errmsg;
       // In this case, something bad probably happened, so we just go
       // with what the other method gave us for an error message.
-      if (BadSigners.empty() && NoPubKeySigners.empty())
+      if (BadSigners.empty() && WorthlessSigners.empty() && NoPubKeySigners.empty())
          errmsg = msg;
       else
       {
@@ -252,6 +273,13 @@
 		 I != BadSigners.end(); I++)
                errmsg += (*I + "\n");
          }
+         if (!WorthlessSigners.empty())
+         {
+            errmsg += _("The following signatures were invalid:\n");
+            for (vector<string>::iterator I = WorthlessSigners.begin();
+		 I != WorthlessSigners.end(); I++)
+               errmsg += (*I + "\n");
+         }
          if (!NoPubKeySigners.empty())
          {
              errmsg += _("The following signatures couldn't be verified because the public key is not available:\n");