[git] GnuPG - branch, STABLE-BRANCH-2-2, updated. gnupg-2.2.7-18-g6a87a0b

by Werner Koch cvs at cvs.gnupg.org
Thu Jun 7 18:53:41 CEST 2018


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, STABLE-BRANCH-2-2 has been updated
       via  6a87a0bd2501d82f4a6263608e4856e841305caf (commit)
       via  cedd754fcb03f6dad6e462efc3d347bcef4ec83a (commit)
       via  36cc730fa516b3a197f3bb1eb6f3881dd128fbb7 (commit)
      from  c5c8fb1ec7c8690495de6189ec2c3a322db4e881 (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 6a87a0bd2501d82f4a6263608e4856e841305caf
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Jun 7 18:41:17 2018 +0200

    gpg: Improve import's repair-key duplicate signature detection.
    
    * g10/key-check.c (key_check_all_keysigs): Factor some code out to ...
    (remove_duplicate_sigs): new.
    (key_check_all_keysigs): Call remove_duplicate_sigs again after
    reordering.
    --
    
    This is a follupup for commit 26bce2f01d2029ea2b8a8dbbe36118e3c83c5cba
    to cleanup the code and to add a second de-duplicate step when needed.
    
    GnuPG-bug-id: 3994
    Signed-off-by: Werner Koch <wk at gnupg.org>
    (cherry picked from commit 26746fe65d14a00773473c2d0d271406a5105bca)

diff --git a/g10/key-check.c b/g10/key-check.c
index 17f2dae..c17b12c 100644
--- a/g10/key-check.c
+++ b/g10/key-check.c
@@ -1,7 +1,7 @@
 /* key-check.c - Detect and fix various problems with keys
  * Copyright (C) 1998-2010 Free Software Foundation, Inc.
  * Copyright (C) 1998-2017 Werner Koch
- * Copyright (C) 2015-2017 g10 Code GmbH
+ * Copyright (C) 2015-2018 g10 Code GmbH
  *
  * This file is part of GnuPG.
  *
@@ -101,6 +101,125 @@ sig_comparison (const void *av, const void *bv)
 }
 
 
+static gpg_error_t
+remove_duplicate_sigs (kbnode_t kb, int *dups, int *modified)
+{
+  gpg_error_t err;
+  kbnode_t n;
+  int nsigs;
+  kbnode_t *sigs;  /* Allocated array with the signature packet.  */
+  int i;
+  int last_i;
+  int block;
+  PKT_signature *sig;
+
+  /* Count the sigs.  */
+  for (nsigs = 0, n = kb; n; n = n->next)
+    {
+      if (is_deleted_kbnode (n))
+        continue;
+      else if (n->pkt->pkttype == PKT_SIGNATURE)
+        nsigs ++;
+    }
+
+  if (!nsigs)
+    return 0; /* No signatures at all.  */
+
+  /* Add them all to the SIGS array.  */
+  sigs = xtrycalloc (nsigs, sizeof *sigs);
+  if (!sigs)
+    {
+      err = gpg_error_from_syserror ();
+      log_error (_("error allocating memory: %s\n"), gpg_strerror (err));
+      return err;
+    }
+
+  block = 0;
+  i = 0;
+  for (n = kb; n; n = n->next)
+    {
+      if (is_deleted_kbnode (n))
+        continue;
+
+      if (n->pkt->pkttype != PKT_SIGNATURE)
+        {
+          switch (n->pkt->pkttype)
+            {
+            case PKT_PUBLIC_SUBKEY:
+            case PKT_SECRET_SUBKEY:
+            case PKT_USER_ID:
+            case PKT_ATTRIBUTE:
+              /* Bump the block number so that we only consider
+               * signatures below the same object as duplicates.  */
+              block++;
+              break;
+            default:
+              break;
+            }
+          continue;
+        }
+      sig = n->pkt->pkt.signature;
+      sig->help_counter = block;
+      sigs[i++] = n;
+    }
+  log_assert (i == nsigs);
+
+  qsort (sigs, nsigs, sizeof (sigs[0]), sig_comparison);
+
+  last_i = 0;
+  for (i = 1; i < nsigs; i ++)
+    {
+      log_assert (sigs[last_i]);
+      log_assert (sigs[last_i]->pkt->pkttype == PKT_SIGNATURE);
+      log_assert (sigs[i]);
+      log_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)
+            {
+              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_t 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);
+  return 0;
+}
+
+
 /* Perform a few sanity checks on a keyblock is okay and possibly
  * repair some damage.  Concretely:
  *
@@ -146,121 +265,11 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
   pk = kb->pkt->pkt.public_key;
 
   /* First we look for duplicates.  */
-  {
-    int nsigs;
-    kbnode_t *sigs;
-    int i;
-    int last_i;
-    int block;
-
-    /* Count the sigs.  */
-    for (nsigs = 0, n = kb; n; n = n->next)
-      {
-        if (is_deleted_kbnode (n))
-          continue;
-        else if (n->pkt->pkttype == PKT_SIGNATURE)
-          nsigs ++;
-      }
-
-    if (!nsigs)
-      return 0; /* No signatures at all.  */
-
-    /* Add them all to the SIGS array.  */
-    sigs = xtrycalloc (nsigs, sizeof *sigs);
-    if (!sigs)
-      {
-        log_error (_("error allocating memory: %s\n"),
-                   gpg_strerror (gpg_error_from_syserror ()));
-        return 0;
-      }
-
-    i = 0;
-    block = 0;
-    for (n = kb; n; n = n->next)
-      {
-        if (is_deleted_kbnode (n))
-          continue;
-
-        if (n->pkt->pkttype != PKT_SIGNATURE)
-          {
-            switch (n->pkt->pkttype)
-              {
-              case PKT_PUBLIC_SUBKEY:
-              case PKT_SECRET_SUBKEY:
-              case PKT_USER_ID:
-              case PKT_ATTRIBUTE:
-                /* Bump the block number so that we only consider
-                 * signatures below the same object as duplicates.  */
-                block++;
-                break;
-              default:
-                break;
-              }
-            continue;
-          }
-        sig = n->pkt->pkt.signature;
-        sig->help_counter = block;
-        sigs[i] = n;
-        i ++;
-      }
-    log_assert (i == nsigs);
-
-    qsort (sigs, nsigs, sizeof (sigs[0]), sig_comparison);
+  if (remove_duplicate_sigs (kb, &dups, &modified))
+    goto leave;  /* Error */
 
-    last_i = 0;
-    for (i = 1; i < nsigs; i ++)
-      {
-        log_assert (sigs[last_i]);
-        log_assert (sigs[last_i]->pkt->pkttype == PKT_SIGNATURE);
-        log_assert (sigs[i]);
-        log_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)
-              {
-                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.  */
+  /* Now make sure the sigs occur after the component (aka block)
+   * (public key, subkey, user id) that they sign.  */
   issuer = NULL;
   last_printed_component = NULL;
   for (n_prevp = &kb, n = kb;
@@ -598,6 +607,14 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
     free_public_key (issuer);
   issuer = NULL;
 
+  /* If we reordered signatures we need to de-duplicate again because
+   * a signature can now be a duplicate in another block.  */
+  if (reordered)
+    {
+      if (remove_duplicate_sigs (kb, &dups, &modified))
+        goto leave;
+    }
+
   /* Identify keys / uids that don't have a self-sig.  */
   {
     int has_selfsig = 0;
@@ -667,6 +684,8 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
       }
   }
 
+
+ leave:
   if (!opt.quiet)
     {
       char prefix[100];

commit cedd754fcb03f6dad6e462efc3d347bcef4ec83a
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Jun 7 17:22:58 2018 +0200

    gpg: Fix import's repair-key duplicate signature detection.
    
    * g10/packet.h (PKG_siganture): Add field 'help_counter'.
    * g10/key-check.c (sig_comparison): Take care of HELP_COUNTER.
    (key_check_all_keysigs): De-duplicate on a per-block base.
    --
    
    The key_check_all_keysigs first does a detection of duplicate
    signature.  This is done over all signatures at once.  The problem
    here is for example:
    
       key
       uid_1
         sig_uid_1.1
         sig_uid_1.2
       subkey_1
         sig_sub_1.1
       subkey_2
         sig_sub_2.1
         sig_sub_2.2  (duplicate of sig_sub_1.1)
    
    Now the de-duplication deletes the first signature and keeps the
    second.  That works in most cases for foreign signature on userids but
    in the above constellation the code simply removes sig_sub_1.1 so that
    subkey_1 has no binding signature anymore.  In a later step during
    import the missing binding is detected and subkey_1 is removed because
    it is not anymore valid.  The sig_sub_2.2 will also be removed later
    because it does not check out for subkey_2 (that is as expected).
    
    The fix is to let the de-duplication work only on blocks (ie. within
    the signatures of a user id or a subkey).  This will not detect all
    duplicates but that does not harm because later steps will detect and
    remove them.
    
    In the above case (with this patch applied) the second phase of
    key_check_all_keysigs will reorder key signatures and move the
    duplicate sig_sub_2.2 directly after sig_sub_1.1.  This duplicates the
    signature and for cleanness we should kick the de-duplication process
    again.  This will be done with a followup patch.
    
    GnuPG-bug-id: 3994
    Signed-off-by: Werner Koch <wk at gnupg.org>
    (cherry picked from commit 26bce2f01d2029ea2b8a8dbbe36118e3c83c5cba)

diff --git a/g10/key-check.c b/g10/key-check.c
index 86b1e76..17f2dae 100644
--- a/g10/key-check.c
+++ b/g10/key-check.c
@@ -72,6 +72,13 @@ sig_comparison (const void *av, const void *bv)
   a = an->pkt->pkt.signature;
   b = bn->pkt->pkt.signature;
 
+  /* Signatures with a different help counter are not identical for
+   * our purpose.  */
+  if (a->help_counter < b->help_counter)
+    return -1;
+  if (a->help_counter > b->help_counter)
+    return 1;
+
   if (a->digest_algo < b->digest_algo)
     return -1;
   if (a->digest_algo > b->digest_algo)
@@ -133,6 +140,7 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
   int bad_signature = 0;
   int missing_selfsig = 0;
   int modified = 0;
+  PKT_signature *sig;
 
   log_assert (kb->pkt->pkttype == PKT_PUBLIC_KEY);
   pk = kb->pkt->pkt.public_key;
@@ -143,6 +151,7 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
     kbnode_t *sigs;
     int i;
     int last_i;
+    int block;
 
     /* Count the sigs.  */
     for (nsigs = 0, n = kb; n; n = n->next)
@@ -166,14 +175,31 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
       }
 
     i = 0;
+    block = 0;
     for (n = kb; n; n = n->next)
       {
         if (is_deleted_kbnode (n))
           continue;
 
         if (n->pkt->pkttype != PKT_SIGNATURE)
-          continue;
-
+          {
+            switch (n->pkt->pkttype)
+              {
+              case PKT_PUBLIC_SUBKEY:
+              case PKT_SECRET_SUBKEY:
+              case PKT_USER_ID:
+              case PKT_ATTRIBUTE:
+                /* Bump the block number so that we only consider
+                 * signatures below the same object as duplicates.  */
+                block++;
+                break;
+              default:
+                break;
+              }
+            continue;
+          }
+        sig = n->pkt->pkt.signature;
+        sig->help_counter = block;
         sigs[i] = n;
         i ++;
       }
@@ -194,7 +220,7 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
           {
             if (DBG_PACKET)
               {
-                PKT_signature *sig = sigs[i]->pkt->pkt.signature;
+                sig = sigs[i]->pkt->pkt.signature;
 
                 log_debug ("Signature appears multiple times, "
                            "deleting duplicate:\n");
@@ -244,7 +270,6 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
     {
       PACKET *p;
       int processed_current_component;
-      PKT_signature *sig;
       int rc;
       int dump_sig_params = 0;
 
@@ -577,7 +602,6 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb,
   {
     int has_selfsig = 0;
     PACKET *p;
-    PKT_signature *sig;
 
     current_component = NULL;
     for (n = kb; n; n = n->next)
diff --git a/g10/packet.h b/g10/packet.h
index 43c097e..dc4ce5f 100644
--- a/g10/packet.h
+++ b/g10/packet.h
@@ -230,6 +230,7 @@ typedef struct
   const byte *trust_regexp;
   struct revocation_key *revkey;
   int numrevkeys;
+  int help_counter;          /* Used internally bu some fucntions.  */
   pka_info_t *pka_info;      /* Malloced PKA data or NULL if not
                                 available.  See also flags.pka_tried. */
   char *signers_uid;         /* Malloced value of the SIGNERS_UID

commit 36cc730fa516b3a197f3bb1eb6f3881dd128fbb7
Author: Werner Koch <wk at gnupg.org>
Date:   Thu Jun 7 10:30:07 2018 +0200

    gpg: Improve verbose output during import.
    
    * g10/import.c (chk_self_sigs): Print the subkeyid in addition to the
    keyid.
    (delete_inv_parts): Ditto.
    
    Signed-off-by: Werner Koch <wk at gnupg.org>
    (cherry picked from commit 1bc6b5174248ba4d83d648ef6d6f4550540d1f20)

diff --git a/g10/import.c b/g10/import.c
index 10fbd42..6d2beaf 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -2731,8 +2731,9 @@ import_revoke_cert (ctrl_t ctrl, kbnode_t node, struct import_stats_s *stats)
 }
 
 
-/* Loop over the keyblock and check all self signatures.  On return
- * the following bis in the node flags are set:
+/* Loop over the KEYBLOCK and check all self signatures.  KEYID is the
+ * keyid of the primary key for reporting purposes. On return the
+ * following bits in the node flags are set:
  *
  * - NODE_GOOD_SELFSIG  :: User ID or subkey has a self-signature
  * - NODE_BAD_SELFSIG   :: Used ID or subkey has an invalid self-signature
@@ -2747,17 +2748,22 @@ import_revoke_cert (ctrl_t ctrl, kbnode_t node, struct import_stats_s *stats)
 static int
 chk_self_sigs (ctrl_t ctrl, kbnode_t keyblock, u32 *keyid, int *non_self)
 {
-  kbnode_t n, knode = NULL;
+  kbnode_t knode = NULL;   /* The node of the current subkey.  */
+  PKT_public_key *subpk = NULL; /* and its packet. */
+  kbnode_t bsnode = NULL;  /* Subkey binding signature node.  */
+  u32 bsdate = 0;          /* Timestamp of that node.   */
+  kbnode_t rsnode = NULL;  /* Subkey recocation signature node.  */
+  u32 rsdate = 0;          /* Timestamp of tha node.  */
   PKT_signature *sig;
   int rc;
-  u32 bsdate=0, rsdate=0;
-  kbnode_t bsnode = NULL, rsnode = NULL;
+  kbnode_t n;
 
   for (n=keyblock; (n = find_next_kbnode (n, 0)); )
     {
       if (n->pkt->pkttype == PKT_PUBLIC_SUBKEY)
 	{
 	  knode = n;
+          subpk = knode->pkt->pkt.public_key;
 	  bsdate = 0;
 	  rsdate = 0;
 	  bsnode = NULL;
@@ -2846,11 +2852,14 @@ chk_self_sigs (ctrl_t ctrl, kbnode_t keyblock, u32 *keyid, int *non_self)
               if ( rc )
                 {
                   if (opt.verbose)
-                    log_info (gpg_err_code (rc) == GPG_ERR_PUBKEY_ALGO ?
-                              _("key %s: unsupported public key"
-                                " algorithm\n"):
-                              _("key %s: invalid subkey binding\n"),
-                              keystr (keyid));
+                    {
+                      keyid_from_pk (subpk, NULL);
+                      log_info (gpg_err_code (rc) == GPG_ERR_PUBKEY_ALGO ?
+                                _("key %s: unsupported public key"
+                                  " algorithm\n"):
+                                _("key %s: invalid subkey binding\n"),
+                                keystr_with_sub (keyid, subpk->keyid));
+                    }
                   n->flag |= NODE_DELETION_MARK;
                 }
               else
@@ -2865,8 +2874,12 @@ chk_self_sigs (ctrl_t ctrl, kbnode_t keyblock, u32 *keyid, int *non_self)
                              one is newer */
                           bsnode->flag |= NODE_DELETION_MARK;
                           if (opt.verbose)
-                            log_info (_("key %s: removed multiple subkey"
-                                        " binding\n"),keystr(keyid));
+                            {
+                              keyid_from_pk (subpk, NULL);
+                              log_info (_("key %s: removed multiple subkey"
+                                          " binding\n"),
+                                        keystr_with_sub (keyid, subpk->keyid));
+                            }
                         }
 
                       bsnode = n;
@@ -2945,6 +2958,7 @@ delete_inv_parts (ctrl_t ctrl, kbnode_t keyblock, u32 *keyid,
 {
   kbnode_t node;
   int nvalid=0, uid_seen=0, subkey_seen=0;
+  PKT_public_key *pk;
 
   for (node=keyblock->next; node; node = node->next )
     {
@@ -2982,7 +2996,12 @@ delete_inv_parts (ctrl_t ctrl, kbnode_t keyblock, u32 *keyid,
               || !(node->flag & NODE_GOOD_SELFSIG))
             {
               if (opt.verbose )
-                log_info( _("key %s: skipped subkey\n"),keystr(keyid));
+                {
+                  pk = node->pkt->pkt.public_key;
+                  keyid_from_pk (pk, NULL);
+                  log_info (_("key %s: skipped subkey\n"),
+                            keystr_with_sub (keyid, pk->keyid));
+                }
 
               delete_kbnode( node ); /* the subkey */
               /* and all following signature packets */

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

Summary of changes:
 g10/import.c    |  45 ++++++++---
 g10/key-check.c | 241 +++++++++++++++++++++++++++++++++-----------------------
 g10/packet.h    |   1 +
 3 files changed, 175 insertions(+), 112 deletions(-)


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




More information about the Gnupg-commits mailing list