summaryrefslogtreecommitdiff
path: root/src/lib
diff options
context:
space:
mode:
authortb <>2024-02-23 10:39:07 +0000
committertb <>2024-02-23 10:39:07 +0000
commit501fb5e3889fa53d51e5261dede7b653c754d22a (patch)
tree757f8cb1cb0fbfb3f9f6c1a21099d615a566c7fd /src/lib
parent2f9cfbbab0541be2daffe9ee996acdac0d1846a2 (diff)
downloadopenbsd-501fb5e3889fa53d51e5261dede7b653c754d22a.tar.gz
openbsd-501fb5e3889fa53d51e5261dede7b653c754d22a.tar.bz2
openbsd-501fb5e3889fa53d51e5261dede7b653c754d22a.zip
Prepare to provide X509_STORE_get1_objects()
The OpenSSL 1.1 API X509_STORE_get0_objects() is not thread safe. It exposes a naked internal pointer containing certificates, CRLs and cached objects added by X509_LOOKUP_hash_dir(). Thus, if the store is shared between threads, it is not possible to inspect this pointer safely since another thread could concurrently add to it. This may happen in particular during certificate verification. This API led to security issues in rust-openssl and is also problematic in current Python. Other consumers of X509_STORE_get0_objects() are haproxy, isync, openvpn. The solution is to take a snapshot of the state under a lock and return that. This is what X509_STORE_get1_objects() does. It returns a newly allocated stack that needs to be freed with sk_X509_OBJECT_pop_free(), passing X509_OBJECT_free as a second argument. Based on a diff by David Benjamin for BoringSSL. https://boringssl-review.googlesource.com/c/boringssl/+/65787 ok beck jsing PS: Variants of this have landed in Python and OpenSSL 3 as well. There the sk_*deep_copy() API is used, which in OpenSSL relies on evaluating function pointers after casts (BoringSSL fixed that). Instead of using this macro insanity and exposing that garbage in public, we can do this by implementing a pedestrian, static sk_X509_OBJECT_deep_copy() by hand.
Diffstat (limited to 'src/lib')
-rw-r--r--src/lib/libcrypto/Symbols.namespace1
-rw-r--r--src/lib/libcrypto/hidden/openssl/x509_vfy.h3
-rw-r--r--src/lib/libcrypto/x509/x509_lu.c67
-rw-r--r--src/lib/libcrypto/x509/x509_vfy.h5
4 files changed, 73 insertions, 3 deletions
diff --git a/src/lib/libcrypto/Symbols.namespace b/src/lib/libcrypto/Symbols.namespace
index 62d6b5a3ad..bcd5b84ba6 100644
--- a/src/lib/libcrypto/Symbols.namespace
+++ b/src/lib/libcrypto/Symbols.namespace
@@ -525,6 +525,7 @@ _libre_X509_STORE_new
525_libre_X509_STORE_free 525_libre_X509_STORE_free
526_libre_X509_STORE_up_ref 526_libre_X509_STORE_up_ref
527_libre_X509_STORE_get0_objects 527_libre_X509_STORE_get0_objects
528_libre_X509_STORE_get1_objects
528_libre_X509_STORE_get_ex_data 529_libre_X509_STORE_get_ex_data
529_libre_X509_STORE_set_ex_data 530_libre_X509_STORE_set_ex_data
530_libre_X509_STORE_set_flags 531_libre_X509_STORE_set_flags
diff --git a/src/lib/libcrypto/hidden/openssl/x509_vfy.h b/src/lib/libcrypto/hidden/openssl/x509_vfy.h
index b5f2ac1a85..3502492133 100644
--- a/src/lib/libcrypto/hidden/openssl/x509_vfy.h
+++ b/src/lib/libcrypto/hidden/openssl/x509_vfy.h
@@ -1,4 +1,4 @@
1/* $OpenBSD: x509_vfy.h,v 1.6 2023/07/05 21:14:54 bcook Exp $ */ 1/* $OpenBSD: x509_vfy.h,v 1.7 2024/02/23 10:39:07 tb Exp $ */
2/* 2/*
3 * Copyright (c) 2022 Bob Beck <beck@openbsd.org> 3 * Copyright (c) 2022 Bob Beck <beck@openbsd.org>
4 * 4 *
@@ -40,6 +40,7 @@ LCRYPTO_USED(X509_STORE_new);
40LCRYPTO_USED(X509_STORE_free); 40LCRYPTO_USED(X509_STORE_free);
41LCRYPTO_USED(X509_STORE_up_ref); 41LCRYPTO_USED(X509_STORE_up_ref);
42LCRYPTO_USED(X509_STORE_get0_objects); 42LCRYPTO_USED(X509_STORE_get0_objects);
43LCRYPTO_USED(X509_STORE_get1_objects);
43LCRYPTO_USED(X509_STORE_get_ex_data); 44LCRYPTO_USED(X509_STORE_get_ex_data);
44LCRYPTO_USED(X509_STORE_set_ex_data); 45LCRYPTO_USED(X509_STORE_set_ex_data);
45LCRYPTO_USED(X509_STORE_set_flags); 46LCRYPTO_USED(X509_STORE_set_flags);
diff --git a/src/lib/libcrypto/x509/x509_lu.c b/src/lib/libcrypto/x509/x509_lu.c
index 6bdae0f5c4..7e7a5dedd0 100644
--- a/src/lib/libcrypto/x509/x509_lu.c
+++ b/src/lib/libcrypto/x509/x509_lu.c
@@ -1,4 +1,4 @@
1/* $OpenBSD: x509_lu.c,v 1.62 2023/12/27 01:55:25 tb Exp $ */ 1/* $OpenBSD: x509_lu.c,v 1.63 2024/02/23 10:39:07 tb Exp $ */
2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) 2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
3 * All rights reserved. 3 * All rights reserved.
4 * 4 *
@@ -246,6 +246,24 @@ X509_OBJECT_free(X509_OBJECT *a)
246} 246}
247LCRYPTO_ALIAS(X509_OBJECT_free); 247LCRYPTO_ALIAS(X509_OBJECT_free);
248 248
249static X509_OBJECT *
250x509_object_dup(const X509_OBJECT *obj)
251{
252 X509_OBJECT *copy;
253
254 if ((copy = X509_OBJECT_new()) == NULL) {
255 X509error(ERR_R_MALLOC_FAILURE);
256 return NULL;
257 }
258
259 copy->type = obj->type;
260 copy->data = obj->data;
261
262 X509_OBJECT_up_ref_count(copy);
263
264 return copy;
265}
266
249void 267void
250X509_STORE_free(X509_STORE *store) 268X509_STORE_free(X509_STORE *store)
251{ 269{
@@ -785,6 +803,53 @@ X509_STORE_get0_objects(X509_STORE *xs)
785} 803}
786LCRYPTO_ALIAS(X509_STORE_get0_objects); 804LCRYPTO_ALIAS(X509_STORE_get0_objects);
787 805
806static STACK_OF(X509_OBJECT) *
807sk_X509_OBJECT_deep_copy(const STACK_OF(X509_OBJECT) *objs)
808{
809 STACK_OF(X509_OBJECT) *copy = NULL;
810 X509_OBJECT *obj = NULL;
811 int i;
812
813 if ((copy = sk_X509_OBJECT_new(x509_object_cmp)) == NULL) {
814 X509error(ERR_R_MALLOC_FAILURE);
815 goto err;
816 }
817
818 for (i = 0; i < sk_X509_OBJECT_num(objs); i++) {
819 if ((obj = x509_object_dup(sk_X509_OBJECT_value(objs, i))) == NULL)
820 goto err;
821 if (!sk_X509_OBJECT_push(copy, obj))
822 goto err;
823 obj = NULL;
824 }
825
826 return copy;
827
828 err:
829 X509_OBJECT_free(obj);
830 sk_X509_OBJECT_pop_free(copy, X509_OBJECT_free);
831
832 return NULL;
833}
834
835STACK_OF(X509_OBJECT) *
836X509_STORE_get1_objects(X509_STORE *store)
837{
838 STACK_OF(X509_OBJECT) *objs;
839
840 if (store == NULL) {
841 X509error(ERR_R_PASSED_NULL_PARAMETER);
842 return NULL;
843 }
844
845 CRYPTO_r_lock(CRYPTO_LOCK_X509_STORE);
846 objs = sk_X509_OBJECT_deep_copy(store->objs);
847 CRYPTO_r_unlock(CRYPTO_LOCK_X509_STORE);
848
849 return objs;
850}
851LCRYPTO_ALIAS(X509_STORE_get1_objects);
852
788void * 853void *
789X509_STORE_get_ex_data(X509_STORE *xs, int idx) 854X509_STORE_get_ex_data(X509_STORE *xs, int idx)
790{ 855{
diff --git a/src/lib/libcrypto/x509/x509_vfy.h b/src/lib/libcrypto/x509/x509_vfy.h
index 1aa29abd3d..d7657a51f0 100644
--- a/src/lib/libcrypto/x509/x509_vfy.h
+++ b/src/lib/libcrypto/x509/x509_vfy.h
@@ -1,4 +1,4 @@
1/* $OpenBSD: x509_vfy.h,v 1.64 2023/05/28 05:25:24 tb Exp $ */ 1/* $OpenBSD: x509_vfy.h,v 1.65 2024/02/23 10:39:07 tb Exp $ */
2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) 2/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
3 * All rights reserved. 3 * All rights reserved.
4 * 4 *
@@ -293,6 +293,9 @@ int X509_STORE_up_ref(X509_STORE *x);
293STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *st, X509_NAME *nm); 293STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *st, X509_NAME *nm);
294STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *st, X509_NAME *nm); 294STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *st, X509_NAME *nm);
295STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *xs); 295STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *xs);
296#if defined(LIBRESSL_INTERNAL) || defined(LIBRESSL_NEXT_API)
297STACK_OF(X509_OBJECT) *X509_STORE_get1_objects(X509_STORE *xs);
298#endif
296void *X509_STORE_get_ex_data(X509_STORE *xs, int idx); 299void *X509_STORE_get_ex_data(X509_STORE *xs, int idx);
297int X509_STORE_set_ex_data(X509_STORE *xs, int idx, void *data); 300int X509_STORE_set_ex_data(X509_STORE *xs, int idx, void *data);
298 301