From 866cb2ce2bf2eae88833731bf24c249de5111bff Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 19 Aug 2025 13:27:50 +0100 Subject: [PATCH] Fix a race in by_store_subject MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When looking in the stack of objects in the store we need to ensure we are holding a read lock for the store. Issue detected via thread sanitizer after the test from the previous commit was added. Reviewed-by: Saša Nedvědický Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/28198) (cherry picked from commit 07f65e16c209e06be9887c2d5f943f528e3f0139) --- crypto/x509/by_store.c | 8 +++++++- crypto/x509/x509_local.h | 1 + crypto/x509/x509_lu.c | 6 +++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crypto/x509/by_store.c b/crypto/x509/by_store.c index 90228ed661b..199317837d4 100644 --- a/crypto/x509/by_store.c +++ b/crypto/x509/by_store.c @@ -231,8 +231,14 @@ static int by_store_subject(X509_LOOKUP *ctx, X509_LOOKUP_TYPE type, OSSL_STORE_SEARCH_free(criterion); - if (ok) + if (ok) { + X509_STORE *store = X509_LOOKUP_get_store(ctx); + + if (!ossl_x509_store_read_lock(store)) + return 0; tmp = X509_OBJECT_retrieve_by_subject(store_objects, type, name); + X509_STORE_unlock(store); + } ok = 0; if (tmp != NULL) { diff --git a/crypto/x509/x509_local.h b/crypto/x509/x509_local.h index 6d602e1d8ef..5c886d86e26 100644 --- a/crypto/x509/x509_local.h +++ b/crypto/x509/x509_local.h @@ -157,3 +157,4 @@ DEFINE_STACK_OF(STACK_OF_X509_NAME_ENTRY) int ossl_x509_likely_issued(X509 *issuer, X509 *subject); int ossl_x509_signing_allowed(const X509 *issuer, const X509 *subject); +int ossl_x509_store_read_lock(X509_STORE *xs); diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 09fa2ee1f74..11f59722d85 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -44,7 +44,7 @@ int X509_STORE_lock(X509_STORE *xs) return CRYPTO_THREAD_write_lock(xs->lock); } -static int x509_store_read_lock(X509_STORE *xs) +int ossl_x509_store_read_lock(X509_STORE *xs) { return CRYPTO_THREAD_read_lock(xs->lock); } @@ -333,7 +333,7 @@ static int ossl_x509_store_ctx_get_by_subject(const X509_STORE_CTX *ctx, stmp.type = X509_LU_NONE; stmp.data.ptr = NULL; - if (!x509_store_read_lock(store)) + if (!ossl_x509_store_read_lock(store)) return 0; /* Should already be sorted...but just in case */ if (!sk_X509_OBJECT_is_sorted(store->objs)) { @@ -604,7 +604,7 @@ STACK_OF(X509_OBJECT) *X509_STORE_get1_objects(X509_STORE *store) return NULL; } - if (!x509_store_read_lock(store)) + if (!ossl_x509_store_read_lock(store)) return NULL; objs = sk_X509_OBJECT_deep_copy(store->objs, x509_object_dup, -- 2.47.3