rpm: further fixes to CVE-2021-3521

- CVE-2021-3521.patch taken from https://git.rockylinux.org/staging/rpms/rpm/-/blob/r8/SOURCES/rpm-4.14.3-validate-and-require-subkey-binding-sigs.patch - And add rpm upstream commit rpm-software-management/rpm@daeddb0 This is based on suggestion rpm-software-management/rpm#1795 (comment) Change-Id: Id9e0fa045c70594bc16a7a4a0f206cb2789b8efa Signed-off-by: Shreenidhi Shedi <[email protected]> Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/17877 Tested-by: gerrit-photon <[email protected]> Reviewed-by: Tapas Kundu <[email protected]>
vmware · Sep 21, 2022 · fbc5ad7 · fbc5ad7
1 parent 0ecbf8c
commit fbc5ad7
Show file tree

Hide file tree

Showing 5 changed files with 183 additions and 439 deletions.
diff --git a/SPECS/rpm/CVE-2021-3521-1.patch b/SPECS/rpm/CVE-2021-3521-1.patch
diff --git a/SPECS/rpm/CVE-2021-3521-2.patch b/SPECS/rpm/CVE-2021-3521-2.patch
diff --git a/SPECS/rpm/CVE-2021-3521-3.patch → SPECS/rpm/CVE-2021-3521.patch b/SPECS/rpm/CVE-2021-3521-3.patch → SPECS/rpm/CVE-2021-3521.patch
@@ -1,7 +1,7 @@
-From e6eb466291a07c22db2549f65295506866aa0616 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
+From 747fbc8734b9016b1ba6a3123af02fdefa0832cc Mon Sep 17 00:00:00 2001
+From: Demi Marie Obenour <demi@invisiblethingslab.com>
+Date: Thu, 6 May 2021 18:34:45 -0400
+Subject: [PATCH 1/2] Validate and require subkey binding signatures on PGP
  public keys
 
 All subkeys must be followed by a binding signature by the primary key
@@ -14,35 +14,82 @@ 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
+Combined with:
+5ff86764b17f31535cb247543a90dd739076ec38
+b5e8bc74b2b05aa557f663fe227b94d2bc64fbd8
+9f03f42e2614a68f589f9db8fe76287146522c0c
+b6dffb6dc5ffa2ddc389743f0507876cab341315 (mem-leak fix)
+ae3d2d234ae47ff85229d3fce97a266fa1aa5a61 (use-after-free fix)
 
 Fixes CVE-2021-3521.
-
-[sshedi: back port to v4.14.3]
-
-Signed-off-by: Shreenidhi Shedi <[email protected]>
 ---
- rpmio/rpmpgp.c                                | 102 ++++++++++++++++--
+ rpmio/rpmpgp.c                                | 122 +++++++++++++++---
+ sign/rpmgensig.c                              |   2 +-
  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, 211 insertions(+), 9 deletions(-)
+ 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 ++++
+ 7 files changed, 224 insertions(+), 18 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 8f48509..ccc4151 100644
+index 9bd7512..e29f3fe 100644
 --- a/rpmio/rpmpgp.c
 +++ b/rpmio/rpmpgp.c
-@@ -1017,37 +1017,121 @@ static pgpDigParams pgpDigParamsNew(uint8_t tag)
-     return digp;
+@@ -542,7 +542,7 @@ pgpDigAlg pgpDigAlgFree(pgpDigAlg alg)
+     return NULL;
+ }
+
+-static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo, uint8_t sigtype,
++static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo,
+ 		const uint8_t *p, const uint8_t *h, size_t hlen,
+ 		pgpDigParams sigp)
+ {
+@@ -555,10 +555,8 @@ static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo, uint8_t sigtype,
+ 	int mpil = pgpMpiLen(p);
+ 	if (p + mpil > pend)
+ 	    break;
+-	if (sigtype == PGPSIGTYPE_BINARY || sigtype == PGPSIGTYPE_TEXT) {
+-	    if (sigalg->setmpi(sigalg, i, p))
+-		break;
+-	}
++	if (sigalg->setmpi(sigalg, i, p))
++	    break;
+ 	p += mpil;
+     }
+
+@@ -618,7 +616,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
+ 	}
+
+ 	p = ((uint8_t *)v) + sizeof(*v);
+-	rc = pgpPrtSigParams(tag, v->pubkey_algo, v->sigtype, p, h, hlen, _digp);
++	rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp);
+     }	break;
+     case 4:
+     {   pgpPktSigV4 v = (pgpPktSigV4)h;
+@@ -676,7 +674,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
+ 	if (p > (h + hlen))
+ 	    return 1;
+
+-	rc = pgpPrtSigParams(tag, v->pubkey_algo, v->sigtype, p, h, hlen, _digp);
++	rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp);
+     }	break;
+     default:
+ 	rpmlog(RPMLOG_WARNING, _("Unsupported version of key: V%d\n"), version);
+@@ -1017,36 +1015,127 @@ unsigned int pgpDigParamsAlgo(pgpDigParams digp, unsigned int algotype)
+     return algo;
  }
 
++static pgpDigParams pgpDigParamsNew(uint8_t tag)
++{
++    pgpDigParams digp = xcalloc(1, sizeof(*digp));
++    digp->tag = tag;
++    return digp;
++}
++
 +static int hashKey(DIGEST_CTX hash, const struct pgpPkt *pkt, int exptag)
 +{
 +    int rc = -1;
@@ -115,7 +162,8 @@ index 8f48509..ccc4151 100644
 +	    if (pkttype && pkt->tag != pkttype) {
  		break;
  	    } else {
--		digp = pgpDigParamsNew(pkt.tag);
+-		digp = xcalloc(1, sizeof(*digp));
+-		digp->tag = pkt.tag;
 +		digp = pgpDigParamsNew(pkt->tag);
  	    }
  	}
@@ -131,8 +179,6 @@ index 8f48509..ccc4151 100644
  	    break;
 
 -	p += (pkt.body - pkt.head) + pkt.blen;
--    if (pkttype == PGPTAG_SIGNATURE)
--        break;
 +	if (selfsig) {
 +	    /* subkeys must be followed by binding signature */
 +	    int xx = 1; /* assume failure */
@@ -166,24 +212,46 @@ index 8f48509..ccc4151 100644
 +    rc = (digp && (p == pend) && expect == 0) ? 0 : -1;
 
 +    free(all);
-+    selfsig = pgpDigParamsFree(selfsig);
      if (ret && rc == 0) {
  	*ret = digp;
      } else {
+@@ -1081,8 +1170,7 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
+ 		digps = xrealloc(digps, alloced * sizeof(*digps));
+ 	    }
+
+-	    digps[count] = xcalloc(1, sizeof(**digps));
+-	    digps[count]->tag = PGPTAG_PUBLIC_SUBKEY;
++	    digps[count] = pgpDigParamsNew(PGPTAG_PUBLIC_SUBKEY);
+ 	    /* Copy UID from main key to subkey */
+ 	    digps[count]->userid = xstrdup(mainkey->userid);
+
+diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
+index 771d010..b33fe99 100644
+--- a/sign/rpmgensig.c
++++ b/sign/rpmgensig.c
+@@ -409,7 +409,7 @@ static int haveSignature(rpmtd sigtd, Header h)
+ 	pgpPrtParams(oldtd.data, oldtd.count, PGPTAG_SIGNATURE, &sig2);
+ 	if (pgpDigParamsCmp(sig1, sig2) == 0)
+ 	    rc = 1;
+-	pgpDigParamsFree(sig2);
++	sig2 = pgpDigParamsFree(sig2);
+     }
+     pgpDigParamsFree(sig1);
+     rpmtdFreeData(&oldtd);
 diff --git a/tests/Makefile.am b/tests/Makefile.am
-index 5f5207e..ea81ec4 100644
+index 5f5207e..3093472 100644
 --- a/tests/Makefile.am
 +++ b/tests/Makefile.am
-@@ -88,6 +88,9 @@ EXTRA_DIST += data/SPECS/hello-cd.spec
+@@ -87,6 +87,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/macros.testfile
 +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
 
  # testsuite voodoo
- AUTOTEST = $(AUTOM4TE) --language=autotest
 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..aea00f9
@@ -331,3 +399,82 @@ index 09fcdd5..a74f400 100644
 -- 
 2.25.1
 
+From 533bcf52b8c00e9281de60e1ac50c50aa9115808 Mon Sep 17 00:00:00 2001
+From: Demi Marie Obenour <[email protected]>
+Date: Thu, 6 May 2021 20:03:10 -0400
+Subject: [PATCH 2/2] verifySignature(): package signatures must be
+ PGPSIGTYPE_BINARY
+
+RPM packages are binary documents and must be signed as such.  To avoid
+having to access the fields of pgpDigParams() directly, this adds a new
+accessor function, pgpSignatureType().  pgpSignatureType() returns the
+type of a signature, or -1 if the PGP data is NULL or is not a
+signature.
+
+Prior to commit b5e8bc74b2b05aa557f663fe227b94d2bc64fbd8 this was kind
+of checked inside the parser but incorrectly.
+---
+ lib/rpmvs.c    |  4 +++-
+ rpmio/rpmpgp.c | 10 ++++++++++
+ rpmio/rpmpgp.h |  9 +++++++++
+ 3 files changed, 22 insertions(+), 1 deletion(-)
+
+diff --git a/lib/rpmvs.c b/lib/rpmvs.c
+index 622e480..70f8e35 100644
+--- a/lib/rpmvs.c
++++ b/lib/rpmvs.c
+@@ -530,7 +530,9 @@ exit:
+ static rpmRC
+ verifySignature(rpmKeyring keyring, struct rpmsinfo_s *sinfo)
+ {
+-    rpmRC res = rpmKeyringVerifySig(keyring, sinfo->sig, sinfo->ctx);
++    rpmRC res = RPMRC_FAIL;
++    if (pgpSignatureType(sinfo->sig) == PGPSIGTYPE_BINARY)
++	res = rpmKeyringVerifySig(keyring, sinfo->sig, sinfo->ctx);
+
+     return res;
+ }
+diff --git a/rpmio/rpmpgp.c b/rpmio/rpmpgp.c
+index e29f3fe..1ed982b 100644
+--- a/rpmio/rpmpgp.c
++++ b/rpmio/rpmpgp.c
+@@ -437,6 +437,16 @@ static int pgpVersion(const uint8_t *h, size_t hlen, uint8_t *version)
+     return 0;
+ }
+
++int pgpSignatureType(pgpDigParams _digp)
++{
++    int rc = -1;
++
++    if (_digp && _digp->tag == PGPTAG_SIGNATURE)
++	rc = _digp->sigtype;
++
++    return rc;
++}
++
+ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype, 
+ 			 pgpDigParams _digp)
+ {
+diff --git a/rpmio/rpmpgp.h b/rpmio/rpmpgp.h
+index d37a2e9..c360c23 100644
+--- a/rpmio/rpmpgp.h
++++ b/rpmio/rpmpgp.h
+@@ -1133,6 +1133,15 @@ rpmRC pgpVerifySignature(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx)
+  */
+ rpmRC pgpVerifySig(pgpDig dig, DIGEST_CTX hashctx);
+
++/** \ingroup rpmpgp
++ * Return the type of a PGP signature. If `sig` is NULL, or is not a signature,
++ * returns -1.
++ *
++ * @param sig		signature
++ * @return 		type of the signature
++ */
++int pgpSignatureType(pgpDigParams sig);
++
+ /** \ingroup rpmpgp
+  * Return a string identification of a PGP signature/pubkey.
+  * @param digp		signature/pubkey container
+-- 
+2.25.1
+