From 6add112349e19cb55cccb26ea60bdcfbee85f366 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 | 109 +++++++++++++++++----------- 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, 178 insertions(+), 41 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..72d0c3b285 100644 --- a/rpmio/rpmkeyring.cc +++ b/rpmio/rpmkeyring.cc @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -31,7 +32,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 +127,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 +159,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,52 +373,69 @@ pgpDigParams rpmPubkeyPgpDigParams(rpmPubkey key) return params; } -static rpmPubkey findbySig(rpmKeyring keyring, pgpDigParams sig) +rpmRC pubKeyVerify(rpmPubkey key, pgpDigParams sig, DIGEST_CTX ctx, std::vector> &results) { - rpmPubkey key = NULL; + rpmRC rc = RPMRC_FAIL; + char *lints = NULL; + rc = pgpVerifySignature2(key ? key->pgpkey : NULL, sig, ctx, &lints); - 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; - } - } - } - return key; + /* found right key, discard error messages from wrong keys */ + if (rc == RPMRC_OK) + results.clear(); + + results.push_back({rc, std::string(lints ? lints : "")}); + free(lints); + 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)); + std::vector> results = {}; + + /* Look for verifying key */ + auto range = keyring->keys.equal_range(keyid); + + for (auto it = range.first; it != range.second; ++it) { + key = it->second; + + /* Do the parameters match the signature? */ + if (pgpDigParamsAlgo(sig, PGPVAL_PUBKEYALGO) + != pgpDigParamsAlgo(key->pgpkey, PGPVAL_PUBKEYALGO)) + { + continue; + } + + rc = pubKeyVerify(key, sig, ctx, results); + + if (rc == RPMRC_OK) + break; + } + + /* No key, sanity check signature*/ + if (results.empty()) { + key = NULL; + rc = pubKeyVerify(NULL, sig, ctx, results); + assert(rc != RPMRC_OK); } - if (keyptr) { - *keyptr = pubkeyPrimarykey(key); + + for (auto const & item : results) { + if (!item.second.empty()) { + rpmlog(item.first ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", + item.second.c_str()); + } } } + if (keyptr) { + *keyptr = pubkeyPrimarykey(key); + } + 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