Page MenuHome GnuPG
Create Task
Maniphest T5000

trustdb,keybox: Adding support of v5key
Closed, ResolvedPublic
Actions

Description

In g10/tdbio.c (and verify_own_keys in g10/trustdb.c), fingerprint max is assumed to be 20.

And the record of 40-byte is defined as:

  • 1-byte: TYPE
  • 1-byte; Reserved
  • 20-byte: fingerprint
  • 1-byte owner trust
  • 1-byte depth
  • 1-byte min_ownertrust
  • 1-byte rsvd
  • 4-byte validlist recnum
  • rest of unused bytes (10)

v5 key's fingerprint length is 32. New record type and structure should be possibly needed here.
Perhaps, we can use half-byte for ownertrust and another half-byte for min_ownertrust, so that it can be packed into 40-byte.

Revisions and Commits

rG GnuPG
rG20982bbd7539 gpg: Fix short key ID for v5key.
rG373c975859a5 gpg: Fix trustdb for v5key.
rGdf531848a961 kbx: Support v5key for short kid and long kid.

Event Timeline

• gniibe created this task.Jul 20 2020, 4:22 AM
• gniibe added a comment.Jul 20 2020, 4:25 AM

Something like:

  • 1-byte: TYPE
  • 1-byte: Reserved
  • 32-byte: fingerprint
  • 1-byte; ownertrust / min_ownertrust
  • 1-byte: depth
  • 4-byte: validlist recnum
• gniibe updated the task description. (Show Details)Jul 20 2020, 4:25 AM
• gniibe renamed this task from trustdb: Adding support of v5 keys to trustdb: Adding support of v5key.Jul 20 2020, 7:18 AM
• gniibe added a comment.Jul 20 2020, 7:21 AM

Here is the patch for trustdb and keybox. Not introduced new record structure, but RECTYPE_TRUST_SHA2 saving only 20-byte.

diff --git a/g10/tdbio.c b/g10/tdbio.c
index bfeede991..a440f3ff4 100644
--- a/g10/tdbio.c
+++ b/g10/tdbio.c
@@ -1172,7 +1172,8 @@ upd_hashtable (ctrl_t ctrl, ulong table, byte *key, int keylen, ulong newrecnum)
 	    } /* end loop over list slots */
 
 	}
-      else if (rec.rectype == RECTYPE_TRUST) /* Insert a list record.  */
+      else if (rec.rectype == RECTYPE_TRUST || rec.rectype == RECTYPE_TRUST_SHA2)
+        /* Insert a list record.  */
         {
           if (rec.recnum == newrecnum)
             {
@@ -1481,6 +1482,7 @@ tdbio_dump_record (TRUSTREC *rec, estream_t fp)
       break;
 
     case RECTYPE_TRUST:
+    case RECTYPE_TRUST_SHA2:
       es_fprintf (fp, "trust ");
       for (i=0; i < 20; i++)
         es_fprintf (fp, "%02X", rec->r.trust.fingerprint[i]);
@@ -1624,6 +1626,7 @@ tdbio_read_record (ulong recnum, TRUSTREC *rec, int expected)
       break;
 
     case RECTYPE_TRUST:
+    case RECTYPE_TRUST_SHA2:
       memcpy (rec->r.trust.fingerprint, p, 20);
       p+=20;
       rec->r.trust.ownertrust = *p++;
@@ -1720,6 +1723,7 @@ tdbio_write_record (ctrl_t ctrl, TRUSTREC *rec)
       break;
 
     case RECTYPE_TRUST:
+    case RECTYPE_TRUST_SHA2:
       memcpy (p, rec->r.trust.fingerprint, 20); p += 20;
       *p++ = rec->r.trust.ownertrust;
       *p++ = rec->r.trust.depth;
@@ -1743,7 +1747,7 @@ tdbio_write_record (ctrl_t ctrl, TRUSTREC *rec)
   rc = put_record_into_cache (recnum, buf);
   if (rc)
     ;
-  else if (rec->rectype == RECTYPE_TRUST)
+  else if (rec->rectype == RECTYPE_TRUST || rec->rectype == RECTYPE_TRUST_SHA2)
     rc = update_trusthashtbl (ctrl, rec);
 
   return rc;
@@ -1765,7 +1769,7 @@ tdbio_delete_record (ctrl_t ctrl, ulong recnum)
   rc = tdbio_read_record (recnum, &rec, 0);
   if (rc)
     ;
-  else if (rec.rectype == RECTYPE_TRUST)
+  else if (rec.rectype == RECTYPE_TRUST || rec.rectype == RECTYPE_TRUST_SHA2)
     {
       rc = drop_from_hashtable (ctrl, get_trusthashrec (ctrl),
                                 rec.r.trust.fingerprint, 20, rec.recnum);
@@ -1877,7 +1881,7 @@ tdbio_new_recnum (ctrl_t ctrl)
 static int
 cmp_trec_fpr ( const void *fpr, const TRUSTREC *rec )
 {
-  return (rec->rectype == RECTYPE_TRUST
+  return ((rec->rectype == RECTYPE_TRUST || rec->rectype == RECTYPE_TRUST_SHA2)
           && !memcmp (rec->r.trust.fingerprint, fpr, 20));
 }
 
diff --git a/g10/tdbio.h b/g10/tdbio.h
index 267a37970..003cbdd2d 100644
--- a/g10/tdbio.h
+++ b/g10/tdbio.h
@@ -38,6 +38,7 @@
 #define RECTYPE_HLST 11
 #define RECTYPE_TRUST 12
 #define RECTYPE_VALID 13
+#define RECTYPE_TRUST_SHA2 14
 #define RECTYPE_FREE 254
 
 
diff --git a/g10/trustdb.c b/g10/trustdb.c
index c4b996a96..8af664da3 100644
--- a/g10/trustdb.c
+++ b/g10/trustdb.c
@@ -277,8 +277,8 @@ verify_own_keys (ctrl_t ctrl)
   /* scan the trustdb to find all ultimately trusted keys */
   for (recnum=1; !tdbio_read_record (recnum, &rec, 0); recnum++ )
     {
-      if ( rec.rectype == RECTYPE_TRUST
-           && (rec.r.trust.ownertrust & TRUST_MASK) == TRUST_ULTIMATE)
+      if ((rec.rectype == RECTYPE_TRUST || rec.rectype == RECTYPE_TRUST_SHA2)
+          && (rec.r.trust.ownertrust & TRUST_MASK) == TRUST_ULTIMATE)
         {
             byte *fpr = rec.r.trust.fingerprint;
             int fprlen;
@@ -291,6 +291,8 @@ verify_own_keys (ctrl_t ctrl)
              * the whole validation code to only work with
              * fingerprints */
             fprlen = (!fpr[16] && !fpr[17] && !fpr[18] && !fpr[19])? 16:20;
+            if (rec.rectype == RECTYPE_TRUST_SHA2)
+              fprlen = 32;
             keyid_from_fingerprint (ctrl, fpr, fprlen, kid);
             if (!add_utk (kid))
 	      log_info(_("key %s occurs more than once in the trustdb\n"),
@@ -676,7 +678,7 @@ read_trust_record (ctrl_t ctrl, PKT_public_key *pk, TRUSTREC *rec)
       return rc;
     }
 
-  if (rec->rectype != RECTYPE_TRUST)
+  if (rec->rectype != RECTYPE_TRUST && rec->rectype != RECTYPE_TRUST_SHA2)
     {
       log_error ("trustdb: record %lu is not a trust record\n",
                  rec->recnum);
@@ -780,6 +782,7 @@ tdb_update_ownertrust (ctrl_t ctrl, PKT_public_key *pk, unsigned int new_trust )
   else if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     { /* no record yet - create a new one */
       size_t dummy;
+      byte fingerprint[MAX_FINGERPRINT_LEN];
 
       if (DBG_TRUST)
         log_debug ("insert ownertrust %u\n", new_trust );
@@ -787,7 +790,10 @@ tdb_update_ownertrust (ctrl_t ctrl, PKT_public_key *pk, unsigned int new_trust )
       memset (&rec, 0, sizeof rec);
       rec.recnum = tdbio_new_recnum (ctrl);
       rec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, rec.r.trust.fingerprint, &dummy);
+      fingerprint_from_pk (pk, fingerprint, &dummy);
+      memcpy (rec.r.trust.fingerprint, fingerprint, 20);
+      if (dummy == 32)
+        rec.rectype = RECTYPE_TRUST_SHA2;
       rec.r.trust.ownertrust = new_trust;
       write_record (ctrl, &rec);
       tdb_revalidation_mark (ctrl);
@@ -838,6 +844,7 @@ update_min_ownertrust (ctrl_t ctrl, u32 *kid, unsigned int new_trust)
   else if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     { /* no record yet - create a new one */
       size_t dummy;
+      byte fingerprint[MAX_FINGERPRINT_LEN];
 
       if (DBG_TRUST)
         log_debug ("insert min_ownertrust %u\n", new_trust );
@@ -845,7 +852,10 @@ update_min_ownertrust (ctrl_t ctrl, u32 *kid, unsigned int new_trust)
       memset (&rec, 0, sizeof rec);
       rec.recnum = tdbio_new_recnum (ctrl);
       rec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, rec.r.trust.fingerprint, &dummy);
+      fingerprint_from_pk (pk, fingerprint, &dummy);
+      memcpy (rec.r.trust.fingerprint, fingerprint, 20);
+      if (dummy == 32)
+        rec.rectype = RECTYPE_TRUST_SHA2;
       rec.r.trust.min_ownertrust = new_trust;
       write_record (ctrl, &rec);
       tdb_revalidation_mark (ctrl);
@@ -926,11 +936,15 @@ update_validity (ctrl_t ctrl, PKT_public_key *pk, PKT_user_id *uid,
     {
       /* No record yet - create a new one. */
       size_t dummy;
+      byte fingerprint[MAX_FINGERPRINT_LEN];
 
       memset (&trec, 0, sizeof trec);
       trec.recnum = tdbio_new_recnum (ctrl);
       trec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, trec.r.trust.fingerprint, &dummy);
+      fingerprint_from_pk (pk, fingerprint, &dummy);
+      memcpy (trec.r.trust.fingerprint, fingerprint, 20);
+      if (dummy == 32)
+        trec.rectype = RECTYPE_TRUST_SHA2;
       trec.r.trust.ownertrust = 0;
       }
 
@@ -1937,7 +1951,7 @@ reset_trust_records (ctrl_t ctrl)
 
   for (recnum=1; !tdbio_read_record (recnum, &rec, 0); recnum++ )
     {
-      if(rec.rectype==RECTYPE_TRUST)
+      if (rec.rectype==RECTYPE_TRUST || rec.rectype==RECTYPE_TRUST_SHA2)
 	{
 	  count++;
 	  if(rec.r.trust.min_ownertrust)
diff --git a/kbx/keybox-search.c b/kbx/keybox-search.c
index 66a383bef..131407cc3 100644
--- a/kbx/keybox-search.c
+++ b/kbx/keybox-search.c
@@ -313,7 +313,7 @@ blob_cmp_fpr_part (KEYBOXBLOB blob, const unsigned char *fpr,
         storedfprlen = (get16 (buffer + off + 32) & 0x80)? 32:20;
       else
         storedfprlen = 20;
-      if (storedfprlen == fproff + fprlen
+      if ((fpr32 || (storedfprlen == fproff + fprlen))
           && !memcmp (buffer + off + fproff, fpr, fprlen))
         return idx+1; /* found */
     }
@@ -685,18 +685,43 @@ blob_x509_has_grip (KEYBOXBLOB blob, const unsigned char *grip)
 static inline int
 has_short_kid (KEYBOXBLOB blob, u32 lkid)
 {
+  const unsigned char *buffer;
+  size_t length;
+  int fpr32;
   unsigned char buf[4];
+
+  buffer = _keybox_get_blob_image (blob, &length);
+  if (length < 48)
+    return 0; /* blob too short */
+  fpr32 = buffer[5] == 2;
+  if (fpr32 && length < 56)
+    return 0; /* blob to short */
+
   buf[0] = lkid >> 24;
   buf[1] = lkid >> 16;
   buf[2] = lkid >> 8;
   buf[3] = lkid;
-  return blob_cmp_fpr_part (blob, buf, 16, 4);
+  if (fpr32)
+    return blob_cmp_fpr_part (blob, buf, 0, 4);
+  else
+    return blob_cmp_fpr_part (blob, buf, 16, 4);
 }
 
 static inline int
 has_long_kid (KEYBOXBLOB blob, u32 mkid, u32 lkid)
 {
+  const unsigned char *buffer;
+  size_t length;
+  int fpr32;
   unsigned char buf[8];
+
+  buffer = _keybox_get_blob_image (blob, &length);
+  if (length < 48)
+    return 0; /* blob too short */
+  fpr32 = buffer[5] == 2;
+  if (fpr32 && length < 56)
+    return 0; /* blob to short */
+
   buf[0] = mkid >> 24;
   buf[1] = mkid >> 16;
   buf[2] = mkid >> 8;
@@ -705,7 +730,10 @@ has_long_kid (KEYBOXBLOB blob, u32 mkid, u32 lkid)
   buf[5] = lkid >> 16;
   buf[6] = lkid >> 8;
   buf[7] = lkid;
-  return blob_cmp_fpr_part (blob, buf, 12, 8);
+  if (fpr32)
+    return blob_cmp_fpr_part (blob, buf, 0, 8);
+  else
+    return blob_cmp_fpr_part (blob, buf, 12, 8);
 }
 
 static inline int
• gniibe renamed this task from trustdb: Adding support of v5key to trustdb,keybox: Adding support of v5key.Jul 20 2020, 7:30 AM
• gniibe updated the task description. (Show Details)
• werner added a subscriber: • werner.Jul 20 2020, 9:26 AM

I deferred this thing because I hoped to implement this in the keyboxd. Another option is to use a truncated fingerprint - for displaying purposes we anyway truncate to 25 byte and 20 byte should also be okay until we can move this to keyboxd. But okay, if you want to add support please go ahead but make sure that there are no fatal conditions if a gpg 2.2 accesses the v5 enabled trustdb.

• gniibe added a commit: rGdf531848a961: kbx: Support v5key for short kid and long kid..Aug 6 2020, 12:26 PM
• gniibe added a comment.EditedAug 6 2020, 12:31 PM

I revise the change, using different approach, so that we can keep better existing implementation compatibility.

No new data structure, but keep using 20-byte fingerprint field for the trust record.
Only save a part of fingerprint for v5key, and re-arrange 32-byte fingerprint to 20-bytee so that extracting kid can be same. A bit tricky, but works.

diff --git a/g10/keydb.h b/g10/keydb.h
index a6c70d682..fe9cac7de 100644
--- a/g10/keydb.h
+++ b/g10/keydb.h
@@ -526,6 +526,8 @@ const char *keystr_from_pk_with_sub (PKT_public_key *main_pk,
    the storage.  */
 const char *pk_keyid_str (PKT_public_key *pk);
 
+void compute_fingerprint (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 d3fc29a98..3c1f4f49e 100644
--- a/g10/keyid.c
+++ b/g10/keyid.c
@@ -519,7 +519,7 @@ keystr_from_desc(KEYDB_SEARCH_DESC *desc)
 
 
 /* Compute the fingerprint and keyid and store it in PK.  */
-static void
+void
 compute_fingerprint (PKT_public_key *pk)
 {
   const byte *dp;
diff --git a/g10/trustdb.c b/g10/trustdb.c
index c4b996a96..fd706e04e 100644
--- a/g10/trustdb.c
+++ b/g10/trustdb.c
@@ -39,6 +39,70 @@
 #include "tofu.h"
 #include "key-clean.h"
 
+static void
+fpr20_from_pk (PKT_public_key *pk, byte *array)
+{
+  if (!pk->fprlen)
+    compute_fingerprint (pk);
+
+  if (!array)
+    array = xmalloc (pk->fprlen);
+
+  if (pk->fprlen == 32)
+    {
+      memcpy (array +  0, pk->fpr + 20, 4);
+      memcpy (array +  4, pk->fpr + 24, 4);
+      memcpy (array +  8, pk->fpr + 28, 4);
+      memcpy (array + 12, pk->fpr +  0, 4);
+      memcpy (array + 16, pk->fpr +  4, 4);
+    }
+  else
+    memcpy (array, pk->fpr, 20);
+}
+
+static u32
+keyid_from_fpr20 (ctrl_t ctrl, const byte *fpr, u32 *keyid)
+{
+  u32 dummy_keyid[2];
+  int fprlen;
+
+  if( !keyid )
+    keyid = dummy_keyid;
+
+  /* Problem: We do only use fingerprints in the trustdb but
+   * we need the keyID here to indetify the key; we can only
+   * use that ugly hack to distinguish between 16 and 20
+   * bytes fpr - it does not work always so we better change
+   * the whole validation code to only work with
+   * fingerprints */
+  fprlen = (!fpr[16] && !fpr[17] && !fpr[18] && !fpr[19])? 16:20;
+
+  if (fprlen != 20)
+    {
+      /* This is special as we have to lookup the key first.  */
+      PKT_public_key pk;
+      int rc;
+
+      memset (&pk, 0, sizeof pk);
+      rc = get_pubkey_byfprint (ctrl, &pk, NULL, fpr, fprlen);
+      if (rc)
+        {
+          log_printhex (fpr, fprlen,
+                        "Oops: keyid_from_fingerprint: no pubkey; fpr:");
+          keyid[0] = 0;
+          keyid[1] = 0;
+        }
+      else
+        keyid_from_pk (&pk, keyid);
+    }
+  else
+    {
+      keyid[0] = buf32_to_u32 (fpr+12);
+      keyid[1] = buf32_to_u32 (fpr+16);
+    }
+
+  return keyid[1];
+}
 
 typedef struct key_item **KeyHashTable; /* see new_key_hash_table() */
 
@@ -277,24 +341,15 @@ verify_own_keys (ctrl_t ctrl)
   /* scan the trustdb to find all ultimately trusted keys */
   for (recnum=1; !tdbio_read_record (recnum, &rec, 0); recnum++ )
     {
-      if ( rec.rectype == RECTYPE_TRUST
-           && (rec.r.trust.ownertrust & TRUST_MASK) == TRUST_ULTIMATE)
+      if (rec.rectype == RECTYPE_TRUST
+          && (rec.r.trust.ownertrust & TRUST_MASK) == TRUST_ULTIMATE)
         {
-            byte *fpr = rec.r.trust.fingerprint;
-            int fprlen;
-            u32 kid[2];
-
-            /* Problem: We do only use fingerprints in the trustdb but
-             * we need the keyID here to indetify the key; we can only
-             * use that ugly hack to distinguish between 16 and 20
-             * butes fpr - it does not work always so we better change
-             * the whole validation code to only work with
-             * fingerprints */
-            fprlen = (!fpr[16] && !fpr[17] && !fpr[18] && !fpr[19])? 16:20;
-            keyid_from_fingerprint (ctrl, fpr, fprlen, kid);
-            if (!add_utk (kid))
-	      log_info(_("key %s occurs more than once in the trustdb\n"),
-		       keystr(kid));
+          u32 kid[2];
+
+          keyid_from_fpr20 (ctrl, rec.r.trust.fingerprint, kid);
+          if (!add_utk (kid))
+            log_info (_("key %s occurs more than once in the trustdb\n"),
+                      keystr(kid));
         }
     }
 
@@ -779,15 +834,13 @@ tdb_update_ownertrust (ctrl_t ctrl, PKT_public_key *pk, unsigned int new_trust )
     }
   else if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     { /* no record yet - create a new one */
-      size_t dummy;
-
       if (DBG_TRUST)
         log_debug ("insert ownertrust %u\n", new_trust );
 
       memset (&rec, 0, sizeof rec);
       rec.recnum = tdbio_new_recnum (ctrl);
       rec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, rec.r.trust.fingerprint, &dummy);
+      fpr20_from_pk (pk, rec.r.trust.fingerprint);
       rec.r.trust.ownertrust = new_trust;
       write_record (ctrl, &rec);
       tdb_revalidation_mark (ctrl);
@@ -837,15 +890,13 @@ update_min_ownertrust (ctrl_t ctrl, u32 *kid, unsigned int new_trust)
     }
   else if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     { /* no record yet - create a new one */
-      size_t dummy;
-
       if (DBG_TRUST)
         log_debug ("insert min_ownertrust %u\n", new_trust );
 
       memset (&rec, 0, sizeof rec);
       rec.recnum = tdbio_new_recnum (ctrl);
       rec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, rec.r.trust.fingerprint, &dummy);
+      fpr20_from_pk (pk, rec.r.trust.fingerprint);
       rec.r.trust.min_ownertrust = new_trust;
       write_record (ctrl, &rec);
       tdb_revalidation_mark (ctrl);
@@ -925,12 +976,10 @@ update_validity (ctrl_t ctrl, PKT_public_key *pk, PKT_user_id *uid,
   if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     {
       /* No record yet - create a new one. */
-      size_t dummy;
-
       memset (&trec, 0, sizeof trec);
       trec.recnum = tdbio_new_recnum (ctrl);
       trec.rectype = RECTYPE_TRUST;
-      fingerprint_from_pk (pk, trec.r.trust.fingerprint, &dummy);
+      fpr20_from_pk (pk, trec.r.trust.fingerprint);
       trec.r.trust.ownertrust = 0;
       }
• gniibe added a comment.Aug 7 2020, 4:53 AM

No, it didn't work, but we need more change:

diff --git a/g10/tdbio.c b/g10/tdbio.c
index bfeede991..9f01667b4 100644
--- a/g10/tdbio.c
+++ b/g10/tdbio.c
@@ -1909,12 +1909,9 @@ tdbio_search_trust_byfpr (ctrl_t ctrl, const byte *fingerprint, TRUSTREC *rec)
 gpg_error_t
 tdbio_search_trust_bypk (ctrl_t ctrl, PKT_public_key *pk, TRUSTREC *rec)
 {
-  byte fingerprint[MAX_FINGERPRINT_LEN];
-  size_t fingerlen;
+  byte fingerprint[20];
 
-  fingerprint_from_pk( pk, fingerprint, &fingerlen );
-  for (; fingerlen < 20; fingerlen++)
-    fingerprint[fingerlen] = 0;
+  fpr20_from_pk (pk, fingerprint);
   return tdbio_search_trust_byfpr (ctrl, fingerprint, rec);
 }
• gniibe added a comment.Aug 7 2020, 6:10 AM

Applied and pushed.

• gniibe added a commit: rG20982bbd7539: gpg: Fix short key ID for v5key..Aug 7 2020, 6:11 AM
• gniibe added a commit: rG373c975859a5: gpg: Fix trustdb for v5key..
• gniibe changed the task status from Open to Testing.Aug 7 2020, 6:11 AM
• gniibe triaged this task as Normal priority.
• gniibe added a project: Restricted Project.
• werner added a comment.Apr 19 2021, 5:51 PM

Has been released with 2.3.0 and we better open a new task if problems show up with v5 key. I am pretty sure that there will be a few v5 key problems after they get in real use.

• werner closed this task as Resolved.Apr 19 2021, 5:51 PM
• werner claimed this task.