Skip to content

Commit bd36c5d

Browse files
committed
Validate and require subkey binding signatures on PGP public keys
All subkeys must be followed by a binding signature by the primary key as per the OpenPGP RFC, enforce the presence and validity in the parser. The implementation is as kludgey as they come to work around our simple-minded parser structure without touching API, to maximise backportability. Store all the raw packets internally as we decode them to be able to access previous elements at will, needed to validate ordering and access the actual data. Add testcases for manipulated keys whose import previously would succeed. Depends on the two previous commits: 7b399fc and 236b802 Fixes CVE-2021-3521.
1 parent 9f03f42 commit bd36c5d

File tree

6 files changed

+209
-7
lines changed

6 files changed

+209
-7
lines changed

Diff for: rpmio/rpmpgp.c

+91-7
Original file line numberDiff line numberDiff line change
@@ -1062,37 +1062,121 @@ static pgpDigParams pgpDigParamsNew(uint8_t tag)
10621062
return digp;
10631063
}
10641064

1065+
static int hashKey(DIGEST_CTX hash, const struct pgpPkt *pkt, int exptag)
1066+
{
1067+
int rc = -1;
1068+
if (pkt->tag == exptag) {
1069+
uint8_t head[] = {
1070+
0x99,
1071+
(pkt->blen >> 8),
1072+
(pkt->blen ),
1073+
};
1074+
1075+
rpmDigestUpdate(hash, head, 3);
1076+
rpmDigestUpdate(hash, pkt->body, pkt->blen);
1077+
rc = 0;
1078+
}
1079+
return rc;
1080+
}
1081+
1082+
static int pgpVerifySelf(pgpDigParams key, pgpDigParams selfsig,
1083+
const struct pgpPkt *all, int i)
1084+
{
1085+
int rc = -1;
1086+
DIGEST_CTX hash = NULL;
1087+
1088+
switch (selfsig->sigtype) {
1089+
case PGPSIGTYPE_SUBKEY_BINDING:
1090+
hash = rpmDigestInit(selfsig->hash_algo, 0);
1091+
if (hash) {
1092+
rc = hashKey(hash, &all[0], PGPTAG_PUBLIC_KEY);
1093+
if (!rc)
1094+
rc = hashKey(hash, &all[i-1], PGPTAG_PUBLIC_SUBKEY);
1095+
}
1096+
break;
1097+
default:
1098+
/* ignore types we can't handle */
1099+
rc = 0;
1100+
break;
1101+
}
1102+
1103+
if (hash && rc == 0)
1104+
rc = pgpVerifySignature(key, selfsig, hash);
1105+
1106+
rpmDigestFinal(hash, NULL, NULL, 0);
1107+
1108+
return rc;
1109+
}
1110+
10651111
int pgpPrtParams(const uint8_t * pkts, size_t pktlen, unsigned int pkttype,
10661112
pgpDigParams * ret)
10671113
{
10681114
const uint8_t *p = pkts;
10691115
const uint8_t *pend = pkts + pktlen;
10701116
pgpDigParams digp = NULL;
1071-
struct pgpPkt pkt;
1117+
pgpDigParams selfsig = NULL;
1118+
int i = 0;
1119+
int alloced = 16; /* plenty for normal cases */
1120+
struct pgpPkt *all = xmalloc(alloced * sizeof(*all));
10721121
int rc = -1; /* assume failure */
1122+
int expect = 0;
1123+
int prevtag = 0;
10731124

10741125
while (p < pend) {
1075-
if (decodePkt(p, (pend - p), &pkt))
1126+
struct pgpPkt *pkt = &all[i];
1127+
if (decodePkt(p, (pend - p), pkt))
10761128
break;
10771129

10781130
if (digp == NULL) {
1079-
if (pkttype && pkt.tag != pkttype) {
1131+
if (pkttype && pkt->tag != pkttype) {
10801132
break;
10811133
} else {
1082-
digp = pgpDigParamsNew(pkt.tag);
1134+
digp = pgpDigParamsNew(pkt->tag);
10831135
}
10841136
}
10851137

1086-
if (pgpPrtPkt(&pkt, digp))
1138+
if (expect) {
1139+
if (pkt->tag != expect)
1140+
break;
1141+
selfsig = pgpDigParamsNew(pkt->tag);
1142+
}
1143+
1144+
if (pgpPrtPkt(pkt, selfsig ? selfsig : digp))
10871145
break;
10881146

1089-
p += (pkt.body - pkt.head) + pkt.blen;
1147+
if (selfsig) {
1148+
/* subkeys must be followed by binding signature */
1149+
if (prevtag == PGPTAG_PUBLIC_SUBKEY) {
1150+
if (selfsig->sigtype != PGPSIGTYPE_SUBKEY_BINDING)
1151+
break;
1152+
}
1153+
1154+
int xx = pgpVerifySelf(digp, selfsig, all, i);
1155+
1156+
selfsig = pgpDigParamsFree(selfsig);
1157+
if (xx)
1158+
break;
1159+
expect = 0;
1160+
}
1161+
1162+
if (pkt->tag == PGPTAG_PUBLIC_SUBKEY)
1163+
expect = PGPTAG_SIGNATURE;
1164+
prevtag = pkt->tag;
1165+
1166+
i++;
1167+
p += (pkt->body - pkt->head) + pkt->blen;
10901168
if (pkttype == PGPTAG_SIGNATURE)
10911169
break;
1170+
1171+
if (alloced <= i) {
1172+
alloced *= 2;
1173+
all = xrealloc(all, alloced * sizeof(*all));
1174+
}
10921175
}
10931176

1094-
rc = (digp && (p == pend)) ? 0 : -1;
1177+
rc = (digp && (p == pend) && expect == 0) ? 0 : -1;
10951178

1179+
free(all);
10961180
if (ret && rc == 0) {
10971181
*ret = digp;
10981182
} else {

Diff for: tests/Makefile.am

+3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ EXTRA_DIST += data/SPECS/hello-config-buildid.spec
108108
EXTRA_DIST += data/SPECS/hello-cd.spec
109109
EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.pub
110110
EXTRA_DIST += data/keys/rpm.org-rsa-2048-test.secret
111+
EXTRA_DIST += data/keys/CVE-2021-3521-badbind.asc
112+
EXTRA_DIST += data/keys/CVE-2021-3521-nosubsig.asc
113+
EXTRA_DIST += data/keys/CVE-2021-3521-nosubsig-last.asc
111114
EXTRA_DIST += data/macros.testfile
112115
EXTRA_DIST += data/macros.debug
113116
EXTRA_DIST += data/SOURCES/foo.c

Diff for: tests/data/keys/CVE-2021-3521-badbind.asc

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
-----BEGIN PGP PUBLIC KEY BLOCK-----
2+
Version: rpm-4.17.90 (NSS-3)
3+
4+
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g
5+
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY
6+
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8
7+
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas
8+
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ
9+
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl
10+
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK
11+
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf
12+
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB
13+
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr
14+
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX
15+
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq
16+
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN
17+
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY
18+
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz
19+
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6
20+
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c
21+
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m
22+
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE=
23+
=WCfs
24+
-----END PGP PUBLIC KEY BLOCK-----
25+

Diff for: tests/data/keys/CVE-2021-3521-nosubsig-last.asc

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
-----BEGIN PGP PUBLIC KEY BLOCK-----
2+
Version: rpm-4.17.90 (NSS-3)
3+
4+
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g
5+
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY
6+
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8
7+
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas
8+
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ
9+
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl
10+
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK
11+
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf
12+
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB
13+
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr
14+
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX
15+
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq
16+
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN
17+
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY
18+
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz
19+
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6
20+
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c
21+
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m
22+
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAE=
23+
=WCfs
24+
-----END PGP PUBLIC KEY BLOCK-----
25+

Diff for: tests/data/keys/CVE-2021-3521-nosubsig.asc

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
-----BEGIN PGP PUBLIC KEY BLOCK-----
2+
Version: rpm-4.17.90 (NSS-3)
3+
4+
mQENBFjmORgBCAC7TMEk6wnjSs8Dr4yqSScWdU2pjcqrkTxuzdWvowcIUPZI0w/g
5+
HkRqGd4apjvY2V15kjL10gk3QhFP3pZ/9p7zh8o8NHX7aGdSGDK7NOq1eFaErPRY
6+
91LW9RiZ0lbOjXEzIL0KHxUiTQEmdXJT43DJMFPyW9fkCWg0OltiX618FUdWWfI8
7+
eySdLur1utnqBvdEbCUvWK2RX3vQZQdvEBODnNk2pxqTyV0w6VPQ96W++lF/5Aas
8+
7rUv3HIyIXxIggc8FRrnH+y9XvvHDonhTIlGnYZN4ubm9i4y3gOkrZlGTrEw7elQ
9+
1QeMyG2QQEbze8YjpTm4iLABCBrRfPRaQpwrABEBAAG0IXJwbS5vcmcgUlNBIHRl
10+
c3RrZXkgPHJzYUBycG0ub3JnPokBNwQTAQgAIQUCWOY5GAIbAwULCQgHAgYVCAkK
11+
CwIEFgIDAQIeAQIXgAAKCRBDRFkeGWTF/MxxCACnjqFL+MmPh9W9JQKT2DcLbBzf
12+
Cqo6wcEBoCOcwgRSk8dSikhARoteoa55JRJhuMyeKhhEAogE9HRmCPFdjezFTwgB
13+
BDVBpO2dZ023mLXDVCYX3S8pShOgCP6Tn4wqCnYeAdLcGg106N4xcmgtcssJE+Pr
14+
XzTZksbZsrTVEmL/Ym+R5w5jBfFnGk7Yw7ndwfQsfNXQb5AZynClFxnX546lcyZX
15+
fEx3/e6ezw57WNOUK6WT+8b+EGovPkbetK/rGxNXuWaP6X4A/QUm8O98nCuHYFQq
16+
+mvNdsCBqGf7mhaRGtpHk/JgCn5rFvArMDqLVrR9hX0LdCSsH7EGE+bR3r7wuQEN
17+
BFjmORgBCACk+vDZrIXQuFXEYToZVwb2attzbbJJCqD71vmZTLsW0QxuPKRgbcYY
18+
zp4K4lVBnHhFrF8MOUOxJ7kQWIJZMZFt+BDcptCYurbD2H4W2xvnWViiC+LzCMzz
19+
iMJT6165uefL4JHTDPxC2fFiM9yrc72LmylJNkM/vepT128J5Qv0gRUaQbHiQuS6
20+
Dm/+WRnUfx3i89SV4mnBxb/Ta93GVqoOciWwzWSnwEnWYAvOb95JL4U7c5J5f/+c
21+
KnQDHsW7sIiIdscsWzvgf6qs2Ra1Zrt7Fdk4+ZS2f/adagLhDO1C24sXf5XfMk5m
22+
L0OGwZSr9m5s17VXxfspgU5ugc8kBJfzABEBAAG5AQ0EWOY5GAEIAKT68NmshdC4
23+
VcRhOhlXBvZq23NtskkKoPvW+ZlMuxbRDG48pGBtxhjOngriVUGceEWsXww5Q7En
24+
uRBYglkxkW34ENym0Ji6tsPYfhbbG+dZWKIL4vMIzPOIwlPrXrm558vgkdMM/ELZ
25+
8WIz3KtzvYubKUk2Qz+96lPXbwnlC/SBFRpBseJC5LoOb/5ZGdR/HeLz1JXiacHF
26+
v9Nr3cZWqg5yJbDNZKfASdZgC85v3kkvhTtzknl//5wqdAMexbuwiIh2xyxbO+B/
27+
qqzZFrVmu3sV2Tj5lLZ/9p1qAuEM7ULbixd/ld8yTmYvQ4bBlKv2bmzXtVfF+ymB
28+
Tm6BzyQEl/MAEQEAAYkBHwQYAQgACQUCWOY5GAIbDAAKCRBDRFkeGWTF/PANB/9j
29+
mifmj6z/EPe0PJFhrpISt9PjiUQCt0IPtiL5zKAkWjHePIzyi+0kCTBF6DDLFxos
30+
3vN4bWnVKT1kBhZAQlPqpJTg+m74JUYeDGCdNx9SK7oRllATqyu+5rncgxjWVPnQ
31+
zu/HRPlWJwcVFYEVXYL8xzfantwQTqefjmcRmBRdA2XJITK+hGWwAmrqAWx+q5xX
32+
Pa8wkNMxVzNS2rUKO9SoVuJ/wlUvfoShkJ/VJ5HDp3qzUqncADfdGN35TDzscngQ
33+
gHvnMwVBfYfSCABV1hNByoZcc/kxkrWMmsd/EnIyLd1Q1baKqc3cEDuC6E6/o4yJ
34+
E4XX4jtDmdZPreZALsiB
35+
=rRop
36+
-----END PGP PUBLIC KEY BLOCK-----
37+

Diff for: tests/rpmsigdig.at

+28
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,34 @@ gpg(185e6146f00650f8) = 4:185e6146f00650f8-58e63918
240240
[])
241241
AT_CLEANUP
242242

243+
AT_SETUP([rpmkeys --import invalid keys])
244+
AT_KEYWORDS([rpmkeys import])
245+
RPMDB_INIT
246+
247+
AT_CHECK([
248+
runroot rpmkeys --import /data/keys/CVE-2021-3521-badbind.asc
249+
],
250+
[1],
251+
[],
252+
[error: /data/keys/CVE-2021-3521-badbind.asc: key 1 import failed.]
253+
)
254+
AT_CHECK([
255+
runroot rpmkeys --import /data/keys/CVE-2021-3521-nosubsig.asc
256+
],
257+
[1],
258+
[],
259+
[error: /data/keys/CVE-2021-3521-nosubsig.asc: key 1 import failed.]
260+
)
261+
262+
AT_CHECK([
263+
runroot rpmkeys --import /data/keys/CVE-2021-3521-nosubsig-last.asc
264+
],
265+
[1],
266+
[],
267+
[error: /data/keys/CVE-2021-3521-nosubsig-last.asc: key 1 import failed.]
268+
)
269+
AT_CLEANUP
270+
243271
# ------------------------------
244272
# Test pre-built package verification
245273
AT_SETUP([rpmkeys -K <signed> 1])

0 commit comments

Comments
 (0)