From 960ed381d596b637f634a1ba0b0ba64922957185 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour <demi@invisiblethingslab.com> Date: Mon, 21 Feb 2022 16:15:30 -0500 Subject: [PATCH] Ignore subkeys that cannot sign This ensures that a signature is only accepted if the key in question can actually be used for signing. Tests 265 and 266 verify that RPM completely ignores keys that are not capable of signing. Fixes #1911. --- rpmio/digest.h | 2 + rpmio/rpmpgp.c | 52 ++++++++++++++--- tests/Makefile.am | 1 + .../data/keys/rpm.org-ed25519-subkey-test.pub | 20 +++++++ tests/rpmsigdig.at | 57 ++++++++++++++++++- 5 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 tests/data/keys/rpm.org-ed25519-subkey-test.pub diff --git a/rpmio/digest.h b/rpmio/digest.h index 3b72a2870c..ce6c8f0c28 100644 --- a/rpmio/digest.h +++ b/rpmio/digest.h @@ -27,6 +27,7 @@ struct pgpDigParams_s { uint8_t * hash; uint8_t tag; + uint8_t flags; /*!< key usae flags */ uint8_t version; /*!< version number. */ uint32_t time; /*!< key/signature creation time. */ uint8_t pubkey_algo; /*!< public key algorithm. */ @@ -39,6 +40,7 @@ struct pgpDigParams_s { uint8_t saved; #define PGPDIG_SAVED_TIME (1 << 0) #define PGPDIG_SAVED_ID (1 << 1) +#define PGPDIG_SAVED_KEY_FLAGS (1 << 2) pgpDigAlg alg; }; diff --git a/rpmio/rpmpgp.c b/rpmio/rpmpgp.c index f297a9ebef..85326202ff 100644 --- a/rpmio/rpmpgp.c +++ b/rpmio/rpmpgp.c @@ -437,7 +437,7 @@ int pgpSignatureType(pgpDigParams _digp) } static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype, - pgpDigParams _digp) + pgpDigParams _digp, int hashed) { const uint8_t *p = h; size_t plen = 0, i; @@ -498,6 +498,17 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype, _digp->saved |= PGPDIG_SAVED_ID; memcpy(_digp->signid, p+1, sizeof(_digp->signid)); } + case PGPSUBTYPE_KEY_FLAGS: /* Key usage flags */ + /* Subpackets in the unhashed section cannot be trusted */ + if (!hashed) + break; + /* Reject duplicate key usage flags */ + if (_digp->saved & PGPDIG_SAVED_KEY_FLAGS) + return 1; + impl = *p; + _digp->saved |= PGPDIG_SAVED_KEY_FLAGS; + _digp->flags = plen >= 2 ? p[1] : 0; + break; case PGPSUBTYPE_EXPORTABLE_CERT: case PGPSUBTYPE_TRUST_SIG: case PGPSUBTYPE_REGEX: @@ -508,7 +519,6 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype, case PGPSUBTYPE_PREFER_KEYSERVER: case PGPSUBTYPE_PRIMARY_USERID: case PGPSUBTYPE_POLICY_URL: - case PGPSUBTYPE_KEY_FLAGS: case PGPSUBTYPE_SIGNER_USERID: case PGPSUBTYPE_REVOKE_REASON: case PGPSUBTYPE_FEATURES: @@ -601,6 +611,9 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, if (pgpVersion(h, hlen, &version)) return rc; + _digp->flags = 0; + _digp->saved &= (PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID); + switch (version) { case 3: { pgpPktSigV3 v = (pgpPktSigV3)h; @@ -630,6 +643,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, _digp->pubkey_algo = v->pubkey_algo; _digp->hash_algo = v->hash_algo; memcpy(_digp->signhash16, v->signhash16, sizeof(_digp->signhash16)); + _digp->saved = PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID; } p = ((uint8_t *)v) + sizeof(*v); @@ -660,7 +674,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, _digp->hashlen = sizeof(*v) + plen; _digp->hash = memcpy(xmalloc(_digp->hashlen), v, _digp->hashlen); } - if (pgpPrtSubType(p, plen, v->sigtype, _digp)) + if (pgpPrtSubType(p, plen, v->sigtype, _digp, 1)) return 1; p += plen; @@ -671,7 +685,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, if ((p + plen) > hend) return 1; - if (pgpPrtSubType(p, plen, v->sigtype, _digp)) + if (pgpPrtSubType(p, plen, v->sigtype, _digp, 0)) return 1; p += plen; @@ -692,7 +706,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, if (p > hend) return 1; - rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp); + rc = tag ? pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp) : 0; } break; default: rpmlog(RPMLOG_WARNING, _("Unsupported version of signature: V%d\n"), version); @@ -1136,7 +1150,8 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype, int xx = 1; /* assume failure */ if (!(prevtag == PGPTAG_PUBLIC_SUBKEY && - selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING)) + (selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING || + selfsig->version != 4))) xx = pgpVerifySelf(digp, selfsig, all, i); selfsig = pgpDigParamsFree(selfsig); @@ -1178,6 +1193,7 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen, const uint8_t *p = pkts; const uint8_t *pend = pkts + pktlen; pgpDigParams *digps = NULL; + pgpDigParams params = NULL; int count = 0; int alloced = 10; struct pgpPkt pkt; @@ -1210,10 +1226,32 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen, pgpDigParamsFree(digps[count]); continue; } - count++; + + if (!params) + params = pgpDigParamsNew(0); + + if (decodePkt(p, (pend - p), &pkt) || + pkt.tag != PGPTAG_SIGNATURE || + pgpPrtSig(0, pkt.body, pkt.blen, params) || + params->sigtype != PGPSIGTYPE_SUBKEY_BINDING || + params->version != 4) { + pgpDigParamsFree(digps[count]); + break; + } + + if (!((params->saved & PGPDIG_SAVED_KEY_FLAGS) && + (params->flags & 0x02))) { + pgpDigParamsFree(digps[count]); + } else { + digps[count]->flags = params->flags; + digps[count]->saved |= PGPDIG_SAVED_KEY_FLAGS; + count++; + } + p += (pkt.body - pkt.head) + pkt.blen; } } rc = (p == pend) ? 0 : -1; + pgpDigParamsFree(params); if (rc == 0) { *subkeys = xrealloc(digps, count * sizeof(*digps)); diff --git a/tests/Makefile.am b/tests/Makefile.am index 43d2cfd3da..a04f547465 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -112,6 +112,7 @@ EXTRA_DIST += data/SOURCES/hello.c EXTRA_DIST += data/SPECS/hello-attr-buildid.spec EXTRA_DIST += data/SPECS/hello-config-buildid.spec EXTRA_DIST += data/SPECS/hello-cd.spec +EXTRA_DIST += data/keys/rpm.org-ed25519-subkey-test.pub EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret EXTRA_DIST += data/keys/CVE-2021-3521-badbind.asc diff --git a/tests/data/keys/rpm.org-ed25519-subkey-test.pub b/tests/data/keys/rpm.org-ed25519-subkey-test.pub new file mode 100644 index 0000000000..1ad7ce5048 --- /dev/null +++ b/tests/data/keys/rpm.org-ed25519-subkey-test.pub @@ -0,0 +1,20 @@ +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mDMEYhf4KRYJKwYBBAHaRw8BAQdAr/MfCGYyI1lNpG4h5Tvdw62G9CgDsn+nweQA +bYtC9bC0NXJwbS5vcmcgZWQyNTUxOSB0ZXN0a2V5IHdpdGggc3Via2V5IDxlZDI1 +NTE5QHJwbS5vcmc+iJoEExYKAEIWIQROGuLaBUTEjXgeGiLaH1/8yyIDHwUCYhf5 +UAIbAwUJA8JnAAULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcCF4AACgkQ2h9f/Msi +Ax/VxQEAm5tVLQRyLHzHpSXz8i8IZsK5kZHGsiDAlvgDubMD5d8A/3ig8MgOBlri +HdsUvvpZma/cJQgfyRHjfh9fkQOOg+cAuDgEYhf4KRIKKwYBBAGXVQEFAQEHQFNp +5tOJofXuDMVlzjnUC1FXp4La5BWkMH469jIbFlhAAwEIB4h4BBgWCgAgFiEEThri +2gVExI14Hhoi2h9f/MsiAx8FAmIX+CkCGwwACgkQ2h9f/MsiAx+kNAEA0eXCnGqX +jdaaPs1cKeGAkgzGwnwvut+YPD7+9H0r1eIA/RQw5yoTIJUButy55lx/mLGUPq0G +N1RhDNg9wosqxfkCuDMEYhf4ehYJKwYBBAHaRw8BAQdASWiPf9/Cd4yD3GtDZFI+ +pcPhdhfWb5Ndv1hP+ndTkSyI7wQYFgoAIBYhBE4a4toFRMSNeB4aItofX/zLIgMf +BQJiF/h6AhsCAIEJENofX/zLIgMfdiAEGRYKAB0WIQTc1zsT3tx9EJeerHXL9SUW +mxSfwgUCYhf4egAKCRDL9SUWmxSfwrjXAPwJf7n8qpEvee1/wCFr0XwObuhOZ+vB +ZeubWyEpWe/XqwEA1WCah/7cgAhYf4u1V7Ciel0kQskE0aAOvtJE/EZqCwix5wEA +14PxZWyO0PrU8AeKR2U/A9jM/81SpWjNMT7X52UYFmgBAL3GaEXFIC5STG9Qsopd +t1vfoExOCG5Ewg7CWOeLF9EB +=Fk+p +-----END PGP PUBLIC KEY BLOCK----- diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at index 24c44c3b20..a9b5a14efb 100644 --- a/tests/rpmsigdig.at +++ b/tests/rpmsigdig.at @@ -247,8 +247,61 @@ UNW2iqnN3BA7guhOv6OMiROF1+I7Q5nWT63mQC7IgQ== gpg(rpm.org RSA testkey <rsa@rpm.org>) = 4:4344591e1964c5fc-58e63918 gpg(1964c5fc) = 4:4344591e1964c5fc-58e63918 gpg(4344591e1964c5fc) = 4:4344591e1964c5fc-58e63918 -gpg(f00650f8) = 4:185e6146f00650f8-58e63918 -gpg(185e6146f00650f8) = 4:185e6146f00650f8-58e63918 +], +[]) +AT_CLEANUP + +# ----------------------------------------- +# Import a public Ed25519 key with a subkey +AT_SETUP([rpmkeys --import ed25519 with subkey]) +AT_KEYWORDS([rpmkeys import]) +AT_CHECK([ +RPMDB_INIT + +runroot rpmkeys --import /data/keys/rpm.org-ed25519-subkey-test.pub +runroot rpm -qi gpg-pubkey-cb22031f-6217f950|grep -v Date|grep -v Version: +runroot rpm -q --provides gpg-pubkey +], +[0], +[Name : gpg-pubkey +Version : cb22031f +Release : 6217f950 +Architecture: (none) +Group : Public Keys +Size : 0 +License : pubkey +Signature : (none) +Source RPM : (none) +Build Host : localhost +Packager : rpm.org ed25519 testkey with subkey <ed25519@rpm.org> +Summary : rpm.org ed25519 testkey with subkey <ed25519@rpm.org> public key +Description : +-----BEGIN PGP PUBLIC KEY BLOCK----- + +mDMEYhf4KRYJKwYBBAHaRw8BAQdAr/MfCGYyI1lNpG4h5Tvdw62G9CgDsn+nweQA +bYtC9bC0NXJwbS5vcmcgZWQyNTUxOSB0ZXN0a2V5IHdpdGggc3Via2V5IDxlZDI1 +NTE5QHJwbS5vcmc+iJoEExYKAEIWIQROGuLaBUTEjXgeGiLaH1/8yyIDHwUCYhf5 +UAIbAwUJA8JnAAULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcCF4AACgkQ2h9f/Msi +Ax/VxQEAm5tVLQRyLHzHpSXz8i8IZsK5kZHGsiDAlvgDubMD5d8A/3ig8MgOBlri +HdsUvvpZma/cJQgfyRHjfh9fkQOOg+cAuDgEYhf4KRIKKwYBBAGXVQEFAQEHQFNp +5tOJofXuDMVlzjnUC1FXp4La5BWkMH469jIbFlhAAwEIB4h4BBgWCgAgFiEEThri +2gVExI14Hhoi2h9f/MsiAx8FAmIX+CkCGwwACgkQ2h9f/MsiAx+kNAEA0eXCnGqX +jdaaPs1cKeGAkgzGwnwvut+YPD7+9H0r1eIA/RQw5yoTIJUButy55lx/mLGUPq0G +N1RhDNg9wosqxfkCuDMEYhf4ehYJKwYBBAHaRw8BAQdASWiPf9/Cd4yD3GtDZFI+ +pcPhdhfWb5Ndv1hP+ndTkSyI7wQYFgoAIBYhBE4a4toFRMSNeB4aItofX/zLIgMf +BQJiF/h6AhsCAIEJENofX/zLIgMfdiAEGRYKAB0WIQTc1zsT3tx9EJeerHXL9SUW +mxSfwgUCYhf4egAKCRDL9SUWmxSfwrjXAPwJf7n8qpEvee1/wCFr0XwObuhOZ+vB +ZeubWyEpWe/XqwEA1WCah/7cgAhYf4u1V7Ciel0kQskE0aAOvtJE/EZqCwix5wEA +14PxZWyO0PrU8AeKR2U/A9jM/81SpWjNMT7X52UYFmgBAL3GaEXFIC5STG9Qsopd +t1vfoExOCG5Ewg7CWOeLF9EB +=Fk+p +-----END PGP PUBLIC KEY BLOCK----- + +gpg(rpm.org ed25519 testkey with subkey <ed25519@rpm.org>) = 4:da1f5ffccb22031f-6217f950 +gpg(cb22031f) = 4:da1f5ffccb22031f-6217f950 +gpg(da1f5ffccb22031f) = 4:da1f5ffccb22031f-6217f950 +gpg(9b149fc2) = 4:cbf525169b149fc2-6217f87a +gpg(cbf525169b149fc2) = 4:cbf525169b149fc2-6217f87a ], []) AT_CLEANUP