Ignore subkeys that cannot sign

This ensures that a signature is only accepted if the key in question
can actually be used for signing.  Tests 265 and 266 verify that RPM
completely ignores keys that are not capable of signing.

A subkey is considered to be capable of signing if, and only if, its
subkey binding signature has a hashed key flags subpacket that contains
the flag 0x02.  No signature, regardless of type, is permitted to have
more than one hashed key flags subpacket.  Unhashed key flags subpackets
are ignored.  The presence of a hashed key flags subpacket is specified
by the flag PGPDIG_HAS_KEY_FLAGS in struct pgpDigParams_s::saved.  A new
`hashed` parameter is added to pgpPrtSubType() to allow hashed and
unhashed subpackets to be distinguished.

Since PGPDIG_HAS_KEY_FLAGS must not be inherited from a previous use of
this `struct pgpDigParams_s`, it is cleared by pgpPrtSig() whenever it
processes a new signature.  All other saved flags, with the exception of
PGPDIG_SAVED_ID and PGPDIG_SAVED_TIME, are also cleared.  This allows
for future changes to store per-key state without worrying that it will
be inherited from a previously used key.  The key usage flags are
cleared too, although this is not strictly necessary.

RFC4880 requires that a subkey binding signature be v4 if the subkey is
capable of signing.  Requiring a hashed key flags subpacket for such
signatures enforces this requirement implicitly, as v3 signatures do not
have subpackets.  RFC4880 also requires that primary key binding
signatures be present and checked.  This is not implemented in this
commit, but may be implemented in a future commit.

To avoid a performance penalty, pgpPrtSig() is changed to ignore the
MPIs of a signature if its `tag` parameter is 0.  The only caller that
sets `tag` to 0 is pgpPrtParamSubkeys(), which does not actually check
any cryptographic signatures.  The subkey binding signature has been
checked earlier in pgpPrtParams().

Finally, use the new `hashed` parameter to pgpPrtSubType() to explicitly
ignore *all* creation time subpackets in the unhashed section, even
those that are of a length RPM does not know about.

Fixes rpm-software-management#1911.

rpmio/digest.h

@@ -27,6 +27,7 @@ struct pgpDigParams_s {
     uint8_t * hash;
     uint8_t tag;
 
+    uint8_t key_flags;		/*!< key usage flags */
     uint8_t version;		/*!< version number. */
     uint32_t time;		/*!< key/signature creation time. */
     uint8_t pubkey_algo;		/*!< public key algorithm. */
@@ -41,6 +42,7 @@ struct pgpDigParams_s {
 #define	PGPDIG_SAVED_TIME	(1 << 0)
 #define	PGPDIG_SAVED_ID		(1 << 1)
 #define	PGPDIG_SIG_HAS_CREATION_TIME	(1 << 2)
+#define	PGPDIG_SIG_HAS_KEY_FLAGS	(1 << 3)
 
     pgpDigAlg alg;
 };

rpmio/rpmpgp.c

@@ -437,7 +437,7 @@ int pgpSignatureType(pgpDigParams _digp)
 }
 
 static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype, 
-			 pgpDigParams _digp)
+			 pgpDigParams _digp, int hashed)
 {
     const uint8_t *p = h;
     size_t plen = 0, i;
@@ -474,6 +474,8 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype,
 		pgpPrtVal(" ", pgpKeyServerPrefsTbl, p[i]);
 	    break;
 	case PGPSUBTYPE_SIG_CREATE_TIME:  /* signature creation time */
+	    if (!hashed)
+		break; /* RFC 4880 §5.2.3.4 creation time MUST be hashed */
 	    if (plen-1 != sizeof(_digp->time))
 		break; /* other lengths not understood */
 	    if (_digp->saved & PGPDIG_SIG_HAS_CREATION_TIME)
@@ -498,6 +500,17 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype,
 		_digp->saved |= PGPDIG_SAVED_ID;
 		memcpy(_digp->signid, p+1, sizeof(_digp->signid));
 	    }
+	case PGPSUBTYPE_KEY_FLAGS: /* Key usage flags */
+	    /* Subpackets in the unhashed section cannot be trusted */
+	    if (!hashed)
+		break;
+	    /* Reject duplicate key usage flags */
+	    if (_digp->saved & PGPDIG_SIG_HAS_KEY_FLAGS)
+		return 1;
+	    impl = *p;
+	    _digp->saved |= PGPDIG_SIG_HAS_KEY_FLAGS;
+	    _digp->key_flags = plen >= 2 ? p[1] : 0;
+	    break;
 	case PGPSUBTYPE_EXPORTABLE_CERT:
 	case PGPSUBTYPE_TRUST_SIG:
 	case PGPSUBTYPE_REGEX:
@@ -508,7 +521,6 @@ static int pgpPrtSubType(const uint8_t *h, size_t hlen, pgpSigType sigtype,
 	case PGPSUBTYPE_PREFER_KEYSERVER:
 	case PGPSUBTYPE_PRIMARY_USERID:
 	case PGPSUBTYPE_POLICY_URL:
-	case PGPSUBTYPE_KEY_FLAGS:
 	case PGPSUBTYPE_SIGNER_USERID:
 	case PGPSUBTYPE_REVOKE_REASON:
 	case PGPSUBTYPE_FEATURES:
@@ -600,6 +612,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
 
     /* Reset the saved flags */
     _digp->saved &= PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID;
+    _digp->key_flags = 0;
 
     if (pgpVersion(h, hlen, &version))
 	return rc;
@@ -639,7 +652,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, p, h, hlen, _digp);
+	rc = tag ? pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp) : 0;
     }	break;
     case 4:
     {   pgpPktSigV4 v = (pgpPktSigV4)h;
@@ -666,7 +679,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
 	    _digp->hashlen = sizeof(*v) + plen;
 	    _digp->hash = memcpy(xmalloc(_digp->hashlen), v, _digp->hashlen);
 	}
-	if (pgpPrtSubType(p, plen, v->sigtype, _digp))
+	if (pgpPrtSubType(p, plen, v->sigtype, _digp, 1))
 	    return 1;
 	p += plen;
 
@@ -680,7 +693,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
 	if ((p + plen) > hend)
 	    return 1;
 
-	if (pgpPrtSubType(p, plen, v->sigtype, _digp))
+	if (pgpPrtSubType(p, plen, v->sigtype, _digp, 0))
 	    return 1;
 	p += plen;
 
@@ -701,7 +714,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
 	if (p > hend)
 	    return 1;
 
-	rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp);
+	rc = tag ? pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp) : 0;
     }	break;
     default:
 	rpmlog(RPMLOG_WARNING, _("Unsupported version of signature: V%d\n"), version);
@@ -1104,6 +1117,29 @@ static int pgpVerifySelf(pgpDigParams key, pgpDigParams selfsig,
     return rc;
 }
 
+static int parseSubkeySig(const struct pgpPkt *pkt, uint8_t tag,
+			  pgpDigParams *params_p) {
+    int rc = -1; /* assume failure */
+    pgpDigParams params = *params_p;
+
+    if (params == NULL)
+	params = *params_p = pgpDigParamsNew(tag);
+
+    if (pkt->tag != PGPTAG_SIGNATURE)
+	goto fail;
+
+    if (pgpPrtSig(tag, pkt->body, pkt->blen, params))
+	goto fail;
+
+    if (params->sigtype != PGPSIGTYPE_SUBKEY_BINDING &&
+	params->sigtype != PGPSIGTYPE_SUBKEY_REVOKE)
+	goto fail;
+
+    rc = 0;
+fail:
+    return rc;
+}
+
 static const size_t RPM_MAX_OPENPGP_BYTES = 65535; /* max number of bytes in a key */
 
 int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
@@ -1193,6 +1229,7 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
     const uint8_t *p = pkts;
     const uint8_t *pend = pkts + pktlen;
     pgpDigParams *digps = NULL;
+    pgpDigParams params = NULL;
     int count = 0;
     int alloced = 10;
     struct pgpPkt pkt;
@@ -1225,10 +1262,29 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
 		pgpDigParamsFree(digps[count]);
 		continue;
 	    }
-	    count++;
+
+	    if (decodePkt(p, pend - p, &pkt) ||
+	        parseSubkeySig(&pkt, 0, &params))
+	    {
+		pgpDigParamsFree(digps[count]);
+		break;
+	    }
+
+	    if (params->sigtype != PGPSIGTYPE_SUBKEY_BINDING ||
+		!((params->saved & PGPDIG_SIG_HAS_KEY_FLAGS) &&
+		  (params->key_flags & 0x02))) {
+		/* Subkey is revoked or incapable of signing */
+		pgpDigParamsFree(digps[count]);
+	    } else {
+		digps[count]->key_flags = params->key_flags;
+		digps[count]->saved |= PGPDIG_SIG_HAS_KEY_FLAGS;
+		count++;
+	    }
+	    p += (pkt.body - pkt.head) + pkt.blen;
 	}
     }
     rc = (p == pend) ? 0 : -1;
+    pgpDigParamsFree(params);
 
     if (rc == 0) {
 	*subkeys = xrealloc(digps, count * sizeof(*digps));

tests/Makefile.am

@@ -113,6 +113,7 @@ EXTRA_DIST += data/SOURCES/hello.c
 EXTRA_DIST += data/SPECS/hello-attr-buildid.spec
 EXTRA_DIST += data/SPECS/hello-config-buildid.spec
 EXTRA_DIST += data/SPECS/hello-cd.spec
+EXTRA_DIST += data/keys/rpm.org-ed25519-subkey-test.pub
 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

tests/data/keys/rpm.org-ed25519-subkey-test.pub

@@ -0,0 +1,20 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMEYhf4KRYJKwYBBAHaRw8BAQdAr/MfCGYyI1lNpG4h5Tvdw62G9CgDsn+nweQA
+bYtC9bC0NXJwbS5vcmcgZWQyNTUxOSB0ZXN0a2V5IHdpdGggc3Via2V5IDxlZDI1
+NTE5QHJwbS5vcmc+iJoEExYKAEIWIQROGuLaBUTEjXgeGiLaH1/8yyIDHwUCYhf5
+UAIbAwUJA8JnAAULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcCF4AACgkQ2h9f/Msi
+Ax/VxQEAm5tVLQRyLHzHpSXz8i8IZsK5kZHGsiDAlvgDubMD5d8A/3ig8MgOBlri
+HdsUvvpZma/cJQgfyRHjfh9fkQOOg+cAuDgEYhf4KRIKKwYBBAGXVQEFAQEHQFNp
+5tOJofXuDMVlzjnUC1FXp4La5BWkMH469jIbFlhAAwEIB4h4BBgWCgAgFiEEThri
+2gVExI14Hhoi2h9f/MsiAx8FAmIX+CkCGwwACgkQ2h9f/MsiAx+kNAEA0eXCnGqX
+jdaaPs1cKeGAkgzGwnwvut+YPD7+9H0r1eIA/RQw5yoTIJUButy55lx/mLGUPq0G
+N1RhDNg9wosqxfkCuDMEYhf4ehYJKwYBBAHaRw8BAQdASWiPf9/Cd4yD3GtDZFI+
+pcPhdhfWb5Ndv1hP+ndTkSyI7wQYFgoAIBYhBE4a4toFRMSNeB4aItofX/zLIgMf
+BQJiF/h6AhsCAIEJENofX/zLIgMfdiAEGRYKAB0WIQTc1zsT3tx9EJeerHXL9SUW
+mxSfwgUCYhf4egAKCRDL9SUWmxSfwrjXAPwJf7n8qpEvee1/wCFr0XwObuhOZ+vB
+ZeubWyEpWe/XqwEA1WCah/7cgAhYf4u1V7Ciel0kQskE0aAOvtJE/EZqCwix5wEA
+14PxZWyO0PrU8AeKR2U/A9jM/81SpWjNMT7X52UYFmgBAL3GaEXFIC5STG9Qsopd
+t1vfoExOCG5Ewg7CWOeLF9EB
+=Fk+p
+-----END PGP PUBLIC KEY BLOCK-----

tests/rpmsigdig.at

@@ -247,8 +247,61 @@ UNW2iqnN3BA7guhOv6OMiROF1+I7Q5nWT63mQC7IgQ==
 gpg(rpm.org RSA testkey <[email protected]>) = 4:4344591e1964c5fc-58e63918
 gpg(1964c5fc) = 4:4344591e1964c5fc-58e63918
 gpg(4344591e1964c5fc) = 4:4344591e1964c5fc-58e63918
-gpg(f00650f8) = 4:185e6146f00650f8-58e63918
-gpg(185e6146f00650f8) = 4:185e6146f00650f8-58e63918
+],
+[])
+AT_CLEANUP
+
+# -----------------------------------------
+# Import a public Ed25519 key with a subkey
+AT_SETUP([rpmkeys --import ed25519 with subkey])
+AT_KEYWORDS([rpmkeys import])
+AT_CHECK([
+RPMDB_INIT
+
+runroot rpmkeys --import /data/keys/rpm.org-ed25519-subkey-test.pub
+runroot rpm -qi gpg-pubkey-cb22031f-6217f950|grep -v Date|grep -v Version:
+runroot rpm -q --provides gpg-pubkey
+],
+[0],
+[Name        : gpg-pubkey
+Version     : cb22031f
+Release     : 6217f950
+Architecture: (none)
+Group       : Public Keys
+Size        : 0
+License     : pubkey
+Signature   : (none)
+Source RPM  : (none)
+Build Host  : localhost
+Packager    : rpm.org ed25519 testkey with subkey <[email protected]>
+Summary     : rpm.org ed25519 testkey with subkey <[email protected]> public key
+Description :
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+
+mDMEYhf4KRYJKwYBBAHaRw8BAQdAr/MfCGYyI1lNpG4h5Tvdw62G9CgDsn+nweQA
+bYtC9bC0NXJwbS5vcmcgZWQyNTUxOSB0ZXN0a2V5IHdpdGggc3Via2V5IDxlZDI1
+NTE5QHJwbS5vcmc+iJoEExYKAEIWIQROGuLaBUTEjXgeGiLaH1/8yyIDHwUCYhf5
+UAIbAwUJA8JnAAULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcCF4AACgkQ2h9f/Msi
+Ax/VxQEAm5tVLQRyLHzHpSXz8i8IZsK5kZHGsiDAlvgDubMD5d8A/3ig8MgOBlri
+HdsUvvpZma/cJQgfyRHjfh9fkQOOg+cAuDgEYhf4KRIKKwYBBAGXVQEFAQEHQFNp
+5tOJofXuDMVlzjnUC1FXp4La5BWkMH469jIbFlhAAwEIB4h4BBgWCgAgFiEEThri
+2gVExI14Hhoi2h9f/MsiAx8FAmIX+CkCGwwACgkQ2h9f/MsiAx+kNAEA0eXCnGqX
+jdaaPs1cKeGAkgzGwnwvut+YPD7+9H0r1eIA/RQw5yoTIJUButy55lx/mLGUPq0G
+N1RhDNg9wosqxfkCuDMEYhf4ehYJKwYBBAHaRw8BAQdASWiPf9/Cd4yD3GtDZFI+
+pcPhdhfWb5Ndv1hP+ndTkSyI7wQYFgoAIBYhBE4a4toFRMSNeB4aItofX/zLIgMf
+BQJiF/h6AhsCAIEJENofX/zLIgMfdiAEGRYKAB0WIQTc1zsT3tx9EJeerHXL9SUW
+mxSfwgUCYhf4egAKCRDL9SUWmxSfwrjXAPwJf7n8qpEvee1/wCFr0XwObuhOZ+vB
+ZeubWyEpWe/XqwEA1WCah/7cgAhYf4u1V7Ciel0kQskE0aAOvtJE/EZqCwix5wEA
+14PxZWyO0PrU8AeKR2U/A9jM/81SpWjNMT7X52UYFmgBAL3GaEXFIC5STG9Qsopd
+t1vfoExOCG5Ewg7CWOeLF9EB
+=Fk+p
+-----END PGP PUBLIC KEY BLOCK-----
+
+gpg(rpm.org ed25519 testkey with subkey <[email protected]>) = 4:da1f5ffccb22031f-6217f950
+gpg(cb22031f) = 4:da1f5ffccb22031f-6217f950
+gpg(da1f5ffccb22031f) = 4:da1f5ffccb22031f-6217f950
+gpg(9b149fc2) = 4:cbf525169b149fc2-6217f87a
+gpg(cbf525169b149fc2) = 4:cbf525169b149fc2-6217f87a
 ],
 [])
 AT_CLEANUP