Skip to content

Commit fbd15ac

Browse files
prasdenprasden
authored
Merge branch 'main' into ocsp-tighten-url-parsing
2 parents 929e18c + 4207584 commit fbd15ac

6 files changed

Lines changed: 100 additions & 11 deletions

File tree

‎crypto/evp_extra/p_kem_asn1.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,14 @@ static int kem_get_pub_raw(const EVP_PKEY *pkey, uint8_t *out,
8686
return 1;
8787
}
8888

89+
// kem_cmp_parameters returns 1 if |a| and |b| hold populated KEM keys with
90+
// the same KEM NID, 0 if their NIDs differ, or -2 if either operand is
91+
// missing its key or parameters. The tri-state return aligns with the
92+
// |EVP_PKEY_cmp| convention (1 = equal, 0 = not equal, negative = error).
8993
static int kem_cmp_parameters(const EVP_PKEY *a, const EVP_PKEY *b) {
94+
if (a == NULL || b == NULL) {
95+
return -2;
96+
}
9097
const KEM_KEY *a_key = a->pkey.kem_key;
9198
const KEM_KEY *b_key = b->pkey.kem_key;
9299
if (a_key == NULL || b_key == NULL) {

‎crypto/evp_extra/p_pqdsa_asn1.c‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,42 @@ static int pqdsa_pub_encode(CBB *out, const EVP_PKEY *pkey) {
137137
return 1;
138138
}
139139

140+
// pqdsa_cmp_parameters returns 1 if |a| and |b| hold populated PQDSA keys
141+
// with the same ML-DSA NID, 0 if their NIDs differ, or -2 if either operand
142+
// is missing its key or parameters. The tri-state return aligns with the
143+
// |EVP_PKEY_cmp| convention (1 = equal, 0 = not equal, negative = error).
144+
static int pqdsa_cmp_parameters(const EVP_PKEY *a, const EVP_PKEY *b) {
145+
if (a == NULL || b == NULL) {
146+
return -2;
147+
}
148+
const PQDSA_KEY *a_key = a->pkey.pqdsa_key;
149+
const PQDSA_KEY *b_key = b->pkey.pqdsa_key;
150+
if (a_key == NULL || b_key == NULL) {
151+
return -2;
152+
}
153+
154+
const PQDSA *a_pqdsa = a_key->pqdsa;
155+
const PQDSA *b_pqdsa = b_key->pqdsa;
156+
if (a_pqdsa == NULL || b_pqdsa == NULL) {
157+
return -2;
158+
}
159+
160+
return a_pqdsa->nid == b_pqdsa->nid;
161+
}
162+
140163
static int pqdsa_pub_cmp(const EVP_PKEY *a, const EVP_PKEY *b) {
141-
PQDSA_KEY *a_key = a->pkey.pqdsa_key;
142-
PQDSA_KEY *b_key = b->pkey.pqdsa_key;
164+
int ret = pqdsa_cmp_parameters(a, b);
165+
if (ret <= 0) {
166+
return ret;
167+
}
143168

144-
return OPENSSL_memcmp(a_key->public_key,
145-
b_key->public_key,
146-
a->pkey.pqdsa_key->pqdsa->public_key_len) == 0;
169+
const PQDSA_KEY *a_key = a->pkey.pqdsa_key;
170+
const PQDSA_KEY *b_key = b->pkey.pqdsa_key;
171+
if (a_key->public_key == NULL || b_key->public_key == NULL) {
172+
return -2;
173+
}
174+
return OPENSSL_memcmp(a_key->public_key, b_key->public_key,
175+
a_key->pqdsa->public_key_len) == 0;
147176
}
148177

149178
static int pqdsa_priv_decode(EVP_PKEY *out, CBS *oid, CBS *params, CBS *key, CBS *pubkey) {

‎crypto/ocsp/ocsp_server.c‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,33 @@ int OCSP_basic_add1_cert(OCSP_BASICRESP *resp, X509 *cert) {
6565
return 1;
6666
}
6767

68+
// ocsp_respid_clear frees any previously-installed content of |respid| using
69+
// the destructor matching the current |respid->type|, so callers can install
70+
// a new value without leaking or triggering a type-confused free.
71+
static void ocsp_respid_clear(OCSP_RESPID *respid) {
72+
switch (respid->type) {
73+
case V_OCSP_RESPID_NAME:
74+
X509_NAME_free(respid->value.byName);
75+
respid->value.byName = NULL;
76+
break;
77+
case V_OCSP_RESPID_KEY:
78+
ASN1_OCTET_STRING_free(respid->value.byKey);
79+
respid->value.byKey = NULL;
80+
break;
81+
}
82+
}
83+
6884
static int OCSP_RESPID_set_by_name(OCSP_RESPID *respid, X509 *cert) {
6985
GUARD_PTR(respid);
7086
GUARD_PTR(cert);
71-
if (!X509_NAME_set(&respid->value.byName, X509_get_subject_name(cert))) {
87+
88+
X509_NAME *byName = X509_NAME_dup(X509_get_subject_name(cert));
89+
if (byName == NULL) {
7290
return 0;
7391
}
74-
92+
ocsp_respid_clear(respid);
7593
respid->type = V_OCSP_RESPID_NAME;
94+
respid->value.byName = byName;
7695
return 1;
7796
}
7897

@@ -95,6 +114,7 @@ static int OCSP_RESPID_set_by_key(OCSP_RESPID *respid, X509 *cert) {
95114
ASN1_OCTET_STRING_free(byKey);
96115
return 0;
97116
}
117+
ocsp_respid_clear(respid);
98118
respid->type = V_OCSP_RESPID_KEY;
99119
respid->value.byKey = byKey;
100120

‎crypto/ocsp/ocsp_test.cc‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1293,6 +1293,38 @@ TEST(OCSPResponseSignTestExtended, OCSPResponseSign) {
12931293
pkey.get(), EVP_sha256(), additional_cert.get(),
12941294
OCSP_NOCERTS));
12951295
EXPECT_EQ((int)sk_X509_num(basic_response.get()->certs), 0);
1296+
1297+
// Regression: re-signing the same |OCSP_BASICRESP| through any sequence of
1298+
// |OCSP_RESPID_KEY| flag combinations must not leak or free the prior
1299+
// responderId union arm through the wrong destructor. ASAN should flag any
1300+
// misuse.
1301+
basic_response.reset(OCSP_BASICRESP_new());
1302+
ASSERT_TRUE(basic_response);
1303+
EXPECT_TRUE(OCSP_basic_sign(basic_response.get(), signer_cert.get(),
1304+
pkey.get(), EVP_sha256(), additional_cert.get(),
1305+
0));
1306+
EXPECT_EQ(basic_response.get()->tbsResponseData->responderId->type,
1307+
V_OCSP_RESPID_NAME);
1308+
EXPECT_TRUE(OCSP_basic_sign(basic_response.get(), signer_cert.get(),
1309+
pkey.get(), EVP_sha256(), additional_cert.get(),
1310+
OCSP_RESPID_KEY));
1311+
EXPECT_EQ(basic_response.get()->tbsResponseData->responderId->type,
1312+
V_OCSP_RESPID_KEY);
1313+
EXPECT_TRUE(OCSP_basic_sign(basic_response.get(), signer_cert.get(),
1314+
pkey.get(), EVP_sha256(), additional_cert.get(),
1315+
OCSP_RESPID_KEY));
1316+
EXPECT_EQ(basic_response.get()->tbsResponseData->responderId->type,
1317+
V_OCSP_RESPID_KEY);
1318+
EXPECT_TRUE(OCSP_basic_sign(basic_response.get(), signer_cert.get(),
1319+
pkey.get(), EVP_sha256(), additional_cert.get(),
1320+
0));
1321+
EXPECT_EQ(basic_response.get()->tbsResponseData->responderId->type,
1322+
V_OCSP_RESPID_NAME);
1323+
EXPECT_TRUE(OCSP_basic_sign(basic_response.get(), signer_cert.get(),
1324+
pkey.get(), EVP_sha256(), additional_cert.get(),
1325+
0));
1326+
EXPECT_EQ(basic_response.get()->tbsResponseData->responderId->type,
1327+
V_OCSP_RESPID_NAME);
12961328
}
12971329

12981330
static const char extended_good_http_request_hdr[] =

‎crypto/pem/pem_lib.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,12 +269,12 @@ int PEM_ASN1_write_bio(i2d_of_void *i2d, const char *name, BIO *bp, void *x,
269269
callback = PEM_def_callback;
270270
}
271271
pass_len = (*callback)(buf, PEM_BUFSIZE, 1, u);
272-
if (pass_len < 0) {
273-
OPENSSL_PUT_ERROR(PEM, PEM_R_READ_KEY);
274-
goto err;
275-
}
276272
pass = (const unsigned char *)buf;
277273
}
274+
if (pass_len < 0) {
275+
OPENSSL_PUT_ERROR(PEM, PEM_R_READ_KEY);
276+
goto err;
277+
}
278278
assert(iv_len <= sizeof(iv));
279279
AWSLC_ABORT_IF_NOT_ONE(RAND_bytes(iv, iv_len)); // Generate a salt
280280

‎crypto/pem/pem_test.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ TEST(PEMTest, WriteReadRSAPem) {
132132
ASSERT_TRUE(write_bio);
133133
const EVP_CIPHER* cipher = EVP_get_cipherbynid(NID_aes_256_cbc);
134134
ASSERT_TRUE(cipher);
135+
EXPECT_FALSE(PEM_write_bio_RSAPrivateKey(write_bio.get(), rsa.get(), cipher, (unsigned char*)SECRET, -1, nullptr, nullptr));
135136
ASSERT_TRUE(PEM_write_bio_RSAPrivateKey(write_bio.get(), rsa.get(), cipher, (unsigned char*)SECRET, (int)BUF_strnlen(SECRET, 256), nullptr, nullptr));
136137

137138
const uint8_t* content;

0 commit comments

Comments
 (0)