Panu Matilainen 8a4b8c
From e75ae70ef1a152dac9a066506cafd2bbf7b2565e Mon Sep 17 00:00:00 2001
Panu Matilainen 8a4b8c
Message-Id: <e75ae70ef1a152dac9a066506cafd2bbf7b2565e.1681989428.git.pmatilai@redhat.com>
Panu Matilainen 8a4b8c
From: "Neal H. Walfield" <neal@pep.foundation>
Panu Matilainen 8a4b8c
Date: Wed, 12 Apr 2023 17:56:19 +0200
Panu Matilainen 8a4b8c
Subject: [PATCH] Add pgpVerifySignature2() and pgpPrtParams2()
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
Add new functions pgpVerifySignature2() and pgpPrtParams2(), which are
Panu Matilainen 8a4b8c
like their earlier versions, but optionally return descriptive error
Panu Matilainen 8a4b8c
messages (in the case of failure) or lints (in the case of success).
Panu Matilainen 8a4b8c
Adjust tests accordingly.
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
This requires rpm-sequoia 1.4 or later.
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
See https://github.com/rpm-software-management/rpm-sequoia/issues/39
Panu Matilainen 8a4b8c
and
Panu Matilainen 8a4b8c
https://github.com/rpm-software-management/rpm/issues/2127#issuecomment-1482646398
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
Fixes #2483.
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
This is a backport of commit 87b9e0c28c3df3937f6676ee1b4164d6154dd9d3
Panu Matilainen 8a4b8c
---
Panu Matilainen 8a4b8c
 configure.ac            |  2 +-
Panu Matilainen 8a4b8c
 include/rpm/rpmpgp.h    | 23 +++++++++++++++++++++++
Panu Matilainen 8a4b8c
 lib/rpmvs.c             | 19 ++++++++++++++++---
Panu Matilainen 8a4b8c
 rpmio/rpmkeyring.c      |  7 ++++++-
Panu Matilainen 8a4b8c
 rpmio/rpmpgp_internal.c | 15 +++++++++++++++
Panu Matilainen 8a4b8c
 rpmio/rpmpgp_sequoia.c  |  7 +++++++
Panu Matilainen 8a4b8c
 tests/rpmi.at           | 10 ++++++++--
Panu Matilainen 8a4b8c
 tests/rpmsigdig.at      | 20 +++++++++++++++++---
Panu Matilainen 8a4b8c
 9 files changed, 95 insertions(+), 10 deletions(-)
Panu Matilainen 8a4b8c
Panu Matilainen 8a4b8c
diff --git a/configure.ac b/configure.ac
Panu Matilainen 8a4b8c
index e6676c581..1d173e4e2 100644
Panu Matilainen 8a4b8c
--- a/configure.ac
Panu Matilainen 8a4b8c
+++ b/configure.ac
Panu Matilainen 8a4b8c
@@ -384,7 +384,7 @@ AC_SUBST(WITH_LIBGCRYPT_LIB)
Panu Matilainen 8a4b8c
 WITH_RPM_SEQUOIA_INCLUDE=
Panu Matilainen 8a4b8c
 WITH_RPM_SEQUOIA_LIB=
Panu Matilainen 8a4b8c
 if test "$with_crypto" = sequoia ; then
Panu Matilainen 8a4b8c
-  PKG_CHECK_MODULES([RPM_SEQUOIA], [rpm-sequoia], [have_rpm_sequoia=yes], [have_rpm_sequoia=no])
Panu Matilainen 8a4b8c
+  PKG_CHECK_MODULES([RPM_SEQUOIA], [rpm-sequoia >= 1.4.0], [have_rpm_sequoia=yes], [have_rpm_sequoia=no])
Panu Matilainen 8a4b8c
   if test "$have_rpm_sequoia" = "yes"; then
Panu Matilainen 8a4b8c
      WITH_RPM_SEQUOIA_INCLUDE="$RPM_SEQUOIA_CFLAGS"
Panu Matilainen 8a4b8c
      WITH_RPM_SEQUOIA_LIB="$RPM_SEQUOIA_LIBS"
Panu Matilainen 8a4b8c
diff --git a/include/rpm/rpmpgp.h b/include/rpm/rpmpgp.h
Panu Matilainen 8a4b8c
index a3238a643..3352129b8 100644
Panu Matilainen 8a4b8c
--- a/include/rpm/rpmpgp.h
Panu Matilainen 8a4b8c
+++ b/include/rpm/rpmpgp.h
Panu Matilainen 8a4b8c
@@ -1013,6 +1013,18 @@ int pgpPubkeyKeyID(const uint8_t * pkt, size_t pktlen, pgpKeyID_t keyid);
Panu Matilainen 8a4b8c
 int pgpPrtParams(const uint8_t *pkts, size_t pktlen, unsigned int pkttype,
Panu Matilainen 8a4b8c
 		 pgpDigParams * ret);
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
+/** \ingroup rpmpgp
Panu Matilainen 8a4b8c
+ * Parse a OpenPGP packet(s).
Panu Matilainen 8a4b8c
+ * @param pkts		OpenPGP packet(s)
Panu Matilainen 8a4b8c
+ * @param pktlen	OpenPGP packet(s) length (no. of bytes)
Panu Matilainen 8a4b8c
+ * @param pkttype	Expected packet type (signature/key) or 0 for any
Panu Matilainen 8a4b8c
+ * @param[out] ret	signature/pubkey packet parameters on success (alloced)
Panu Matilainen 8a4b8c
+ * @param[out] lints	error messages and lints
Panu Matilainen 8a4b8c
+ * @return		-1 on error, 0 on success
Panu Matilainen 8a4b8c
+ */
Panu Matilainen 8a4b8c
+int pgpPrtParams2(const uint8_t *pkts, size_t pktlen, unsigned int pkttype,
Panu Matilainen 8a4b8c
+		 pgpDigParams * ret, char **lints);
Panu Matilainen 8a4b8c
+
Panu Matilainen 8a4b8c
 /** \ingroup rpmpgp
Panu Matilainen 8a4b8c
  * Parse subkey parameters from OpenPGP packet(s).
Panu Matilainen 8a4b8c
  * @param pkts		OpenPGP packet(s)
Panu Matilainen 8a4b8c
@@ -1191,6 +1203,17 @@ const uint8_t *pgpDigParamsSignID(pgpDigParams digp);
Panu Matilainen 8a4b8c
  */
Panu Matilainen 8a4b8c
 const char *pgpDigParamsUserID(pgpDigParams digp);
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
+/** \ingroup rpmpgp
Panu Matilainen 8a4b8c
+ * Verify a PGP signature and return a error message or lint.
Panu Matilainen 8a4b8c
+ * @param key		public key
Panu Matilainen 8a4b8c
+ * @param sig		signature
Panu Matilainen 8a4b8c
+ * @param hashctx	digest context
Panu Matilainen 8a4b8c
+ * @param lints	error messages and lints
Panu Matilainen 8a4b8c
+ * @return 		RPMRC_OK on success
Panu Matilainen 8a4b8c
+ */
Panu Matilainen 8a4b8c
+rpmRC pgpVerifySignature2(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx,
Panu Matilainen 8a4b8c
+                          char **lints);
Panu Matilainen 8a4b8c
+
Panu Matilainen 8a4b8c
 /** \ingroup rpmpgp
Panu Matilainen 8a4b8c
  * Retrieve the object's version.
Panu Matilainen 8a4b8c
  *
Panu Matilainen 8a4b8c
diff --git a/lib/rpmvs.c b/lib/rpmvs.c
Panu Matilainen 8a4b8c
index a1425ea17..9b2106927 100644
Panu Matilainen 8a4b8c
--- a/lib/rpmvs.c
Panu Matilainen 8a4b8c
+++ b/lib/rpmvs.c
Panu Matilainen 8a4b8c
@@ -193,10 +193,23 @@ static void rpmsinfoInit(const struct vfyinfo_s *vinfo,
Panu Matilainen 8a4b8c
     }
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
     if (sinfo->type == RPMSIG_SIGNATURE_TYPE) {
Panu Matilainen 8a4b8c
-	if (pgpPrtParams(data, dlen, PGPTAG_SIGNATURE, &sinfo->sig)) {
Panu Matilainen 8a4b8c
-	    rasprintf(&sinfo->msg, _("%s tag %u: invalid OpenPGP signature"),
Panu Matilainen 8a4b8c
-		    origin, td->tag);
Panu Matilainen 8a4b8c
+	char *lints = NULL;
Panu Matilainen 8a4b8c
+        int ec = pgpPrtParams2(data, dlen, PGPTAG_SIGNATURE, &sinfo->sig, &lints);
Panu Matilainen 8a4b8c
+	if (ec) {
Panu Matilainen 8a4b8c
+	    if (lints) {
Panu Matilainen 8a4b8c
+		rasprintf(&sinfo->msg,
Panu Matilainen 8a4b8c
+			("%s tag %u: invalid OpenPGP signature: %s"),
Panu Matilainen 8a4b8c
+			origin, td->tag, lints);
Panu Matilainen 8a4b8c
+		free(lints);
Panu Matilainen 8a4b8c
+	    } else {
Panu Matilainen 8a4b8c
+		rasprintf(&sinfo->msg,
Panu Matilainen 8a4b8c
+			_("%s tag %u: invalid OpenPGP signature"),
Panu Matilainen 8a4b8c
+			origin, td->tag);
Panu Matilainen 8a4b8c
+	    }
Panu Matilainen 8a4b8c
 	    goto exit;
Panu Matilainen 8a4b8c
+	} else if (lints) {
Panu Matilainen 8a4b8c
+	    rpmlog(RPMLOG_WARNING, "%s\n", lints);
Panu Matilainen 8a4b8c
+	    free(lints);
Panu Matilainen 8a4b8c
 	}
Panu Matilainen 8a4b8c
 	sinfo->hashalgo = pgpDigParamsAlgo(sinfo->sig, PGPVAL_HASHALGO);
Panu Matilainen 8a4b8c
 	sinfo->keyid = pgpGrab(pgpDigParamsSignID(sinfo->sig)+4, 4);
Panu Matilainen 8a4b8c
diff --git a/rpmio/rpmkeyring.c b/rpmio/rpmkeyring.c
Panu Matilainen 8a4b8c
index db72892d9..712004bc8 100644
Panu Matilainen 8a4b8c
--- a/rpmio/rpmkeyring.c
Panu Matilainen 8a4b8c
+++ b/rpmio/rpmkeyring.c
Panu Matilainen 8a4b8c
@@ -328,7 +328,12 @@ rpmRC rpmKeyringVerifySig(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx)
Panu Matilainen 8a4b8c
 	    pgpkey = key->pgpkey;
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
 	/* We call verify even if key not found for a signature sanity check */
Panu Matilainen 8a4b8c
-	rc = pgpVerifySignature(pgpkey, sig, ctx);
Panu Matilainen 8a4b8c
+	char *lints = NULL;
Panu Matilainen 8a4b8c
+	rc = pgpVerifySignature2(pgpkey, sig, ctx, &lints);
Panu Matilainen 8a4b8c
+	if (lints) {
Panu Matilainen 8a4b8c
+	    rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints);
Panu Matilainen 8a4b8c
+	    free(lints);
Panu Matilainen 8a4b8c
+	}
Panu Matilainen 8a4b8c
     }
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
     if (keyring)
Panu Matilainen 8a4b8c
diff --git a/rpmio/rpmpgp_internal.c b/rpmio/rpmpgp_internal.c
Panu Matilainen 8a4b8c
index 0fcd220e4..a049c09b2 100644
Panu Matilainen 8a4b8c
--- a/rpmio/rpmpgp_internal.c
Panu Matilainen 8a4b8c
+++ b/rpmio/rpmpgp_internal.c
Panu Matilainen 8a4b8c
@@ -1095,6 +1095,14 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
Panu Matilainen 8a4b8c
     return rc;
Panu Matilainen 8a4b8c
 }
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
+int pgpPrtParams2(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
Panu Matilainen 8a4b8c
+                  pgpDigParams * ret, char **lints)
Panu Matilainen 8a4b8c
+{
Panu Matilainen 8a4b8c
+    if (lints)
Panu Matilainen 8a4b8c
+        *lints = NULL;
Panu Matilainen 8a4b8c
+    return pgpPrtParams(pkts, pktlen, pkttype, ret);
Panu Matilainen 8a4b8c
+}
Panu Matilainen 8a4b8c
+
Panu Matilainen 8a4b8c
 int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
Panu Matilainen 8a4b8c
 			pgpDigParams mainkey, pgpDigParams **subkeys,
Panu Matilainen 8a4b8c
 			int *subkeysCount)
Panu Matilainen 8a4b8c
@@ -1264,6 +1272,13 @@ rpmRC pgpVerifySig(pgpDig dig, DIGEST_CTX hashctx)
Panu Matilainen 8a4b8c
 			      pgpDigGetParams(dig, PGPTAG_SIGNATURE), hashctx);
Panu Matilainen 8a4b8c
 }
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
+rpmRC pgpVerifySignature2(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx, char **lints)
Panu Matilainen 8a4b8c
+{
Panu Matilainen 8a4b8c
+    if (lints)
Panu Matilainen 8a4b8c
+        *lints = NULL;
Panu Matilainen 8a4b8c
+    return pgpVerifySignature(key, sig, hashctx);
Panu Matilainen 8a4b8c
+}
Panu Matilainen 8a4b8c
+
Panu Matilainen 8a4b8c
 static pgpArmor decodePkts(uint8_t *b, uint8_t **pkt, size_t *pktlen)
Panu Matilainen 8a4b8c
 {
Panu Matilainen 8a4b8c
     const char * enc = NULL;
Panu Matilainen 8a4b8c
diff --git a/rpmio/rpmpgp_sequoia.c b/rpmio/rpmpgp_sequoia.c
Panu Matilainen 8a4b8c
index e01acd0e9..2141bbf30 100644
Panu Matilainen 8a4b8c
--- a/rpmio/rpmpgp_sequoia.c
Panu Matilainen 8a4b8c
+++ b/rpmio/rpmpgp_sequoia.c
Panu Matilainen 8a4b8c
@@ -36,6 +36,9 @@ W(uint32_t, pgpDigParamsCreationTime, (pgpDigParams digp), (digp))
Panu Matilainen 8a4b8c
 W(rpmRC, pgpVerifySignature,
Panu Matilainen 8a4b8c
   (pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx),
Panu Matilainen 8a4b8c
   (key, sig, hashctx))
Panu Matilainen 8a4b8c
+W(rpmRC, pgpVerifySignature2,
Panu Matilainen 8a4b8c
+  (pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx, char **lints),
Panu Matilainen 8a4b8c
+  (key, sig, hashctx, lints))
Panu Matilainen 8a4b8c
 W(int, pgpPubkeyKeyID,
Panu Matilainen 8a4b8c
   (const uint8_t * pkt, size_t pktlen, pgpKeyID_t keyid),
Panu Matilainen 8a4b8c
   (pkt, pktlen, keyid))
Panu Matilainen 8a4b8c
@@ -51,6 +54,10 @@ W(int, pgpPubKeyCertLen,
Panu Matilainen 8a4b8c
 W(int, pgpPrtParams,
Panu Matilainen 8a4b8c
   (const uint8_t *pkts, size_t pktlen, unsigned int pkttype, pgpDigParams *ret),
Panu Matilainen 8a4b8c
   (pkts, pktlen, pkttype, ret))
Panu Matilainen 8a4b8c
+W(int, pgpPrtParams2,
Panu Matilainen 8a4b8c
+  (const uint8_t *pkts, size_t pktlen, unsigned int pkttype, pgpDigParams *ret,
Panu Matilainen 8a4b8c
+   char **lints),
Panu Matilainen 8a4b8c
+  (pkts, pktlen, pkttype, ret, lints))
Panu Matilainen 8a4b8c
 W(int, pgpPrtParamsSubkeys,
Panu Matilainen 8a4b8c
   (const uint8_t *pkts, size_t pktlen,
Panu Matilainen 8a4b8c
    pgpDigParams mainkey, pgpDigParams **subkeys,
Panu Matilainen 8a4b8c
diff --git a/tests/rpmi.at b/tests/rpmi.at
Panu Matilainen 8a4b8c
index 7c8f25eff..d67185d5b 100644
Panu Matilainen 8a4b8c
--- a/tests/rpmi.at
Panu Matilainen 8a4b8c
+++ b/tests/rpmi.at
Panu Matilainen 8a4b8c
@@ -254,7 +254,7 @@ AT_CLEANUP
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
 AT_SETUP([rpm -U <corrupted signed 1>])
Panu Matilainen 8a4b8c
 AT_KEYWORDS([install])
Panu Matilainen 8a4b8c
-AT_CHECK([
Panu Matilainen 8a4b8c
+AT_CHECK_UNQUOTED([
Panu Matilainen 8a4b8c
 RPMDB_INIT
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
 pkg="hello-2.0-1.x86_64-signed.rpm"
Panu Matilainen 8a4b8c
@@ -267,7 +267,13 @@ runroot rpm -U --ignorearch --ignoreos --nodeps \
Panu Matilainen 8a4b8c
 ],
Panu Matilainen 8a4b8c
 [1],
Panu Matilainen 8a4b8c
 [],
Panu Matilainen 8a4b8c
-[error: /tmp/hello-2.0-1.x86_64-signed.rpm: Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
Panu Matilainen 8a4b8c
+[`if test x$PGP = xinternal; then
Panu Matilainen 8a4b8c
+    echo 'error: /tmp/hello-2.0-1.x86_64-signed.rpm: Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
Panu Matilainen 8a4b8c
+else
Panu Matilainen 8a4b8c
+    echo 'error: /tmp/hello-2.0-1.x86_64-signed.rpm: Header RSA signature: BAD (package tag 268: invalid OpenPGP signature: Parsing an OpenPGP packet:'
Panu Matilainen 8a4b8c
+    echo '  Failed to parse Signature Packet'
Panu Matilainen 8a4b8c
+    echo '      because: Malformed packet: Subpacket extends beyond the end of the subpacket area)'
Panu Matilainen 8a4b8c
+fi`
Panu Matilainen 8a4b8c
 error: /tmp/hello-2.0-1.x86_64-signed.rpm cannot be installed
Panu Matilainen 8a4b8c
 ])
Panu Matilainen 8a4b8c
 AT_CLEANUP
Panu Matilainen 8a4b8c
diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at
Panu Matilainen 8a4b8c
index 5b1c6c4a6..e5482735a 100644
Panu Matilainen 8a4b8c
--- a/tests/rpmsigdig.at
Panu Matilainen 8a4b8c
+++ b/tests/rpmsigdig.at
Panu Matilainen 8a4b8c
@@ -539,7 +539,7 @@ AT_CLEANUP
Panu Matilainen 8a4b8c
 # Test pre-built corrupted package verification (corrupted signature)
Panu Matilainen 8a4b8c
 AT_SETUP([rpmkeys -Kv <corrupted signed> 1])
Panu Matilainen 8a4b8c
 AT_KEYWORDS([rpmkeys digest signature])
Panu Matilainen 8a4b8c
-AT_CHECK([
Panu Matilainen 8a4b8c
+AT_CHECK_UNQUOTED([
Panu Matilainen 8a4b8c
 RPMDB_INIT
Panu Matilainen 8a4b8c
 
Panu Matilainen 8a4b8c
 pkg="hello-2.0-1.x86_64-signed.rpm"
Panu Matilainen 8a4b8c
@@ -553,14 +553,28 @@ runroot rpmkeys -Kv /tmp/${pkg}
Panu Matilainen 8a4b8c
 ],
Panu Matilainen 8a4b8c
 [1],
Panu Matilainen 8a4b8c
 [/tmp/hello-2.0-1.x86_64-signed.rpm:
Panu Matilainen 8a4b8c
-    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
Panu Matilainen 8a4b8c
+`if test x$PGP = xinternal; then
Panu Matilainen 8a4b8c
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
Panu Matilainen 8a4b8c
+else
Panu Matilainen 8a4b8c
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature: Parsing an OpenPGP packet:'
Panu Matilainen 8a4b8c
+    echo '  Failed to parse Signature Packet'
Panu Matilainen 8a4b8c
+    echo '      because: Signature appears to be created by a non-conformant OpenPGP implementation, see <https://github.com/rpm-software-management/rpm/issues/2351>.'
Panu Matilainen 8a4b8c
+    echo '      because: Malformed MPI: leading bit is not set: expected bit 1 to be set in        0 (0))'
Panu Matilainen 8a4b8c
+fi`
Panu Matilainen 8a4b8c
     Header SHA256 digest: OK
Panu Matilainen 8a4b8c
     Header SHA1 digest: OK
Panu Matilainen 8a4b8c
     Payload SHA256 digest: OK
Panu Matilainen 8a4b8c
     V4 RSA/SHA256 Signature, key ID 1964c5fc: NOKEY
Panu Matilainen 8a4b8c
     MD5 digest: OK
Panu Matilainen 8a4b8c
 /tmp/hello-2.0-1.x86_64-signed.rpm:
Panu Matilainen 8a4b8c
-    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
Panu Matilainen 8a4b8c
+`if test x$PGP = xinternal; then
Panu Matilainen 8a4b8c
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
Panu Matilainen 8a4b8c
+else
Panu Matilainen 8a4b8c
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature: Parsing an OpenPGP packet:'
Panu Matilainen 8a4b8c
+    echo '  Failed to parse Signature Packet'
Panu Matilainen 8a4b8c
+    echo '      because: Signature appears to be created by a non-conformant OpenPGP implementation, see <https://github.com/rpm-software-management/rpm/issues/2351>.'
Panu Matilainen 8a4b8c
+    echo '      because: Malformed MPI: leading bit is not set: expected bit 1 to be set in        0 (0))'
Panu Matilainen 8a4b8c
+fi`
Panu Matilainen 8a4b8c
     Header SHA256 digest: OK
Panu Matilainen 8a4b8c
     Header SHA1 digest: OK
Panu Matilainen 8a4b8c
     Payload SHA256 digest: OK
Panu Matilainen 8a4b8c
-- 
Panu Matilainen 8a4b8c
2.40.0
Panu Matilainen 8a4b8c