gpg: Fix a verification DoS due to a malicious subkey in the keyring.

* g10/getkey.c (get_pubkey): Factor code out to ...
(get_pubkey_bykid): new.  Add feature to return the keyblock.
(get_pubkey_for_sig): Add arg r_keyblock to return the used keyblock.
Request a signing usage.
(get_pubkeyblock_for_sig): Remove.
(finish_lookup): Improve debug output.
* g10/sig-check.c (check_signature): Add arg r_keyblock and pass it
down.
* g10/mainproc.c (do_check_sig): Ditto.
(check_sig_and_print): Use the keyblock returned by do_check_sig to
show further information instead of looking it up again with
get_pubkeyblock_for_sig.  Also re-check the signature after the import
of an included keyblock.
--

The problem here is that it is possible to import a key from someone
who added a signature subkey from another public key and thus inhibits
that a good signature good be verified.

Such a malicious key signature subkey must have been created w/o the
mandatory backsig which bind a signature subkey to its primary key.
For encryption subkeys this is not an issue because the existence of a
decryption private key is all you need to decrypt something and then
it does not matter if the public subkey or its binding signature has
been put below another primary key; in fact we do the latter for
ADSKs.

GnuPG-bug-id: 7527

1 files changed, 67 insertions, 41 deletions

diff --git a/g10/getkey.c b/g10/getkey.c
index bee5ff356..21f996a5b 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -316,27 +316,50 @@ pk_from_block (PKT_public_key *pk, kbnode_t keyblock, kbnode_t found_key)
 
 /* Specialized version of get_pubkey which retrieves the key based on
  * information in SIG.  In contrast to get_pubkey PK is required.  IF
- * FORCED_PK is not NULL, this public key is used and copied to PK. */
+ * FORCED_PK is not NULL, this public key is used and copied to PK.
+ * If R_KEYBLOCK is not NULL the entire keyblock is stored there if
+ * found and FORCED_PK is not used; if not used or on error NULL is
+ * stored there. */
 gpg_error_t
 get_pubkey_for_sig (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig,
-                    PKT_public_key *forced_pk)
+                    PKT_public_key *forced_pk, kbnode_t *r_keyblock)
 {
+  gpg_error_t err;
   const byte *fpr;
   size_t fprlen;
 
+  if (r_keyblock)
+    *r_keyblock = NULL;
+
   if (forced_pk)
     {
       copy_public_key (pk, forced_pk);
       return 0;
     }
 
+  /* Make sure to request only keys cabable of signing.  This makes
+   * sure that a subkey w/o a valid backsig or with bad usage flags
+   * will be skipped.  */
+  pk->req_usage = PUBKEY_USAGE_SIG;
+
   /* First try the ISSUER_FPR info.  */
   fpr = issuer_fpr_raw (sig, &fprlen);
-  if (fpr && !get_pubkey_byfpr (ctrl, pk, NULL, fpr, fprlen))
+  if (fpr && !get_pubkey_byfpr (ctrl, pk, r_keyblock, fpr, fprlen))
     return 0;
+  if (r_keyblock)
+    {
+      release_kbnode (*r_keyblock);
+      *r_keyblock = NULL;
+    }
 
   /* Fallback to use the ISSUER_KEYID.  */
-  return get_pubkey (ctrl, pk, sig->keyid);
+  err = get_pubkey_bykid (ctrl, pk, r_keyblock, sig->keyid);
+  if (err && r_keyblock)
+    {
+      release_kbnode (*r_keyblock);
+      *r_keyblock = NULL;
+    }
+  return err;
 }
 
 
@@ -354,6 +377,10 @@ get_pubkey_for_sig (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig,
  * usage will be returned.  As such, it is essential that
  * PK->REQ_USAGE be correctly initialized!
  *
+ * If R_KEYBLOCK is not NULL, then the first result's keyblock is
+ * returned in *R_KEYBLOCK.  This should be freed using
+ * release_kbnode().
+ *
  * Returns 0 on success, GPG_ERR_NO_PUBKEY if there is no public key
  * with the specified key id, or another error code if an error
  * occurs.
@@ -361,24 +388,30 @@ get_pubkey_for_sig (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig,
  * If the data was not read from the cache, then the self-signed data
  * has definitely been merged into the public key using
  * merge_selfsigs.  */
-int
-get_pubkey (ctrl_t ctrl, PKT_public_key * pk, u32 * keyid)
+gpg_error_t
+get_pubkey_bykid (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock,
+                  u32 *keyid)
 {
   int internal = 0;
-  int rc = 0;
+  gpg_error_t rc = 0;
+
+  if (r_keyblock)
+    *r_keyblock = NULL;
 
 #if MAX_PK_CACHE_ENTRIES
-  if (pk)
+  if (pk && !r_keyblock)
     {
       /* Try to get it from the cache.  We don't do this when pk is
-         NULL as it does not guarantee that the user IDs are
-         cached. */
+       * NULL as it does not guarantee that the user IDs are cached.
+       * The old get_pubkey_function did not check PK->REQ_USAGE when
+       * reading form the caceh.  This is probably a bug.  Note that
+       * the cache is not used when the caller asked to return the
+       * entire keyblock.  This is because the cache does not
+       * associate the public key wit its primary key.  */
       pk_cache_entry_t ce;
       for (ce = pk_cache; ce; ce = ce->next)
 	{
 	  if (ce->keyid[0] == keyid[0] && ce->keyid[1] == keyid[1])
-	    /* XXX: We don't check PK->REQ_USAGE here, but if we don't
-	       read from the cache, we do check it!  */
 	    {
 	      copy_public_key (pk, ce->pk);
 	      return 0;
@@ -386,6 +419,7 @@ get_pubkey (ctrl_t ctrl, PKT_public_key * pk, u32 * keyid)
 	}
     }
 #endif
+
   /* More init stuff.  */
   if (!pk)
     {
@@ -431,16 +465,18 @@ get_pubkey (ctrl_t ctrl, PKT_public_key * pk, u32 * keyid)
     ctx.req_usage = pk->req_usage;
     rc = lookup (ctrl, &ctx, 0, &kb, &found_key);
     if (!rc)
+      pk_from_block (pk, kb, found_key);
+    getkey_end (ctrl, &ctx);
+    if (!rc && r_keyblock)
       {
-	pk_from_block (pk, kb, found_key);
+        *r_keyblock = kb;
+        kb = NULL;
       }
-    getkey_end (ctrl, &ctx);
     release_kbnode (kb);
   }
-  if (!rc)
-    goto leave;
 
-  rc = GPG_ERR_NO_PUBKEY;
+  if (rc)  /* Return a more useful error code.  */
+    rc = gpg_error (GPG_ERR_NO_PUBKEY);
 
 leave:
   if (!rc)
@@ -451,6 +487,14 @@ leave:
 }
 
 
+/* Wrapper for get_pubkey_bykid w/o keyblock return feature.  */
+int
+get_pubkey (ctrl_t ctrl, PKT_public_key *pk, u32 *keyid)
+{
+  return get_pubkey_bykid (ctrl, pk, NULL, keyid);
+}
+
+
 /* Same as get_pubkey but if the key was not found the function tries
  * to import it from LDAP.  FIXME: We should not need this but switch
  * to a fingerprint lookup.  */
@@ -563,28 +607,6 @@ get_pubkey_fast (ctrl_t ctrl, PKT_public_key * pk, u32 * keyid)
 }
 
 
-/* Return the entire keyblock used to create SIG.  This is a
- * specialized version of get_pubkeyblock.
- *
- * FIXME: This is a hack because get_pubkey_for_sig was already called
- * and it could have used a cache to hold the key.  */
-kbnode_t
-get_pubkeyblock_for_sig (ctrl_t ctrl, PKT_signature *sig)
-{
-  const byte *fpr;
-  size_t fprlen;
-  kbnode_t keyblock;
-
-  /* First try the ISSUER_FPR info.  */
-  fpr = issuer_fpr_raw (sig, &fprlen);
-  if (fpr && !get_pubkey_byfpr (ctrl, NULL, &keyblock, fpr, fprlen))
-    return keyblock;
-
-  /* Fallback to use the ISSUER_KEYID.  */
-  return get_pubkeyblock (ctrl, sig->keyid);
-}
-
-
 /* Return the key block for the key with key id KEYID or NULL, if an
  * error occurs.  Use release_kbnode() to release the key block.
  *
@@ -3700,6 +3722,7 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact,
   kbnode_t latest_key;
   PKT_public_key *pk;
   int req_prim;
+  int diag_exactfound = 0;
   u32 curtime = make_timestamp ();
 
   if (r_flags)
@@ -3730,11 +3753,10 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact,
         {
           if (want_exact)
             {
-              if (DBG_LOOKUP)
-                log_debug ("finish_lookup: exact search requested and found\n");
               foundk = k;
               pk = k->pkt->pkt.public_key;
               pk->flags.exact = 1;
+              diag_exactfound = 1;
               break;
             }
           else if (!allow_adsk && (k->pkt->pkt.public_key->pubkey_usage
@@ -3764,10 +3786,14 @@ finish_lookup (kbnode_t keyblock, unsigned int req_usage, int want_exact,
     log_debug ("finish_lookup: checking key %08lX (%s)(req_usage=%x)\n",
 	       (ulong) keyid_from_pk (keyblock->pkt->pkt.public_key, NULL),
 	       foundk ? "one" : "all", req_usage);
+  if (diag_exactfound && DBG_LOOKUP)
+    log_debug ("\texact search requested and found\n");
 
   if (!req_usage)
     {
       latest_key = foundk ? foundk : keyblock;
+      if (DBG_LOOKUP)
+        log_debug ("\tno usage requested - accepting key\n");
       goto found;
     }