contexts: Forbid destroying, cloning and randomizing the static context

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+#### 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
 
 #### Added

include/secp256k1.h

@@ -291,8 +291,11 @@ SECP256K1_API secp256k1_context* secp256k1_context_create(
  * called at most once for every call of this function. If you need to avoid dynamic
  * memory allocation entirely, see the functions in secp256k1_preallocated.h.
  *
+ * Cloning secp256k1_context_static is not possible, and should not be emulated by
+ * the caller (e.g., using memcpy). Create a new context instead.
+ *
  * Returns: a newly created context object.
- * Args: ctx: an existing context to copy
+ * Args: ctx: an existing context to copy (not secp256k1_context_static)
  */
 SECP256K1_API secp256k1_context* secp256k1_context_clone(
  const secp256k1_context* ctx
@@ -310,6 +313,7 @@ SECP256K1_API secp256k1_context* secp256k1_context_clone(
  *
  * Args: ctx: an existing context to destroy, constructed using
  * secp256k1_context_create or secp256k1_context_clone
+ * (i.e., not secp256k1_context_static).
  */
 SECP256K1_API void secp256k1_context_destroy(
  secp256k1_context* ctx
@@ -820,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,
@@ -838,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,

include/secp256k1_preallocated.h

@@ -88,8 +88,11 @@ SECP256K1_API size_t secp256k1_context_preallocated_clone_size(
  * the lifetime of this context object, see the description of
  * secp256k1_context_preallocated_create for details.
  *
+ * Cloning secp256k1_context_static is not possible, and should not be emulated by
+ * the caller (e.g., using memcpy). Create a new context instead.
+ *
  * Returns: a newly created context object.
- * Args: ctx: an existing context to copy.
+ * Args: ctx: an existing context to copy (not secp256k1_context_static).
  * In: prealloc: a pointer to a rewritable contiguous block of memory of
  * size at least secp256k1_context_preallocated_size(flags)
  * bytes, as detailed above.
@@ -117,7 +120,8 @@ SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone(
  *
  * Args: ctx: an existing context to destroy, constructed using
  * secp256k1_context_preallocated_create or
- * secp256k1_context_preallocated_clone.
+ * secp256k1_context_preallocated_clone
+ * (i.e., not secp256k1_context_static).
  */
 SECP256K1_API void secp256k1_context_preallocated_destroy(
  secp256k1_context* ctx

src/secp256k1.c

@@ -109,9 +109,9 @@ size_t secp256k1_context_preallocated_size(unsigned int flags) {
 }
 
 size_t secp256k1_context_preallocated_clone_size(const secp256k1_context* ctx) {
- size_t ret = sizeof(secp256k1_context);
  VERIFY_CHECK(ctx != NULL);
- return ret;
+ ARG_CHECK(secp256k1_context_is_proper(ctx));
+ return sizeof(secp256k1_context);
 }
 
 secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigned int flags) {
@@ -152,6 +152,7 @@ secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context*
  secp256k1_context* ret;
  VERIFY_CHECK(ctx != NULL);
  ARG_CHECK(prealloc != NULL);
+ ARG_CHECK(secp256k1_context_is_proper(ctx));
 
  ret = (secp256k1_context*)prealloc;
  *ret = *ctx;
@@ -163,24 +164,35 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) {
  size_t prealloc_size;
 
  VERIFY_CHECK(ctx != NULL);
+ ARG_CHECK(secp256k1_context_is_proper(ctx));
+
  prealloc_size = secp256k1_context_preallocated_clone_size(ctx);
  ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, prealloc_size);
  ret = secp256k1_context_preallocated_clone(ctx, ret);
  return ret;
 }
 
 void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) {
- ARG_CHECK_VOID(ctx != secp256k1_context_static);
- if (ctx != NULL) {
- secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx);
+ ARG_CHECK_VOID(ctx == NULL || secp256k1_context_is_proper(ctx));
+
+ /* Defined as noop */
+ if (ctx == NULL) {
+ return;
  }
+
+ secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx);
 }
 
 void secp256k1_context_destroy(secp256k1_context* ctx) {
- if (ctx != NULL) {
- secp256k1_context_preallocated_destroy(ctx);
- free(ctx);
+ ARG_CHECK_VOID(ctx == NULL || secp256k1_context_is_proper(ctx));
+
+ /* Defined as noop */
+ if (ctx == NULL) {
+ return;
  }
+
+ secp256k1_context_preallocated_destroy(ctx);
+ free(ctx);
 }
 
 void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
@@ -737,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

@@ -32,11 +32,43 @@ static int COUNT = 64;
 static secp256k1_context *CTX = NULL;
 static secp256k1_context *STATIC_CTX = NULL;
 
+static int all_bytes_equal(const void* s, unsigned char value, size_t n) {
+ const unsigned char *p = s;
+ size_t i;
+
+ for (i = 0; i < n; i++) {
+ if (p[i] != value) {
+ return 0;
+ }
+ }
+ return 1;
+}
+
+/* TODO Use CHECK_ILLEGAL(_VOID) everywhere and get rid of the uncounting callback */
+/* CHECK that expr_or_stmt calls the illegal callback of ctx exactly once
+ *
+ * For checking functions that use ARG_CHECK_VOID */
+#define CHECK_ILLEGAL_VOID(ctx, expr_or_stmt) do { \
+ int32_t _calls_to_illegal_callback = 0; \
+ secp256k1_callback _saved_illegal_cb = ctx->illegal_callback; \
+ secp256k1_context_set_illegal_callback(ctx, \
+ counting_illegal_callback_fn, &_calls_to_illegal_callback); \
+ { expr_or_stmt; } \
+ ctx->illegal_callback = _saved_illegal_cb; \
+ CHECK(_calls_to_illegal_callback == 1); \
+} while(0);
+
+/* CHECK that expr calls the illegal callback of ctx exactly once and that expr == 0
+ *
+ * For checking functions that use ARG_CHECK */
+#define CHECK_ILLEGAL(ctx, expr) CHECK_ILLEGAL_VOID(ctx, CHECK((expr) == 0))
+
 static void counting_illegal_callback_fn(const char* str, void* data) {
  /* Dummy callback function that just counts. */
  int32_t *p;
  (void)str;
  p = data;
+ CHECK(*p != INT32_MAX);
  (*p)++;
 }
 
@@ -45,6 +77,7 @@ static void uncounting_illegal_callback_fn(const char* str, void* data) {
  int32_t *p;
  (void)str;
  p = data;
+ CHECK(*p != INT32_MIN);
  (*p)--;
 }
 
@@ -229,31 +262,61 @@ static void run_ec_illegal_argument_tests(void) {
  secp256k1_context_set_illegal_callback(CTX, NULL, NULL);
 }
 
-static void run_static_context_tests(void) {
- int32_t dummy = 0;
-
+static void run_static_context_tests(int use_prealloc) {
  /* Check that deprecated secp256k1_context_no_precomp is an alias to secp256k1_context_static. */
  CHECK(secp256k1_context_no_precomp == secp256k1_context_static);
 
- /* check if sizes for cloning are consistent */
- CHECK(secp256k1_context_preallocated_clone_size(STATIC_CTX) >= sizeof(secp256k1_context));
+ {
+  unsigned char seed[32] = {0x17};
 
- /* Verify that setting and resetting illegal callback works */
- secp256k1_context_set_illegal_callback(STATIC_CTX, counting_illegal_callback_fn, &dummy);
- CHECK(STATIC_CTX->illegal_callback.fn == counting_illegal_callback_fn);
- secp256k1_context_set_illegal_callback(STATIC_CTX, NULL, NULL);
- CHECK(STATIC_CTX->illegal_callback.fn == secp256k1_default_illegal_callback_fn);
+ /* Randomizing secp256k1_context_static is not supported. */
+ CHECK_ILLEGAL(STATIC_CTX, secp256k1_context_randomize(STATIC_CTX, seed));
+ CHECK_ILLEGAL(STATIC_CTX, secp256k1_context_randomize(STATIC_CTX, NULL));
+
+ /* Destroying or cloning secp256k1_context_static is not supported. */
+ if (use_prealloc) {
+ CHECK_ILLEGAL(STATIC_CTX, secp256k1_context_preallocated_clone_size(STATIC_CTX));
+ {
+ secp256k1_context *my_static_ctx = malloc(sizeof(*STATIC_CTX));
+ CHECK(my_static_ctx != NULL);
+ memset(my_static_ctx, 0x2a, sizeof(*my_static_ctx));
+ CHECK_ILLEGAL(STATIC_CTX, secp256k1_context_preallocated_clone(STATIC_CTX, my_static_ctx));
+ CHECK(all_bytes_equal(my_static_ctx, 0x2a, sizeof(*my_static_ctx)));
+ free(my_static_ctx);
+ }
+ CHECK_ILLEGAL_VOID(STATIC_CTX, secp256k1_context_preallocated_destroy(STATIC_CTX));
+ } else {
+ CHECK_ILLEGAL(STATIC_CTX, secp256k1_context_clone(STATIC_CTX));
+ CHECK_ILLEGAL_VOID(STATIC_CTX, secp256k1_context_destroy(STATIC_CTX));
+ }
+ }
+
+ {
+ /* Verify that setting and resetting illegal callback works */
+ int32_t dummy = 0;
+ secp256k1_context_set_illegal_callback(STATIC_CTX, counting_illegal_callback_fn, &dummy);
+ CHECK(STATIC_CTX->illegal_callback.fn == counting_illegal_callback_fn);
+ CHECK(STATIC_CTX->illegal_callback.data == &dummy);
+ secp256k1_context_set_illegal_callback(STATIC_CTX, NULL, NULL);
+ CHECK(STATIC_CTX->illegal_callback.fn == secp256k1_default_illegal_callback_fn);
+ CHECK(STATIC_CTX->illegal_callback.data == NULL);
+ }
 }
 
 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);
@@ -262,6 +325,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);
@@ -276,16 +346,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);
  }
  }
@@ -296,12 +383,16 @@ 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);
  CHECK(my_ctx->illegal_callback.fn == counting_illegal_callback_fn);
+ CHECK(my_ctx->illegal_callback.data == &dummy);
  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);
@@ -327,6 +418,8 @@ 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);
  secp256k1_context_preallocated_destroy(NULL);
@@ -7389,9 +7482,8 @@ int main(int argc, char **argv) {
  run_selftest_tests();
 
  /* context tests */
- run_proper_context_tests(0);
- run_proper_context_tests(1);
- run_static_context_tests();
+ run_proper_context_tests(0); run_proper_context_tests(1);
+ run_static_context_tests(0); run_static_context_tests(1);
  run_deprecated_context_flags_test();
 
  /* scratch tests */