gpg: Split check_key_signature2.

* g10/sig-check.c (hash_uid_node): Rename from this...
(hash_uid_packet): ... to this.  Take a PKT_user_id instead of a
KBNODE.
(check_key_signature2): Split the basic signature checking
functionality into...
(check_signature_over_key_or_uid): ... this new function.

--
Signed-off-by: Neal H. Walfield <neal@g10code.com>

g10/main.h

@@ -263,6 +263,19 @@ int check_key_signature2( KBNODE root, KBNODE node, PKT_public_key *check_pk,
 			  PKT_public_key *ret_pk, int *is_selfsig,
 			  u32 *r_expiredate, int *r_expired );
 
+/* Returns whether SIGNER generated the signature SIG over the packet
+   PACKET, which is a key, subkey or uid, and comes from the key block
+   KB.  If SIGNER is NULL, it is looked up based on the information in
+   SIG.  If not NULL, sets *IS_SELFSIG to indicate whether the
+   signature is a self-signature and *RET_PK to a copy of the signer's
+   key.  */
+gpg_error_t check_signature_over_key_or_uid (PKT_public_key *signer,
+                                             PKT_signature *sig,
+                                             KBNODE kb, PACKET *packet,
+                                             int *is_selfsig,
+                                             PKT_public_key *ret_pk);
+
+
 /*-- delkey.c --*/
 gpg_error_t delete_keys (strlist_t names, int secret, int allow_both);
 

g10/sig-check.c

@@ -1,7 +1,7 @@
 /* sig-check.c -  Check a signature
  * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003,
  *               2004, 2006 Free Software Foundation, Inc.
- * Copyright (C) 2015 g10 Code GmbH
+ * Copyright (C) 2015, 2016 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -481,11 +481,8 @@ check_signature_end_simple (PKT_public_key *pk, PKT_signature *sig,
 /* Add a uid node to a hash context.  See section 5.2.4, paragraph 4
    of RFC 4880.  */
 static void
-hash_uid_node( KBNODE unode, gcry_md_hd_t md, PKT_signature *sig )
+hash_uid_packet (PKT_user_id *uid, gcry_md_hd_t md, PKT_signature *sig )
 {
-    PKT_user_id *uid = unode->pkt->pkt.user_id;
-
-    assert( unode->pkt->pkttype == PKT_USER_ID );
     if( uid->attrib_data ) {
 	if( sig->version >=4 ) {
 	    byte buf[5];
@@ -691,6 +688,232 @@ check_key_signature (KBNODE root, KBNODE node, int *is_selfsig)
 }
 
 
+/* Returns whether SIGNER generated the signature SIG over the packet
+   PACKET, which is a key, subkey or uid, and comes from the key block
+   KB.  (KB is PACKET's corresponding keyblock; we don't assume that
+   SIG has been added to the keyblock.)
+
+   If SIGNER is set, then checks whether SIGNER generated the
+   signature.  Otherwise, uses SIG->KEYID to find the alleged signer.
+   This parameter can be used to effectively override the alleged
+   signer that is stored in SIG.
+
+   KB may be NULL if SIGNER is set.
+
+   Unlike check_key_signature, this function ignores any cached
+   results!  That is, it does not consider SIG->FLAGS.CHECKED and
+   SIG->FLAGS.VALID nor does it set them.
+
+   This doesn't check the signature's semantic mean.  Concretely, it
+   doesn't check whether a non-self signed revocation signature was
+   created by a designated revoker.  In fact, it doesn't return an
+   error for a binding generated by a completely different key!
+
+   Returns 0 if the signature is valid.  Returns GPG_ERR_SIG_CLASS if
+   this signature can't be over PACKET.  Returns GPG_ERR_NOT_FOUND if
+   the key that generated the signature (according to SIG) could not
+   be found.  Returns GPG_ERR_BAD_SIGNATURE if the signature is bad.
+   Other errors codes may be returned if something else goes wrong.
+
+   IF IS_SELFSIG is not NULL, sets *IS_SELFSIG to 1 if this is a
+   self-signature (by the key's primary key) or 0 if not.
+
+   If RET_PK is not NULL, returns a copy of the public key that
+   generated the signature (i.e., the signer) on success.  This must
+   be released by the caller using release_public_key_parts ().  */
+gpg_error_t
+check_signature_over_key_or_uid (PKT_public_key *signer,
+                                 PKT_signature *sig, KBNODE kb, PACKET *packet,
+                                 int *is_selfsig, PKT_public_key *ret_pk)
+{
+  int rc;
+  PKT_public_key *pripk = kb->pkt->pkt.public_key;
+  gcry_md_hd_t md;
+  int signer_alloced = 0;
+
+  rc = openpgp_pk_test_algo (sig->pubkey_algo);
+  if (rc)
+    return rc;
+  rc = openpgp_md_test_algo (sig->digest_algo);
+  if (rc)
+    return rc;
+
+  /* A signature's class indicates the type of packet that it
+     signs.  */
+  if (/* Primary key binding (made by a subkey).  */
+      sig->sig_class == 0x19
+      /* Direct key signature.  */
+      || sig->sig_class == 0x1f
+      /* Primary key revocation.  */
+      || sig->sig_class == 0x20)
+    {
+      if (packet->pkttype != PKT_PUBLIC_KEY)
+        /* Key revocations can only be over primary keys.  */
+        return gpg_error (GPG_ERR_SIG_CLASS);
+    }
+  else if (/* Subkey binding.  */
+           sig->sig_class == 0x18
+           /* Subkey revocation.  */
+           || sig->sig_class == 0x28)
+    {
+      if (packet->pkttype != PKT_PUBLIC_SUBKEY)
+        return gpg_error (GPG_ERR_SIG_CLASS);
+    }
+  else if (/* Certification.  */
+           sig->sig_class == 0x10
+           || sig->sig_class == 0x11
+           || sig->sig_class == 0x12
+           || sig->sig_class == 0x13
+           /* Certification revocation.  */
+           || sig->sig_class == 0x30)
+    {
+      if (packet->pkttype != PKT_USER_ID)
+        return gpg_error (GPG_ERR_SIG_CLASS);
+    }
+  else
+    return gpg_error (GPG_ERR_SIG_CLASS);
+
+  /* PACKET is the right type for SIG.  */
+
+  if (signer)
+    {
+      if (is_selfsig)
+        {
+          if (signer->keyid[0] == pripk->keyid[0]
+              && signer->keyid[1] == pripk->keyid[1])
+            *is_selfsig = 1;
+          else
+            *is_selfsig = 0;
+        }
+    }
+  else
+    {
+      /* Get the signer.  If possible, avoid a look up.  */
+      if (sig->keyid[0] == pripk->keyid[0]
+          && sig->keyid[1] == pripk->keyid[1])
+        /* Issued by the primary key.  */
+        {
+          signer = pripk;
+          if (is_selfsig)
+            *is_selfsig = 1;
+        }
+      else
+        /* See if one of the subkeys was the signer (although this is
+           extremely unlikely).  */
+        {
+          kbnode_t ctx = NULL;
+          kbnode_t n;
+
+          while ((n = walk_kbnode (kb, &ctx, PKT_PUBLIC_SUBKEY)))
+            {
+              PKT_public_key *subk = n->pkt->pkt.public_key;
+              if (sig->keyid[0] == subk->keyid[0]
+                  && sig->keyid[1] == subk->keyid[1])
+                /* Issued by a subkey.  */
+                {
+                  signer = subk;
+                  break;
+                }
+            }
+
+          if (! signer)
+            /* Signer by some other key.  */
+            {
+              if (is_selfsig)
+                *is_selfsig = 0;
+              if (ret_pk)
+                {
+                  signer = ret_pk;
+                  memset (signer, 0, sizeof (*signer));
+                  signer_alloced = 1;
+                }
+              else
+                {
+                  signer = xmalloc_clear (sizeof (*signer));
+                  signer_alloced = 2;
+                }
+
+              rc = get_pubkey (signer, sig->keyid);
+              if (rc)
+                {
+                  xfree (signer);
+                  signer = NULL;
+                  signer_alloced = 0;
+                  goto out;
+                }
+            }
+        }
+    }
+
+  /* We checked above that we supported this algo, so an error here is
+     a bug.  */
+  if (gcry_md_open (&md, sig->digest_algo, 0))
+    BUG ();
+
+  /* Hash the relevant data.  */
+
+  if (/* Direct key signature.  */
+      sig->sig_class == 0x1f
+      /* Primary key revocation.  */
+      || sig->sig_class == 0x20)
+    {
+      assert (packet->pkttype == PKT_PUBLIC_KEY);
+      hash_public_key (md, packet->pkt.public_key);
+      rc = check_signature_end_simple (signer, sig, md);
+    }
+  else if (/* Primary key binding (made by a subkey).  */
+      sig->sig_class == 0x19)
+    {
+      assert (packet->pkttype == PKT_PUBLIC_KEY);
+      hash_public_key (md, packet->pkt.public_key);
+      hash_public_key (md, signer);
+      rc = check_signature_end_simple (signer, sig, md);
+    }
+  else if (/* Subkey binding.  */
+           sig->sig_class == 0x18
+           /* Subkey revocation.  */
+           || sig->sig_class == 0x28)
+    {
+      assert (packet->pkttype == PKT_PUBLIC_SUBKEY);
+      hash_public_key (md, pripk);
+      hash_public_key (md, packet->pkt.public_key);
+      rc = check_signature_end_simple (signer, sig, md);
+    }
+  else if (/* Certification.  */
+           sig->sig_class == 0x10
+           || sig->sig_class == 0x11
+           || sig->sig_class == 0x12
+           || sig->sig_class == 0x13
+           /* Certification revocation.  */
+           || sig->sig_class == 0x30)
+    {
+      assert (packet->pkttype == PKT_USER_ID);
+      hash_public_key (md, pripk);
+      hash_uid_packet (packet->pkt.user_id, md, sig);
+      rc = check_signature_end_simple (signer, sig, md);
+    }
+  else
+    /* We should never get here.  (The first if above should have
+       already caught this error.)  */
+    BUG ();
+
+  gcry_md_close (md);
+
+ out:
+  if (! rc && ret_pk && (signer_alloced == -1 || ret_pk != signer))
+    copy_public_key (ret_pk, signer);
+  if (signer_alloced == 1)
+    /* We looked up SIGNER; it is not a pointer into KB.  */
+    {
+      release_public_key_parts (signer);
+      if (signer_alloced == 2)
+        /* We also allocated the memory.  */
+        xfree (signer);
+    }
+
+  return rc;
+}
+
 /* Check that a signature over a key (e.g., a key revocation, key
  * binding, user id certification, etc.) is valid.  If the function
  * detects a self-signature, it uses the public key from the specified
@@ -730,7 +953,6 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk,
                       PKT_public_key *ret_pk, int *is_selfsig,
                       u32 *r_expiredate, int *r_expired )
 {
-  gcry_md_hd_t md;
   PKT_public_key *pk;
   PKT_signature *sig;
   int algo;
@@ -791,114 +1013,69 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk,
         rc = check_revocation_keys (pk, sig);
       else
         {
-          if (gcry_md_open (&md, algo, 0))
+          rc = check_signature_metadata_validity (pk, sig,
-            BUG ();
+                                                  r_expired, NULL);
-          hash_public_key (md, pk);
+          if (! rc)
-          rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
+            rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt,
-          cache_sig_result (sig, rc);
+                                                  is_selfsig, ret_pk);
-          gcry_md_close (md);
         }
     }
-  else if (sig->sig_class == 0x28) /* subkey revocation */
+  else if (sig->sig_class == 0x28  /* subkey revocation */
+           || sig->sig_class == 0x18) /* key binding */
     {
       kbnode_t snode = find_prev_kbnode (root, node, PKT_PUBLIC_SUBKEY);
 
       if (snode)
         {
-          if (gcry_md_open (&md, algo, 0))
+          rc = check_signature_metadata_validity (pk, sig,
-            BUG ();
+                                                  r_expired, NULL);
-          hash_public_key (md, pk);
+          if (! rc)
-          hash_public_key (md, snode->pkt->pkt.public_key);
+            /* 0x28 must be a self-sig, but 0x18 needn't be.  */
-          rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
+            rc = check_signature_over_key_or_uid (sig->sig_class == 0x18
-          cache_sig_result (sig, rc);
+                                                  ? NULL : pk,
-          gcry_md_close (md);
+                                                  sig, root, snode->pkt,
+                                                  is_selfsig, ret_pk);
 	}
       else
         {
           if (opt.verbose)
-            log_info (_("key %s: no subkey for subkey"
+            {
-                        " revocation signature\n"), keystr_from_pk(pk));
+              if (sig->sig_class == 0x28)
+                log_info (_("key %s: no subkey for subkey"
+                            " revocation signature\n"), keystr_from_pk(pk));
+              else if (sig->sig_class == 0x18)
+                log_info(_("key %s: no subkey for subkey"
+                           " binding signature\n"), keystr_from_pk(pk));
+            }
           rc = GPG_ERR_SIG_CLASS;
         }
     }
-    else if (sig->sig_class == 0x18) /* key binding */
-      {
-	kbnode_t snode = find_prev_kbnode (root, node, PKT_PUBLIC_SUBKEY);
-
-	if (snode)
-          {
-	    if (is_selfsig)
-              {
-                /* Does this make sense?  It should always be a
-                   selfsig.  Yes: We can't be sure about this and we
-                   need to be able to indicate that it is a selfsig.
-                   FIXME: The question is whether we should reject
-                   such a signature if it is not a selfsig.  */
-		u32 keyid[2];
-
-		keyid_from_pk (pk, keyid);
-		if (keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1])
-                  *is_selfsig = 1;
-              }
-	    if (gcry_md_open (&md, algo, 0))
-              BUG ();
-	    hash_public_key (md, pk);
-	    hash_public_key (md, snode->pkt->pkt.public_key);
-	    rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
-            cache_sig_result ( sig, rc );
-	    gcry_md_close (md);
-          }
-	else
-	  {
-            if (opt.verbose)
-	      log_info(_("key %s: no subkey for subkey"
-			 " binding signature\n"), keystr_from_pk(pk));
-	    rc = GPG_ERR_SIG_CLASS;
-	  }
-      }
     else if (sig->sig_class == 0x1f) /* direct key signature */
       {
-        if (gcry_md_open (&md, algo, 0 ))
+        rc = check_signature_metadata_validity (pk, sig,
-          BUG ();
+                                                r_expired, NULL);
-	hash_public_key( md, pk );
+        if (! rc)
-	rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
+          rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt,
-        cache_sig_result (sig, rc);
+                                                is_selfsig, ret_pk);
-	gcry_md_close (md);
       }
-    else /* all other classes */
+    else if (/* Certification.  */
+             sig->sig_class == 0x10
+             || sig->sig_class == 0x11
+             || sig->sig_class == 0x12
+             || sig->sig_class == 0x13
+             /* Certification revocation.  */
+             || sig->sig_class == 0x30)
       {
 	kbnode_t unode = find_prev_kbnode (root, node, PKT_USER_ID);
 
 	if (unode)
           {
-	    u32 keyid[2];
+            rc = check_signature_metadata_validity (pk, sig, r_expired, NULL);
-
+            if (! rc)
-	    keyid_from_pk (pk, keyid);
+              /* If this is a self-sig, ignore check_pk.  */
-	    if (gcry_md_open (&md, algo, 0))
+              rc = check_signature_over_key_or_uid
-              BUG ();
+                (keyid_cmp (pk_keyid (pk), sig->keyid) == 0 ? pk : check_pk,
-	    hash_public_key (md, pk);
+                 sig, root, unode->pkt, NULL, ret_pk);
-	    hash_uid_node (unode, md, sig);
-	    if (keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1])
-	      { /* The primary key is the signing key.  */
-
-		if (is_selfsig)
-		  *is_selfsig = 1;
-		rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
-	      }
-	    else if (check_pk)
-              { /* The caller specified a key.  Try that.  */
-
-                rc = check_signature_end (check_pk, sig, md,
-                                          r_expired, NULL, ret_pk);
-              }
-	    else
-              { /* Look up the key.  */
-                rc = check_signature2 (sig, md, r_expiredate, r_expired,
-                                       NULL, ret_pk);
-              }
-
-            cache_sig_result  (sig, rc);
-	    gcry_md_close (md);
           }
 	else
 	  {
@@ -908,6 +1085,10 @@ check_key_signature2 (kbnode_t root, kbnode_t node, PKT_public_key *check_pk,
 	    rc = GPG_ERR_SIG_CLASS;
 	  }
       }
+  else
+      BUG ();
+
+  cache_sig_result  (sig, rc);
 
   return rc;
 }