From 61841fc9ee5aa1ffde3ccd512660207034125ebd Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 7 Dec 2022 14:50:14 +0100 Subject: [PATCH] contexts: Forbid randomizing secp256k1_context_static --- CHANGELOG.md | 1 + include/secp256k1.h | 16 +++++-------- src/secp256k1.c | 2 ++ src/tests.c | 55 +++++++++++++++++++++++++++++++++++++++------ 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7755f61e7b5dd..62a89f83c50f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### Changed - Forbade cloning or destroying `secp256k1_context_static`. Create a new context instead of cloning the static context. (If this change breaks your code, your code is probably wrong.) + - Forbade randomizing (copies of) `secp256k1_context_static`. Randomizing a copy of `secp256k1_context_static` did not have any effect and did not provide defense-in-depth protection against side-channel attacks. Create a new context if you want to benefit from randomization. ## [0.2.0] - 2022-12-12 diff --git a/include/secp256k1.h b/include/secp256k1.h index a228ef638c423..3a75b05000c3e 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -824,10 +824,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( /** Randomizes the context to provide enhanced protection against side-channel leakage. * - * Returns: 1: randomization successful (or called on copy of secp256k1_context_static) + * Returns: 1: randomization successful * 0: error - * Args: ctx: pointer to a context object. - * In: seed32: pointer to a 32-byte random seed (NULL resets to initial state) + * Args: ctx: pointer to a context object (not secp256k1_context_static). + * In: seed32: pointer to a 32-byte random seed (NULL resets to initial state). * * While secp256k1 code is written and tested to be constant-time no matter what * secret values are, it is possible that a compiler may output code which is not, @@ -842,21 +842,17 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( * functions that perform computations involving secret keys, e.g., signing and * public key generation. It is possible to call this function more than once on * the same context, and doing so before every few computations involving secret - * keys is recommended as a defense-in-depth measure. + * keys is recommended as a defense-in-depth measure. Randomization of the static + * context secp256k1_context_static is not supported. * * Currently, the random seed is mainly used for blinding multiplications of a * secret scalar with the elliptic curve base point. Multiplications of this * kind are performed by exactly those API functions which are documented to - * require a context that is not the secp256k1_context_static. As a rule of thumb, + * require a context that is not secp256k1_context_static. As a rule of thumb, * these are all functions which take a secret key (or a keypair) as an input. * A notable exception to that rule is the ECDH module, which relies on a different * kind of elliptic curve point multiplication and thus does not benefit from * enhanced protection against side-channel leakage currently. - * - * It is safe to call this function on a copy of secp256k1_context_static in writable - * memory (e.g., obtained via secp256k1_context_clone). In that case, this - * function is guaranteed to return 1, but the call will have no effect because - * the static context (or a copy thereof) is not meant to be randomized. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize( secp256k1_context* ctx, diff --git a/src/secp256k1.c b/src/secp256k1.c index 3e9e7ccc94e68..7af333ca900fd 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -749,6 +749,8 @@ int secp256k1_ec_pubkey_tweak_mul(const secp256k1_context* ctx, secp256k1_pubkey int secp256k1_context_randomize(secp256k1_context* ctx, const unsigned char *seed32) { VERIFY_CHECK(ctx != NULL); + ARG_CHECK(secp256k1_context_is_proper(ctx)); + if (secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)) { secp256k1_ecmult_gen_blind(&ctx->ecmult_gen_ctx, seed32); } diff --git a/src/tests.c b/src/tests.c index c33d1dc42cc6a..22bc042b671e8 100644 --- a/src/tests.c +++ b/src/tests.c @@ -247,7 +247,16 @@ static void run_static_context_tests(int use_prealloc) { { int ecount = 0; + unsigned char seed[32] = {0x17}; secp256k1_context_set_illegal_callback(STATIC_CTX, counting_illegal_callback_fn, &ecount); + + /* Randomizing secp256k1_context_static is not supported. */ + CHECK(secp256k1_context_randomize(STATIC_CTX, seed) == 0); + CHECK(ecount == 1); + CHECK(secp256k1_context_randomize(STATIC_CTX, NULL) == 0); + CHECK(ecount == 2); + ecount = 0; + /* Destroying or cloning secp256k1_context_static is not supported. */ if (use_prealloc) { CHECK(secp256k1_context_preallocated_clone_size(STATIC_CTX) == 0); @@ -286,13 +295,18 @@ static void run_static_context_tests(int use_prealloc) { static void run_proper_context_tests(int use_prealloc) { int32_t dummy = 0; - secp256k1_context *my_ctx; + secp256k1_context *my_ctx, *my_ctx_fresh; void *my_ctx_prealloc = NULL; + unsigned char seed[32] = {0x17}; secp256k1_gej pubj; secp256k1_ge pub; secp256k1_scalar msg, key, nonce; secp256k1_scalar sigr, sigs; + + /* Fresh reference context for comparison */ + my_ctx_fresh = secp256k1_context_create(SECP256K1_CONTEXT_NONE); + if (use_prealloc) { my_ctx_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(my_ctx_prealloc != NULL); @@ -301,6 +315,13 @@ static void run_proper_context_tests(int use_prealloc) { my_ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); } + /* Randomize and reset randomization */ + CHECK(context_eq(my_ctx, my_ctx_fresh)); + CHECK(secp256k1_context_randomize(my_ctx, seed) == 1); + CHECK(!context_eq(my_ctx, my_ctx_fresh)); + CHECK(secp256k1_context_randomize(my_ctx, NULL) == 1); + CHECK(context_eq(my_ctx, my_ctx_fresh)); + /* set error callback (to a function that still aborts in case malloc() fails in secp256k1_context_clone() below) */ secp256k1_context_set_error_callback(my_ctx, secp256k1_default_illegal_callback_fn, NULL); CHECK(my_ctx->error_callback.fn != secp256k1_default_error_callback_fn); @@ -315,16 +336,33 @@ static void run_proper_context_tests(int use_prealloc) { if (use_prealloc) { /* clone into a non-preallocated context and then again into a new preallocated one. */ - ctx_tmp = my_ctx; my_ctx = secp256k1_context_clone(my_ctx); secp256k1_context_preallocated_destroy(ctx_tmp); - free(my_ctx_prealloc); my_ctx_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(my_ctx_prealloc != NULL); - ctx_tmp = my_ctx; my_ctx = secp256k1_context_preallocated_clone(my_ctx, my_ctx_prealloc); secp256k1_context_destroy(ctx_tmp); + ctx_tmp = my_ctx; + my_ctx = secp256k1_context_clone(my_ctx); + CHECK(context_eq(ctx_tmp, my_ctx)); + secp256k1_context_preallocated_destroy(ctx_tmp); + + free(my_ctx_prealloc); + my_ctx_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); + CHECK(my_ctx_prealloc != NULL); + ctx_tmp = my_ctx; + my_ctx = secp256k1_context_preallocated_clone(my_ctx, my_ctx_prealloc); + CHECK(context_eq(ctx_tmp, my_ctx)); + secp256k1_context_destroy(ctx_tmp); } else { /* clone into a preallocated context and then again into a new non-preallocated one. */ void *prealloc_tmp; - prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(prealloc_tmp != NULL); - ctx_tmp = my_ctx; my_ctx = secp256k1_context_preallocated_clone(my_ctx, prealloc_tmp); secp256k1_context_destroy(ctx_tmp); - ctx_tmp = my_ctx; my_ctx = secp256k1_context_clone(my_ctx); secp256k1_context_preallocated_destroy(ctx_tmp); + prealloc_tmp = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); + CHECK(prealloc_tmp != NULL); + ctx_tmp = my_ctx; + my_ctx = secp256k1_context_preallocated_clone(my_ctx, prealloc_tmp); + CHECK(context_eq(ctx_tmp, my_ctx)); + secp256k1_context_destroy(ctx_tmp); + + ctx_tmp = my_ctx; + my_ctx = secp256k1_context_clone(my_ctx); + CHECK(context_eq(ctx_tmp, my_ctx)); + secp256k1_context_preallocated_destroy(ctx_tmp); free(prealloc_tmp); } } @@ -335,6 +373,7 @@ static void run_proper_context_tests(int use_prealloc) { /* And that it resets back to default. */ secp256k1_context_set_error_callback(my_ctx, NULL, NULL); CHECK(my_ctx->error_callback.fn == secp256k1_default_error_callback_fn); + CHECK(context_eq(my_ctx, my_ctx_fresh)); /* Verify that setting and resetting illegal callback works */ secp256k1_context_set_illegal_callback(my_ctx, counting_illegal_callback_fn, &dummy); @@ -343,6 +382,7 @@ static void run_proper_context_tests(int use_prealloc) { secp256k1_context_set_illegal_callback(my_ctx, NULL, NULL); CHECK(my_ctx->illegal_callback.fn == secp256k1_default_illegal_callback_fn); CHECK(my_ctx->illegal_callback.data == NULL); + CHECK(context_eq(my_ctx, my_ctx_fresh)); /*** attempt to use them ***/ random_scalar_order_test(&msg); @@ -368,6 +408,7 @@ static void run_proper_context_tests(int use_prealloc) { } else { secp256k1_context_destroy(my_ctx); } + secp256k1_context_destroy(my_ctx_fresh); /* Defined as no-op. */ secp256k1_context_destroy(NULL);