From: Richard Levitte Date: Wed, 30 Apr 2025 09:38:04 +0000 (+0200) Subject: Rework the "by store" X509_LOOKUP method to open the given URI early X-Git-Tag: openssl-3.4.2~80 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f57419dad8e10df5762601a4765ceea64d5c796;p=thirdparty%2Fopenssl.git Rework the "by store" X509_LOOKUP method to open the given URI early The cached X509_LOOKUP method data is no longer just the URI, but now includes the OSSL_STORE_CTX pointer, and required parameters to reopen the URI at any time. cache_objects() is modified to handle this, and only (re)open the URI when it wasn't previously opened, or when it was closed by an earlier call. This way, we can call OSSL_STORE_open_ex() in by_store_ctrl_ex(), and get to see possible errors when the URI is loaded. This assumes that if the URI could be opened once, it can be opened again. Fixes #27461 Reviewed-by: David von Oheimb Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/27550) --- diff --git a/crypto/x509/by_store.c b/crypto/x509/by_store.c index d1e186f4fc2..75d08a346b1 100644 --- a/crypto/x509/by_store.c +++ b/crypto/x509/by_store.c @@ -7,23 +7,34 @@ * https://www.openssl.org/source/license.html */ +#include #include #include "internal/cryptlib.h" #include "crypto/x509.h" #include "x509_local.h" +typedef struct cached_store_st { + char *uri; + OSSL_LIB_CTX *libctx; + char *propq; + OSSL_STORE_CTX *ctx; +} CACHED_STORE; + +DEFINE_STACK_OF(CACHED_STORE) + /* Generic object loader, given expected type and criterion */ -static int cache_objects(X509_LOOKUP *lctx, const char *uri, - const OSSL_STORE_SEARCH *criterion, - int depth, OSSL_LIB_CTX *libctx, const char *propq) +static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, + const OSSL_STORE_SEARCH *criterion, int depth) { int ok = 0; - OSSL_STORE_CTX *ctx = NULL; + OSSL_STORE_CTX *ctx = store->ctx; X509_STORE *xstore = X509_LOOKUP_get_store(lctx); - if ((ctx = OSSL_STORE_open_ex(uri, libctx, propq, NULL, NULL, NULL, - NULL, NULL)) == NULL) + if (ctx == NULL + && (ctx = OSSL_STORE_open_ex(store->uri, store->libctx, store->propq, + NULL, NULL, NULL, NULL, NULL)) == NULL) return 0; + store->ctx = ctx; /* * We try to set the criterion, but don't care if it was valid or not. @@ -62,9 +73,15 @@ static int cache_objects(X509_LOOKUP *lctx, const char *uri, * This is an entry in the "directory" represented by the current * uri. if |depth| allows, dive into it. */ - if (depth > 0) - ok = cache_objects(lctx, OSSL_STORE_INFO_get0_NAME(info), - criterion, depth - 1, libctx, propq); + if (depth > 0) { + CACHED_STORE substore; + + substore.uri = (char *)OSSL_STORE_INFO_get0_NAME(info); + substore.libctx = store->libctx; + substore.propq = store->propq; + substore.ctx = NULL; + ok = cache_objects(lctx, &substore, criterion, depth - 1); + } } else { /* * We know that X509_STORE_add_{cert|crl} increments the object's @@ -88,21 +105,26 @@ static int cache_objects(X509_LOOKUP *lctx, const char *uri, break; } OSSL_STORE_close(ctx); + store->ctx = NULL; return ok; } -/* Because OPENSSL_free is a macro and for C type match */ -static void free_uri(OPENSSL_STRING data) +static void free_store(CACHED_STORE *store) { - OPENSSL_free(data); + if (store != NULL) { + OSSL_STORE_close(store->ctx); + OPENSSL_free(store->uri); + OPENSSL_free(store->propq); + OPENSSL_free(store); + } } static void by_store_free(X509_LOOKUP *ctx) { - STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); - sk_OPENSSL_STRING_pop_free(uris, free_uri); + STACK_OF(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); + sk_CACHED_STORE_pop_free(stores, free_store); } static int by_store_ctrl_ex(X509_LOOKUP *ctx, int cmd, const char *argp, @@ -112,27 +134,49 @@ static int by_store_ctrl_ex(X509_LOOKUP *ctx, int cmd, const char *argp, switch (cmd) { case X509_L_ADD_STORE: if (argp != NULL) { - STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); - char *data = OPENSSL_strdup(argp); + STACK_OF(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); + CACHED_STORE *store = OPENSSL_zalloc(sizeof(*store)); - if (data == NULL) { + if (store == NULL) { return 0; } - if (uris == NULL) { - uris = sk_OPENSSL_STRING_new_null(); - X509_LOOKUP_set_method_data(ctx, uris); + + store->uri = OPENSSL_strdup(argp); + store->libctx = libctx; + if (propq != NULL) + store->propq = OPENSSL_strdup(propq); + store->ctx = OSSL_STORE_open_ex(argp, libctx, propq, NULL, NULL, + NULL, NULL, NULL); + if (store->ctx == NULL + || (propq != NULL && store->propq == NULL) + || store->uri == NULL) { + free_store(store); + return 0; + } + + if (stores == NULL) { + stores = sk_CACHED_STORE_new_null(); + if (stores != NULL) + X509_LOOKUP_set_method_data(ctx, stores); } - if (sk_OPENSSL_STRING_push(uris, data) <= 0) { - OPENSSL_free(data); + if (stores == NULL || sk_CACHED_STORE_push(stores, store) <= 0) { + free_store(store); return 0; } return 1; } /* NOP if no URI is given. */ return 1; - case X509_L_LOAD_STORE: + case X509_L_LOAD_STORE: { /* This is a shortcut for quick loading of specific containers */ - return cache_objects(ctx, argp, NULL, 0, libctx, propq); + CACHED_STORE store; + + store.uri = (char *)argp; + store.libctx = libctx; + store.propq = (char *)propq; + store.ctx = NULL; + return cache_objects(ctx, &store, NULL, 0); + } default: /* Unsupported command */ return 0; @@ -149,13 +193,13 @@ static int by_store(X509_LOOKUP *ctx, X509_LOOKUP_TYPE type, const OSSL_STORE_SEARCH *criterion, X509_OBJECT *ret, OSSL_LIB_CTX *libctx, const char *propq) { - STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); + STACK_OF(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); int i; int ok = 0; - for (i = 0; i < sk_OPENSSL_STRING_num(uris); i++) { - ok = cache_objects(ctx, sk_OPENSSL_STRING_value(uris, i), criterion, - 1 /* depth */, libctx, propq); + for (i = 0; i < sk_CACHED_STORE_num(stores); i++) { + ok = cache_objects(ctx, sk_CACHED_STORE_value(stores, i), criterion, + 1 /* depth */); if (ok) break;