Refactor X509_STORE_get1_certs()

Split the retrieval of the certs in the store's cache that match the
desired subject into a separate function. This greatly simplifies
locking, error handling and the flow of the function.

with/ok jsing

1 files changed, 30 insertions, 27 deletions

diff --git a/lib/libcrypto/x509/x509_lu.c b/lib/libcrypto/x509/x509_lu.c
index 9c18c16eeb3..1a8c079fde3 100644
--- a/lib/libcrypto/x509/x509_lu.c
+++ b/lib/libcrypto/x509/x509_lu.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_lu.c,v 1.48 2021/11/05 21:39:45 tb Exp $ */
+/* $OpenBSD: x509_lu.c,v 1.49 2021/11/06 07:18:18 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -532,41 +532,20 @@ X509_OBJECT_get0_X509_CRL(X509_OBJECT *xo)
 	return NULL;
 }
 
-STACK_OF(X509) *
-X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *name)
+static STACK_OF(X509) *
+X509_get1_certs_from_cache(X509_STORE *store, X509_NAME *name)
 {
-	X509_STORE *store = ctx->ctx;
-	STACK_OF(X509) *sk;
+	STACK_OF(X509) *sk = NULL;
 	X509 *x = NULL;
 	X509_OBJECT *obj;
 	int i, idx, cnt;
 
-	if (store == NULL)
-		return NULL;
-
 	CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
-	idx = x509_object_idx_cnt(store->objs, X509_LU_X509, name, &cnt);
-	if (idx >= 0)
-		goto found;
-	CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
-
-	/* Nothing found: do lookup to possibly add new objects to cache. */
-	obj = X509_STORE_CTX_get_obj_by_subject(ctx, X509_LU_X509, name);
-	if (obj == NULL)
-		return NULL;
-
-	X509_OBJECT_free(obj);
-	obj = NULL;
 
-	CRYPTO_w_lock(CRYPTO_LOCK_X509_STORE);
 	idx = x509_object_idx_cnt(store->objs, X509_LU_X509, name, &cnt);
-	if (idx >= 0)
-		goto found;
-	CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
-
-	return NULL;
+	if (idx < 0)
+		goto err;
 
- found:
 	if ((sk = sk_X509_new_null()) == NULL)
 		goto err;
 
@@ -583,15 +562,39 @@ X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *name)
 	}
 
 	CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
+
 	return sk;
 
  err:
 	CRYPTO_w_unlock(CRYPTO_LOCK_X509_STORE);
 	sk_X509_pop_free(sk, X509_free);
 	X509_free(x);
+
 	return NULL;
 }
 
+STACK_OF(X509) *
+X509_STORE_get1_certs(X509_STORE_CTX *ctx, X509_NAME *name)
+{
+	X509_STORE *store = ctx->ctx;
+	STACK_OF(X509) *sk;
+	X509_OBJECT *obj;
+
+	if (store == NULL)
+		return NULL;
+
+	if ((sk = X509_get1_certs_from_cache(store, name)) != NULL)
+		return sk;
+
+	/* Nothing found: do lookup to possibly add new objects to cache. */
+	obj = X509_STORE_CTX_get_obj_by_subject(ctx, X509_LU_X509, name);
+	if (obj == NULL)
+		return NULL;
+	X509_OBJECT_free(obj);
+
+	return X509_get1_certs_from_cache(store, name);
+}
+
 STACK_OF(X509_CRL) *
 X509_STORE_get1_crls(X509_STORE_CTX *ctx, X509_NAME *name)
 {