gpg: Prepare revocation keys for use with v5 keys.
authorWerner Koch <wk@gnupg.org>
Tue, 4 Dec 2018 14:43:19 +0000 (15:43 +0100)
committerWerner Koch <wk@gnupg.org>
Tue, 4 Dec 2018 14:43:19 +0000 (15:43 +0100)
* g10/packet.h (struct revocation_key): Add field 'fprlen'.
* g10/parse-packet.c (parse_revkeys): Set fprlen and allow for v5
keys.  Also fix reading of unitialized data at place where
MAX_FINGERPRINT_LEN is used.
* g10/revoke.c (gen_desig_revoke): Allow for v5 keys and use fprlen.
Do an explicit compare to avoid reading unitialized data.
* g10/sig-check.c (check_revocation_keys): Use the fprlen.
* g10/getkey.c (merge_selfsigs_main): Do an explicit copy to avoid
reading unitialized data.
* g10/import.c (revocation_present): Use fprlen.
* g10/keyedit.c (show_key_with_all_names): Use fprlen.
(menu_addrevoker): Use fprlen.  Allow for v5 keys.
* g10/keygen.c (keygen_add_revkey): Use fprlen.
(parse_revocation_key): Allow for v5 keys.
* g10/keyid.c (keyid_from_fingerprint): Allow for v5 keys.  Print a
better error message in case of bogus fingerprints.
* g10/keylist.c (print_revokers): Use fprlen.
--

The reading of uninitialized data is harmless but we better fix it to
make valgrind happy.  More serious was that we always passed
MAX_FINGERPRINT_LEN but we will need to support 20 and 32 octet
fingerprints and MAX_FINGERPRINT_LEN would be too large for a v4.

Signed-off-by: Werner Koch <wk@gnupg.org>
g10/getkey.c
g10/import.c
g10/keyedit.c
g10/keygen.c
g10/keyid.c
g10/keylist.c
g10/packet.h
g10/parse-packet.c
g10/revoke.c
g10/sig-check.c

index c776a61..039a5ed 100644 (file)
@@ -2577,10 +2577,22 @@ merge_selfsigs_main (ctrl_t ctrl, kbnode_t keyblock, int *r_revoked,
                        xrealloc (pk->revkey, sizeof (struct revocation_key) *
                                  (pk->numrevkeys + sig->numrevkeys));
 
-                     for (i = 0; i < sig->numrevkeys; i++)
-                       memcpy (&pk->revkey[pk->numrevkeys++],
-                               &sig->revkey[i],
-                               sizeof (struct revocation_key));
+                     for (i = 0; i < sig->numrevkeys; i++, pk->numrevkeys++)
+                        {
+                          pk->revkey[pk->numrevkeys].class
+                            = sig->revkey[i].class;
+                          pk->revkey[pk->numrevkeys].algid
+                            = sig->revkey[i].algid;
+                          pk->revkey[pk->numrevkeys].fprlen
+                            = sig->revkey[i].fprlen;
+                          memcpy (pk->revkey[pk->numrevkeys].fpr,
+                                  sig->revkey[i].fpr, sig->revkey[i].fprlen);
+                          memset (pk->revkey[pk->numrevkeys].fpr
+                                  + sig->revkey[i].fprlen,
+                                  0,
+                                  sizeof (sig->revkey[i].fpr)
+                                  - sig->revkey[i].fprlen);
+                        }
                    }
 
                  if (sig->timestamp >= sigdate)
index 8ea5144..64cc4b0 100644 (file)
@@ -3614,7 +3614,7 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
              u32 keyid[2];
 
              keyid_from_fingerprint (ctrl, sig->revkey[idx].fpr,
-                                      MAX_FINGERPRINT_LEN, keyid);
+                                      sig->revkey[idx].fprlen, keyid);
 
              for(inode=keyblock->next;inode;inode=inode->next)
                {
@@ -3634,7 +3634,7 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
 
                      err = get_pubkey_byfprint_fast (NULL,
                                                       sig->revkey[idx].fpr,
-                                                      MAX_FINGERPRINT_LEN);
+                                                      sig->revkey[idx].fprlen);
                      if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
                           || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY)
                        {
@@ -3650,13 +3650,13 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
                                       tempkeystr,keystr(keyid));
                              keyserver_import_fprint (ctrl,
                                                        sig->revkey[idx].fpr,
-                                                       MAX_FINGERPRINT_LEN,
+                                                       sig->revkey[idx].fprlen,
                                                        opt.keyserver, 0);
 
                              /* Do we have it now? */
                              err = get_pubkey_byfprint_fast (NULL,
                                                     sig->revkey[idx].fpr,
-                                                    MAX_FINGERPRINT_LEN);
+                                                     sig->revkey[idx].fprlen);
                            }
 
                          if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
index 9716ed9..63a54fa 100644 (file)
@@ -3524,7 +3524,7 @@ show_key_with_all_names (ctrl_t ctrl, estream_t fp,
 
                    algo = gcry_pk_algo_name (pk->revkey[i].algid);
                    keyid_from_fingerprint (ctrl, pk->revkey[i].fpr,
-                                           MAX_FINGERPRINT_LEN, r_keyid);
+                                           pk->revkey[i].fprlen, r_keyid);
 
                    user = get_user_id_string_native (ctrl, r_keyid);
                    tty_fprintf (fp,
@@ -4309,13 +4309,14 @@ menu_addrevoker (ctrl_t ctrl, kbnode_t pub_keyblock, int sensitive)
       xfree (answer);
 
       fingerprint_from_pk (revoker_pk, revkey.fpr, &fprlen);
-      if (fprlen != 20)
+      if (fprlen != 20 && fprlen != 32)
        {
          log_error (_("cannot appoint a PGP 2.x style key as a "
                       "designated revoker\n"));
          continue;
        }
 
+      revkey.fprlen = fprlen;
       revkey.class = 0x80;
       if (sensitive)
        revkey.class |= 0x40;
index 145b871..61f839a 100644 (file)
@@ -943,11 +943,13 @@ keygen_add_revkey (PKT_signature *sig, void *opaque)
   struct revocation_key *revkey = opaque;
   byte buf[2+MAX_FINGERPRINT_LEN];
 
+  log_assert (revkey->fprlen <= MAX_FINGERPRINT_LEN);
   buf[0] = revkey->class;
   buf[1] = revkey->algid;
-  memcpy (&buf[2], revkey->fpr, MAX_FINGERPRINT_LEN);
+  memcpy (buf + 2, revkey->fpr, revkey->fprlen);
+  memset (buf + 2 + revkey->fprlen, 0, sizeof (revkey->fpr) - revkey->fprlen);
 
-  build_sig_subpkt (sig, SIGSUBPKT_REV_KEY, buf, 2+MAX_FINGERPRINT_LEN);
+  build_sig_subpkt (sig, SIGSUBPKT_REV_KEY, buf, 2+revkey->fprlen);
 
   /* All sigs with revocation keys set are nonrevocable.  */
   sig->flags.revocable = 0;
@@ -3526,6 +3528,8 @@ parse_revocation_key (const char *fname,
 
       revkey.fpr[i]=c;
     }
+  if (i != 20 && i != 32)
+    goto fail;
 
   /* skip to the tag */
   while(*pn && *pn!='s' && *pn!='S')
@@ -3538,7 +3542,7 @@ parse_revocation_key (const char *fname,
 
   return 0;
 
 fail:
+ fail:
   log_error("%s:%d: invalid revocation key\n", fname, r->lnr );
   return -1; /* error */
 }
index 3694c26..e099c7d 100644 (file)
@@ -534,8 +534,9 @@ keyid_from_pk (PKT_public_key *pk, u32 *keyid)
 
 
 /*
- * Get the keyid from the fingerprint. This function is simple for most
- * keys, but has to do a keylookup for old stayle keys.
+ * Get the keyid from the fingerprint.  This function is simple for
+ * most keys, but has to do a key lookup for old v3 keys where the
+ * keyid is not part of the fingerprint.
  */
 u32
 keyid_from_fingerprint (ctrl_t ctrl, const byte *fprint,
@@ -546,7 +547,7 @@ keyid_from_fingerprint (ctrl_t ctrl, const byte *fprint,
   if( !keyid )
     keyid = dummy_keyid;
 
-  if (fprint_len != 20)
+  if (fprint_len != 20 && fprint_len != 32)
     {
       /* This is special as we have to lookup the key first.  */
       PKT_public_key pk;
@@ -556,7 +557,8 @@ keyid_from_fingerprint (ctrl_t ctrl, const byte *fprint,
       rc = get_pubkey_byfprint (ctrl, &pk, NULL, fprint, fprint_len);
       if( rc )
         {
-          log_error("Oops: keyid_from_fingerprint: no pubkey\n");
+          log_printhex (fprint, fprint_len,
+                        "Oops: keyid_from_fingerprint: no pubkey; fpr:");
           keyid[0] = 0;
           keyid[1] = 0;
         }
index 9a21663..92a8d4c 100644 (file)
@@ -1321,7 +1321,7 @@ print_revokers (estream_t fp, PKT_public_key * pk)
 
          es_fprintf (fp, "rvk:::%d::::::", pk->revkey[i].algid);
          p = pk->revkey[i].fpr;
-         for (j = 0; j < 20; j++, p++)
+         for (j = 0; j < pk->revkey[i].fprlen; j++, p++)
            es_fprintf (fp, "%02X", *p);
          es_fprintf (fp, ":%02x%s:\n",
                       pk->revkey[i].class,
index d088bf4..1ec12d6 100644 (file)
@@ -180,6 +180,8 @@ struct revocation_key {
   byte class;
   /* The public-key algorithm ID.  */
   byte algid;
+  /* The length of the fingerprint.  */
+  byte fprlen;
   /* The fingerprint of the authorized key.  */
   byte fpr[MAX_FINGERPRINT_LEN];
 };
index c0f2ca1..21a26c7 100644 (file)
@@ -1905,21 +1905,23 @@ parse_revkeys (PKT_signature * sig)
   while ((revkey = enum_sig_subpkt (sig->hashed, SIGSUBPKT_REV_KEY,
                                    &len, &seq, NULL)))
     {
-      if (/* The only valid length is 22 bytes.  See RFC 4880
-            5.2.3.15.  */
-         len == 22
-         /* 0x80 bit must be set on the class.  */
+      /* Consider only valid packets.  They must have a length of
+       * either 2+20 or 2+32 octets and bit 7 of the class octet must
+       * be set.  */
+      if ((len == 22 || len == 34)
           && (revkey[0] & 0x80))
        {
          sig->revkey = xrealloc (sig->revkey,
                                  sizeof (struct revocation_key) *
                                  (sig->numrevkeys + 1));
 
-         /* Copy the individual fields.  */
          sig->revkey[sig->numrevkeys].class = revkey[0];
          sig->revkey[sig->numrevkeys].algid = revkey[1];
-         memcpy (sig->revkey[sig->numrevkeys].fpr, &revkey[2], 20);
-
+          len -= 2;
+         sig->revkey[sig->numrevkeys].fprlen = len;
+         memcpy (sig->revkey[sig->numrevkeys].fpr, revkey+2, len);
+         memset (sig->revkey[sig->numrevkeys].fpr+len, 0,
+                  sizeof (sig->revkey[sig->numrevkeys].fpr) - len);
          sig->numrevkeys++;
        }
     }
index b778684..e8ce354 100644 (file)
@@ -277,12 +277,12 @@ gen_desig_revoke (ctrl_t ctrl, const char *uname, strlist_t locusr)
 
                fingerprint_from_pk (list->pk, fpr, &fprlen);
 
-               /* Don't get involved with keys that don't have 160
-                  bit fingerprints */
-               if(fprlen!=20)
+               /* Don't get involved with keys that don't have a v4
+                * or v5 fingerprint */
+               if (fprlen != 20 && fprlen != 32)
                  continue;
 
-               if(memcmp(fpr,pk->revkey[i].fpr,20)==0)
+               if (!memcmp(fpr,pk->revkey[i].fpr, fprlen))
                  break;
              }
 
@@ -295,7 +295,7 @@ gen_desig_revoke (ctrl_t ctrl, const char *uname, strlist_t locusr)
          {
            pk2 = xmalloc_clear (sizeof *pk2);
            rc = get_pubkey_byfprint (ctrl, pk2, NULL,
-                                      pk->revkey[i].fpr, MAX_FINGERPRINT_LEN);
+                                      pk->revkey[i].fpr, pk->revkey[i].fprlen);
          }
 
        /* We have the revocation key.  */
@@ -388,15 +388,18 @@ gen_desig_revoke (ctrl_t ctrl, const char *uname, strlist_t locusr)
 
                    for(j=0;j<signode->pkt->pkt.signature->numrevkeys;j++)
                      {
-                       if(pk->revkey[i].class==
-                          signode->pkt->pkt.signature->revkey[j].class &&
-                          pk->revkey[i].algid==
-                          signode->pkt->pkt.signature->revkey[j].algid &&
-                          memcmp(pk->revkey[i].fpr,
-                                 signode->pkt->pkt.signature->revkey[j].fpr,
-                                 MAX_FINGERPRINT_LEN)==0)
+                       if (pk->revkey[i].class
+                               == signode->pkt->pkt.signature->revkey[j].class
+                            && pk->revkey[i].algid
+                                == signode->pkt->pkt.signature->revkey[j].algid
+                            && pk->revkey[i].fprlen
+                               == signode->pkt->pkt.signature->revkey[j].fprlen
+                            && !memcmp
+                                 (pk->revkey[i].fpr,
+                                  signode->pkt->pkt.signature->revkey[j].fpr,
+                                  pk->revkey[i].fprlen))
                          {
-                           revkey=signode->pkt->pkt.signature;
+                           revkey = signode->pkt->pkt.signature;
                            break;
                          }
                      }
index 6d7f1af..d02c68e 100644 (file)
@@ -716,8 +716,8 @@ check_revocation_keys (ctrl_t ctrl, PKT_public_key *pk, PKT_signature *sig)
          /* The revoker's keyid.  */
           u32 keyid[2];
 
-          keyid_from_fingerprint (ctrl, pk->revkey[i].fpr,
-                                  MAX_FINGERPRINT_LEN, keyid);
+          keyid_from_fingerprint (ctrl, pk->revkey[i].fpr, pk->revkey[i].fprlen,
+                                  keyid);
 
           if(keyid[0]==sig->keyid[0] && keyid[1]==sig->keyid[1])
            /* The signature was generated by a designated revoker.