Blob Blame History Raw
From 87b9e0c28c3df3937f6676ee1b4164d6154dd9d3 Mon Sep 17 00:00:00 2001
From: "Neal H. Walfield" <neal@pep.foundation>
Date: Wed, 12 Apr 2023 17:56:19 +0200
Subject: [PATCH] Add pgpVerifySignature2() and pgpPrtParams2()

Add new functions pgpVerifySignature2() and pgpPrtParams2(), which are
like their earlier versions, but optionally return descriptive error
messages (in the case of failure) or lints (in the case of success).
Adjust tests accordingly.

This requires rpm-sequoia 1.4 or later.

See https://github.com/rpm-software-management/rpm-sequoia/issues/39
and
https://github.com/rpm-software-management/rpm/issues/2127#issuecomment-1482646398

Fixes #2483.
---
 ci/Dockerfile           |  2 ++
 include/rpm/rpmpgp.h    | 23 +++++++++++++++++++
 lib/rpmvs.c             | 19 +++++++++++++---
 rpmio/CMakeLists.txt    |  2 +-
 rpmio/rpmkeyring.c      |  7 +++++-
 rpmio/rpmpgp_internal.c | 15 +++++++++++++
 rpmio/rpmpgp_sequoia.c  |  7 ++++++
 tests/rpmi.at           | 10 +++++++--
 tests/rpmsigdig.at      | 50 +++++++++++++++++++++++++++++++----------
 9 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/ci/Dockerfile b/ci/Dockerfile
index d8f808962..552934fcd 100644
--- a/ci/Dockerfile
+++ b/ci/Dockerfile
@@ -7,6 +7,8 @@ RUN sed -i -e "s:^enabled=.$:enabled=0:g" /etc/yum.repos.d/*openh264.repo
 # dummy for controlling per-repo gpgcheck via Semaphore setup
 RUN sed -i -e "s:^gpgcheck=.$:gpgcheck=1:g" /etc/yum.repos.d/*.repo
 RUN dnf -y update
+# until 1.4.0 lands in stable
+RUN dnf -y --enablerepo=updates-testing install "rpm-sequoia-devel >= 1.4.0"
 RUN dnf -y install \
   autoconf \
   cmake \
diff --git a/include/rpm/rpmpgp.h b/include/rpm/rpmpgp.h
index 87a2a5bd2..675cbad73 100644
--- a/include/rpm/rpmpgp.h
+++ b/include/rpm/rpmpgp.h
@@ -1009,6 +1009,18 @@ int pgpPubkeyKeyID(const uint8_t * pkt, size_t pktlen, pgpKeyID_t keyid);
 int pgpPrtParams(const uint8_t *pkts, size_t pktlen, unsigned int pkttype,
 		 pgpDigParams * ret);
 
+/** \ingroup rpmpgp
+ * Parse a OpenPGP packet(s).
+ * @param pkts		OpenPGP packet(s)
+ * @param pktlen	OpenPGP packet(s) length (no. of bytes)
+ * @param pkttype	Expected packet type (signature/key) or 0 for any
+ * @param[out] ret	signature/pubkey packet parameters on success (alloced)
+ * @param[out] lints	error messages and lints
+ * @return		-1 on error, 0 on success
+ */
+int pgpPrtParams2(const uint8_t *pkts, size_t pktlen, unsigned int pkttype,
+		 pgpDigParams * ret, char **lints);
+
 /** \ingroup rpmpgp
  * Parse subkey parameters from OpenPGP packet(s).
  * @param pkts		OpenPGP packet(s)
@@ -1186,6 +1198,17 @@ pgpDigParams pgpDigParamsFree(pgpDigParams digp);
  */
 rpmRC pgpVerifySignature(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx);
 
+/** \ingroup rpmpgp
+ * Verify a PGP signature and return a error message or lint.
+ * @param key		public key
+ * @param sig		signature
+ * @param hashctx	digest context
+ * @param lints	error messages and lints
+ * @return 		RPMRC_OK on success
+ */
+rpmRC pgpVerifySignature2(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx,
+                          char **lints);
+
 /** \ingroup rpmpgp
  * Return the type of a PGP signature. If `sig` is NULL, or is not a signature,
  * returns -1.
diff --git a/lib/rpmvs.c b/lib/rpmvs.c
index a1425ea17..9b2106927 100644
--- a/lib/rpmvs.c
+++ b/lib/rpmvs.c
@@ -193,10 +193,23 @@ static void rpmsinfoInit(const struct vfyinfo_s *vinfo,
     }
 
     if (sinfo->type == RPMSIG_SIGNATURE_TYPE) {
-	if (pgpPrtParams(data, dlen, PGPTAG_SIGNATURE, &sinfo->sig)) {
-	    rasprintf(&sinfo->msg, _("%s tag %u: invalid OpenPGP signature"),
-		    origin, td->tag);
+	char *lints = NULL;
+        int ec = pgpPrtParams2(data, dlen, PGPTAG_SIGNATURE, &sinfo->sig, &lints);
+	if (ec) {
+	    if (lints) {
+		rasprintf(&sinfo->msg,
+			("%s tag %u: invalid OpenPGP signature: %s"),
+			origin, td->tag, lints);
+		free(lints);
+	    } else {
+		rasprintf(&sinfo->msg,
+			_("%s tag %u: invalid OpenPGP signature"),
+			origin, td->tag);
+	    }
 	    goto exit;
+	} else if (lints) {
+	    rpmlog(RPMLOG_WARNING, "%s\n", lints);
+	    free(lints);
 	}
 	sinfo->hashalgo = pgpDigParamsAlgo(sinfo->sig, PGPVAL_HASHALGO);
 	sinfo->keyid = pgpGrab(pgpDigParamsSignID(sinfo->sig)+4, 4);
diff --git a/rpmio/CMakeLists.txt b/rpmio/CMakeLists.txt
index 2fb5794b0..6aa9ab1f1 100644
--- a/rpmio/CMakeLists.txt
+++ b/rpmio/CMakeLists.txt
@@ -21,7 +21,7 @@ if (WITH_INTERNAL_OPENPGP)
 		target_link_libraries(librpmio PRIVATE PkgConfig::LIBGCRYPT)
 	endif()
 else()
-	pkg_check_modules(RPMSEQUOIA REQUIRED IMPORTED_TARGET rpm-sequoia>=1.3.0)
+	pkg_check_modules(RPMSEQUOIA REQUIRED IMPORTED_TARGET rpm-sequoia>=1.4.0)
 	target_sources(librpmio PRIVATE rpmpgp_sequoia.c)
 	target_link_libraries(librpmio PRIVATE PkgConfig::RPMSEQUOIA)
 endif()
diff --git a/rpmio/rpmkeyring.c b/rpmio/rpmkeyring.c
index 166ee43a2..e3eb9e6ea 100644
--- a/rpmio/rpmkeyring.c
+++ b/rpmio/rpmkeyring.c
@@ -276,7 +276,12 @@ rpmRC rpmKeyringVerifySig(rpmKeyring keyring, pgpDigParams sig, DIGEST_CTX ctx)
 	    pgpkey = key->pgpkey;
 
 	/* We call verify even if key not found for a signature sanity check */
-	rc = pgpVerifySignature(pgpkey, sig, ctx);
+	char *lints = NULL;
+	rc = pgpVerifySignature2(pgpkey, sig, ctx, &lints);
+	if (lints) {
+	    rpmlog(rc ? RPMLOG_ERR : RPMLOG_WARNING, "%s\n", lints);
+	    free(lints);
+	}
     }
 
     if (keyring)
diff --git a/rpmio/rpmpgp_internal.c b/rpmio/rpmpgp_internal.c
index ce1d3c27d..82972bcc8 100644
--- a/rpmio/rpmpgp_internal.c
+++ b/rpmio/rpmpgp_internal.c
@@ -1043,6 +1043,14 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
     return rc;
 }
 
+int pgpPrtParams2(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
+                  pgpDigParams * ret, char **lints)
+{
+    if (lints)
+        *lints = NULL;
+    return pgpPrtParams(pkts, pktlen, pkttype, ret);
+}
+
 int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
 			pgpDigParams mainkey, pgpDigParams **subkeys,
 			int *subkeysCount)
@@ -1179,6 +1187,13 @@ exit:
 
 }
 
+rpmRC pgpVerifySignature2(pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx, char **lints)
+{
+    if (lints)
+        *lints = NULL;
+    return pgpVerifySignature(key, sig, hashctx);
+}
+
 static pgpArmor decodePkts(uint8_t *b, uint8_t **pkt, size_t *pktlen)
 {
     const char * enc = NULL;
diff --git a/rpmio/rpmpgp_sequoia.c b/rpmio/rpmpgp_sequoia.c
index c6434270a..d0b673953 100644
--- a/rpmio/rpmpgp_sequoia.c
+++ b/rpmio/rpmpgp_sequoia.c
@@ -36,6 +36,9 @@ W(uint32_t, pgpDigParamsCreationTime, (pgpDigParams digp), (digp))
 W(rpmRC, pgpVerifySignature,
   (pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx),
   (key, sig, hashctx))
+W(rpmRC, pgpVerifySignature2,
+  (pgpDigParams key, pgpDigParams sig, DIGEST_CTX hashctx, char **lints),
+  (key, sig, hashctx, lints))
 W(int, pgpPubkeyKeyID,
   (const uint8_t * pkt, size_t pktlen, pgpKeyID_t keyid),
   (pkt, pktlen, keyid))
@@ -51,6 +54,10 @@ W(int, pgpPubKeyCertLen,
 W(int, pgpPrtParams,
   (const uint8_t *pkts, size_t pktlen, unsigned int pkttype, pgpDigParams *ret),
   (pkts, pktlen, pkttype, ret))
+W(int, pgpPrtParams2,
+  (const uint8_t *pkts, size_t pktlen, unsigned int pkttype, pgpDigParams *ret,
+   char **lints),
+  (pkts, pktlen, pkttype, ret, lints))
 W(int, pgpPrtParamsSubkeys,
   (const uint8_t *pkts, size_t pktlen,
    pgpDigParams mainkey, pgpDigParams **subkeys,
diff --git a/tests/rpmi.at b/tests/rpmi.at
index 9d74cf689..423d97bca 100644
--- a/tests/rpmi.at
+++ b/tests/rpmi.at
@@ -342,7 +342,7 @@ AT_CLEANUP
 
 AT_SETUP([rpm -U <corrupted signed 1>])
 AT_KEYWORDS([install])
-AT_CHECK([
+AT_CHECK_UNQUOTED([
 RPMDB_INIT
 
 pkg="hello-2.0-1.x86_64-signed.rpm"
@@ -355,7 +355,13 @@ runroot rpm -U --ignorearch --ignoreos --nodeps \
 ],
 [1],
 [],
-[error: /tmp/hello-2.0-1.x86_64-signed.rpm: Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
+[`if test x$PGP = xinternal; then
+    echo 'error: /tmp/hello-2.0-1.x86_64-signed.rpm: Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
+else
+    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:'
+    echo '  Failed to parse Signature Packet'
+    echo '      because: Malformed packet: Subpacket extends beyond the end of the subpacket area)'
+fi`
 error: /tmp/hello-2.0-1.x86_64-signed.rpm cannot be installed
 ])
 AT_CLEANUP
diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at
index 9fb3febc9..df1f669e4 100644
--- a/tests/rpmsigdig.at
+++ b/tests/rpmsigdig.at
@@ -386,17 +386,17 @@ AT_CHECK([
 RPMDB_INIT
 
 echo Checking package before importing key:
-runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 echo Importing key:
-runroot rpmkeys --quiet --import /data/keys/alice-expired-subkey.asc; echo $?
+runroot rpmkeys --quiet --import /data/keys/alice-expired-subkey.asc 2>&1; echo $?
 echo Checking for key:
 runroot rpm -qi gpg-pubkey-eb04e625-* | grep Version | head -n1
 echo Checking package after importing key:
-runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 echo Checking package after importing key, no digest:
-runroot rpmkeys --define '_pkgverify_level all' -Kv --nodigest /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv --nodigest /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 echo Checking package after importing key, no signature:
-runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 ],
 [0],
 [[Checking package before importing key:
@@ -416,6 +416,10 @@ Checking for key:
 Version     : eb04e625
 Checking package after importing key:
 /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
+error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <alice@example.org>):
+  Key 1F71177215217EE0 invalid: key is not alive
+      because: The subkey is not live
+      because: Expired on 2022-04-12T00:00:15Z
     Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
     Header DSA signature: NOTFOUND
     Header SHA256 digest: OK
@@ -427,6 +431,10 @@ Checking package after importing key:
 1
 Checking package after importing key, no digest:
 /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
+error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <alice@example.org>):
+  Key 1F71177215217EE0 invalid: key is not alive
+      because: The subkey is not live
+      because: Expired on 2022-04-12T00:00:15Z
     Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
     Header DSA signature: NOTFOUND
     RSA signature: NOTFOUND
@@ -455,15 +463,15 @@ RPMDB_INIT
 echo Checking package before importing key:
 runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
 echo Importing key:
-runroot rpmkeys --quiet --import /data/keys/alice-revoked-subkey.asc; echo $?
+runroot rpmkeys --quiet --import /data/keys/alice-revoked-subkey.asc 2>&1; echo $?
 echo Checking for key:
 runroot rpm -qi gpg-pubkey-eb04e625-* | grep Version | head -n1
 echo Checking package after importing key:
-runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 echo Checking package after importing key, no digest:
-runroot rpmkeys --define '_pkgverify_level all' -Kv --nodigest /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv --nodigest /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 echo Checking package after importing key, no signature:
-runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm; echo $?
+runroot rpmkeys --define '_pkgverify_level all' -Kv --nosignature /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm 2>&1; echo $?
 ],
 [0],
 [[Checking package before importing key:
@@ -483,6 +491,8 @@ Checking for key:
 Version     : eb04e625
 Checking package after importing key:
 /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
+error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <alice@example.org>):
+  Key 1F71177215217EE0 is invalid: key is revoked
     Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
     Header DSA signature: NOTFOUND
     Header SHA256 digest: OK
@@ -494,6 +504,8 @@ Checking package after importing key:
 1
 Checking package after importing key, no digest:
 /data/RPMS/hello-2.0-1.x86_64-signed-with-subkey.rpm:
+error: Verifying a signature using certificate B6542F92F30650C36B6F41BCB3A771BFEB04E625 (Alice <alice@example.org>):
+  Key 1F71177215217EE0 is invalid: key is revoked
     Header V4 RSA/SHA512 Signature, key ID 15217ee0: NOTTRUSTED
     Header DSA signature: NOTFOUND
     RSA signature: NOTFOUND
@@ -740,7 +752,7 @@ AT_CLEANUP
 # Test pre-built corrupted package verification (corrupted signature)
 AT_SETUP([rpmkeys -Kv <corrupted signed> 1])
 AT_KEYWORDS([rpmkeys digest signature])
-AT_CHECK([
+AT_CHECK_UNQUOTED([
 RPMDB_INIT
 
 pkg="hello-2.0-1.x86_64-signed.rpm"
@@ -754,14 +766,28 @@ runroot rpmkeys -Kv /tmp/${pkg}
 ],
 [1],
 [/tmp/hello-2.0-1.x86_64-signed.rpm:
-    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
+`if test x$PGP = xinternal; then
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
+else
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature: Parsing an OpenPGP packet:'
+    echo '  Failed to parse Signature Packet'
+    echo '      because: Signature appears to be created by a non-conformant OpenPGP implementation, see <https://github.com/rpm-software-management/rpm/issues/2351>.'
+    echo '      because: Malformed MPI: leading bit is not set: expected bit 1 to be set in        0 (0))'
+fi`
     Header SHA256 digest: OK
     Header SHA1 digest: OK
     Payload SHA256 digest: OK
     V4 RSA/SHA256 Signature, key ID 1964c5fc: NOKEY
     MD5 digest: OK
 /tmp/hello-2.0-1.x86_64-signed.rpm:
-    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)
+`if test x$PGP = xinternal; then
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature)'
+else
+    echo '    Header RSA signature: BAD (package tag 268: invalid OpenPGP signature: Parsing an OpenPGP packet:'
+    echo '  Failed to parse Signature Packet'
+    echo '      because: Signature appears to be created by a non-conformant OpenPGP implementation, see <https://github.com/rpm-software-management/rpm/issues/2351>.'
+    echo '      because: Malformed MPI: leading bit is not set: expected bit 1 to be set in        0 (0))'
+fi`
     Header SHA256 digest: OK
     Header SHA1 digest: OK
     Payload SHA256 digest: OK
-- 
2.40.1