From e233fb844adda74a5199057d1fd7fa20d994564d Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmatilai@redhat.com>
Date: Thu, 30 Sep 2021 09:59:30 +0300
Subject: [PATCH] Validate and require subkey binding signatures on PGP public keys

---
 rpmio/rpmpgp.c                                | 98 +++++++++++++++++--
 tests/Makefile.am                             |  3 +
 tests/data/keys/CVE-2021-3521-badbind.asc     | 24 +++++
 .../data/keys/CVE-2021-3521-nosubsig-last.asc | 25 +++++
 tests/data/keys/CVE-2021-3521-nosubsig.asc    | 36 +++++++
 tests/rpmsigdig.at                            | 28 ++++++
 6 files changed, 207 insertions(+), 7 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 ee97f07..2b6e173 100644
--- a/rpmio/rpmpgp.c
+++ b/rpmio/rpmpgp.c
@@ -1007,37 +1007,121 @@ 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 (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;
+    rc = (digp && (p == pend) && expect == 0) ? 0 : -1;
 
+    free(all);
     if (ret && rc == 0) {
 	*ret = digp;
     } else {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ab92353..585e7a2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -104,6 +104,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-2021-3521-nosubsig.asc
+EXTRA_DIST += data/keys/CVE-2021-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 0000000..701ffda
--- /dev/null
+++ b/tests/data/keys/CVE-2021-3521-badbind.asc
@@ -0,0 +1,24 @@
+-----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 0000000..2aaa0a0
--- /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 0000000..9743d55
--- /dev/null
+++ b/tests/data/keys/CVE-2021-3521-nosubsig.asc
@@ -0,0 +1,36 @@
+-----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 91c205e..c780deb 100644
--- a/tests/rpmsigdig.at
+++ b/tests/rpmsigdig.at
@@ -225,6 +225,34 @@ gpg(185e6146f00650f8) = 4:185e6146f00650f8-58e63918
 [])
 AT_CLEANUP
 
+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 -K <signed> 1])
-- 
2.27.0

