From: Sebastian Andrzej Siewior Date: Mon, 4 Aug 2025 20:02:43 +0200 Subject: Revert "Rework the "by store" X509_LOOKUP method to open the given URI early" This reverts commit 340383f5f49f8 ("Rework the "by store" X509_LOOKUP method to open the given URI early") Avoid crashes in users, see https://bugs.debian.org/1110254 Signed-off-by: Sebastian Andrzej Siewior --- crypto/x509/by_store.c | 111 ++++++++++++------------------------------------- 1 file changed, 27 insertions(+), 84 deletions(-) diff --git a/crypto/x509/by_store.c b/crypto/x509/by_store.c index fc6942a17855..e538e0f6d1b7 100644 --- a/crypto/x509/by_store.c +++ b/crypto/x509/by_store.c @@ -7,34 +7,23 @@ * 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, CACHED_STORE *store, - const OSSL_STORE_SEARCH *criterion, int depth) +static int cache_objects(X509_LOOKUP *lctx, const char *uri, + const OSSL_STORE_SEARCH *criterion, + int depth, OSSL_LIB_CTX *libctx, const char *propq) { int ok = 0; - OSSL_STORE_CTX *ctx = store->ctx; + OSSL_STORE_CTX *ctx = NULL; X509_STORE *xstore = X509_LOOKUP_get_store(lctx); - if (ctx == NULL - && (ctx = OSSL_STORE_open_ex(store->uri, store->libctx, store->propq, - NULL, NULL, NULL, NULL, NULL)) == NULL) + if ((ctx = OSSL_STORE_open_ex(uri, libctx, 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. @@ -73,15 +62,9 @@ static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, * This is an entry in the "directory" represented by the current * uri. if |depth| allows, dive into it. */ - 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); - } + if (depth > 0) + ok = cache_objects(lctx, OSSL_STORE_INFO_get0_NAME(info), + criterion, depth - 1, libctx, propq); } else { /* * We know that X509_STORE_add_{cert|crl} increments the object's @@ -105,38 +88,27 @@ static int cache_objects(X509_LOOKUP *lctx, CACHED_STORE *store, break; } OSSL_STORE_close(ctx); - store->ctx = NULL; return ok; } -static void free_store(CACHED_STORE *store) +/* Because OPENSSL_free is a macro and for C type match */ +static void free_uri(OPENSSL_STRING data) { - if (store != NULL) { - OSSL_STORE_close(store->ctx); - OPENSSL_free(store->uri); - OPENSSL_free(store->propq); - OPENSSL_free(store); - } + OPENSSL_free(data); } static void by_store_free(X509_LOOKUP *ctx) { - STACK_OF(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); - sk_CACHED_STORE_pop_free(stores, free_store); + STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); + sk_OPENSSL_STRING_pop_free(uris, free_uri); } static int by_store_ctrl_ex(X509_LOOKUP *ctx, int cmd, const char *argp, long argl, char **retp, OSSL_LIB_CTX *libctx, const char *propq) { - /* - * In some cases below, failing to use the defaults shouldn't result in - * an error. |use_default| is used as the return code in those cases. - */ - int use_default = argp == NULL; - switch (cmd) { case X509_L_ADD_STORE: /* If no URI is given, use the default cert dir as default URI */ @@ -146,50 +118,21 @@ static int by_store_ctrl_ex(X509_LOOKUP *ctx, int cmd, const char *argp, argp = X509_get_default_cert_dir(); { - STACK_OF(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); - CACHED_STORE *store = OPENSSL_zalloc(sizeof(*store)); + STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); + char *data = OPENSSL_strdup(argp); - if (store == NULL) { + if (data == NULL) { return 0; } - - 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 use_default; + if (uris == NULL) { + uris = sk_OPENSSL_STRING_new_null(); + X509_LOOKUP_set_method_data(ctx, uris); } - - if (stores == NULL) { - stores = sk_CACHED_STORE_new_null(); - if (stores != NULL) - X509_LOOKUP_set_method_data(ctx, stores); - } - if (stores == NULL || sk_CACHED_STORE_push(stores, store) <= 0) { - free_store(store); - return 0; - } - return 1; + return sk_OPENSSL_STRING_push(uris, data) > 0; } - case X509_L_LOAD_STORE: { + case X509_L_LOAD_STORE: /* This is a shortcut for quick loading of specific containers */ - 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; + return cache_objects(ctx, argp, NULL, 0, libctx, propq); } return 0; @@ -205,13 +148,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(CACHED_STORE) *stores = X509_LOOKUP_get_method_data(ctx); + STACK_OF(OPENSSL_STRING) *uris = X509_LOOKUP_get_method_data(ctx); int i; int ok = 0; - for (i = 0; i < sk_CACHED_STORE_num(stores); i++) { - ok = cache_objects(ctx, sk_CACHED_STORE_value(stores, i), criterion, - 1 /* depth */); + 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); if (ok) break;