[git] GnuPG - branch, master, updated. gnupg-2.1.11-44-g2d1d795

by Neal H. Walfield cvs at cvs.gnupg.org
Fri Feb 19 16:40:05 CET 2016


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".

The branch, master has been updated
       via  2d1d795481bc011447284f8ce0a3ae96a08daf17 (commit)
       via  44cdb9d73f1a0b7d2c8483a119b9c4d6caabc1ec (commit)
       via  5fbd80579aea0f75ca1d2700515c5b8747a75c7d (commit)
       via  ac5aea95455372145f3f06df2b4c1584d759d660 (commit)
       via  10671c3a4c18ea26035a5819a9f2b8fd6c7e41ea (commit)
       via  bf9d1248c80205795e26156f67aff0b3f796cfce (commit)
       via  c45633a571bf663bc7f3610fc481acded6acfc19 (commit)
      from  7e7f35a2d7d40267a4dd30791df77420efeebfa7 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 2d1d795481bc011447284f8ce0a3ae96a08daf17
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 15:52:08 2016 +0100

    gpg: Systematically detect and fix signatures that are out of order.
    
    * g10/keyedit.c (sig_comparison): New function.
    (fix_key_signature_order): Merge functionality into...
    (check_all_keysigs): ... this function.  Rewrite to eliminate
    duplicates and use a systematic approach to detecting and moving
    signatures that are out of order instead of a heuristic.
    (fix_keyblock): Don't call fix_key_signature_order.  Call
    check_all_keysigs instead after collapsing the uids.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>
    GnuPG-bug-id: 2236

diff --git a/g10/keyedit.c b/g10/keyedit.c
index 8cec66a..d7c2a4b 100644
--- a/g10/keyedit.c
+++ b/g10/keyedit.c
@@ -322,97 +322,584 @@ print_and_check_one_sig (KBNODE keyblock, KBNODE node,
 
 
 
-/*
- * Check the keysigs and set the flags to indicate errors.
- * Returns true if error found.
- */
+/* Order two signatures.  The actual ordering isn't important.  Our
+   goal is to ensure that identical signatures occur together.  */
 static int
-check_all_keysigs (KBNODE keyblock, int only_selected, int only_selfsigs)
+sig_comparison (const void *av, const void *bv)
 {
-  KBNODE kbctx;
-  KBNODE node;
-  int inv_sigs = 0;
-  int no_key = 0;
-  int oth_err = 0;
-  int has_selfsig = 0;
-  int mis_selfsig = 0;
-  int selected = !only_selected;
-  int anyuid = 0;
-  u32 keyid[2];
+  const KBNODE an = *(const KBNODE *) av;
+  const KBNODE bn = *(const KBNODE *) bv;
+  const PKT_signature *a;
+  const PKT_signature *b;
+  int ndataa;
+  int ndatab;
+  int i;
+
+  assert (an->pkt->pkttype == PKT_SIGNATURE);
+  assert (bn->pkt->pkttype == PKT_SIGNATURE);
+
+  a = an->pkt->pkt.signature;
+  b = bn->pkt->pkt.signature;
 
-  for (kbctx = NULL; (node = walk_kbnode (keyblock, &kbctx, 0));)
+  if (a->digest_algo < b->digest_algo)
+    return -1;
+  if (a->digest_algo > b->digest_algo)
+    return 1;
+
+  ndataa = pubkey_get_nsig (a->pubkey_algo);
+  ndatab = pubkey_get_nsig (a->pubkey_algo);
+  assert (ndataa == ndatab);
+
+  for (i = 0; i < ndataa; i ++)
     {
-      if (node->pkt->pkttype == PKT_PUBLIC_KEY)
+      int c = gcry_mpi_cmp (a->data[i], b->data[i]);
+      if (c != 0)
+        return c;
+    }
+
+  /* Okay, they are equal.  */
+  return 0;
+}
+
+/* Perform a few sanity checks on a keyblock is okay and possibly
+   repair some damage.  Concretely:
+
+     - Detect duplicate signatures and remove them.
+
+     - Detect out of order signatures and relocate them (e.g., a sig
+       over user id X located under subkey Y).
+
+   Note: this function does not remove signatures that don't belong or
+   components that are not signed!  (Although it would be trivial to
+   do so.)
+
+   If ONLY_SELFSIGS is true, then this function only reorders self
+   signatures (it still checks all signatures for duplicates,
+   however).
+
+   Returns 1 if the keyblock was modified, 0 otherwise.  */
+static int
+check_all_keysigs (KBNODE kb, int only_selected, int only_selfsigs)
+{
+  gpg_error_t err;
+  PKT_public_key *pk;
+  KBNODE n, n_next, *n_prevp, n2;
+  char *pending_desc = NULL;
+  PKT_public_key *issuer;
+  KBNODE last_printed_component;
+  KBNODE current_component = NULL;
+  int dups = 0;
+  int missing_issuer = 0;
+  int reordered = 0;
+  int bad_signature = 0;
+  int missing_selfsig = 0;
+  int modified = 0;
+
+  assert (kb->pkt->pkttype == PKT_PUBLIC_KEY);
+  pk = kb->pkt->pkt.public_key;
+
+  /* First we look for duplicates.  */
+  {
+    int nsigs = 0;
+    KBNODE *sigs;
+    int i;
+    int last_i;
+
+    /* Count the sigs.  */
+    for (n = kb; n; n = n->next)
+      if (is_deleted_kbnode (n))
+        continue;
+      else if (n->pkt->pkttype == PKT_SIGNATURE)
+        nsigs ++;
+
+    /* Add them all to the SIGS array.  */
+    sigs = xmalloc_clear (sizeof (*sigs) * nsigs);
+
+    i = 0;
+    for (n = kb; n; n = n->next)
+      {
+        if (is_deleted_kbnode (n))
+          continue;
+
+        if (n->pkt->pkttype != PKT_SIGNATURE)
+          continue;
+
+        sigs[i] = n;
+        i ++;
+      }
+    assert (i == nsigs);
+
+    qsort (sigs, nsigs, sizeof (sigs[0]), sig_comparison);
+
+    last_i = 0;
+    for (i = 1; i < nsigs; i ++)
+      {
+        assert (sigs[last_i]);
+        assert (sigs[last_i]->pkt->pkttype == PKT_SIGNATURE);
+        assert (sigs[i]);
+        assert (sigs[i]->pkt->pkttype == PKT_SIGNATURE);
+
+        if (sig_comparison (&sigs[last_i], &sigs[i]) == 0)
+          /* They are the same.  Kill the latter.  */
+          {
+            if (DBG_PACKET)
+              {
+                PKT_signature *sig = sigs[i]->pkt->pkt.signature;
+
+                log_debug ("Signature appears multiple times, deleting duplicate:\n");
+                log_debug ("  sig: class 0x%x, issuer: %s, timestamp: %s (%lld), digest: %02x %02x\n",
+                           sig->sig_class, keystr (sig->keyid),
+                           isotimestamp (sig->timestamp),
+                           (long long) sig->timestamp,
+                           sig->digest_start[0], sig->digest_start[1]);
+              }
+
+            /* Remove sigs[i] from the keyblock.  */
+            {
+              KBNODE z, *prevp;
+              int to_kill = last_i;
+              last_i = i;
+
+              for (prevp = &kb, z = kb; z; prevp = &z->next, z = z->next)
+                if (z == sigs[to_kill])
+                  break;
+
+              *prevp = sigs[to_kill]->next;
+
+              sigs[to_kill]->next = NULL;
+              release_kbnode (sigs[to_kill]);
+              sigs[to_kill] = NULL;
+
+              dups ++;
+              modified = 1;
+            }
+          }
+        else
+          last_i = i;
+      }
+
+    xfree (sigs);
+  }
+
+  /* Make sure the sigs occur after the component (public key, subkey,
+     user id) that they sign.  */
+  issuer = NULL;
+  last_printed_component = NULL;
+  for (n_prevp = &kb, n = kb;
+       n;
+       /* If we moved n, then n_prevp is need valid.  */
+       n_prevp = (n->next == n_next ? &n->next : n_prevp), n = n_next)
+    {
+      PACKET *p;
+      int processed_current_component;
+      PKT_signature *sig;
+      int rc;
+      int dump_sig_params = 0;
+
+      n_next = n->next;
+
+      if (is_deleted_kbnode (n))
+        continue;
+
+      p = n->pkt;
+
+      if (issuer && issuer != pk)
         {
-          if (only_selfsigs)
-            keyid_from_pk (node->pkt->pkt.public_key, keyid);
+          free_public_key (issuer);
+          issuer = NULL;
         }
-      else if (node->pkt->pkttype == PKT_USER_ID)
-	{
-	  PKT_user_id *uid = node->pkt->pkt.user_id;
 
-	  if (only_selected)
-	    selected = (node->flag & NODFLG_SELUID);
-	  if (selected)
-	    {
-	      tty_printf ("uid  ");
-	      tty_print_utf8_string (uid->name, uid->len);
-	      tty_printf ("\n");
-	      if (anyuid && !has_selfsig)
-		mis_selfsig++;
-	      has_selfsig = 0;
-	      anyuid = 1;
-	    }
-	}
-      else if (selected && node->pkt->pkttype == PKT_SIGNATURE
-	       && ((node->pkt->pkt.signature->sig_class & ~3) == 0x10
-		   || node->pkt->pkt.signature->sig_class == 0x30))
-	{
-	  int selfsig;
-          PKT_signature *sig = node->pkt->pkt.signature;
+      xfree (pending_desc);
+      pending_desc = NULL;
 
-          if (only_selfsigs
-              && !(keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1]))
+      switch (p->pkttype)
+        {
+        case PKT_PUBLIC_KEY:
+          assert (p->pkt.public_key == pk);
+          if (only_selected && ! (n->flag & NODFLG_SELKEY))
             {
-              /* Not a selfsig but we want only selfsigs - skip.  */
-              /* Static analyzer note: A claim that KEYID above has
-                 garbage is not correct because KEYID is set from the
-                 public key packet which is always the first packet in
-                 a keyblock and thus parsed before this signature.  */
+              current_component = NULL;
+              break;
             }
-	  else if (print_and_check_one_sig (keyblock, node, &inv_sigs,
-                                            &no_key, &oth_err, &selfsig,
-                                            0, only_selfsigs))
-	    {
-	      if (selfsig)
-		has_selfsig = 1;
-	    }
-	  /* Hmmm: should we update the trustdb here? */
-	}
+
+          if (DBG_PACKET)
+            log_debug ("public key %s: timestamp: %s (%lld)\n",
+                       pk_keyid_str (pk),
+                       isotimestamp (pk->timestamp),
+                       (long long) pk->timestamp);
+          current_component = n;
+          break;
+        case PKT_PUBLIC_SUBKEY:
+          if (only_selected && ! (n->flag & NODFLG_SELKEY))
+            {
+              current_component = NULL;
+              break;
+            }
+
+          if (DBG_PACKET)
+            log_debug ("subkey %s: timestamp: %s (%lld)\n",
+                       pk_keyid_str (p->pkt.public_key),
+                       isotimestamp (p->pkt.public_key->timestamp),
+                       (long long) p->pkt.public_key->timestamp);
+          current_component = n;
+          break;
+        case PKT_USER_ID:
+          if (only_selected && ! (n->flag & NODFLG_SELUID))
+            {
+              current_component = NULL;
+              break;
+            }
+
+          if (DBG_PACKET)
+            log_debug ("user id: %s\n",
+                       p->pkt.user_id->attrib_data
+                       ? "[ photo id ]"
+                       : p->pkt.user_id->name);
+          current_component = n;
+          break;
+        case PKT_SIGNATURE:
+          if (! current_component)
+            /* The current component is not selected, don't check the
+               sigs under it.  */
+            break;
+
+          sig = n->pkt->pkt.signature;
+
+          pending_desc = xasprintf ("  sig: class: 0x%x, issuer: %s, timestamp: %s (%lld), digest: %02x %02x",
+                                    sig->sig_class,
+                                    keystr (sig->keyid),
+                                    isotimestamp (sig->timestamp),
+                                    (long long) sig->timestamp,
+                                    sig->digest_start[0], sig->digest_start[1]);
+
+
+          if (keyid_cmp (pk_keyid (pk), sig->keyid) == 0)
+            issuer = pk;
+          else
+            /* Issuer is a different key.  */
+            {
+              if (only_selfsigs)
+                continue;
+
+              issuer = xmalloc (sizeof (*issuer));
+              err = get_pubkey (issuer, sig->keyid);
+              if (err)
+                {
+                  xfree (issuer);
+                  issuer = NULL;
+                  if (DBG_PACKET)
+                    {
+                      if (pending_desc)
+                        log_debug ("%s", pending_desc);
+                      log_debug ("    Can't check signature allegedly issued by %s: %s\n",
+                                keystr (sig->keyid), gpg_strerror (err));
+                    }
+                  missing_issuer ++;
+                  break;
+                }
+            }
+
+          if ((err = openpgp_pk_test_algo (sig->pubkey_algo)))
+            {
+              if (DBG_PACKET && pending_desc)
+                log_debug ("%s", pending_desc);
+              tty_printf (_("can't check signature with unsupported public-key algorithm (%d): %s.\n"),
+                          sig->pubkey_algo, gpg_strerror (err));
+              break;
+            }
+          if ((err = openpgp_md_test_algo (sig->digest_algo)))
+            {
+              if (DBG_PACKET && pending_desc)
+                log_debug ("%s", pending_desc);
+              tty_printf (_("can't check signature with unsupported message-digest algorithm %d: %s.\n"),
+                          sig->digest_algo, gpg_strerror (err));
+              break;
+            }
+
+          /* We iterate over the keyblock.  Most likely, the matching
+             component is the current component so always try that
+             first.  */
+          processed_current_component = 0;
+          for (n2 = current_component;
+               n2;
+               n2 = (processed_current_component ? n2->next : kb),
+                 processed_current_component = 1)
+            if (is_deleted_kbnode (n2))
+              continue;
+            else if (processed_current_component && n2 == current_component)
+              /* Don't process it twice.  */
+              continue;
+            else
+              {
+                err = check_signature_over_key_or_uid (issuer, sig, kb, n2->pkt,
+                                                       NULL, NULL);
+                if (! err)
+                  break;
+              }
+
+          /* n/sig is a signature and n2 is the component (public key,
+             subkey or user id) that it signs, if any.
+             current_component is that component that it appears to
+             apply to (according to the ordering).  */
+
+          if (current_component == n2)
+            {
+              if (DBG_PACKET)
+                {
+                  log_debug ("%s", pending_desc);
+                  log_debug ("    Good signature over last key or uid!\n");
+                }
+
+              rc = 0;
+            }
+          else if (n2)
+            {
+              assert (n2->pkt->pkttype == PKT_USER_ID
+                      || n2->pkt->pkttype == PKT_PUBLIC_KEY
+                      || n2->pkt->pkttype == PKT_PUBLIC_SUBKEY);
+
+              if (DBG_PACKET)
+                {
+                  log_debug ("%s", pending_desc);
+                  log_debug ("    Good signature out of order!  (Over %s (%d) '%s')\n",
+                             n2->pkt->pkttype == PKT_USER_ID
+                             ? "user id"
+                             : n2->pkt->pkttype == PKT_PUBLIC_SUBKEY
+                             ? "subkey"
+                             : "primary key",
+                             n2->pkt->pkttype,
+                             n2->pkt->pkttype == PKT_USER_ID
+                             ? n2->pkt->pkt.user_id->name
+                             : pk_keyid_str (n2->pkt->pkt.public_key));
+                }
+
+              /* Reorder the packets: move the signature n to be just
+                 after n2.  */
+
+              /* Unlink the signature.  */
+              assert (n_prevp);
+              *n_prevp = n->next;
+
+              /* Insert the sig immediately after the component.  */
+              n->next = n2->next;
+              n2->next = n;
+
+              reordered ++;
+              modified = 1;
+
+              rc = 0;
+            }
+          else
+            {
+              if (DBG_PACKET)
+                {
+                  log_debug ("%s", pending_desc);
+                  log_debug ("    Bad signature.\n");
+                }
+
+              if (DBG_PACKET)
+                dump_sig_params = 1;
+
+              bad_signature ++;
+
+              rc = GPG_ERR_BAD_SIGNATURE;
+            }
+
+          /* We don't cache the result here, because we haven't
+             completely checked that the signature is legitimate.  For
+             instance, if we have a revocation certificate on Alice's
+             key signed by Bob, the signature may be good, but we
+             haven't checked that Bob is a designated revoker.  */
+          /* cache_sig_result (sig, rc); */
+
+          {
+            int has_selfsig = 0;
+            if (! rc && issuer == pk)
+              {
+                if (n2->pkt->pkttype == PKT_PUBLIC_KEY
+                    && (/* Direct key signature.  */
+                        sig->sig_class == 0x1f
+                        /* Key revocation signature.  */
+                        || sig->sig_class == 0x20))
+                  has_selfsig = 1;
+                if (n2->pkt->pkttype == PKT_PUBLIC_SUBKEY
+                    && (/* Subkey binding sig.  */
+                        sig->sig_class == 0x18
+                        /* Subkey revocation sig.  */
+                        || sig->sig_class == 0x28))
+                  has_selfsig = 1;
+                if (n2->pkt->pkttype == PKT_USER_ID
+                    && (/* Certification sigs.  */
+                        sig->sig_class == 0x10
+                        || sig->sig_class == 0x11
+                        || sig->sig_class == 0x12
+                        || sig->sig_class == 0x13
+                        /* Certification revocation sig.  */
+                        || sig->sig_class == 0x30))
+                  has_selfsig = 1;
+              }
+
+            if ((n2 && n2 != last_printed_component)
+                || (! n2 && last_printed_component != current_component))
+              {
+                int is_reordered = n2 && n2 != current_component;
+                if (n2)
+                  last_printed_component = n2;
+                else
+                  last_printed_component = current_component;
+
+                if (last_printed_component->pkt->pkttype == PKT_USER_ID)
+                  {
+                    tty_printf ("uid  ");
+                    tty_print_utf8_string (last_printed_component
+                                           ->pkt->pkt.user_id->name,
+                                           last_printed_component
+                                           ->pkt->pkt.user_id->len);
+                  }
+                else if (last_printed_component->pkt->pkttype
+                         == PKT_PUBLIC_KEY)
+                  tty_printf ("pub  %s",
+                              pk_keyid_str (last_printed_component
+                                            ->pkt->pkt.public_key));
+                else
+                  tty_printf ("sub  %s",
+                              pk_keyid_str (last_printed_component
+                                            ->pkt->pkt.public_key));
+
+                if (is_reordered)
+                  tty_printf (_(" (reordered signatures follow)"));
+                tty_printf ("\n");
+              }
+
+            print_one_sig (rc, kb, n, NULL, NULL, NULL, has_selfsig,
+                           0, only_selfsigs);
+          }
+
+          if (dump_sig_params)
+            {
+              int i;
+
+              for (i = 0; i < pubkey_get_nsig (sig->pubkey_algo); i ++)
+                {
+                  char buffer[1024];
+                  size_t len;
+                  char *printable;
+                  gcry_mpi_print (GCRYMPI_FMT_USG,
+                                  buffer, sizeof (buffer), &len,
+                                  sig->data[i]);
+                  printable = bin2hex (buffer, len, NULL);
+                  log_info ("        %d: %s\n", i, printable);
+                  xfree (printable);
+                }
+            }
+          break;
+        default:
+          if (DBG_PACKET)
+            log_debug ("unhandled packet: %d\n", p->pkttype);
+          break;
+        }
     }
-  if (!has_selfsig)
-    mis_selfsig++;
 
-  if (inv_sigs)
-    tty_printf (ngettext("%d bad signature\n",
-                         "%d bad signatures\n", inv_sigs), inv_sigs);
+  xfree (pending_desc);
+  pending_desc = NULL;
 
-  if (no_key)
-    tty_printf (ngettext("%d signature not checked due to a missing key\n",
-                         "%d signatures not checked due to missing keys\n",
-                         no_key), no_key);
+  if (issuer != pk)
+    free_public_key (issuer);
+  issuer = NULL;
 
-  if (oth_err)
-    tty_printf (ngettext("%d signature not checked due to an error\n",
-                         "%d signatures not checked due to errors\n",
-                         oth_err), oth_err);
+  /* Identify keys / uids that don't have a self-sig.  */
+  {
+    int has_selfsig = 0;
+    PACKET *p;
+    PKT_signature *sig;
+
+    current_component = NULL;
+    for (n = kb; n; n = n->next)
+      {
+        if (is_deleted_kbnode (n))
+          continue;
+
+        p = n->pkt;
+
+        switch (p->pkttype)
+          {
+          case PKT_PUBLIC_KEY:
+          case PKT_PUBLIC_SUBKEY:
+          case PKT_USER_ID:
+            if (current_component && ! has_selfsig)
+              missing_selfsig ++;
+            current_component = n;
+            has_selfsig = 0;
+            break;
 
-  if (mis_selfsig)
-    tty_printf (ngettext("%d user ID without valid self-signature detected\n",
-                         "%d user IDs without valid self-signatures detected\n",
-                         mis_selfsig), mis_selfsig);
+          case PKT_SIGNATURE:
+            if (! current_component || has_selfsig)
+              break;
+
+            sig = n->pkt->pkt.signature;
+
+            if (! (sig->flags.checked && sig->flags.valid))
+              break;
+
+            if (keyid_cmp (pk_keyid (pk), sig->keyid) != 0)
+              /* Different issuer, couldn't be a self-sig.  */
+              break;
+
+            if (current_component->pkt->pkttype == PKT_PUBLIC_KEY
+                && (/* Direct key signature.  */
+                    sig->sig_class == 0x1f
+                    /* Key revocation signature.  */
+                    || sig->sig_class == 0x20))
+              has_selfsig = 1;
+            if (current_component->pkt->pkttype == PKT_PUBLIC_SUBKEY
+                && (/* Subkey binding sig.  */
+                    sig->sig_class == 0x18
+                    /* Subkey revocation sig.  */
+                    || sig->sig_class == 0x28))
+              has_selfsig = 1;
+            if (current_component->pkt->pkttype == PKT_USER_ID
+                && (/* Certification sigs.  */
+                    sig->sig_class == 0x10
+                    || sig->sig_class == 0x11
+                    || sig->sig_class == 0x12
+                    || sig->sig_class == 0x13
+                    /* Certification revocation sig.  */
+                    || sig->sig_class == 0x30))
+              has_selfsig = 1;
+
+            break;
+
+          default:
+            if (current_component && ! has_selfsig)
+              missing_selfsig ++;
+            current_component = NULL;
+          }
+      }
+  }
 
-  return inv_sigs || no_key || oth_err || mis_selfsig;
+  if (dups || missing_issuer || bad_signature || reordered)
+    tty_printf (_("key %s:\n"), pk_keyid_str (pk));
+
+  if (dups)
+    tty_printf (ngettext ("%d duplicate signature removed\n",
+                          "%d duplicate signatures removed\n", dups), dups);
+  if (missing_issuer)
+    tty_printf (ngettext ("%d signature not checked due to a missing key\n",
+                          "%d signatures not checked due to missing keys\n",
+                          missing_issuer), missing_issuer);
+  if (bad_signature)
+    tty_printf (ngettext ("%d bad signature\n",
+                          "%d bad signatures\n",
+                          bad_signature), bad_signature);
+  if (reordered)
+    tty_printf (ngettext ("%d signature reordered\n",
+                          "%d signatures reordered\n",
+                          reordered), reordered);
+
+  if (only_selfsigs && (bad_signature || reordered))
+    tty_printf (_("Warning: errors found and only checked self-signatures, run 'check' to check all signatures.\n"));
+
+  return modified;
 }
 
 
@@ -1247,54 +1734,6 @@ change_passphrase (ctrl_t ctrl, kbnode_t keyblock)
 
 
 

-/*
- * There are some keys out (due to a bug in gnupg), where the sequence
- * of the packets is wrong.  This function fixes that.
- * Returns: true if the keyblock has been fixed.
- *
- * Note:  This function does not work if there is more than one user ID.
- */
-static int
-fix_key_signature_order (KBNODE keyblock)
-{
-  KBNODE node, last, subkey;
-  int fixed = 0;
-
-  /* Locate key signatures of class 0x10..0x13 behind sub key packets.  */
-  for (subkey = last = NULL, node = keyblock; node;
-       last = node, node = node->next)
-    {
-      switch (node->pkt->pkttype)
-	{
-	case PKT_PUBLIC_SUBKEY:
-	case PKT_SECRET_SUBKEY:
-	  if (!subkey)
-	    subkey = last; /* Actually it is the one before the subkey.  */
-	  break;
-	case PKT_SIGNATURE:
-	  if (subkey)
-	    {
-	      PKT_signature *sig = node->pkt->pkt.signature;
-	      if (sig->sig_class >= 0x10 && sig->sig_class <= 0x13)
-		{
-		  log_info (_("moving a key signature to the correct place\n"));
-		  last->next = node->next;
-		  node->next = subkey->next;
-		  subkey->next = node;
-		  node = last;
-		  fixed = 1;
-		}
-	    }
-	  break;
-	default:
-	  break;
-	}
-    }
-
-  return fixed;
-}
-
-
 /* Fix various problems in the keyblock.  Returns true if the keyblock
    was changed.  Note that a pointer to the keyblock must be given and
    the function may change it (i.e. replacing the first node).  */
@@ -1303,10 +1742,10 @@ fix_keyblock (kbnode_t *keyblockp)
 {
   int changed = 0;
 
-  if (fix_key_signature_order (*keyblockp))
-    changed++;
   if (collapse_uids (keyblockp))
     changed++;
+  if (check_all_keysigs (*keyblockp, 0, 1))
+    changed++;
   reorder_keyblock (*keyblockp);
   /* If we modified the keyblock, make sure the flags are right. */
   if (changed)

commit 44cdb9d73f1a0b7d2c8483a119b9c4d6caabc1ec
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 15:30:03 2016 +0100

    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 at g10code.com>

diff --git a/g10/main.h b/g10/main.h
index 863afa9..f7c47e9 100644
--- a/g10/main.h
+++ b/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);
 
diff --git a/g10/sig-check.c b/g10/sig-check.c
index 262afed..4530a64 100644
--- a/g10/sig-check.c
+++ b/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))
-            BUG ();
-          hash_public_key (md, pk);
-          rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
-          cache_sig_result (sig, rc);
-          gcry_md_close (md);
+          rc = check_signature_metadata_validity (pk, sig,
+                                                  r_expired, NULL);
+          if (! rc)
+            rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt,
+                                                  is_selfsig, ret_pk);
         }
     }
-  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))
-            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);
+          rc = check_signature_metadata_validity (pk, sig,
+                                                  r_expired, NULL);
+          if (! rc)
+            /* 0x28 must be a self-sig, but 0x18 needn't be.  */
+            rc = check_signature_over_key_or_uid (sig->sig_class == 0x18
+                                                  ? NULL : pk,
+                                                  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 ))
-          BUG ();
-	hash_public_key( md, pk );
-	rc = check_signature_end (pk, sig, md, r_expired, NULL, ret_pk);
-        cache_sig_result (sig, rc);
-	gcry_md_close (md);
+        rc = check_signature_metadata_validity (pk, sig,
+                                                r_expired, NULL);
+        if (! rc)
+          rc = check_signature_over_key_or_uid (pk, sig, root, root->pkt,
+                                                is_selfsig, ret_pk);
       }
-    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];
-
-	    keyid_from_pk (pk, keyid);
-	    if (gcry_md_open (&md, algo, 0))
-              BUG ();
-	    hash_public_key (md, 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);
+            rc = check_signature_metadata_validity (pk, sig, r_expired, NULL);
+            if (! rc)
+              /* If this is a self-sig, ignore check_pk.  */
+              rc = check_signature_over_key_or_uid
+                (keyid_cmp (pk_keyid (pk), sig->keyid) == 0 ? pk : check_pk,
+                 sig, root, unode->pkt, NULL, ret_pk);
           }
 	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;
 }

commit 5fbd80579aea0f75ca1d2700515c5b8747a75c7d
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 15:13:22 2016 +0100

    gpg: Split print_and_check_one_sig.
    
    * g10/keyedit.c (print_and_check_one_sig): Split the print
    functionality into...
    (print_one_sig): ... this new function.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>

diff --git a/g10/keyedit.c b/g10/keyedit.c
index f2ef613..8cec66a 100644
--- a/g10/keyedit.c
+++ b/g10/keyedit.c
@@ -1,7 +1,7 @@
 /* keyedit.c - Edit properties of a key
  * Copyright (C) 1998-2010 Free Software Foundation, Inc.
  * Copyright (C) 1998-2015 Werner Koch
- * Copyright (C) 2015 g10 Code GmbH
+ * Copyright (C) 2015, 2016 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -189,24 +189,23 @@ print_and_check_one_sig_colon (KBNODE keyblock, KBNODE node,
 
 
 /*
- * Print information about a signature, check it and return true if
- * the signature is okay.  NODE must be a signature packet.  With
- * EXTENDED set all possible signature list options will always be
- * printed.
+ * Print information about a signature (rc is its status), check it
+ * and return true if the signature is okay.  NODE must be a signature
+ * packet.  With EXTENDED set all possible signature list options will
+ * always be printed.
  */
 static int
-print_and_check_one_sig (KBNODE keyblock, KBNODE node,
-			 int *inv_sigs, int *no_key, int *oth_err,
-			 int *is_selfsig, int print_without_key, int extended)
+print_one_sig (int rc, KBNODE keyblock, KBNODE node,
+               int *inv_sigs, int *no_key, int *oth_err,
+               int is_selfsig, int print_without_key, int extended)
 {
   PKT_signature *sig = node->pkt->pkt.signature;
-  int rc, sigrc;
+  int sigrc;
   int is_rev = sig->sig_class == 0x30;
 
   /* TODO: Make sure a cached sig record here still has the pk that
      issued it.  See also keylist.c:list_keyblock_print */
 
-  rc = check_key_signature (keyblock, node, is_selfsig);
   switch (gpg_err_code (rc))
     {
     case 0:
@@ -257,7 +256,7 @@ print_and_check_one_sig (KBNODE keyblock, KBNODE node,
 	tty_printf ("[%s] ", gpg_strerror (rc));
       else if (sigrc == '?')
 	;
-      else if (*is_selfsig)
+      else if (is_selfsig)
 	{
 	  tty_printf (is_rev ? _("[revocation]") : _("[self-signature]"));
           if (extended && sig->flags.chosen_selfsig)
@@ -311,6 +310,16 @@ print_and_check_one_sig (KBNODE keyblock, KBNODE node,
   return (sigrc == '!');
 }
 
+static int
+print_and_check_one_sig (KBNODE keyblock, KBNODE node,
+			 int *inv_sigs, int *no_key, int *oth_err,
+			 int *is_selfsig, int print_without_key, int extended)
+{
+  return print_one_sig (check_key_signature (keyblock, node, is_selfsig),
+                        keyblock, node, inv_sigs, no_key, oth_err,
+                        *is_selfsig, print_without_key, extended);
+}
+
 
 
 /*

commit ac5aea95455372145f3f06df2b4c1584d759d660
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 14:10:09 2016 +0100

    gpg: Split the function check_signature_end.
    
    * g10/sig-check.c (check_signature_end): Break the basic signature
    check into...
    (check_signature_end_simple): ... this new function.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>

diff --git a/g10/sig-check.c b/g10/sig-check.c
index 292adb9..262afed 100644
--- a/g10/sig-check.c
+++ b/g10/sig-check.c
@@ -40,6 +40,9 @@ static int check_signature_end (PKT_public_key *pk, PKT_signature *sig,
 				int *r_expired, int *r_revoked,
 				PKT_public_key *ret_pk);
 
+static int check_signature_end_simple (PKT_public_key *pk, PKT_signature *sig,
+                                       gcry_md_hd_t digest);
+
 /* Check a signature.  This is shorthand for check_signature2 with
    the unnamed arguments passed as NULL.  */
 int
@@ -376,14 +379,32 @@ check_signature_end (PKT_public_key *pk, PKT_signature *sig,
 		     gcry_md_hd_t digest,
 		     int *r_expired, int *r_revoked, PKT_public_key *ret_pk)
 {
-    gcry_mpi_t result = NULL;
     int rc = 0;
-    const struct weakhash *weak;
 
     if ((rc = check_signature_metadata_validity (pk, sig,
 						 r_expired, r_revoked)))
         return rc;
 
+    if ((rc = check_signature_end_simple (pk, sig, digest)))
+      return rc;
+
+    if(!rc && ret_pk)
+      copy_public_key(ret_pk,pk);
+
+    return rc;
+}
+
+/* This function is similar to check_signature_end, but it only checks
+   whether the signature was generated by PK.  It does not check
+   expiration, revocation, etc.  */
+static int
+check_signature_end_simple (PKT_public_key *pk, PKT_signature *sig,
+                            gcry_md_hd_t digest)
+{
+    gcry_mpi_t result = NULL;
+    int rc = 0;
+    const struct weakhash *weak;
+
     if (!opt.flags.allow_weak_digest_algos)
       for (weak = opt.weak_digests; weak; weak = weak->next)
         if (sig->digest_algo == weak->algo)
@@ -453,9 +474,6 @@ check_signature_end (PKT_public_key *pk, PKT_signature *sig,
 	rc = GPG_ERR_BAD_SIGNATURE;
       }
 
-    if(!rc && ret_pk)
-      copy_public_key(ret_pk,pk);
-
     return rc;
 }
 

commit 10671c3a4c18ea26035a5819a9f2b8fd6c7e41ea
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 14:59:19 2016 +0100

    gpg: Use format_keyid rather than manually formatting the keyid.
    
    * g10/keyedit.c (menu_addrevoker): Use format_keyid rather than
    manually formatting the keyid.
    * g10/keygen.c (card_write_key_to_backup_file): Likewise.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>

diff --git a/g10/keyedit.c b/g10/keyedit.c
index 19ddf29..f2ef613 100644
--- a/g10/keyedit.c
+++ b/g10/keyedit.c
@@ -3953,8 +3953,7 @@ menu_addrevoker (ctrl_t ctrl, kbnode_t pub_keyblock, int sensitive)
 		  log_error (_("this key has already been designated "
 			       "as a revoker\n"));
 
-		  sprintf (buf, "%08lX%08lX",
-			   (ulong) pk->keyid[0], (ulong) pk->keyid[1]);
+                  format_keyid (pk_keyid (pk), KF_LONG, buf, sizeof (buf));
 		  write_status_text (STATUS_ALREADY_SIGNED, buf);
 
 		  break;
diff --git a/g10/keygen.c b/g10/keygen.c
index be132bb..8f25fe9 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -3840,15 +3840,15 @@ card_write_key_to_backup_file (PKT_public_key *sk, const char *backup_dir)
 {
   gpg_error_t err = 0;
   int rc;
+  char keyid_buffer[2 * 8 + 1];
   char name_buffer[50];
   char *fname;
   IOBUF fp;
   mode_t oldmask;
   PACKET *pkt = NULL;
 
-  keyid_from_pk (sk, NULL);
-  snprintf (name_buffer, sizeof name_buffer, "sk_%08lX%08lX.gpg",
-            (ulong)sk->keyid[0], (ulong)sk->keyid[1]);
+  format_keyid (pk_keyid (sk), KF_LONG, keyid_buffer, sizeof (keyid_buffer));
+  snprintf (name_buffer, sizeof name_buffer, "sk_%s.gpg", keyid_buffer);
 
   fname = make_filename (backup_dir, name_buffer, NULL);
   /* Note that the umask call is not anymore needed because

commit bf9d1248c80205795e26156f67aff0b3f796cfce
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 14:53:29 2016 +0100

    gpg: Initialize the primary key when generating a key.
    
    * g10/keygen.c (do_generate_keypair): Initialize
    pri_psk->flags.primary, pri_psk->keyid and pri_psk->main_keyid.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>

diff --git a/g10/keygen.c b/g10/keygen.c
index 7b5a35b..be132bb 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -4079,6 +4079,13 @@ do_generate_keypair (ctrl_t ctrl, struct para_data_s *para,
       assert (pri_psk);
     }
 
+  /* Make sure a few fields are correctly set up before going further.  */
+  pri_psk->flags.primary = 1;
+  keyid_from_pk (pri_psk, NULL);
+  /* We don't use pk_keyid to get keyid, because it also asserts that
+     main_keyid is set!  */
+  keyid_copy (pri_psk->main_keyid, pri_psk->keyid);
+
   if (!err && (revkey = get_parameter_revkey (para, pREVOKER)))
     err = write_direct_sig (pub_root, pri_psk, revkey, timestamp, cache_nonce);
 

commit c45633a571bf663bc7f3610fc481acded6acfc19
Author: Neal H. Walfield <neal at g10code.com>
Date:   Fri Feb 19 14:48:56 2016 +0100

    gpg: Add accessor & utility functions for pk->keyid and pk->main_keyid.
    
    * g10/keydb.h (keyid_cmp): New function.
    * g10/keyid.c (pk_keyid): New function.
    (pk_main_keyid): New function.
    (keyid_copy): New function.
    (pk_keyid_str): New function.
    * g10/packet.h (PKT_public_key): Update comments for main_keyid and
    keyid.
    
    --
    Signed-off-by: Neal H. Walfield <neal at g10code.com>
    
    Before accessing pk->keyid, it is necessary to call keyid_from_pk (pk,
    NULL) to ensure that pk->keyid is valid.  Because it is easy to forget
    to do this, these accessor functions take care of it.

diff --git a/g10/keydb.h b/g10/keydb.h
index 8d4e36c..8896eea 100644
--- a/g10/keydb.h
+++ b/g10/keydb.h
@@ -1,7 +1,7 @@
 /* keydb.h - Key database
  * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
  *               2006, 2010 Free Software Foundation, Inc.
- * Copyright (C) 2015 g10 Code GmbH
+ * Copyright (C) 2015, 2016 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -396,12 +396,44 @@ char *pubkey_string (PKT_public_key *pk, char *buffer, size_t bufsize);
 u32 v3_keyid (gcry_mpi_t a, u32 *ki);
 void hash_public_key( gcry_md_hd_t md, PKT_public_key *pk );
 char *format_keyid (u32 *keyid, int format, char *buffer, int len);
+
+/* Return PK's keyid.  The memory is owned by PK.  */
+u32 *pk_keyid (PKT_public_key *pk);
+
+/* Return the keyid of the primary key associated with PK.  The memory
+   is owned by PK.  */
+u32 *pk_main_keyid (PKT_public_key *pk);
+
+/* Order A and B.  If A < B then return -1, if A == B then return 0,
+   and if A > B then return 1.  */
+static int GPGRT_ATTR_UNUSED
+keyid_cmp (const u32 *a, const u32 *b)
+{
+  if (a[0] < b[0])
+    return -1;
+  if (a[0] > b[0])
+    return 1;
+  if (a[1] < b[1])
+    return -1;
+  if (a[1] > b[1])
+    return 1;
+  return 0;
+}
+
+/* Copy the keyid in SRC to DEST and return DEST.  */
+u32 *keyid_copy (u32 *dest, const u32 *src);
+
 size_t keystrlen(void);
 const char *keystr(u32 *keyid);
 const char *keystr_with_sub (u32 *main_kid, u32 *sub_kid);
 const char *keystr_from_pk(PKT_public_key *pk);
 const char *keystr_from_pk_with_sub (PKT_public_key *main_pk,
                                      PKT_public_key *sub_pk);
+
+/* Return PK's key id as a string using the default format.  PK owns
+   the storage.  */
+const char *pk_keyid_str (PKT_public_key *pk);
+
 const char *keystr_from_desc(KEYDB_SEARCH_DESC *desc);
 u32 keyid_from_pk( PKT_public_key *pk, u32 *keyid );
 u32 keyid_from_sig( PKT_signature *sig, u32 *keyid );
diff --git a/g10/keyid.c b/g10/keyid.c
index 49eb5f6..f2a5e03 100644
--- a/g10/keyid.c
+++ b/g10/keyid.c
@@ -2,6 +2,7 @@
  * Copyright (C) 1998, 1999, 2000, 2001, 2003,
  *               2004, 2006, 2010 Free Software Foundation, Inc.
  * Copyright (C) 2014 Werner Koch
+ * Copyright (C) 2016 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -274,6 +275,52 @@ v3_keyid (gcry_mpi_t a, u32 *ki)
 }
 
 
+/* Return PK's keyid.  The memory is owned by PK.  */
+u32 *
+pk_keyid (PKT_public_key *pk)
+{
+  keyid_from_pk (pk, NULL);
+
+  /* Uncomment this for help tracking down bugs related to keyid or
+     main_keyid not being set correctly.  */
+#if 0
+  if (! (pk->main_keyid[0] || pk->main_keyid[1]))
+    log_bug ("pk->main_keyid not set!\n");
+  if (keyid_cmp (pk->keyid, pk->main_keyid) == 0
+      && ! pk->flags.primary)
+    log_bug ("keyid and main_keyid are the same, but primary flag not set!\n");
+  if (keyid_cmp (pk->keyid, pk->main_keyid) != 0
+      && pk->flags.primary)
+    log_bug ("keyid and main_keyid are different, but primary flag set!\n");
+#endif
+
+  return pk->keyid;
+}
+
+/* Return the keyid of the primary key associated with PK.  The memory
+   is owned by PK.  */
+u32 *
+pk_main_keyid (PKT_public_key *pk)
+{
+  /* Uncomment this for help tracking down bugs related to keyid or
+     main_keyid not being set correctly.  */
+#if 0
+  if (! (pk->main_keyid[0] || pk->main_keyid[1]))
+    log_bug ("pk->main_keyid not set!\n");
+#endif
+
+  return pk->main_keyid;
+}
+
+/* Copy the keyid in SRC to DEST and return DEST.  */
+u32 *
+keyid_copy (u32 *dest, const u32 *src)
+{
+  dest[0] = src[0];
+  dest[1] = src[1];
+  return dest;
+}
+
 char *
 format_keyid (u32 *keyid, int format, char *buffer, int len)
 {
@@ -396,6 +443,14 @@ keystr_from_pk_with_sub (PKT_public_key *main_pk, PKT_public_key *sub_pk)
 }
 
 
+/* Return PK's key id as a string using the default format.  PK owns
+   the storage.  */
+const char *
+pk_keyid_str (PKT_public_key *pk)
+{
+  return keystr (pk_keyid (pk));
+}
+
 
 const char *
 keystr_from_desc(KEYDB_SEARCH_DESC *desc)
diff --git a/g10/packet.h b/g10/packet.h
index 16524f8..dfd3a00 100644
--- a/g10/packet.h
+++ b/g10/packet.h
@@ -279,8 +279,12 @@ typedef struct
   byte    pubkey_usage;   /* for now only used to pass it to getkey() */
   byte    req_usage;      /* hack to pass a request to getkey() */
   u32     has_expired;    /* set to the expiration date if expired */
-  u32     main_keyid[2];  /* keyid of the primary key */
-  u32     keyid[2];	    /* calculated by keyid_from_pk() */
+  /* keyid of the primary key.  Never access this value directly.
+     Instead, use pk_main_keyid().  */
+  u32     main_keyid[2];
+  /* keyid of this key.  Never access this value directly!  Instead,
+     use pk_keyid().  */
+  u32     keyid[2];
   prefitem_t *prefs;      /* list of preferences (may be NULL) */
   struct
   {

-----------------------------------------------------------------------

Summary of changes:
 g10/keydb.h     |  34 ++-
 g10/keyedit.c   | 723 +++++++++++++++++++++++++++++++++++++++++++++-----------
 g10/keygen.c    |  13 +-
 g10/keyid.c     |  55 +++++
 g10/main.h      |  13 +
 g10/packet.h    |   8 +-
 g10/sig-check.c | 393 ++++++++++++++++++++++--------
 7 files changed, 998 insertions(+), 241 deletions(-)


hooks/post-receive
-- 
The GNU Privacy Guard
http://git.gnupg.org




More information about the Gnupg-commits mailing list