Skip to content

Commit

Permalink
rand: add callbacks to cleanup the user entropy resp. nonce
Browse files Browse the repository at this point in the history
The `get_user_{entropy,nonce}` callbacks were add recently to the
dispatch table in commit 4cde758. 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 <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #22423)

(cherry picked from commit 5516d20)
  • Loading branch information
mspncp authored and mattcaswell committed Oct 20, 2023
1 parent 307048c commit 16d9c8a
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 21 deletions.
26 changes: 22 additions & 4 deletions crypto/provider_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1854,12 +1854,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;
Expand Down Expand Up @@ -2043,6 +2045,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,
Expand All @@ -2068,6 +2077,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);
Expand Down Expand Up @@ -2162,11 +2178,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 },
Expand Down
12 changes: 12 additions & 0 deletions crypto/rand/prov_seed.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
23 changes: 17 additions & 6 deletions doc/internal/man3/ossl_rand_get_entropy.pod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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

Expand All @@ -41,8 +47,12 @@ DRBG seed source. By default this is the operating system but it can
be changed by calling L<RAND_set_seed_source_type(3)>.

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<buf> and is of length I<len> bytes.
ossl_rand_get_entropy(). The entropy buffer is pointed to by I<buf>
and is of length I<len> 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<buf>
and is of length I<len> bytes.

ossl_rand_get_nonce() retrieves a nonce using the passed I<salt> parameter
of length I<salt_len> and operating system specific information.
Expand Down Expand Up @@ -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.

Expand Down
26 changes: 19 additions & 7 deletions doc/man7/provider-base.pod
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -315,9 +321,12 @@ attempt to gather seed material via the seed source specified by a call to
L<RAND_set_seed_source_type(3)> or via L<config(5)/Random Configuration>.

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<buf> and its length
in B<len>.
get_entropy(). The entropy pointer returned by get_entropy()
is passed in B<buf> and its length in B<len>.

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<buf> and its length in B<len>.

get_nonce() retrieves a nonce using the passed I<salt> parameter
of length I<salt_len> and operating system specific information.
Expand All @@ -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<RAND_set_seed_source_type(3)> or via L<config(5)/Random Configuration>.

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<buf> and its length
in B<len>.
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<buf> and its length in B<len>.

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<buf> and its length in B<len>.

provider_register_child_cb() registers callbacks for being informed about the
loading and unloading of providers in the application's library context.
Expand Down
4 changes: 4 additions & 0 deletions include/crypto/rand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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.
Expand Down
6 changes: 6 additions & 0 deletions include/openssl/core_dispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand All @@ -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
Expand Down
24 changes: 20 additions & 4 deletions providers/common/provider_seeding.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
/*
Expand Down Expand Up @@ -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;
Expand All @@ -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
}
Expand All @@ -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,
Expand All @@ -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);
}

0 comments on commit 16d9c8a

Please sign in to comment.