Add X509_STORE_get1_objects

X509_STORE_get0_objects returns a pointer to the X509_STORE's storage,
but this function is a bit deceptive. It is practically unusable in a
multi-threaded program. See, for example, RUSTSEC-2023-0072, a security
vulnerability caused by this OpenSSL API.

One might think that, if no other threads are mutating the X509_STORE,
it is safe to read the resulting list. However, the documention does not
mention that other logically-const operations on the X509_STORE, notably
certifcate verifications when a hash_dir is installed, will, under a
lock, write to the X509_STORE. The X509_STORE also internally re-sorts
the list on the first query.

If the caller knows to call X509_STORE_lock and X509_STORE_unlock, it
can work around this. But this is not obvious, and the documentation
does not discuss how X509_STORE_lock is very rarely safe to use. E.g.
one cannot call any APIs like X509_STORE_add_cert or
X509_STORE_CTX_get1_issuer while holding the lock because those
functions internally expect to take the lock. (X509_STORE_lock is
another such API which is not safe to export as public API.)

Rather than leave all this to the caller to figure out, the API should
have returned a shallow copy of the list, refcounting the values. Then
it could be internally locked and the caller can freely inspect the
result without synchronization with the X509_STORE.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23224)

1 files changed, 20 insertions, 7 deletions

diff --git a/test/x509_load_cert_file_test.c b/test/x509_load_cert_file_test.c
index 4a736071ae..001ed570d3 100644
--- a/test/x509_load_cert_file_test.c
+++ b/test/x509_load_cert_file_test.c
@@ -15,19 +15,32 @@ static const char *chain;
 
 static int test_load_cert_file(void)
 {
-    int ret = 0;
+    int ret = 0, i;
     X509_STORE *store = NULL;
     X509_LOOKUP *lookup = NULL;
     STACK_OF(X509) *certs = NULL;
+    STACK_OF(X509_OBJECT) *objs = NULL;
+
+    if (!TEST_ptr(store = X509_STORE_new())
+        || !TEST_ptr(lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()))
+        || !TEST_true(X509_load_cert_file(lookup, chain, X509_FILETYPE_PEM))
+        || !TEST_ptr(certs = X509_STORE_get1_all_certs(store))
+        || !TEST_int_eq(sk_X509_num(certs), 4)
+        || !TEST_ptr(objs = X509_STORE_get1_objects(store))
+        || !TEST_int_eq(sk_X509_OBJECT_num(objs), 4))
+        goto err;
+
+    for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
+        const X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
+        if (!TEST_int_eq(X509_OBJECT_get_type(obj), X509_LU_X509))
+            goto err;
+    }
 
-    if (TEST_ptr(store = X509_STORE_new())
-        && TEST_ptr(lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()))
-        && TEST_true(X509_load_cert_file(lookup, chain, X509_FILETYPE_PEM))
-        && TEST_ptr(certs = X509_STORE_get1_all_certs(store))
-        && TEST_int_eq(sk_X509_num(certs), 4))
-        ret = 1;
+    ret = 1;
 
+err:
     OSSL_STACK_OF_X509_free(certs);
+    sk_X509_OBJECT_pop_free(objs, X509_OBJECT_free);
     X509_STORE_free(store);
     return ret;
 }