Skip to content

Commit d4f233c

Browse files
justsmthjustsmth
authored
Upstream merge 2024-03-21 (#1506)
### Description of changes: Merging from Upstream considering commits from google/boringssl@aa533e0 (Nov 20, 2023) to google/boringssl@d9b81bb (Nov 27, 2023). ### Call-outs: See internal document as well as "AWS-LC" notes inserted in some of the commit messages for additions/deviations from the upstream commit. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.
2 parents d3ad9d3 + 18e6bf5 commit d4f233c

File tree

9 files changed

+406
-436
lines changed

9 files changed

+406
-436
lines changed

‎crypto/CMakeLists.txt

-2
Original file line numberDiff line numberDiff line change
@@ -484,9 +484,7 @@ add_library(
484484
x509/x_attrib.c
485485
x509/x_crl.c
486486
x509/x_exten.c
487-
x509/x_info.c
488487
x509/x_name.c
489-
x509/x_pkey.c
490488
x509/x_pubkey.c
491489
x509/x_req.c
492490
x509/x_sig.c

‎crypto/pem/pem_info.c

+31
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,37 @@
6969
#include <openssl/rsa.h>
7070
#include <openssl/x509.h>
7171

72+
73+
static X509_PKEY *X509_PKEY_new(void) {
74+
return OPENSSL_zalloc(sizeof(X509_PKEY));
75+
}
76+
77+
static void X509_PKEY_free(X509_PKEY *x) {
78+
if (x == NULL) {
79+
return;
80+
}
81+
82+
EVP_PKEY_free(x->dec_pkey);
83+
OPENSSL_free(x);
84+
}
85+
86+
static X509_INFO *X509_INFO_new(void) {
87+
return OPENSSL_zalloc(sizeof(X509_INFO));
88+
}
89+
90+
void X509_INFO_free(X509_INFO *x) {
91+
if (x == NULL) {
92+
return;
93+
}
94+
95+
X509_free(x->x509);
96+
X509_CRL_free(x->crl);
97+
X509_PKEY_free(x->x_pkey);
98+
OPENSSL_free(x->enc_data);
99+
OPENSSL_free(x);
100+
}
101+
102+
72103
STACK_OF(X509_INFO) *PEM_X509_INFO_read(FILE *fp, STACK_OF(X509_INFO) *sk,
73104
pem_password_cb *cb, void *u) {
74105
BIO *b = BIO_new_fp(fp, BIO_NOCLOSE);

‎crypto/x509/x509_lu.c

+41-51
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,22 @@
6565
#include "../internal.h"
6666
#include "internal.h"
6767

68-
X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method) {
69-
X509_LOOKUP *ret;
7068

71-
ret = (X509_LOOKUP *)OPENSSL_zalloc(sizeof(X509_LOOKUP));
69+
static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
70+
X509_NAME *name);
71+
static X509_OBJECT *X509_OBJECT_retrieve_by_subject(
72+
STACK_OF(X509_OBJECT) *h, int type, X509_NAME *name);
73+
static X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
74+
X509_OBJECT *x);
75+
static int X509_OBJECT_up_ref_count(X509_OBJECT *a);
76+
77+
static X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method);
78+
static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name,
79+
X509_OBJECT *ret);
80+
static int X509_LOOKUP_shutdown(X509_LOOKUP *ctx);
81+
82+
static X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method) {
83+
X509_LOOKUP *ret = OPENSSL_zalloc(sizeof(X509_LOOKUP));
7284
if (ret == NULL) {
7385
return NULL;
7486
}
@@ -91,18 +103,7 @@ void X509_LOOKUP_free(X509_LOOKUP *ctx) {
91103
OPENSSL_free(ctx);
92104
}
93105

94-
int X509_LOOKUP_init(X509_LOOKUP *ctx) {
95-
if (ctx->method == NULL) {
96-
return 0;
97-
}
98-
if (ctx->method->init != NULL) {
99-
return ctx->method->init(ctx);
100-
} else {
101-
return 1;
102-
}
103-
}
104-
105-
int X509_LOOKUP_shutdown(X509_LOOKUP *ctx) {
106+
static int X509_LOOKUP_shutdown(X509_LOOKUP *ctx) {
106107
if (ctx->method == NULL) {
107108
return 0;
108109
}
@@ -125,14 +126,18 @@ int X509_LOOKUP_ctrl(X509_LOOKUP *ctx, int cmd, const char *argc, long argl,
125126
}
126127
}
127128

128-
int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name,
129-
X509_OBJECT *ret) {
129+
static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name,
130+
X509_OBJECT *ret) {
130131
if ((ctx->method == NULL) || (ctx->method->get_by_subject == NULL)) {
131132
return 0;
132133
}
133134
if (ctx->skip) {
134135
return 0;
135136
}
137+
// Note |get_by_subject| leaves |ret| in an inconsistent state. It has
138+
// pointers to an |X509| or |X509_CRL|, but has not bumped the refcount yet.
139+
// For now, the caller is expected to fix this, but ideally we'd fix the
140+
// |X509_LOOKUP| convention itself.
136141
return ctx->method->get_by_subject(ctx, type, name, ret) > 0;
137142
}
138143

@@ -217,21 +222,6 @@ int X509_STORE_up_ref(X509_STORE *store) {
217222
return 1;
218223
}
219224

220-
static void cleanup(X509_OBJECT *a) {
221-
if (a == NULL) {
222-
return;
223-
}
224-
if (a->type == X509_LU_X509) {
225-
X509_free(a->data.x509);
226-
} else if (a->type == X509_LU_CRL) {
227-
X509_CRL_free(a->data.crl);
228-
} else {
229-
// abort();
230-
}
231-
232-
OPENSSL_free(a);
233-
}
234-
235225
void X509_STORE_free(X509_STORE *vfy) {
236226
size_t j;
237227
STACK_OF(X509_LOOKUP) *sk;
@@ -254,7 +244,7 @@ void X509_STORE_free(X509_STORE *vfy) {
254244
X509_LOOKUP_free(lu);
255245
}
256246
sk_X509_LOOKUP_free(sk);
257-
sk_X509_OBJECT_pop_free(vfy->objs, cleanup);
247+
sk_X509_OBJECT_pop_free(vfy->objs, X509_OBJECT_free);
258248

259249
if (vfy->param) {
260250
X509_VERIFY_PARAM_free(vfy->param);
@@ -328,7 +318,7 @@ static int x509_store_add(X509_STORE *ctx, void *x, int is_crl) {
328318
return 0;
329319
}
330320

331-
X509_OBJECT *const obj = (X509_OBJECT *)OPENSSL_malloc(sizeof(X509_OBJECT));
321+
X509_OBJECT *const obj = X509_OBJECT_new();
332322
if (obj == NULL) {
333323
return 0;
334324
}
@@ -354,8 +344,7 @@ static int x509_store_add(X509_STORE *ctx, void *x, int is_crl) {
354344
CRYPTO_MUTEX_unlock_write(&ctx->objs_lock);
355345

356346
if (!added) {
357-
X509_OBJECT_free_contents(obj);
358-
OPENSSL_free(obj);
347+
X509_OBJECT_free(obj);
359348
}
360349

361350
return ret;
@@ -370,14 +359,18 @@ int X509_STORE_add_crl(X509_STORE *ctx, X509_CRL *x) {
370359
}
371360

372361
X509_OBJECT *X509_OBJECT_new(void) {
373-
X509_OBJECT *ret = OPENSSL_zalloc(sizeof(X509_OBJECT));
374-
if (ret == NULL) {
375-
return NULL;
362+
return OPENSSL_zalloc(sizeof(X509_OBJECT));
363+
}
364+
365+
void X509_OBJECT_free(X509_OBJECT *obj) {
366+
if (obj == NULL) {
367+
return;
376368
}
377-
return ret;
369+
X509_OBJECT_free_contents(obj);
370+
OPENSSL_free(obj);
378371
}
379372

380-
int X509_OBJECT_up_ref_count(X509_OBJECT *a) {
373+
static int X509_OBJECT_up_ref_count(X509_OBJECT *a) {
381374
switch (a->type) {
382375
case X509_LU_X509:
383376
X509_up_ref(a->data.x509);
@@ -398,11 +391,8 @@ void X509_OBJECT_free_contents(X509_OBJECT *a) {
398391
X509_CRL_free(a->data.crl);
399392
break;
400393
}
401-
}
402394

403-
void X509_OBJECT_free(X509_OBJECT *a) {
404-
X509_OBJECT_free_contents(a);
405-
OPENSSL_free(a);
395+
OPENSSL_memset(a, 0, sizeof(X509_OBJECT));
406396
}
407397

408398
int X509_OBJECT_get_type(const X509_OBJECT *a) { return a->type; }
@@ -488,13 +478,13 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, int type,
488478
return (int)idx;
489479
}
490480

491-
int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
492-
X509_NAME *name) {
481+
static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type,
482+
X509_NAME *name) {
493483
return x509_object_idx_cnt(h, type, name, NULL);
494484
}
495485

496-
X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h, int type,
497-
X509_NAME *name) {
486+
X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h,
487+
int type, X509_NAME *name) {
498488
int idx;
499489
idx = X509_OBJECT_idx_by_subject(h, type, name);
500490
if (idx == -1) {
@@ -589,8 +579,8 @@ STACK_OF(X509_CRL) *X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) {
589579
return sk;
590580
}
591581

592-
X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
593-
X509_OBJECT *x) {
582+
static X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h,
583+
X509_OBJECT *x) {
594584
sk_X509_OBJECT_sort(h);
595585
size_t idx;
596586
if (!sk_X509_OBJECT_find_awslc(h, &idx, x)) {

‎crypto/x509/x509_test.cc

+14-9
Original file line numberDiff line numberDiff line change
@@ -1851,7 +1851,7 @@ TEST(X509Test, MatchFoundSetsPeername) {
18511851
char *peername = nullptr;
18521852
EXPECT_NE(1, X509_check_host(leaf.get(), kWrongHostname, strlen(kWrongHostname), 0, &peername));
18531853
ASSERT_EQ(nullptr, peername);
1854-
1854+
18551855
EXPECT_EQ(1, X509_check_host(leaf.get(), kHostname, strlen(kHostname), 0, &peername));
18561856
EXPECT_STREQ(peername, kHostname);
18571857
OPENSSL_free(peername);
@@ -3888,6 +3888,18 @@ TEST(X509Test, NullStore) {
38883888
EXPECT_FALSE(X509_STORE_CTX_init(ctx.get(), nullptr, leaf.get(), nullptr));
38893889
}
38903890

3891+
TEST(X509Test, StoreCtxReuse) {
3892+
bssl::UniquePtr<X509> leaf(CertFromPEM(kLeafPEM));
3893+
ASSERT_TRUE(leaf);
3894+
bssl::UniquePtr<X509_STORE> store(X509_STORE_new());
3895+
ASSERT_TRUE(store);
3896+
bssl::UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
3897+
ASSERT_TRUE(ctx);
3898+
ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), nullptr));
3899+
// Re-initializing |ctx| should not leak memory.
3900+
ASSERT_TRUE(X509_STORE_CTX_init(ctx.get(), store.get(), leaf.get(), nullptr));
3901+
}
3902+
38913903
TEST(X509Test, BasicConstraints) {
38923904
const uint32_t kFlagMask = EXFLAG_CA | EXFLAG_BCONS | EXFLAG_INVALID;
38933905

@@ -5074,15 +5086,8 @@ TEST(X509Test, AddDuplicates) {
50745086
ASSERT_TRUE(X509_STORE_lock(store.get()));
50755087
// Sleep after taking the lock to cause contention. Sleep longer than the
50765088
// adder half of threads to ensure we hold the lock while they contend
5077-
// for it. |X509_OBJECT_retrieve_by_subject| is called because it doesn't
5078-
// take a lock on the store, thus avoiding deadlock.
5089+
// for it.
50795090
std::this_thread::sleep_for(std::chrono::microseconds(11 + (sleep_buf[0] % 5)));
5080-
EXPECT_TRUE(X509_OBJECT_retrieve_by_subject(
5081-
store->objs, X509_LU_X509, X509_get_subject_name(a.get())
5082-
));
5083-
EXPECT_TRUE(X509_OBJECT_retrieve_by_subject(
5084-
store->objs, X509_LU_X509, X509_get_subject_name(b.get())
5085-
));
50865091
ASSERT_TRUE(X509_STORE_unlock(store.get()));
50875092
});
50885093
}

‎crypto/x509/x509_vfy.c

+4-14
Original file line numberDiff line numberDiff line change
@@ -1659,18 +1659,7 @@ int X509_STORE_CTX_purpose_inherit(X509_STORE_CTX *ctx, int def_purpose,
16591659
}
16601660

16611661
X509_STORE_CTX *X509_STORE_CTX_new(void) {
1662-
X509_STORE_CTX *ctx;
1663-
ctx = (X509_STORE_CTX *)OPENSSL_zalloc(sizeof(X509_STORE_CTX));
1664-
if (!ctx) {
1665-
return NULL;
1666-
}
1667-
// NO-OP: struct already zeroed
1668-
//X509_STORE_CTX_zero(ctx);
1669-
return ctx;
1670-
}
1671-
1672-
void X509_STORE_CTX_zero(X509_STORE_CTX *ctx) {
1673-
OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
1662+
return OPENSSL_zalloc(sizeof(X509_STORE_CTX));
16741663
}
16751664

16761665
void X509_STORE_CTX_free(X509_STORE_CTX *ctx) {
@@ -1683,7 +1672,8 @@ void X509_STORE_CTX_free(X509_STORE_CTX *ctx) {
16831672

16841673
int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
16851674
STACK_OF(X509) *chain) {
1686-
X509_STORE_CTX_zero(ctx);
1675+
X509_STORE_CTX_cleanup(ctx);
1676+
16871677
ctx->ctx = store;
16881678
ctx->cert = x509;
16891679
ctx->untrusted = chain;
@@ -1810,7 +1800,7 @@ void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) {
18101800
sk_X509_pop_free(ctx->chain, X509_free);
18111801
ctx->chain = NULL;
18121802
CRYPTO_free_ex_data(&g_ex_data_class, ctx, &(ctx->ex_data));
1813-
OPENSSL_memset(&ctx->ex_data, 0, sizeof(CRYPTO_EX_DATA));
1803+
OPENSSL_memset(ctx, 0, sizeof(X509_STORE_CTX));
18141804
}
18151805

18161806
void X509_STORE_CTX_set_depth(X509_STORE_CTX *ctx, int depth) {

0 commit comments

Comments
 (0)