diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 024c3e7a4c1..d8dac39eaf4 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -65,10 +65,22 @@ #include "../internal.h" #include "internal.h" -X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method) { - X509_LOOKUP *ret; - ret = (X509_LOOKUP *)OPENSSL_zalloc(sizeof(X509_LOOKUP)); +static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type, + X509_NAME *name); +static X509_OBJECT *X509_OBJECT_retrieve_by_subject( + STACK_OF(X509_OBJECT) *h, int type, X509_NAME *name); +static X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, + X509_OBJECT *x); +static int X509_OBJECT_up_ref_count(X509_OBJECT *a); + +static X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method); +static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name, + X509_OBJECT *ret); +static int X509_LOOKUP_shutdown(X509_LOOKUP *ctx); + +static X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method) { + X509_LOOKUP *ret = OPENSSL_zalloc(sizeof(X509_LOOKUP)); if (ret == NULL) { return NULL; } @@ -91,18 +103,7 @@ void X509_LOOKUP_free(X509_LOOKUP *ctx) { OPENSSL_free(ctx); } -int X509_LOOKUP_init(X509_LOOKUP *ctx) { - if (ctx->method == NULL) { - return 0; - } - if (ctx->method->init != NULL) { - return ctx->method->init(ctx); - } else { - return 1; - } -} - -int X509_LOOKUP_shutdown(X509_LOOKUP *ctx) { +static int X509_LOOKUP_shutdown(X509_LOOKUP *ctx) { if (ctx->method == NULL) { return 0; } @@ -125,14 +126,18 @@ int X509_LOOKUP_ctrl(X509_LOOKUP *ctx, int cmd, const char *argc, long argl, } } -int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name, - X509_OBJECT *ret) { +static int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, X509_NAME *name, + X509_OBJECT *ret) { if ((ctx->method == NULL) || (ctx->method->get_by_subject == NULL)) { return 0; } if (ctx->skip) { return 0; } + // Note |get_by_subject| leaves |ret| in an inconsistent state. It has + // pointers to an |X509| or |X509_CRL|, but has not bumped the refcount yet. + // For now, the caller is expected to fix this, but ideally we'd fix the + // |X509_LOOKUP| convention itself. return ctx->method->get_by_subject(ctx, type, name, ret) > 0; } @@ -365,7 +370,7 @@ void X509_OBJECT_free(X509_OBJECT *obj) { OPENSSL_free(obj); } -int X509_OBJECT_up_ref_count(X509_OBJECT *a) { +static int X509_OBJECT_up_ref_count(X509_OBJECT *a) { switch (a->type) { case X509_LU_X509: X509_up_ref(a->data.x509); @@ -473,13 +478,13 @@ static int x509_object_idx_cnt(STACK_OF(X509_OBJECT) *h, int type, return (int)idx; } -int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type, - X509_NAME *name) { +static int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, int type, + X509_NAME *name) { return x509_object_idx_cnt(h, type, name, NULL); } -X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h, int type, - X509_NAME *name) { +X509_OBJECT *X509_OBJECT_retrieve_by_subject(STACK_OF(X509_OBJECT) *h, + int type, X509_NAME *name) { int idx; idx = X509_OBJECT_idx_by_subject(h, type, name); if (idx == -1) { @@ -574,8 +579,8 @@ STACK_OF(X509_CRL) *X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *nm) { return sk; } -X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, - X509_OBJECT *x) { +static X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, + X509_OBJECT *x) { sk_X509_OBJECT_sort(h); size_t idx; if (!sk_X509_OBJECT_find_awslc(h, &idx, x)) { diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index 2fd4a6ac1c5..2d4bb95e855 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -1851,7 +1851,7 @@ TEST(X509Test, MatchFoundSetsPeername) { char *peername = nullptr; EXPECT_NE(1, X509_check_host(leaf.get(), kWrongHostname, strlen(kWrongHostname), 0, &peername)); ASSERT_EQ(nullptr, peername); - + EXPECT_EQ(1, X509_check_host(leaf.get(), kHostname, strlen(kHostname), 0, &peername)); EXPECT_STREQ(peername, kHostname); OPENSSL_free(peername); @@ -5086,15 +5086,8 @@ TEST(X509Test, AddDuplicates) { ASSERT_TRUE(X509_STORE_lock(store.get())); // Sleep after taking the lock to cause contention. Sleep longer than the // adder half of threads to ensure we hold the lock while they contend - // for it. |X509_OBJECT_retrieve_by_subject| is called because it doesn't - // take a lock on the store, thus avoiding deadlock. + // for it. std::this_thread::sleep_for(std::chrono::microseconds(11 + (sleep_buf[0] % 5))); - EXPECT_TRUE(X509_OBJECT_retrieve_by_subject( - store->objs, X509_LU_X509, X509_get_subject_name(a.get()) - )); - EXPECT_TRUE(X509_OBJECT_retrieve_by_subject( - store->objs, X509_LU_X509, X509_get_subject_name(b.get()) - )); ASSERT_TRUE(X509_STORE_unlock(store.get())); }); } diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 9ebe5d4189b..9e3091a4cfe 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -2658,6 +2658,13 @@ OPENSSL_EXPORT int X509_NAME_get_text_by_NID(const X509_NAME *name, int nid, OPENSSL_EXPORT X509_STORE_CTX *X509_STORE_CTX_get0_parent_ctx( X509_STORE_CTX *ctx); +// X509_LOOKUP_free releases memory associated with |ctx|. This function should +// never be used outside the library. No function in the public API hands +// ownership of an |X509_LOOKUP| to the caller. +// +// TODO(davidben): Unexport this function after rust-openssl is fixed to no +// longer call it. +OPENSSL_EXPORT void X509_LOOKUP_free(X509_LOOKUP *ctx); // Private structures. @@ -3016,13 +3023,6 @@ OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_new(void); // X509_OBJECT_free frees an |X509_OBJECT| from the heap. OPENSSL_EXPORT void X509_OBJECT_free(X509_OBJECT *a); -OPENSSL_EXPORT int X509_OBJECT_idx_by_subject(STACK_OF(X509_OBJECT) *h, - int type, X509_NAME *name); -OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_retrieve_by_subject( - STACK_OF(X509_OBJECT) *h, int type, X509_NAME *name); -OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, - X509_OBJECT *x); - // X509_OBJECT_new returns a newly-allocated, empty |X509_OBJECT| or NULL on // error. OPENSSL_EXPORT X509_OBJECT *X509_OBJECT_new(void); @@ -3038,7 +3038,6 @@ OPENSSL_EXPORT int X509_OBJECT_get_type(const X509_OBJECT *obj); // a certificate. OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *obj); -OPENSSL_EXPORT int X509_OBJECT_up_ref_count(X509_OBJECT *a); OPENSSL_EXPORT int X509_OBJECT_get_type(const X509_OBJECT *a); OPENSSL_EXPORT X509 *X509_OBJECT_get0_X509(const X509_OBJECT *a); // X509_OBJECT_get0_X509_CRL returns the |X509_CRL| associated with |a| @@ -3182,13 +3181,6 @@ OPENSSL_EXPORT int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, OPENSSL_EXPORT int X509_load_cert_crl_file(X509_LOOKUP *ctx, const char *file, int type); -OPENSSL_EXPORT X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method); -OPENSSL_EXPORT void X509_LOOKUP_free(X509_LOOKUP *ctx); -OPENSSL_EXPORT int X509_LOOKUP_init(X509_LOOKUP *ctx); -OPENSSL_EXPORT int X509_LOOKUP_by_subject(X509_LOOKUP *ctx, int type, - X509_NAME *name, X509_OBJECT *ret); -OPENSSL_EXPORT int X509_LOOKUP_shutdown(X509_LOOKUP *ctx); - OPENSSL_EXPORT int X509_STORE_load_locations(X509_STORE *ctx, const char *file, const char *dir); OPENSSL_EXPORT int X509_STORE_set_default_paths(X509_STORE *ctx); @@ -3379,7 +3371,6 @@ BORINGSSL_MAKE_DELETER(X509_ATTRIBUTE, X509_ATTRIBUTE_free) BORINGSSL_MAKE_DELETER(X509_CRL, X509_CRL_free) BORINGSSL_MAKE_UP_REF(X509_CRL, X509_CRL_up_ref) BORINGSSL_MAKE_DELETER(X509_EXTENSION, X509_EXTENSION_free) -BORINGSSL_MAKE_DELETER(X509_LOOKUP, X509_LOOKUP_free) BORINGSSL_MAKE_DELETER(X509_OBJECT, X509_OBJECT_free) BORINGSSL_MAKE_DELETER(X509_NAME, X509_NAME_free) BORINGSSL_MAKE_DELETER(X509_NAME_ENTRY, X509_NAME_ENTRY_free)