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, 30 insertions, 0 deletions

diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c
index 0ca7cb960d..3fa4fee1e1 100644
--- a/crypto/x509/x509_lu.c
+++ b/crypto/x509/x509_lu.c
@@ -583,6 +583,36 @@ STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(const X509_STORE *xs)
     return xs->objs;
 }
 
+static X509_OBJECT *x509_object_dup(const X509_OBJECT *obj)
+{
+    X509_OBJECT *ret = X509_OBJECT_new();
+    if (ret == NULL)
+        return NULL;
+
+    ret->type = obj->type;
+    ret->data = obj->data;
+    X509_OBJECT_up_ref_count(ret);
+    return ret;
+}
+
+STACK_OF(X509_OBJECT) *X509_STORE_get1_objects(X509_STORE *store)
+{
+    STACK_OF(X509_OBJECT) *objs;
+
+    if (store == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_PASSED_NULL_PARAMETER);
+        return NULL;
+    }
+
+    if (!x509_store_read_lock(store))
+        return NULL;
+
+    objs = sk_X509_OBJECT_deep_copy(store->objs, x509_object_dup,
+                                    X509_OBJECT_free);
+    X509_STORE_unlock(store);
+    return objs;
+}
+
 STACK_OF(X509) *X509_STORE_get1_all_certs(X509_STORE *store)
 {
     STACK_OF(X509) *sk;