-
Notifications
You must be signed in to change notification settings - Fork 375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate self-signatures and require subkey bindings on PGP public keys #1788
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -543,7 +543,7 @@ pgpDigAlg pgpDigAlgFree(pgpDigAlg alg) | |||||||||
return NULL; | ||||||||||
} | ||||||||||
|
||||||||||
static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo, uint8_t sigtype, | ||||||||||
static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo, | ||||||||||
const uint8_t *p, const uint8_t *h, size_t hlen, | ||||||||||
pgpDigParams sigp) | ||||||||||
{ | ||||||||||
|
@@ -556,10 +556,8 @@ static int pgpPrtSigParams(pgpTag tag, uint8_t pubkey_algo, uint8_t sigtype, | |||||||||
int mpil = pgpMpiLen(p); | ||||||||||
if (pend - p < mpil) | ||||||||||
break; | ||||||||||
if (sigtype == PGPSIGTYPE_BINARY || sigtype == PGPSIGTYPE_TEXT) { | ||||||||||
if (sigalg->setmpi(sigalg, i, p)) | ||||||||||
break; | ||||||||||
} | ||||||||||
if (sigalg->setmpi(sigalg, i, p)) | ||||||||||
break; | ||||||||||
p += mpil; | ||||||||||
} | ||||||||||
|
||||||||||
|
@@ -619,7 +617,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, v->sigtype, p, h, hlen, _digp); | ||||||||||
rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp); | ||||||||||
} break; | ||||||||||
case 4: | ||||||||||
{ pgpPktSigV4 v = (pgpPktSigV4)h; | ||||||||||
|
@@ -678,7 +676,7 @@ static int pgpPrtSig(pgpTag tag, const uint8_t *h, size_t hlen, | |||||||||
if (p > hend) | ||||||||||
return 1; | ||||||||||
|
||||||||||
rc = pgpPrtSigParams(tag, v->pubkey_algo, v->sigtype, p, h, hlen, _digp); | ||||||||||
rc = pgpPrtSigParams(tag, v->pubkey_algo, p, h, hlen, _digp); | ||||||||||
} break; | ||||||||||
default: | ||||||||||
rpmlog(RPMLOG_WARNING, _("Unsupported version of signature: V%d\n"), version); | ||||||||||
|
@@ -1057,38 +1055,154 @@ unsigned int pgpDigParamsAlgo(pgpDigParams digp, unsigned int algotype) | |||||||||
return algo; | ||||||||||
} | ||||||||||
|
||||||||||
static pgpDigParams pgpDigParamsNew(uint8_t tag) | ||||||||||
{ | ||||||||||
pgpDigParams digp = xcalloc(1, sizeof(*digp)); | ||||||||||
digp->tag = tag; | ||||||||||
return digp; | ||||||||||
} | ||||||||||
|
||||||||||
static int hashKey(DIGEST_CTX hash, const struct pgpPkt *pkt, int exptag) | ||||||||||
{ | ||||||||||
int rc = -1; | ||||||||||
if (pkt->tag == exptag) { | ||||||||||
uint8_t head[] = { | ||||||||||
0x99, | ||||||||||
(pkt->blen >> 8), | ||||||||||
(pkt->blen ), | ||||||||||
}; | ||||||||||
|
||||||||||
rpmDigestUpdate(hash, head, 3); | ||||||||||
rpmDigestUpdate(hash, pkt->body, pkt->blen); | ||||||||||
rc = 0; | ||||||||||
} | ||||||||||
return rc; | ||||||||||
} | ||||||||||
|
||||||||||
static int hashUser(DIGEST_CTX hash, const struct pgpPkt *pkt, int exptag) | ||||||||||
{ | ||||||||||
int rc = -1; | ||||||||||
if (pkt->tag == exptag) { | ||||||||||
uint8_t head[] = { | ||||||||||
0xb4, | ||||||||||
(pkt->blen >> 24), | ||||||||||
(pkt->blen >> 16), | ||||||||||
(pkt->blen >> 8), | ||||||||||
(pkt->blen ), | ||||||||||
}; | ||||||||||
rpmDigestUpdate(hash, head, 5); | ||||||||||
rpmDigestUpdate(hash, pkt->body, pkt->blen); | ||||||||||
rc = 0; | ||||||||||
} | ||||||||||
return rc; | ||||||||||
} | ||||||||||
|
||||||||||
static int pgpVerifySelf(pgpDigParams key, pgpDigParams selfsig, | ||||||||||
const struct pgpPkt *all, int i) | ||||||||||
{ | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're suggesting to add two lines of a comment to explain what an assert() is? Come on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Edited There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The point is that we don't sprinkle material like this around. This is just redundant clutter in the codebase which makes it unreadable. Whenever you feel the need to add a comment or an assert, it's more likely because the code in question is dumb and could be written in a better way. Such as here. |
||||||||||
int rc = -1; | ||||||||||
DIGEST_CTX hash = NULL; | ||||||||||
|
||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to verify the package types of the package you're hashing, i.e. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it actually matter though? I mean, if the type is wrong then the hash will also be wrong and simply fail to pass? Okay, as a belt and suspenders kind of thing I suppose... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also think that it does not matter, but it's the right thing to do for security relevant code ;-) Anyway, for the _CERT verification you can't use all[i-1] as there may be multiple signatures. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, good point on the CERT verification (annoying as it may be...) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changed it to check for the types and more errors overall, userid lookup in the CERT case still missing. |
||||||||||
switch (selfsig->sigtype) { | ||||||||||
case PGPSIGTYPE_SUBKEY_BINDING: | ||||||||||
hash = rpmDigestInit(selfsig->hash_algo, 0); | ||||||||||
if (hash) { | ||||||||||
rc = hashKey(hash, &all[0], PGPTAG_PUBLIC_KEY); | ||||||||||
if (!rc) | ||||||||||
rc = hashKey(hash, &all[i-1], PGPTAG_PUBLIC_SUBKEY); | ||||||||||
} | ||||||||||
break; | ||||||||||
case PGPSIGTYPE_GENERIC_CERT: | ||||||||||
case PGPSIGTYPE_PERSONA_CERT: | ||||||||||
case PGPSIGTYPE_CASUAL_CERT: | ||||||||||
case PGPSIGTYPE_POSITIVE_CERT: | ||||||||||
hash = rpmDigestInit(selfsig->hash_algo, 0); | ||||||||||
if (hash) { | ||||||||||
rc = hashKey(hash, &all[0], PGPTAG_PUBLIC_KEY); | ||||||||||
if (!rc) | ||||||||||
rc = hashUser(hash, &all[i-1], PGPTAG_USER_ID); | ||||||||||
} | ||||||||||
break; | ||||||||||
default: | ||||||||||
/* ignore unknown types */ | ||||||||||
rc = 0; | ||||||||||
Comment on lines
+1127
to
+1128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, rejecting types we cannot handle would only cause us to fail on perfectly legitimate keys. IIRC the PGP spec quite specifically tells you to ignore what you don't know, which generally is the key to future expandable standards. |
||||||||||
break; | ||||||||||
} | ||||||||||
|
||||||||||
if (hash && rc == 0) | ||||||||||
rc = pgpVerifySignature(key, selfsig, hash); | ||||||||||
|
||||||||||
rpmDigestFinal(hash, NULL, NULL, 0); | ||||||||||
|
||||||||||
return rc; | ||||||||||
} | ||||||||||
|
||||||||||
int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype, | ||||||||||
pgpDigParams * ret) | ||||||||||
{ | ||||||||||
const uint8_t *p = pkts; | ||||||||||
const uint8_t *pend = pkts + pktlen; | ||||||||||
pgpDigParams digp = NULL; | ||||||||||
struct pgpPkt pkt; | ||||||||||
pgpDigParams selfsig = NULL; | ||||||||||
int i = 0; | ||||||||||
int alloced = 16; /* plenty for normal cases */ | ||||||||||
struct pgpPkt *all = xmalloc(alloced * sizeof(*all)); | ||||||||||
int rc = -1; /* assume failure */ | ||||||||||
int expect = 0; | ||||||||||
|
||||||||||
while (p < pend) { | ||||||||||
if (decodePkt(p, (pend - p), &pkt)) | ||||||||||
struct pgpPkt *pkt = &all[i]; | ||||||||||
if (decodePkt(p, (pend - p), pkt)) | ||||||||||
break; | ||||||||||
|
||||||||||
if (pkt->tag == PGPTAG_PUBLIC_SUBKEY) | ||||||||||
expect = PGPTAG_SIGNATURE; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also enforce a self-sig on a User ID Packet? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered that, but it'd be against the spec:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be good to differentiate between verified and non-verified somehow though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe GPG ignores User ID packets that have no self-signatures. RPM should either reject such packets or ignore them; processing them as if they were verified is a bad idea. |
||||||||||
|
||||||||||
if (digp == NULL) { | ||||||||||
if (pkttype && pkt.tag != pkttype) { | ||||||||||
if (pkttype && pkt->tag != pkttype) { | ||||||||||
break; | ||||||||||
} else { | ||||||||||
digp = xcalloc(1, sizeof(*digp)); | ||||||||||
digp->tag = pkt.tag; | ||||||||||
digp = pgpDigParamsNew(pkt->tag); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
if (pgpPrtPkt(&pkt, digp)) | ||||||||||
if (digp->tag == PGPTAG_PUBLIC_KEY && pkt->tag == PGPTAG_SIGNATURE) | ||||||||||
selfsig = pgpDigParamsNew(pkt->tag); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code assumes that the self-sig always comes first if there are multiple signatures. Does the standard say something about this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you should also check that the issuer id matches and ignore non-selfsigned sigs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://datatracker.ietf.org/doc/html/rfc4880#section-11.1 says:
Surely a signature with a different issuer will simply not pass? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, but that's a problem as the signature checking code below is done for all signatures, not just the subkey binding one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm probably Monday dense here, but I fail to see the problem. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it's me ;) My point is that selfsig will also be set for signatures that use a different key (i.e. not self-signed) and the code below will call pgpVerifySelf on them and then break the loop as the verification will fail. (Of course I'm talking about _CERT sigs, not the subkey binding one) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fine with assuming and requiring that the self-signature comes first, provided that doing so is interoperable. Keys with non-self-signatures obviously need to be accepted, but I am fine with RPM ignoring the extra signatures and omitting them from the parsed result. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right, got it. I think. This is all a surprisingly nasty tangle of logic. Perhaps I should split the _CERT part out of this to simplify, that's not part of the CVE anyway and that's what I want to get out of the way. |
||||||||||
|
||||||||||
if (pgpPrtPkt(pkt, selfsig ? selfsig : digp)) | ||||||||||
break; | ||||||||||
|
||||||||||
p += (pkt.body - pkt.head) + pkt.blen; | ||||||||||
/* subkeys must be followed by binding signature */ | ||||||||||
if (i > 0 && all[i-1].tag == PGPTAG_PUBLIC_SUBKEY) | ||||||||||
if (!(selfsig && selfsig->sigtype == PGPSIGTYPE_SUBKEY_BINDING)) | ||||||||||
break; | ||||||||||
|
||||||||||
if (selfsig) { | ||||||||||
int xx = -1; | ||||||||||
|
||||||||||
if (i > 0) | ||||||||||
xx = pgpVerifySelf(digp, selfsig, all, i); | ||||||||||
|
||||||||||
selfsig = pgpDigParamsFree(selfsig); | ||||||||||
if (xx) | ||||||||||
break; | ||||||||||
expect = 0; | ||||||||||
} | ||||||||||
|
||||||||||
i++; | ||||||||||
p += (pkt->body - pkt->head) + pkt->blen; | ||||||||||
if (pkttype == PGPTAG_SIGNATURE) | ||||||||||
break; | ||||||||||
|
||||||||||
if (alloced <= i) { | ||||||||||
alloced *= 2; | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is a last-ditch check against memory corruption due to integer overflow. Such long keys should be rejected earlier in any case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. Just no. I've told you before, we wont be littering the code-base with stuff like this. If we did, nobody could find the actual code from underneath. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To elaborate on that a bit, the suggested change is simply absurd when you could simply place an upper bound and error out if exceeded. |
||||||||||
all = xrealloc(all, alloced * sizeof(*all)); | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
rc = (digp && (p == pend)) ? 0 : -1; | ||||||||||
rc = (digp && (p == pend) && expect == 0) ? 0 : -1; | ||||||||||
|
||||||||||
free(all); | ||||||||||
if (ret && rc == 0) { | ||||||||||
*ret = digp; | ||||||||||
} else { | ||||||||||
|
@@ -1123,8 +1237,7 @@ int pgpPrtParamsSubkeys(const uint8_t *pkts, size_t pktlen, | |||||||||
digps = xrealloc(digps, alloced * sizeof(*digps)); | ||||||||||
} | ||||||||||
|
||||||||||
digps[count] = xcalloc(1, sizeof(**digps)); | ||||||||||
digps[count]->tag = PGPTAG_PUBLIC_SUBKEY; | ||||||||||
digps[count] = pgpDigParamsNew(PGPTAG_PUBLIC_SUBKEY); | ||||||||||
/* Copy UID from main key to subkey */ | ||||||||||
digps[count]->userid = xstrdup(mainkey->userid); | ||||||||||
|
||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
-----BEGIN PGP PUBLIC KEY BLOCK----- | ||
Version: rpm-4.17.90 (NSS-3) | ||
|
||
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g | ||
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY | ||
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 | ||
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas | ||
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ | ||
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl | ||
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK | ||
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf | ||
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB | ||
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr | ||
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX | ||
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq | ||
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN | ||
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY | ||
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz | ||
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 | ||
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c | ||
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m | ||
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE= | ||
=WCfs | ||
-----END PGP PUBLIC KEY BLOCK----- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
-----BEGIN PGP PUBLIC KEY BLOCK----- | ||
Version: rpm-4.17.90 (NSS-3) | ||
|
||
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g | ||
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY | ||
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 | ||
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas | ||
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ | ||
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IWYwMGYucmcgUlNBIHRl | ||
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK | ||
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf | ||
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB | ||
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr | ||
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX | ||
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq | ||
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN | ||
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY | ||
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz | ||
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 | ||
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c | ||
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m | ||
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAGJAR8EGAEIAAkFAljmORgCGwwA | ||
CgkQQ0RZHhlkxfzwDQf/Y5on5o+s/xD3tDyRYa6SErfT44lEArdCD7Yi+cygJFox | ||
3jyM8ovtJAkwRegwyxcaLN7zeG1p1Sk9ZAYWQEJT6qSU4Ppu+CVGHgxgnTcfUiu6 | ||
EZZQE6srvua53IMY1lT50M7vx0T5VicHFRWBFV2C/Mc32p7cEE6nn45nEZgUXQNl | ||
ySEyvoRlsAJq6gFsfqucVz2vMJDTMVczUtq1CjvUqFbif8JVL36EoZCf1SeRw6d6 | ||
s1Kp3AA33Rjd+Uw87HJ4EIB75zMFQX2H0ggAVdYTQcqGXHP5MZK1jJrHfxJyMi3d | ||
UNW2iqnN3BA7guhOv6OMiROF1+I7Q5nWT63mQC7IgQ== | ||
=sE8L | ||
-----END PGP PUBLIC KEY BLOCK----- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
-----BEGIN PGP PUBLIC KEY BLOCK----- | ||
Version: rpm-4.17.90 (NSS-3) | ||
|
||
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g | ||
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY | ||
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 | ||
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas | ||
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ | ||
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl | ||
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK | ||
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf | ||
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB | ||
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr | ||
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX | ||
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq | ||
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN | ||
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY | ||
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz | ||
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 | ||
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c | ||
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m | ||
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE= | ||
=WCfs | ||
-----END PGP PUBLIC KEY BLOCK----- | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
-----BEGIN PGP PUBLIC KEY BLOCK----- | ||
Version: rpm-4.17.90 (NSS-3) | ||
|
||
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g | ||
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY | ||
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8 | ||
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas | ||
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ | ||
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl | ||
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK | ||
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf | ||
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB | ||
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr | ||
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX | ||
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq | ||
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN | ||
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY | ||
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz | ||
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6 | ||
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c | ||
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m | ||
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAG5AQ0EWOY5GAEIAKT68NmshdC4 | ||
VcRhOhlXBvZq23NtskkKoPvW+ZlMuxbRDG48pGBtxhjOngriVUGceEWsXww5Q7En | ||
uRBYglkxkW34ENym0Ji6tsPYfhbbG+dZWKIL4vMIzPOIwlPrXrm558vgkdMM/ELZ | ||
8WIz3KtzvYubKUk2Qz+96lPXbwnlC/SBFRpBseJC5LoOb/5ZGdR/HeLz1JXiacHF | ||
v9Nr3cZWqg5yJbDNZKfASdZgC85v3kkvhTtzknl//5wqdAMexbuwiIh2xyxbO+B/ | ||
qqzZFrVmu3sV2Tj5lLZ/9p1qAuEM7ULbixd/ld8yTmYvQ4bBlKv2bmzXtVfF+ymB | ||
Tm6BzyQEl/MAEQEAAYkBHwQYAQgACQUCWOY5GAIbDAAKCRBDRFkeGWTF/PANB/9j | ||
mifmj6z/EPe0PJFhrpISt9PjiUQCt0IPtiL5zKAkWjHePIzyi+0kCTBF6DDLFxos | ||
3vN4bWnVKT1kBhZAQlPqpJTg+m74JUYeDGCdNx9SK7oRllATqyu+5rncgxjWVPnQ | ||
zu/HRPlWJwcVFYEVXYL8xzfantwQTqefjmcRmBRdA2XJITK+hGWwAmrqAWx+q5xX | ||
Pa8wkNMxVzNS2rUKO9SoVuJ/wlUvfoShkJ/VJ5HDp3qzUqncADfdGN35TDzscngQ | ||
gHvnMwVBfYfSCABV1hNByoZcc/kxkrWMmsd/EnIyLd1Q1baKqc3cEDuC6E6/o4yJ | ||
E4XX4jtDmdZPreZALsiB | ||
=rRop | ||
-----END PGP PUBLIC KEY BLOCK----- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, RPM needs to check that package signatures are in fact signatures of binary documents. I am not aware of an actual exploit, but it is the Right Thing To Do, since a security patch should not regress security elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, instead of removing the check maybe we should check for the kinds that we actually support in the PGP parser. We dont do TEXT anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A certification or binding signature isn’t a valid package signature and should not be accepted where a package signature is required. The parser doesn’t currently know what kind of signature is expected and so can’t make that decision. That said, another option would be to add a new API function that only parses signatures, and which expects the type of the signature as an argument.