| From 07676ca03ad8afcf1ca95a2353c83fbb1d970b9b Mon Sep 17 00:00:00 2001 |
| From: Panu Matilainen <pmatilai@redhat.com> |
| Date: Thu, 30 Sep 2021 09:59:30 +0300 |
| Subject: [PATCH 3/3] Validate and require subkey binding signatures on PGP |
| public keys |
| |
| All subkeys must be followed by a binding signature by the primary key |
| as per the OpenPGP RFC, enforce the presence and validity in the parser. |
| |
| The implementation is as kludgey as they come to work around our |
| simple-minded parser structure without touching API, to maximise |
| backportability. Store all the raw packets internally as we decode them |
| to be able to access previous elements at will, needed to validate ordering |
| and access the actual data. Add testcases for manipulated keys whose |
| import previously would succeed. |
| |
| Depends on the two previous commits: |
| 7b399fcb8f52566e6f3b4327197a85facd08db91 and |
| 236b802a4aa48711823a191d1b7f753c82a89ec5 |
| |
| Fixes CVE-2021-3521. |
| |
| Upstream-Status: Backport [https://github.com/rpm-software-management/rpm/commit/bd36c5dc9] |
| CVE:CVE-2021-3521 |
| |
| Signed-off-by: Changqing Li <changqing.li@windriver.com> |
| |
| --- |
| rpmio/rpmpgp.c | 99 +++++++++++++++++-- |
| tests/Makefile.am | 3 + |
| tests/data/keys/CVE-2021-3521-badbind.asc | 25 +++++ |
| .../data/keys/CVE-2021-3521-nosubsig-last.asc | 25 +++++ |
| tests/data/keys/CVE-2021-3521-nosubsig.asc | 37 +++++++ |
| tests/rpmsigdig.at | 28 ++++++ |
| 6 files changed, 209 insertions(+), 8 deletions(-) |
| create mode 100644 tests/data/keys/CVE-2021-3521-badbind.asc |
| create mode 100644 tests/data/keys/CVE-2021-3521-nosubsig-last.asc |
| create mode 100644 tests/data/keys/CVE-2021-3521-nosubsig.asc |
| |
| diff --git a/rpmio/rpmpgp.c b/rpmio/rpmpgp.c |
| index 509e777e6d..371ad4d9b6 100644 |
| --- a/rpmio/rpmpgp.c |
| +++ b/rpmio/rpmpgp.c |
| @@ -1061,33 +1061,116 @@ static pgpDigParams pgpDigParamsNew(uint8_t tag) |
| return digp; |
| } |
| |
| +static int hashKey(DIGEST_CTX hash, const struct pgpPkt *pkt, int exptag) |
| +{ |
| + int rc = -1; |
| + if (pkt->tag == exptag) { |
| + uint8_t head[] = { |
| + 0x99, |
| + (pkt->blen >> 8), |
| + (pkt->blen ), |
| + }; |
| + |
| + rpmDigestUpdate(hash, head, 3); |
| + rpmDigestUpdate(hash, pkt->body, pkt->blen); |
| + rc = 0; |
| + } |
| + return rc; |
| +} |
| + |
| +static int pgpVerifySelf(pgpDigParams key, pgpDigParams selfsig, |
| + const struct pgpPkt *all, int i) |
| +{ |
| + int rc = -1; |
| + DIGEST_CTX hash = NULL; |
| + |
| + switch (selfsig->sigtype) { |
| + case PGPSIGTYPE_SUBKEY_BINDING: |
| + hash = rpmDigestInit(selfsig->hash_algo, 0); |
| + if (hash) { |
| + rc = hashKey(hash, &all[0], PGPTAG_PUBLIC_KEY); |
| + if (!rc) |
| + rc = hashKey(hash, &all[i-1], PGPTAG_PUBLIC_SUBKEY); |
| + } |
| + break; |
| + default: |
| + /* ignore types we can't handle */ |
| + rc = 0; |
| + break; |
| + } |
| + |
| + if (hash && rc == 0) |
| + rc = pgpVerifySignature(key, selfsig, hash); |
| + |
| + rpmDigestFinal(hash, NULL, NULL, 0); |
| + |
| + return rc; |
| +} |
| + |
| int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype, |
| pgpDigParams * ret) |
| { |
| const uint8_t *p = pkts; |
| const uint8_t *pend = pkts + pktlen; |
| pgpDigParams digp = NULL; |
| - struct pgpPkt pkt; |
| + pgpDigParams selfsig = NULL; |
| + int i = 0; |
| + int alloced = 16; /* plenty for normal cases */ |
| + struct pgpPkt *all = xmalloc(alloced * sizeof(*all)); |
| int rc = -1; /* assume failure */ |
| + int expect = 0; |
| + int prevtag = 0; |
| |
| while (p < pend) { |
| - if (decodePkt(p, (pend - p), &pkt)) |
| + struct pgpPkt *pkt = &all[i]; |
| + if (decodePkt(p, (pend - p), pkt)) |
| break; |
| |
| if (digp == NULL) { |
| - if (pkttype && pkt.tag != pkttype) { |
| + if (pkttype && pkt->tag != pkttype) { |
| break; |
| } else { |
| - digp = pgpDigParamsNew(pkt.tag); |
| + digp = pgpDigParamsNew(pkt->tag); |
| } |
| } |
| |
| - if (pgpPrtPkt(&pkt, digp)) |
| + if (expect) { |
| + if (pkt->tag != expect) |
| + break; |
| + selfsig = pgpDigParamsNew(pkt->tag); |
| + } |
| + if (pgpPrtPkt(pkt, selfsig ? selfsig : digp)) |
| break; |
| |
| - p += (pkt.body - pkt.head) + pkt.blen; |
| - if (pkttype == PGPTAG_SIGNATURE) |
| - break; |
| + if (selfsig) { |
| + /* subkeys must be followed by binding signature */ |
| + if (prevtag == PGPTAG_PUBLIC_SUBKEY) { |
| + if (selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING) |
| + break; |
| + } |
| + |
| + int xx = pgpVerifySelf(digp, selfsig, all, i); |
| + |
| + selfsig = pgpDigParamsFree(selfsig); |
| + if (xx) |
| + break; |
| + expect = 0; |
| + } |
| + |
| + if (pkt->tag == PGPTAG_PUBLIC_SUBKEY) |
| + expect = PGPTAG_SIGNATURE; |
| + prevtag = pkt->tag; |
| + |
| + i++; |
| + p += (pkt->body - pkt->head) + pkt->blen; |
| + if (pkttype == PGPTAG_SIGNATURE) |
| + break; |
| + |
| + if (alloced <= i) { |
| + alloced *= 2; |
| + all = xrealloc(all, alloced * sizeof(*all)); |
| + } |
| + |
| } |
| |
| rc = (digp && (p == pend)) ? 0 : -1; |
| diff --git a/tests/Makefile.am b/tests/Makefile.am |
| index a41ce10de8..7bb23247f1 100644 |
| --- a/tests/Makefile.am |
| +++ b/tests/Makefile.am |
| @@ -107,6 +107,9 @@ EXTRA_DIST += data/SPECS/hello-config-buildid.spec |
| EXTRA_DIST += data/SPECS/hello-cd.spec |
| 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 |
| +EXTRA_DIST += data/keys/CVE-2022-3521-nosubsig.asc |
| +EXTRA_DIST += data/keys/CVE-2022-3521-nosubsig-last.asc |
| EXTRA_DIST += data/macros.testfile |
| EXTRA_DIST += data/macros.debug |
| EXTRA_DIST += data/SOURCES/foo.c |
| diff --git a/tests/data/keys/CVE-2021-3521-badbind.asc b/tests/data/keys/CVE-2021-3521-badbind.asc |
| new file mode 100644 |
| index 0000000000..aea00f9d7a |
| --- /dev/null |
| +++ b/tests/data/keys/CVE-2021-3521-badbind.asc |
| @@ -0,0 +1,25 @@ |
| +-----BEGIN PGP PUBLIC KEY BLOCK----- |
| +Version: rpm-4.17.90 (NSS-3) |
| + |
| +mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g |
| +HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY |
| +91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 |
| +eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas |
| +7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ |
| +1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl |
| +c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK |
| +CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf |
| +Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB |
| +BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr |
| +XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX |
| +fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq |
| ++mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN |
| +BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY |
| +zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz |
| +iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 |
| +Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c |
| +KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m |
| +L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE= |
| +=WCfs |
| +-----END PGP PUBLIC KEY BLOCK----- |
| + |
| diff --git a/tests/data/keys/CVE-2021-3521-nosubsig-last.asc b/tests/data/keys/CVE-2021-3521-nosubsig-last.asc |
| new file mode 100644 |
| index 0000000000..aea00f9d7a |
| --- /dev/null |
| +++ b/tests/data/keys/CVE-2021-3521-nosubsig-last.asc |
| @@ -0,0 +1,25 @@ |
| +-----BEGIN PGP PUBLIC KEY BLOCK----- |
| +Version: rpm-4.17.90 (NSS-3) |
| + |
| +mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g |
| +HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY |
| +91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 |
| +eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas |
| +7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ |
| +1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl |
| +c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK |
| +CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf |
| +Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB |
| +BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr |
| +XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX |
| +fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq |
| ++mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN |
| +BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY |
| +zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz |
| +iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 |
| +Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c |
| +KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m |
| +L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE= |
| +=WCfs |
| +-----END PGP PUBLIC KEY BLOCK----- |
| + |
| diff --git a/tests/data/keys/CVE-2021-3521-nosubsig.asc b/tests/data/keys/CVE-2021-3521-nosubsig.asc |
| new file mode 100644 |
| index 0000000000..3a2e7417f8 |
| --- /dev/null |
| +++ b/tests/data/keys/CVE-2021-3521-nosubsig.asc |
| @@ -0,0 +1,37 @@ |
| +-----BEGIN PGP PUBLIC KEY BLOCK----- |
| +Version: rpm-4.17.90 (NSS-3) |
| + |
| +mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g |
| +HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY |
| +91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 |
| +eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas |
| +7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ |
| +1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl |
| +c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK |
| +CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf |
| +Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB |
| +BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr |
| +XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX |
| +fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq |
| ++mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN |
| +BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY |
| +zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz |
| +iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 |
| +Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c |
| +KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m |
| +L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAG5AQ0EWOY5GAEIAKT68NmshdC4 |
| +VcRhOhlXBvZq23NtskkKoPvW+ZlMuxbRDG48pGBtxhjOngriVUGceEWsXww5Q7En |
| +uRBYglkxkW34ENym0Ji6tsPYfhbbG+dZWKIL4vMIzPOIwlPrXrm558vgkdMM/ELZ |
| +8WIz3KtzvYubKUk2Qz+96lPXbwnlC/SBFRpBseJC5LoOb/5ZGdR/HeLz1JXiacHF |
| +v9Nr3cZWqg5yJbDNZKfASdZgC85v3kkvhTtzknl//5wqdAMexbuwiIh2xyxbO+B/ |
| +qqzZFrVmu3sV2Tj5lLZ/9p1qAuEM7ULbixd/ld8yTmYvQ4bBlKv2bmzXtVfF+ymB |
| +Tm6BzyQEl/MAEQEAAYkBHwQYAQgACQUCWOY5GAIbDAAKCRBDRFkeGWTF/PANB/9j |
| +mifmj6z/EPe0PJFhrpISt9PjiUQCt0IPtiL5zKAkWjHePIzyi+0kCTBF6DDLFxos |
| +3vN4bWnVKT1kBhZAQlPqpJTg+m74JUYeDGCdNx9SK7oRllATqyu+5rncgxjWVPnQ |
| +zu/HRPlWJwcVFYEVXYL8xzfantwQTqefjmcRmBRdA2XJITK+hGWwAmrqAWx+q5xX |
| +Pa8wkNMxVzNS2rUKO9SoVuJ/wlUvfoShkJ/VJ5HDp3qzUqncADfdGN35TDzscngQ |
| +gHvnMwVBfYfSCABV1hNByoZcc/kxkrWMmsd/EnIyLd1Q1baKqc3cEDuC6E6/o4yJ |
| +E4XX4jtDmdZPreZALsiB |
| +=rRop |
| +-----END PGP PUBLIC KEY BLOCK----- |
| + |
| diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at |
| index 8e7c759b8f..e2d30a7f1b 100644 |
| --- a/tests/rpmsigdig.at |
| +++ b/tests/rpmsigdig.at |
| @@ -2,6 +2,34 @@ |
| |
| AT_BANNER([RPM signatures and digests]) |
| |
| +AT_SETUP([rpmkeys --import invalid keys]) |
| +AT_KEYWORDS([rpmkeys import]) |
| +RPMDB_INIT |
| + |
| +AT_CHECK([ |
| +runroot rpmkeys --import /data/keys/CVE-2021-3521-badbind.asc |
| +], |
| +[1], |
| +[], |
| +[error: /data/keys/CVE-2021-3521-badbind.asc: key 1 import failed.] |
| +) |
| +AT_CHECK([ |
| +runroot rpmkeys --import /data/keys/CVE-2021-3521-nosubsig.asc |
| +], |
| +[1], |
| +[], |
| +[error: /data/keys/CVE-2021-3521-nosubsig.asc: key 1 import failed.] |
| +) |
| + |
| +AT_CHECK([ |
| +runroot rpmkeys --import /data/keys/CVE-2021-3521-nosubsig-last.asc |
| +], |
| +[1], |
| +[], |
| +[error: /data/keys/CVE-2021-3521-nosubsig-last.asc: key 1 import failed.] |
| +) |
| +AT_CLEANUP |
| + |
| # ------------------------------ |
| # Test pre-built package verification |
| AT_SETUP([rpmkeys -Kv <unsigned> 1]) |
| -- |
| 2.17.1 |
| |