contexts: Forbid randomizing secp256k1_context_static

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
 

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,

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);
     }

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);