Skip to content

Commit

Permalink
Restrict packets following a subkey
Browse files Browse the repository at this point in the history
According to RFC 4880, subkeys are supposed to only be followed by
binding signatures, revocation signatures, and other subkeys.  In
practice, the most likely reason to get something else is a revocation
signature of a type that RPM does not implement.  This could include a
revocation of a subkey binding signature or a revocation made by a
designated revoker.  In this case, it is not safe to import the key, as
RPM might accept a signature made by a subkey whose secret portion has
been compromised.  Therefore, return an error in this case.
  • Loading branch information
DemiMarie committed Apr 12, 2022
1 parent 2e8ba35 commit ae35666
Showing 1 changed file with 22 additions and 13 deletions.
35 changes: 22 additions & 13 deletions rpmio/rpmpgp_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -927,8 +927,12 @@ static int parseSubkeySig(const struct pgpPkt *pkt, uint8_t tag,
pgpDigParams *params_p) {
pgpDigParams params = *params_p = NULL; /* assume failure */

if (pkt->tag != PGPTAG_SIGNATURE)
if (pkt->tag != PGPTAG_SIGNATURE) {
rpmlog(RPMLOG_WARNING, "Key had an unexpected packet (type %u) "
"where a subkey signature was expected",
(unsigned int)pkt->tag);
goto fail;
}

params = pgpDigParamsNew(tag);

Expand All @@ -938,6 +942,12 @@ static int parseSubkeySig(const struct pgpPkt *pkt, uint8_t tag,
if (params->sigtype != PGPSIGTYPE_SUBKEY_BINDING &&
params->sigtype != PGPSIGTYPE_SUBKEY_REVOKE)
{
/* It isn't safe to ignore this, because it could be a
* revocation of a binding signature or something else that
* should cause the subkey to be ignored. Return an error
* here, but log a warning so that users know what is going on. */
rpmlog(RPMLOG_WARNING, _("Unsupported subkey signature type %u\n"),
(unsigned int)params->sigtype);
goto fail;
}

Expand Down Expand Up @@ -970,7 +980,6 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
const uint8_t *p = pkts;
const uint8_t *pend = pkts + pktlen;
pgpDigParams digp = NULL;
pgpDigParams selfsig = NULL;
int i = 0;
int alloced = 16; /* plenty for normal cases */
int rc = -1; /* assume failure */
Expand All @@ -994,24 +1003,25 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
}
}

if (expect) {
if (pkt->tag != expect)
break;
selfsig = pgpDigParamsNew(pkt->tag);
}

if (pgpPrtPkt(pkt, selfsig ? selfsig : digp))
if (expect && pkt->tag != expect)
break;
expect = 0;

if (selfsig) {
/* Do we need to verify a self-signature? */
if (prevtag == PGPTAG_PUBLIC_SUBKEY &&
pkt->tag != PGPTAG_PUBLIC_SUBKEY)
{
pgpDigParams selfsig = NULL;
if (parseSubkeySig(pkt, pkt->tag, &selfsig))
break;
/* subkeys must be followed by binding or revocation signature */
int xx = pgpVerifySelf(digp, selfsig, all, i, prevtag);

selfsig = pgpDigParamsFree(selfsig);
if (xx)
break;
expect = 0;
}
} else if (pgpPrtPkt(pkt, digp))
break;

if (pkt->tag == PGPTAG_PUBLIC_SUBKEY)
expect = PGPTAG_SIGNATURE;
Expand All @@ -1032,7 +1042,6 @@ int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
rc = (digp && (p == pend) && expect == 0) ? 0 : -1;

free(all);
selfsig = pgpDigParamsFree(selfsig);
if (ret && rc == 0) {
*ret = digp;
} else {
Expand Down

0 comments on commit ae35666

Please sign in to comment.