From 5516d20226c496c2b22fa741698b4d48dad0428f Mon Sep 17 00:00:00 2001 From: "Matthias St. Pierre" Date: Mon, 16 Oct 2023 23:48:03 +0200 Subject: [PATCH] rand: add callbacks to cleanup the user entropy resp. nonce The `get_user_{entropy,nonce}` callbacks were add recently to the dispatch table in commit 4cde7585ce8e. Instead of adding corresponding `cleanup_user_{entropy,nonce}` callbacks, the `cleanup_{entropy,nonce}` callbacks were reused. This can cause a problem in the case where the seed source is replaced by a provider: the buffer gets allocated by the provider but cleared by the core. Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/22423) --- crypto/provider_core.c | 26 +++++++++++++++++---- crypto/rand/prov_seed.c | 12 ++++++++++ doc/internal/man3/ossl_rand_get_entropy.pod | 23 +++++++++++++----- doc/man7/provider-base.pod | 26 +++++++++++++++------ include/crypto/rand.h | 4 ++++ include/openssl/core_dispatch.h | 6 +++++ providers/common/provider_seeding.c | 24 +++++++++++++++---- 7 files changed, 100 insertions(+), 21 deletions(-) diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 266934937cb89..838bcd161c810 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -1930,12 +1930,14 @@ OSSL_FUNC_BIO_free_fn ossl_core_bio_free; OSSL_FUNC_BIO_vprintf_fn ossl_core_bio_vprintf; OSSL_FUNC_BIO_vsnprintf_fn BIO_vsnprintf; static OSSL_FUNC_self_test_cb_fn core_self_test_get_callback; -static OSSL_FUNC_get_user_entropy_fn rand_get_user_entropy; static OSSL_FUNC_get_entropy_fn rand_get_entropy; +static OSSL_FUNC_get_user_entropy_fn rand_get_user_entropy; static OSSL_FUNC_cleanup_entropy_fn rand_cleanup_entropy; -static OSSL_FUNC_get_user_nonce_fn rand_get_user_nonce; +static OSSL_FUNC_cleanup_user_entropy_fn rand_cleanup_user_entropy; static OSSL_FUNC_get_nonce_fn rand_get_nonce; +static OSSL_FUNC_get_user_nonce_fn rand_get_user_nonce; static OSSL_FUNC_cleanup_nonce_fn rand_cleanup_nonce; +static OSSL_FUNC_cleanup_user_nonce_fn rand_cleanup_user_nonce; #endif OSSL_FUNC_CRYPTO_malloc_fn CRYPTO_malloc; OSSL_FUNC_CRYPTO_zalloc_fn CRYPTO_zalloc; @@ -2119,6 +2121,13 @@ static void rand_cleanup_entropy(const OSSL_CORE_HANDLE *handle, buf, len); } +static void rand_cleanup_user_entropy(const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len) +{ + ossl_rand_cleanup_user_entropy((OSSL_LIB_CTX *)core_get_libctx(handle), + buf, len); +} + static size_t rand_get_nonce(const OSSL_CORE_HANDLE *handle, unsigned char **pout, size_t min_len, size_t max_len, @@ -2144,6 +2153,13 @@ static void rand_cleanup_nonce(const OSSL_CORE_HANDLE *handle, buf, len); } +static void rand_cleanup_user_nonce(const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len) +{ + ossl_rand_cleanup_user_nonce((OSSL_LIB_CTX *)core_get_libctx(handle), + buf, len); +} + static const char *core_provider_get0_name(const OSSL_CORE_HANDLE *prov) { return OSSL_PROVIDER_get0_name((const OSSL_PROVIDER *)prov); @@ -2238,11 +2254,13 @@ static const OSSL_DISPATCH core_dispatch_[] = { { OSSL_FUNC_BIO_VSNPRINTF, (void (*)(void))BIO_vsnprintf }, { OSSL_FUNC_SELF_TEST_CB, (void (*)(void))core_self_test_get_callback }, { OSSL_FUNC_GET_ENTROPY, (void (*)(void))rand_get_entropy }, + { OSSL_FUNC_GET_USER_ENTROPY, (void (*)(void))rand_get_user_entropy }, { OSSL_FUNC_CLEANUP_ENTROPY, (void (*)(void))rand_cleanup_entropy }, + { OSSL_FUNC_CLEANUP_USER_ENTROPY, (void (*)(void))rand_cleanup_user_entropy }, { OSSL_FUNC_GET_NONCE, (void (*)(void))rand_get_nonce }, - { OSSL_FUNC_CLEANUP_NONCE, (void (*)(void))rand_cleanup_nonce }, - { OSSL_FUNC_GET_USER_ENTROPY, (void (*)(void))rand_get_user_entropy }, { OSSL_FUNC_GET_USER_NONCE, (void (*)(void))rand_get_user_nonce }, + { OSSL_FUNC_CLEANUP_NONCE, (void (*)(void))rand_cleanup_nonce }, + { OSSL_FUNC_CLEANUP_USER_NONCE, (void (*)(void))rand_cleanup_user_nonce }, #endif { OSSL_FUNC_CRYPTO_MALLOC, (void (*)(void))CRYPTO_malloc }, { OSSL_FUNC_CRYPTO_ZALLOC, (void (*)(void))CRYPTO_zalloc }, diff --git a/crypto/rand/prov_seed.c b/crypto/rand/prov_seed.c index af35e0247595a..a8128119b5101 100644 --- a/crypto/rand/prov_seed.c +++ b/crypto/rand/prov_seed.c @@ -77,6 +77,12 @@ void ossl_rand_cleanup_entropy(ossl_unused OSSL_LIB_CTX *ctx, OPENSSL_secure_clear_free(buf, len); } +void ossl_rand_cleanup_user_entropy(OSSL_LIB_CTX *ctx, + unsigned char *buf, size_t len) +{ + OPENSSL_secure_clear_free(buf, len); +} + size_t ossl_rand_get_nonce(ossl_unused OSSL_LIB_CTX *ctx, unsigned char **pout, size_t min_len, ossl_unused size_t max_len, @@ -130,3 +136,9 @@ void ossl_rand_cleanup_nonce(ossl_unused OSSL_LIB_CTX *ctx, { OPENSSL_clear_free(buf, len); } + +void ossl_rand_cleanup_user_nonce(ossl_unused OSSL_LIB_CTX *ctx, + unsigned char *buf, size_t len) +{ + OPENSSL_clear_free(buf, len); +} diff --git a/doc/internal/man3/ossl_rand_get_entropy.pod b/doc/internal/man3/ossl_rand_get_entropy.pod index 5c7a076336df0..be39369f2b700 100644 --- a/doc/internal/man3/ossl_rand_get_entropy.pod +++ b/doc/internal/man3/ossl_rand_get_entropy.pod @@ -2,8 +2,10 @@ =head1 NAME -ossl_rand_get_entropy, ossl_rand_get_user_entropy, ossl_rand_cleanup_entropy, -ossl_rand_get_nonce, ossl_rand_get_user_nonce, ossl_rand_cleanup_nonce +ossl_rand_get_entropy, ossl_rand_get_user_entropy, +ossl_rand_cleanup_entropy, ossl_rand_cleanup_user_entropy, +ossl_rand_get_nonce, ossl_rand_get_user_nonce, +ossl_rand_cleanup_nonce, ossl_rand_cleanup_user_nonce - get seed material from the operating system =head1 SYNOPSIS @@ -18,6 +20,8 @@ ossl_rand_get_nonce, ossl_rand_get_user_nonce, ossl_rand_cleanup_nonce size_t min_len, size_t max_len); void ossl_rand_cleanup_entropy(OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len); + void ossl_rand_cleanup_user_entropy(OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len); size_t ossl_rand_get_nonce(OSSL_CORE_HANDLE *handle, unsigned char **pout, size_t min_len, size_t max_len, const void *salt, size_t salt_len); @@ -26,6 +30,8 @@ ossl_rand_get_nonce, ossl_rand_get_user_nonce, ossl_rand_cleanup_nonce const void *salt, size_t salt_len); void ossl_rand_cleanup_nonce(OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len); + void ossl_rand_cleanup_user_nonce(OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len); =head1 DESCRIPTION @@ -41,8 +47,12 @@ DRBG seed source. By default this is the operating system but it can be changed by calling L. ossl_rand_cleanup_entropy() cleanses and frees any storage allocated by -ossl_rand_get_entropy() or ossl_rand_get_user_entropy(). The entropy -buffer is pointed to by I and is of length I bytes. +ossl_rand_get_entropy(). The entropy buffer is pointed to by I +and is of length I bytes. + +ossl_rand_cleanup_user_entropy() cleanses and frees any storage allocated by +ossl_rand_get_user_entropy(). The entropy buffer is pointed to by I +and is of length I bytes. ossl_rand_get_nonce() retrieves a nonce using the passed I parameter of length I and operating system specific information. @@ -76,8 +86,9 @@ of bytes in I<*pout> or 0 on error. =head1 HISTORY -The functions ossl_rand_get_user_entropy() and ossl_rand_get_user_nonce() -were added in OpenSSL 3.0.12, 3.1.4 and 3.2.0. +The functions ossl_rand_get_user_entropy(), ossl_rand_get_user_nonce(), +ossl_rand_cleanup_user_entropy(), and ossl_rand_cleanup_user_nonce() +were added in OpenSSL 3.1.4 and 3.2.0. The remaining functions described here were all added in OpenSSL 3.0. diff --git a/doc/man7/provider-base.pod b/doc/man7/provider-base.pod index eb9e8d35758ff..5dcbbed221be2 100644 --- a/doc/man7/provider-base.pod +++ b/doc/man7/provider-base.pod @@ -81,6 +81,8 @@ provider-base size_t min_len, size_t max_len); void cleanup_entropy(const OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len); + void cleanup_user_entropy(const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len); size_t get_nonce(const OSSL_CORE_HANDLE *handle, unsigned char **pout, size_t min_len, size_t max_len, const void *salt, size_t salt_len); @@ -89,6 +91,8 @@ provider-base const void *salt, size_t salt_len); void cleanup_nonce(const OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len); + void cleanup_user_nonce(const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len); /* Functions for querying the providers in the application library context */ int provider_register_child_cb(const OSSL_CORE_HANDLE *handle, @@ -179,9 +183,11 @@ provider): ossl_rand_get_entropy OSSL_FUNC_GET_ENTROPY ossl_rand_get_user_entropy OSSL_FUNC_GET_USER_ENTROPY ossl_rand_cleanup_entropy OSSL_FUNC_CLEANUP_ENTROPY + ossl_rand_cleanup_user_entropy OSSL_FUNC_CLEANUP_USER_ENTROPY ossl_rand_get_nonce OSSL_FUNC_GET_NONCE ossl_rand_get_user_nonce OSSL_FUNC_GET_USER_NONCE ossl_rand_cleanup_nonce OSSL_FUNC_CLEANUP_NONCE + ossl_rand_cleanup_user_nonce OSSL_FUNC_CLEANUP_USER_NONCE provider_register_child_cb OSSL_FUNC_PROVIDER_REGISTER_CHILD_CB provider_deregister_child_cb OSSL_FUNC_PROVIDER_DEREGISTER_CHILD_CB provider_name OSSL_FUNC_PROVIDER_NAME @@ -315,9 +321,12 @@ attempt to gather seed material via the seed source specified by a call to L or via L. cleanup_entropy() is used to clean up and free the buffer returned by -get_entropy() or get_user_entropy(). The entropy pointer returned by -get_entropy() or get_user_entropy() is passed in B and its length -in B. +get_entropy(). The entropy pointer returned by get_entropy() +is passed in B and its length in B. + +cleanup_user_entropy() is used to clean up and free the buffer returned by +get_user_entropy(). The entropy pointer returned by get_user_entropy() +is passed in B and its length in B. get_nonce() retrieves a nonce using the passed I parameter of length I and operating system specific information. @@ -331,10 +340,13 @@ get_user_nonce() is the same as get_nonce() except that it will attempt to gather seed material via the seed source specified by a call to L or via L. -cleanup_nonce() is used to clean up and free the buffer returned -by get_nonce() or get_user_nonce(). The nonce pointer returned by -get_nonce() or get_user_nonce() is passed in B and its length -in B. +cleanup_nonce() is used to clean up and free the buffer returned by +get_nonce(). The nonce pointer returned by get_nonce() +is passed in B and its length in B. + +cleanup_user_nonce() is used to clean up and free the buffer returned by +get_user_nonce(). The nonce pointer returned by get_user_nonce() +is passed in B and its length in B. provider_register_child_cb() registers callbacks for being informed about the loading and unloading of providers in the application's library context. diff --git a/include/crypto/rand.h b/include/crypto/rand.h index 5841cccaa66e9..215b3b7af34f5 100644 --- a/include/crypto/rand.h +++ b/include/crypto/rand.h @@ -116,6 +116,8 @@ size_t ossl_rand_get_user_entropy(OSSL_LIB_CTX *ctx, size_t min_len, size_t max_len); void ossl_rand_cleanup_entropy(OSSL_LIB_CTX *ctx, unsigned char *buf, size_t len); +void ossl_rand_cleanup_user_entropy(OSSL_LIB_CTX *ctx, + unsigned char *buf, size_t len); size_t ossl_rand_get_nonce(OSSL_LIB_CTX *ctx, unsigned char **pout, size_t min_len, size_t max_len, const void *salt, size_t salt_len); @@ -124,6 +126,8 @@ size_t ossl_rand_get_user_nonce(OSSL_LIB_CTX *ctx, unsigned char **pout, const void *salt, size_t salt_len); void ossl_rand_cleanup_nonce(OSSL_LIB_CTX *ctx, unsigned char *buf, size_t len); +void ossl_rand_cleanup_user_nonce(OSSL_LIB_CTX *ctx, + unsigned char *buf, size_t len); /* * Get seeding material from the operating system sources. diff --git a/include/openssl/core_dispatch.h b/include/openssl/core_dispatch.h index 6c952f18aac00..9b03f20c3b7da 100644 --- a/include/openssl/core_dispatch.h +++ b/include/openssl/core_dispatch.h @@ -177,6 +177,8 @@ OSSL_CORE_MAKE_FUNC(int, BIO_ctrl, (OSSL_CORE_BIO *bio, int cmd, long num, void *ptr)) /* New seeding functions prototypes with the 101-104 series */ +#define OSSL_FUNC_CLEANUP_USER_ENTROPY 96 +#define OSSL_FUNC_CLEANUP_USER_NONCE 97 #define OSSL_FUNC_GET_USER_ENTROPY 98 #define OSSL_FUNC_GET_USER_NONCE 99 @@ -197,6 +199,8 @@ OSSL_CORE_MAKE_FUNC(size_t, get_user_entropy, (const OSSL_CORE_HANDLE *handle, size_t min_len, size_t max_len)) OSSL_CORE_MAKE_FUNC(void, cleanup_entropy, (const OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len)) +OSSL_CORE_MAKE_FUNC(void, cleanup_user_entropy, (const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len)) OSSL_CORE_MAKE_FUNC(size_t, get_nonce, (const OSSL_CORE_HANDLE *handle, unsigned char **pout, size_t min_len, size_t max_len, const void *salt, @@ -207,6 +211,8 @@ OSSL_CORE_MAKE_FUNC(size_t, get_user_nonce, (const OSSL_CORE_HANDLE *handle, size_t salt_len)) OSSL_CORE_MAKE_FUNC(void, cleanup_nonce, (const OSSL_CORE_HANDLE *handle, unsigned char *buf, size_t len)) +OSSL_CORE_MAKE_FUNC(void, cleanup_user_nonce, (const OSSL_CORE_HANDLE *handle, + unsigned char *buf, size_t len)) /* Functions to access the core's providers */ #define OSSL_FUNC_PROVIDER_REGISTER_CHILD_CB 105 diff --git a/providers/common/provider_seeding.c b/providers/common/provider_seeding.c index c7b2ea6da62a3..544344f30a996 100644 --- a/providers/common/provider_seeding.c +++ b/providers/common/provider_seeding.c @@ -14,9 +14,11 @@ static OSSL_FUNC_get_entropy_fn *c_get_entropy = NULL; static OSSL_FUNC_get_user_entropy_fn *c_get_user_entropy = NULL; static OSSL_FUNC_cleanup_entropy_fn *c_cleanup_entropy = NULL; +static OSSL_FUNC_cleanup_user_entropy_fn *c_cleanup_user_entropy = NULL; static OSSL_FUNC_get_nonce_fn *c_get_nonce = NULL; static OSSL_FUNC_get_user_nonce_fn *c_get_user_nonce = NULL; static OSSL_FUNC_cleanup_nonce_fn *c_cleanup_nonce = NULL; +static OSSL_FUNC_cleanup_user_nonce_fn *c_cleanup_user_nonce = NULL; #ifdef FIPS_MODULE /* @@ -56,6 +58,9 @@ int ossl_prov_seeding_from_dispatch(const OSSL_DISPATCH *fns) case OSSL_FUNC_CLEANUP_ENTROPY: set_func(c_cleanup_entropy, OSSL_FUNC_cleanup_entropy(fns)); break; + case OSSL_FUNC_CLEANUP_USER_ENTROPY: + set_func(c_cleanup_user_entropy, OSSL_FUNC_cleanup_user_entropy(fns)); + break; case OSSL_FUNC_GET_NONCE: set_func(c_get_nonce, OSSL_FUNC_get_nonce(fns)); break; @@ -65,6 +70,9 @@ int ossl_prov_seeding_from_dispatch(const OSSL_DISPATCH *fns) case OSSL_FUNC_CLEANUP_NONCE: set_func(c_cleanup_nonce, OSSL_FUNC_cleanup_nonce(fns)); break; + case OSSL_FUNC_CLEANUP_USER_NONCE: + set_func(c_cleanup_user_nonce, OSSL_FUNC_cleanup_user_nonce(fns)); + break; } #undef set_func } @@ -86,8 +94,12 @@ size_t ossl_prov_get_entropy(PROV_CTX *prov_ctx, unsigned char **pout, void ossl_prov_cleanup_entropy(PROV_CTX *prov_ctx, unsigned char *buf, size_t len) { - if (c_cleanup_entropy != NULL) - c_cleanup_entropy(CORE_HANDLE(prov_ctx), buf, len); + const OSSL_CORE_HANDLE *handle = CORE_HANDLE(prov_ctx); + + if (c_cleanup_user_entropy != NULL) + c_cleanup_user_entropy(handle, buf, len); + else if (c_cleanup_entropy != NULL) + c_cleanup_entropy(handle, buf, len); } size_t ossl_prov_get_nonce(PROV_CTX *prov_ctx, unsigned char **pout, @@ -105,6 +117,10 @@ size_t ossl_prov_get_nonce(PROV_CTX *prov_ctx, unsigned char **pout, void ossl_prov_cleanup_nonce(PROV_CTX *prov_ctx, unsigned char *buf, size_t len) { - if (c_cleanup_nonce != NULL) - c_cleanup_nonce(CORE_HANDLE(prov_ctx), buf, len); + const OSSL_CORE_HANDLE *handle = CORE_HANDLE(prov_ctx); + + if (c_cleanup_user_nonce != NULL) + c_cleanup_user_nonce(handle, buf, len); + else if (c_cleanup_nonce != NULL) + c_cleanup_nonce(handle, buf, len); }