Skip to content

Commit

Permalink
Ignore subkeys that cannot sign
Browse files Browse the repository at this point in the history
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.

Fixes rpm-software-management#1911.
  • Loading branch information
DemiMarie committed Feb 26, 2022
1 parent d44be2c commit 960ed38
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 9 deletions.
2 changes: 2 additions & 0 deletions rpmio/digest.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct pgpDigParams_s {
uint8_t * hash;
uint8_t tag;

uint8_t flags; /*!< key usae flags */
uint8_t version; /*!< version number. */
uint32_t time; /*!< key/signature creation time. */
uint8_t pubkey_algo; /*!< public key algorithm. */
Expand All @@ -39,6 +40,7 @@ struct pgpDigParams_s {
uint8_t saved;
#define PGPDIG_SAVED_TIME (1 << 0)
#define PGPDIG_SAVED_ID (1 << 1)
#define PGPDIG_SAVED_KEY_FLAGS (1 << 2)

pgpDigAlg alg;
};
Expand Down
52 changes: 45 additions & 7 deletions rpmio/rpmpgp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -498,6 +498,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_SAVED_KEY_FLAGS)
return 1;
impl = *p;
_digp->saved |= PGPDIG_SAVED_KEY_FLAGS;
_digp->flags = plen >= 2 ? p[1] : 0;
break;
case PGPSUBTYPE_EXPORTABLE_CERT:
case PGPSUBTYPE_TRUST_SIG:
case PGPSUBTYPE_REGEX:
Expand All @@ -508,7 +519,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:
Expand Down Expand Up @@ -601,6 +611,9 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
if (pgpVersion(h, hlen, &version))
return rc;

_digp->flags = 0;
_digp->saved &= (PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID);

switch (version) {
case 3:
{ pgpPktSigV3 v = (pgpPktSigV3)h;
Expand Down Expand Up @@ -630,6 +643,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen,
_digp->pubkey_algo = v->pubkey_algo;
_digp->hash_algo = v->hash_algo;
memcpy(_digp->signhash16, v->signhash16, sizeof(_digp->signhash16));
_digp->saved = PGPDIG_SAVED_TIME | PGPDIG_SAVED_ID;
}

p = ((uint8_t *)v) + sizeof(*v);
Expand Down Expand Up @@ -660,7 +674,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;

Expand All @@ -671,7 +685,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;

Expand All @@ -692,7 +706,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);
Expand Down Expand Up @@ -1136,7 +1150,8 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
int xx = 1; /* assume failure */

if (!(prevtag == PGPTAG_PUBLIC_SUBKEY &&
selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING))
(selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING ||
selfsig->version != 4)))
xx = pgpVerifySelf(digp, selfsig, all, i);

selfsig = pgpDigParamsFree(selfsig);
Expand Down Expand Up @@ -1178,6 +1193,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;
Expand Down Expand Up @@ -1210,10 +1226,32 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen,
pgpDigParamsFree(digps[count]);
continue;
}
count++;

if (!params)
params = pgpDigParamsNew(0);

if (decodePkt(p, (pend - p), &pkt) ||
pkt.tag != PGPTAG_SIGNATURE ||
pgpPrtSig(0, pkt.body, pkt.blen, params) ||
params->sigtype != PGPSIGTYPE_SUBKEY_BINDING ||
params->version != 4) {
pgpDigParamsFree(digps[count]);
break;
}

if (!((params->saved & PGPDIG_SAVED_KEY_FLAGS) &&
(params->flags & 0x02))) {
pgpDigParamsFree(digps[count]);
} else {
digps[count]->flags = params->flags;
digps[count]->saved |= PGPDIG_SAVED_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));
Expand Down
1 change: 1 addition & 0 deletions tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,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
Expand Down
20 changes: 20 additions & 0 deletions tests/data/keys/rpm.org-ed25519-subkey-test.pub
Original file line number Diff line number Diff line change
@@ -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-----
57 changes: 55 additions & 2 deletions tests/rpmsigdig.at
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 960ed38

Please sign in to comment.