From ed5cc0cc98e534ced89407595899c93971ba721e Mon Sep 17 00:00:00 2001 From: Florian Festi Date: Wed, 16 Oct 2024 15:25:39 +0200 Subject: [PATCH] rpmKeyring: Support keys with the same key ID Loop over the candidates during signature verification and use the one verifying it - iff any. Otherwise print error messages from all candidates. Resolves: #3334 --- rpmio/rpmkeyring.cc | 100 ++++++++++++++++++---------- tests/data/keys/keyidcollision1.asc | 8 +++ tests/data/keys/keyidcollision1.pub | 8 +++ tests/data/keys/keyidcollision2.asc | 8 +++ tests/data/keys/keyidcollision2.pub | 8 +++ tests/rpmsigdig.at | 78 ++++++++++++++++++++++ 6 files changed, 174 insertions(+), 36 deletions(-) create mode 100644 tests/data/keys/keyidcollision1.asc create mode 100644 tests/data/keys/keyidcollision1.pub create mode 100644 tests/data/keys/keyidcollision2.asc create mode 100644 tests/data/keys/keyidcollision2.pub diff --git a/rpmio/rpmkeyring.cc b/rpmio/rpmkeyring.cc index 6e6977985d..7ef8094dcf 100644 --- a/rpmio/rpmkeyring.cc +++ b/rpmio/rpmkeyring.cc @@ -31,7 +31,7 @@ using wrlock = std::unique_lock; using rdlock = std::shared_lock; struct rpmKeyring_s { - std::map keys; /* pointers to keys */ + std::multimap 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,9 +158,13 @@ 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); - return rkey; + + auto range = keyring->keys.equal_range(key->keyid); + for (auto item = range.first; item != range.second; ++item) { + if (item->second->fp == key->fp) + return rpmPubkeyLink(item->second); + } + return NULL; } rpmKeyring rpmKeyringLink(rpmKeyring keyring) @@ -363,50 +372,69 @@ pgpDigParams rpmPubkeyPgpDigParams(rpmPubkey key) return params; } -static rpmPubkey findbySig(rpmKeyring keyring, pgpDigParams sig) +rpmRC rpmKeyringVerifySig2(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx, rpmPubkey * keyptr) { + rpmRC rc = RPMRC_FAIL; rpmPubkey key = NULL; + char *lints = NULL; + std::vector> results = {}; - if (keyring && sig) { + if (sig && ctx) { rdlock lock(keyring->mutex); auto keyid = key2str(pgpDigParamsSignID(sig)); - auto item = keyring->keys.find(keyid); - if (item != keyring->keys.end()) { + + /* use first verifying key */ + auto range = keyring->keys.equal_range(keyid); + + for (auto item = range.first; item != range.second; ++item) { 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)) { + if (pgpDigParamsAlgo(sig, PGPVAL_PUBKEYALGO) + != pgpDigParamsAlgo(key->pgpkey, PGPVAL_PUBKEYALGO)) + { key = NULL; + continue; } - } - } - return key; -} -rpmRC rpmKeyringVerifySig2(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx, rpmPubkey * keyptr) -{ - rpmRC rc = RPMRC_FAIL; + rc = pgpVerifySignature2(key->pgpkey, sig, ctx, &lints); - if (sig && ctx) { - pgpDigParams pgpkey = NULL; - rpmPubkey key = findbySig(keyring, sig); + if (rc == RPMRC_OK) { + break; + } else { + results.push_back({rc, lints}); + } + } - 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) { + if (rc != RPMRC_OK) { + /* Only failing keys */ + for (auto item : results) { + if (item.second) { + rpmlog(item.first ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", item.second); + } + } + /* No key, sanity check signature*/ + if (results.empty()) { + rc = pgpVerifySignature2(key ? key->pgpkey : NULL, + sig, ctx, &lints); + if (lints) { + rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints); + free(lints); + } + } + } else if (lints) { rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints); free(lints); } - if (keyptr) { - *keyptr = pubkeyPrimarykey(key); - } + } + + if (keyptr) { + *keyptr = pubkeyPrimarykey(key); + } + + for (auto item : results) { + free(item.second); } return rc; diff --git a/tests/data/keys/keyidcollision1.asc b/tests/data/keys/keyidcollision1.asc new file mode 100644 index 0000000000..715b1711b4 --- /dev/null +++ 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 new file mode 100644 index 0000000000..ccfd40930e --- /dev/null +++ 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 new file mode 100644 index 0000000000..27bc9a69da --- /dev/null +++ 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 new file mode 100644 index 0000000000..b8a4e83b55 --- /dev/null +++ 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 index ae68cef50e..f6d723a8e4 100644 --- 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