From 09066cf2a1f9f3d13ea2898304250f5916d6de70 Mon Sep 17 00:00:00 2001 From: "Dr. Matthias St. Pierre" Date: Fri, 31 Jan 2020 13:32:11 +0100 Subject: [PATCH] tests/drbgtest: use new RAND_DRBG callback_data API instead of ex_data It took me a little while to realize why the test_rand_drbg_reseed test kept crashing after replacing the RAND_DRBG_{gs}et_ex_data() calls by RAND_DRBG_{gs}et_callback_data(). The reason was that the ex_data API prohibits modifying the callbacks or callback data of chained DRBGs and returned an error which was ignored by the `test_rand_drbg_reseed` test, for good reasons. The `test_rand_drbg_reseed` test is special in this respect, because it needs to install callbacks for all DRBGs, in order to intercept and count the reseeding events. Since the drbgtest module has access to the internal structures of the DRBG anyway, the problem could be solved by accessing the members directly. I added a warning comment in hook_drbg(). [extended tests] Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/10950) --- doc/man3/RAND_DRBG_set_callbacks.pod | 4 ++-- test/drbgtest.c | 31 ++++++++++++++-------------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/doc/man3/RAND_DRBG_set_callbacks.pod b/doc/man3/RAND_DRBG_set_callbacks.pod index c899a1adcb..51414bcfc1 100644 --- a/doc/man3/RAND_DRBG_set_callbacks.pod +++ b/doc/man3/RAND_DRBG_set_callbacks.pod @@ -62,7 +62,7 @@ RAND_DRBG_set_callback_data() can be used to store a pointer to some context specific data, which can subsequently be retrieved by the entropy and nonce callbacks using RAND_DRBG_get_callback_data(). The ownership of the context data remains with the caller, i.e., it is the -caller's responsibility to keep it available as long as it is need by the +caller's responsibility to keep it available as long as it is needed by the callbacks and free it after use. For more information about the the callback data see the NOTES section. @@ -140,7 +140,7 @@ In this case the DRBG will automatically request an extra amount of entropy utilize for the nonce, following the recommendations of [NIST SP 800-90A Rev. 1], section 8.6.7. -The callback data a rather specialized feature, because in general the +The callback data is a rather specialized feature, because in general the random sources don't (and in fact, they must not) depend on any state provided by the DRBG. There are however exceptional cases where this feature is useful, most notably diff --git a/test/drbgtest.c b/test/drbgtest.c index b8dcbf7b7b..34a5cc744d 100644 --- a/test/drbgtest.c +++ b/test/drbgtest.c @@ -127,8 +127,6 @@ static DRBG_SELFTEST_DATA drbg_test[] = { make_drbg_test_data_hash(NID_sha512, sha512, 0), }; -static int app_data_index; - /* * Test context data, attached as EXDATA to the RAND_DRBG */ @@ -145,7 +143,7 @@ static size_t kat_entropy(RAND_DRBG *drbg, unsigned char **pout, int entropy, size_t min_len, size_t max_len, int prediction_resistance) { - TEST_CTX *t = (TEST_CTX *)RAND_DRBG_get_ex_data(drbg, app_data_index); + TEST_CTX *t = (TEST_CTX *)RAND_DRBG_get_callback_data(drbg); t->entropycnt++; *pout = (unsigned char *)t->entropy; @@ -155,7 +153,7 @@ static size_t kat_entropy(RAND_DRBG *drbg, unsigned char **pout, static size_t kat_nonce(RAND_DRBG *drbg, unsigned char **pout, int entropy, size_t min_len, size_t max_len) { - TEST_CTX *t = (TEST_CTX *)RAND_DRBG_get_ex_data(drbg, app_data_index); + TEST_CTX *t = (TEST_CTX *)RAND_DRBG_get_callback_data(drbg); t->noncecnt++; *pout = (unsigned char *)t->nonce; @@ -213,6 +211,7 @@ static int single_kat(DRBG_SELFTEST_DATA *td) return 0; if (!TEST_true(RAND_DRBG_set_callbacks(drbg, kat_entropy, NULL, kat_nonce, NULL)) + || !TEST_true(RAND_DRBG_set_callback_data(drbg, &t)) || !TEST_true(disable_crngt(drbg))) { failures++; goto err; @@ -222,7 +221,6 @@ static int single_kat(DRBG_SELFTEST_DATA *td) t.entropylen = td->entropylen; t.nonce = td->nonce; t.noncelen = td->noncelen; - RAND_DRBG_set_ex_data(drbg, app_data_index, &t); if (!TEST_true(RAND_DRBG_instantiate(drbg, td->pers, td->perslen)) || !TEST_true(RAND_DRBG_generate(drbg, buff, td->exlen, 0, @@ -246,9 +244,9 @@ static int single_kat(DRBG_SELFTEST_DATA *td) */ if (!TEST_true(RAND_DRBG_set(drbg, td->nid, td->flags)) || !TEST_true(RAND_DRBG_set_callbacks(drbg, kat_entropy, NULL, - kat_nonce, NULL))) + kat_nonce, NULL)) + || !TEST_true(RAND_DRBG_set_callback_data(drbg, &t))) failures++; - RAND_DRBG_set_ex_data(drbg, app_data_index, &t); t.entropy = td->entropy_pr; t.entropylen = td->entropylen_pr; t.nonce = td->nonce_pr; @@ -296,7 +294,7 @@ static int init(RAND_DRBG *drbg, DRBG_SELFTEST_DATA *td, TEST_CTX *t) || !TEST_true(RAND_DRBG_set_callbacks(drbg, kat_entropy, NULL, kat_nonce, NULL))) return 0; - RAND_DRBG_set_ex_data(drbg, app_data_index, t); + RAND_DRBG_set_callback_data(drbg, t); t->entropy = td->entropy; t->entropylen = td->entropylen; t->nonce = td->nonce; @@ -551,7 +549,7 @@ static HOOK_CTX master_ctx, public_ctx, private_ctx; static HOOK_CTX *get_hook_ctx(RAND_DRBG *drbg) { - return (HOOK_CTX *)RAND_DRBG_get_ex_data(drbg, app_data_index); + return (HOOK_CTX *)RAND_DRBG_get_callback_data(drbg); } /* Intercepts and counts calls to the get_entropy() callback */ @@ -579,17 +577,22 @@ static void hook_drbg(RAND_DRBG *drbg, HOOK_CTX *ctx) memset(ctx, 0, sizeof(*ctx)); ctx->drbg = drbg; ctx->get_entropy = drbg->get_entropy; + + /* + * We can't use the public API here, since it prohibits modifying + * the callbacks or the callback data of chained DRBGs. + */ drbg->get_entropy = get_entropy_hook; - RAND_DRBG_set_ex_data(drbg, app_data_index, ctx); + drbg->callback_data = ctx; } /* Installs the hook for the get_entropy() callback of the given drbg */ static void unhook_drbg(RAND_DRBG *drbg) { - HOOK_CTX *ctx = get_hook_ctx(drbg); + HOOK_CTX *ctx = drbg->callback_data; - drbg->get_entropy = ctx->get_entropy; - CRYPTO_free_ex_data(CRYPTO_EX_INDEX_RAND_DRBG, drbg, &drbg->ex_data); + if (ctx != NULL) + drbg->get_entropy = ctx->get_entropy; } /* Resets the given hook context */ @@ -1382,8 +1385,6 @@ err: int setup_tests(void) { - app_data_index = RAND_DRBG_get_ex_new_index(0L, NULL, NULL, NULL, NULL); - ADD_ALL_TESTS(test_kats, OSSL_NELEM(drbg_test)); ADD_ALL_TESTS(test_error_checks, OSSL_NELEM(drbg_test)); ADD_TEST(test_rand_drbg_reseed); -- 2.39.5