rpmKeyring: Support keys with the same key ID

Loop over the candidates during signature verification and use the one verifying it - iff any. Otherwise use last key with matching key ID (basically a random one). Resolves: rpm-software-management#3334
ffesti · Oct 22, 2024 · f48524f · f48524f
1 parent 027ef64
commit f48524f
Show file tree

Hide file tree

Showing 6 changed files with 171 additions and 39 deletions.
diff --git a/rpmio/rpmkeyring.cc b/rpmio/rpmkeyring.cc
@@ -31,7 +31,7 @@ using wrlock = std::unique_lock<std::shared_mutex>;
 using rdlock = std::shared_lock<std::shared_mutex>;
 
 struct rpmKeyring_s {
-    std::map<std::string,rpmPubkey> keys; /* pointers to keys */
+    std::multimap<std::string,rpmPubkey> keys; /* pointers to keys */
     std::atomic_int nrefs;
     std::shared_mutex mutex;
 };
@@ -126,16 +126,21 @@ int rpmKeyringModify(rpmKeyring keyring, rpmPubkey key, rpmKeyringModifyMode mod
 
     /* check if we already have this key, but always wrlock for simplicity */
     wrlock lock(keyring->mutex);
-    auto item = keyring->keys.find(key->keyid);
-    if (item != keyring->keys.end() && mode == RPMKEYRING_DELETE) {
+    auto range = keyring->keys.equal_range(key->keyid);
+    auto item = range.first;
+    for (; item != range.second; item++) {
+	if (item->second->fp == key->fp)
+	    break;
+    }
+    if (item != range.second && mode == RPMKEYRING_DELETE) {
 	rpmPubkeyFree(item->second);
 	keyring->keys.erase(item);
 	rc = 0;
-    } else if (item != keyring->keys.end() && mode == RPMKEYRING_REPLACE) {
+    } else if (item != range.second && mode == RPMKEYRING_REPLACE) {
 	rpmPubkeyFree(item->second);
 	item->second = rpmPubkeyLink(key);
 	rc = 0;
-    } else if (item == keyring->keys.end() && (mode == RPMKEYRING_ADD ||mode == RPMKEYRING_REPLACE) ) {
+    } else if (item == range.second && (mode == RPMKEYRING_ADD ||mode == RPMKEYRING_REPLACE) ) {
 	keyring->keys.insert({key->keyid, rpmPubkeyLink(key)});
 	rc = 0;
     }
@@ -153,8 +158,14 @@ rpmPubkey rpmKeyringLookupKey(rpmKeyring keyring, rpmPubkey key)
     if (keyring == NULL || key == NULL)
 	return NULL;
     rdlock lock(keyring->mutex);
-    auto item = keyring->keys.find(key->keyid);
-    rpmPubkey rkey = item == keyring->keys.end() ? NULL : rpmPubkeyLink(item->second);
+
+    auto range = keyring->keys.equal_range(key->keyid);
+    auto item = range.first;
+    for (; item != range.second; item++) {
+	if (item->second->fp == key->fp)
+	    break;
+    }
+    rpmPubkey rkey = (item == range.second) ? NULL : rpmPubkeyLink(item->second);
     return rkey;
 }
 
@@ -363,51 +374,62 @@ pgpDigParams rpmPubkeyPgpDigParams(rpmPubkey key)
     return params;
 }
 
-static rpmPubkey findbySig(rpmKeyring keyring, pgpDigParams sig)
+
+static rpmRC pubkeyVerifySig(rpmPubkey key, pgpDigParams sig, DIGEST_CTX ctx)
 {
-    rpmPubkey key = NULL;
+    pgpDigParams pgpkey = key ? key->pgpkey : NULL;
+    char *lints = NULL;
+    rpmRC rc = RPMRC_FAIL;
 
-    if (keyring && sig) {
-	rdlock lock(keyring->mutex);
-	auto keyid = key2str(pgpDigParamsSignID(sig));
-	auto item = keyring->keys.find(keyid);
-	if (item != keyring->keys.end()) {
-	    key = item->second;
-	    pgpDigParams pub = key->pgpkey;
-	    /* Do the parameters match the signature? */
-	    if ((pgpDigParamsAlgo(sig, PGPVAL_PUBKEYALGO)
-                 != pgpDigParamsAlgo(pub, PGPVAL_PUBKEYALGO)) ||
-                memcmp(pgpDigParamsSignID(sig), pgpDigParamsSignID(pub),
-                       PGP_KEYID_LEN)) {
-		key = NULL;
-	    }
-	}
+    rc = pgpVerifySignature2(pgpkey, sig, ctx, &lints);
+    if (lints) {
+	rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints);
+	free(lints);
     }
-    return key;
+    return rc;
 }
 
 rpmRC rpmKeyringVerifySig2(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx, rpmPubkey * keyptr)
 {
     rpmRC rc = RPMRC_FAIL;
+    rpmPubkey key = NULL;
 
     if (sig && ctx) {
-	pgpDigParams pgpkey = NULL;
-	rpmPubkey key = findbySig(keyring, sig);
-
-	if (key)
-	    pgpkey = key->pgpkey;
-
-	/* We call verify even if key not found for a signature sanity check */
-	char *lints = NULL;
-	rc = pgpVerifySignature2(pgpkey, sig, ctx, &lints);
-	if (lints) {
-	    rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints);
-	    free(lints);
+	rdlock lock(keyring->mutex);
+	auto keyid = key2str(pgpDigParamsSignID(sig));
+
+	/* use first verifying key */
+	auto range = keyring->keys.equal_range(keyid);
+	auto item = range.first;
+	for (; item != range.second; item++) {
+	    key = item->second;
+
+	    /* Do the parameters match the signature? */
+	    if (pgpDigParamsAlgo(sig, PGPVAL_PUBKEYALGO)
+		!= pgpDigParamsAlgo(key->pgpkey, PGPVAL_PUBKEYALGO)) {
+		key = NULL;
+		continue;
+	    }
+
+	    if (pgpVerifySignature(key->pgpkey, sig, ctx) == RPMRC_OK)
+		break;
+	    key = NULL;
 	}
-	if (keyptr) {
-	    *keyptr = pubkeyPrimarykey(key);
+
+	/* keys but no one verifies: show errors from all */
+	if (range.first != range.second && !key) {
+	    for (item = range.first; item != range.second; item++) {
+		key = item->second;
+		rc = pubkeyVerifySig(key, sig, ctx);
+	    }
+	} else {
+	    /* We call verify even if key not found for a signature sanity check */
+	    rc = pubkeyVerifySig(key, sig, ctx);
 	}
     }
+    if (keyptr) {
+	*keyptr = pubkeyPrimarykey(key);
+    }
 
     return rc;
 }

diff --git a/tests/data/keys/keyidcollision1.asc b/tests/data/keys/keyidcollision1.asc
@@ -0,0 +1,8 @@
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xVgEZxYoexYJKwYBBAHaRw8BAQdAw36CCvOf5G1jhEQe/j5CfVQi3DrYCdQriSwp
+G/C/aB8AAP9fqZy7LBMwXaJ4ozz4Edw+zRaWXRMW4etZUmZMyI+FcQ+7zQDCYQQT
+FggAEwUCZxYoewkQQmVadRVrPeACGwMAAEy6AQD6lIb8pOjIwIV+DrsKhuXLtCsX
+Q2gUwumc/6WLB8m1RAEA4VNRScVPrw69XloBSFY8Q1RVNfxWiYfpy/OrLD/QFAg=
+=KcD6
+-----END PGP PRIVATE KEY BLOCK-----
diff --git a/tests/data/keys/keyidcollision1.pub b/tests/data/keys/keyidcollision1.pub
@@ -0,0 +1,8 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMEZxYoexYJKwYBBAHaRw8BAQdAw36CCvOf5G1jhEQe/j5CfVQi3DrYCdQriSwp
+G/C/aB+0AIhhBBMWCAATBQJnFih7CRBCZVp1FWs94AIbAwAATLoBAPqUhvyk6MjA
+hX4OuwqG5cu0KxdDaBTC6Zz/pYsHybVEAQDhU1FJxU+vDr1eWgFIVjxDVFU1/FaJ
+h+nL86ssP9AUCA==
+=fev2
+-----END PGP PUBLIC KEY BLOCK-----
diff --git a/tests/data/keys/keyidcollision2.asc b/tests/data/keys/keyidcollision2.asc
@@ -0,0 +1,8 @@
+-----BEGIN PGP PRIVATE KEY BLOCK-----
+
+xVgEZxYoexYJKwYBBAHaRw8BAQdAGGMA/Vn1CsPFvLJ6jNawd7rWgHmO+ar3epiy
+D7wvXy4AAQD5L8KyW9QkHBYY6mxTeAaz9AZv22IstQZE84SAx2VB7g83zQDCYQQT
+FggAEwUCZxYoewkQQmVadRVrPeACGwMAALddAQDGGgUSvspZUepR5VF8JlWd2sz1
+Vhu5n9dphlNg/0Q7xgD/TrlZI+x+BMAQZ/62tHHX1d36JgTwX40D5PBD+rSjLg4=
+=PNIB
+-----END PGP PRIVATE KEY BLOCK-----
diff --git a/tests/data/keys/keyidcollision2.pub b/tests/data/keys/keyidcollision2.pub
@@ -0,0 +1,8 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMEZxYoexYJKwYBBAHaRw8BAQdAGGMA/Vn1CsPFvLJ6jNawd7rWgHmO+ar3epiy
+D7wvXy60AIhhBBMWCAATBQJnFih7CRBCZVp1FWs94AIbAwAAt10BAMYaBRK+yllR
+6lHlUXwmVZ3azPVWG7mf12mGU2D/RDvGAP9OuVkj7H4EwBBn/ra0cdfV3fomBPBf
+jQPk8EP6tKMuDg==
+=mfU5
+-----END PGP PUBLIC KEY BLOCK-----
diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at
@@ -1431,3 +1431,81 @@ POST-IMPORT
 
 gpgconf --kill gpg-agent
 RPMTEST_CLEANUP
+
+# ------------------------------
+# Test key id collision
+AT_SETUP([key id collision])
+AT_KEYWORDS([rpmsign signature])
+AT_SKIP_IF([test x$PGP = xdummy])
+RPMDB_INIT
+gpg2 --import ${RPMTEST}/data/keys/keyidcollision1.asc
+# Our keys have no passphrases to be asked, silence GPG_TTY warning
+export GPG_TTY=""
+RPMTEST_CHECK([
+RPMDB_INIT
+
+cp "${RPMTEST}"/data/RPMS/hello-2.0-1.x86_64.rpm "${RPMTEST}"/tmp/
+run rpmsign --key-id 79cc07f167fee8841829acaa42655a75156b3de0 --digest-algo sha256 --addsign "${RPMTEST}"/tmp/hello-2.0-1.x86_64.rpm > /dev/null
+echo PRE-IMPORT
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+echo POST-IMPORT
+runroot rpmkeys --import /data/keys/keyidcollision2.pub
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+runroot rpmkeys --import /data/keys/keyidcollision1.pub
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+],
+[0],
+[PRE-IMPORT
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, key ID 42655a75156b3de0: NOKEY
+POST-IMPORT
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, Key Fingerprint: 94706f8da571389e8642bdfd42655a75156b3de0: BAD
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, Key Fingerprint: 79cc07f167fee8841829acaa42655a75156b3de0: OK
+],
+[error: Verifying a signature using certificate 94706F8DA571389E8642BDFD42655A75156B3DE0 ():
+  Certificate 42655A75156B3DE0 does not contain key 79CC07F167FEE8841829ACAA42655A75156B3DE0 or it is not valid
+])
+
+gpgconf --kill gpg-agent
+RPMTEST_CLEANUP
+
+# ------------------------------
+# Test key id collision
+AT_SETUP([key id collision])
+AT_KEYWORDS([rpmsign signature])
+AT_SKIP_IF([test x$PGP = xdummy])
+RPMDB_INIT
+gpg2 --import ${RPMTEST}/data/keys/keyidcollision2.asc
+# Our keys have no passphrases to be asked, silence GPG_TTY warning
+export GPG_TTY=""
+RPMTEST_CHECK([
+RPMDB_INIT
+
+cp "${RPMTEST}"/data/RPMS/hello-2.0-1.x86_64.rpm "${RPMTEST}"/tmp/
+run rpmsign --key-id 94706f8da571389e8642bdfd42655a75156b3de0 --digest-algo sha256 --addsign "${RPMTEST}"/tmp/hello-2.0-1.x86_64.rpm > /dev/null
+echo PRE-IMPORT
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+echo POST-IMPORT
+runroot rpmkeys --import /data/keys/keyidcollision1.pub
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+runroot rpmkeys --import /data/keys/keyidcollision2.pub
+runroot rpmkeys -Kv /tmp/hello-2.0-1.x86_64.rpm|grep -v digest
+],
+[0],
+[PRE-IMPORT
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, key ID 42655a75156b3de0: NOKEY
+POST-IMPORT
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, Key Fingerprint: 79cc07f167fee8841829acaa42655a75156b3de0: BAD
+/tmp/hello-2.0-1.x86_64.rpm:
+    Header V4 EdDSA/SHA256 Signature, Key Fingerprint: 94706f8da571389e8642bdfd42655a75156b3de0: OK
+],
+[error: Verifying a signature using certificate 79CC07F167FEE8841829ACAA42655A75156B3DE0 ():
+  Certificate 42655A75156B3DE0 does not contain key 94706F8DA571389E8642BDFD42655A75156B3DE0 or it is not valid
+])
+
+gpgconf --kill gpg-agent
+RPMTEST_CLEANUP