Unexport various unused X509_OBJECT and X509_LOOKUP functions. Some things of note: - Anyone calling X509_OBJECT_up_ref_count is breaking X509_OBJECT's internal invariants, or relying on someone else handing back an X509_OBJECT with broken invariants. - X509_LOOKUP_by_subject hands back an X509_OBJECT with broken internal invariants. Fortunately, it is never called, so unexport it as a the first step to cleaning this up. Change-Id: Ia67693f802671cf857bf51aec6e20f27d1525212 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64130 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index d113cee..2f5ba58 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c
@@ -65,20 +65,28 @@ #include "../internal.h" #include "internal.h" -X509_LOOKUP *X509_LOOKUP_new(X509_LOOKUP_METHOD *method) { - X509_LOOKUP *ret; - ret = (X509_LOOKUP *)OPENSSL_malloc(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; } - ret->init = 0; - ret->skip = 0; ret->method = method; - ret->method_data = NULL; - ret->store_ctx = NULL; - if ((method->new_item != NULL) && !method->new_item(ret)) { + if (method->new_item != NULL && !method->new_item(ret)) { OPENSSL_free(ret); return NULL; } @@ -95,18 +103,7 @@ 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; } @@ -129,14 +126,18 @@ } } -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; } @@ -353,7 +354,7 @@ 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); @@ -432,13 +433,13 @@ 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) { +static 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) { @@ -533,8 +534,8 @@ 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(h, &idx, x)) {
diff --git a/include/openssl/x509.h b/include/openssl/x509.h index dfe66c5..a7e4f01 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h
@@ -2589,6 +2589,14 @@ // longer call it. OPENSSL_EXPORT void X509_OBJECT_free_contents(X509_OBJECT *obj); +// 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. @@ -2902,13 +2910,6 @@ #define X509_VP_FLAG_LOCKED 0x8 #define X509_VP_FLAG_ONCE 0x10 -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); @@ -2924,7 +2925,6 @@ // 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 X509_STORE *X509_STORE_new(void); OPENSSL_EXPORT int X509_STORE_up_ref(X509_STORE *store); OPENSSL_EXPORT void X509_STORE_free(X509_STORE *v); @@ -3048,13 +3048,6 @@ 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);