diff options
author | tb <> | 2024-02-23 10:39:07 +0000 |
---|---|---|
committer | tb <> | 2024-02-23 10:39:07 +0000 |
commit | 501fb5e3889fa53d51e5261dede7b653c754d22a (patch) | |
tree | 757f8cb1cb0fbfb3f9f6c1a21099d615a566c7fd /src/lib | |
parent | 2f9cfbbab0541be2daffe9ee996acdac0d1846a2 (diff) | |
download | openbsd-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.namespace | 1 | ||||
-rw-r--r-- | src/lib/libcrypto/hidden/openssl/x509_vfy.h | 3 | ||||
-rw-r--r-- | src/lib/libcrypto/x509/x509_lu.c | 67 | ||||
-rw-r--r-- | src/lib/libcrypto/x509/x509_vfy.h | 5 |
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); | |||
40 | LCRYPTO_USED(X509_STORE_free); | 40 | LCRYPTO_USED(X509_STORE_free); |
41 | LCRYPTO_USED(X509_STORE_up_ref); | 41 | LCRYPTO_USED(X509_STORE_up_ref); |
42 | LCRYPTO_USED(X509_STORE_get0_objects); | 42 | LCRYPTO_USED(X509_STORE_get0_objects); |
43 | LCRYPTO_USED(X509_STORE_get1_objects); | ||
43 | LCRYPTO_USED(X509_STORE_get_ex_data); | 44 | LCRYPTO_USED(X509_STORE_get_ex_data); |
44 | LCRYPTO_USED(X509_STORE_set_ex_data); | 45 | LCRYPTO_USED(X509_STORE_set_ex_data); |
45 | LCRYPTO_USED(X509_STORE_set_flags); | 46 | LCRYPTO_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 | } |
247 | LCRYPTO_ALIAS(X509_OBJECT_free); | 247 | LCRYPTO_ALIAS(X509_OBJECT_free); |
248 | 248 | ||
249 | static X509_OBJECT * | ||
250 | x509_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 | |||
249 | void | 267 | void |
250 | X509_STORE_free(X509_STORE *store) | 268 | X509_STORE_free(X509_STORE *store) |
251 | { | 269 | { |
@@ -785,6 +803,53 @@ X509_STORE_get0_objects(X509_STORE *xs) | |||
785 | } | 803 | } |
786 | LCRYPTO_ALIAS(X509_STORE_get0_objects); | 804 | LCRYPTO_ALIAS(X509_STORE_get0_objects); |
787 | 805 | ||
806 | static STACK_OF(X509_OBJECT) * | ||
807 | sk_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 | |||
835 | STACK_OF(X509_OBJECT) * | ||
836 | X509_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 | } | ||
851 | LCRYPTO_ALIAS(X509_STORE_get1_objects); | ||
852 | |||
788 | void * | 853 | void * |
789 | X509_STORE_get_ex_data(X509_STORE *xs, int idx) | 854 | X509_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); | |||
293 | STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *st, X509_NAME *nm); | 293 | STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *st, X509_NAME *nm); |
294 | STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *st, X509_NAME *nm); | 294 | STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(X509_STORE_CTX *st, X509_NAME *nm); |
295 | STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *xs); | 295 | STACK_OF(X509_OBJECT) *X509_STORE_get0_objects(X509_STORE *xs); |
296 | #if defined(LIBRESSL_INTERNAL) || defined(LIBRESSL_NEXT_API) | ||
297 | STACK_OF(X509_OBJECT) *X509_STORE_get1_objects(X509_STORE *xs); | ||
298 | #endif | ||
296 | void *X509_STORE_get_ex_data(X509_STORE *xs, int idx); | 299 | void *X509_STORE_get_ex_data(X509_STORE *xs, int idx); |
297 | int X509_STORE_set_ex_data(X509_STORE *xs, int idx, void *data); | 300 | int X509_STORE_set_ex_data(X509_STORE *xs, int idx, void *data); |
298 | 301 | ||