From b19806122e9065c6f434fc6160cd0c57fa3fea8c Mon Sep 17 00:00:00 2001 From: Tim Ruffing Date: Wed, 7 Dec 2022 14:56:43 +0100 Subject: [PATCH] tests: Use global copy of secp256k1_context_static instead of clone --- src/modules/extrakeys/tests_impl.h | 5 +++-- src/modules/recovery/tests_impl.h | 4 ++-- src/modules/schnorrsig/tests_impl.h | 6 +++--- src/tests.c | 20 +++++++++++--------- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/modules/extrakeys/tests_impl.h b/src/modules/extrakeys/tests_impl.h index 8030aedad..dd535b9ad 100644 --- a/src/modules/extrakeys/tests_impl.h +++ b/src/modules/extrakeys/tests_impl.h @@ -336,7 +336,6 @@ void test_keypair(void) { secp256k1_xonly_pubkey xonly_pk, xonly_pk_tmp; int pk_parity, pk_parity_tmp; int ecount; - secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static); set_counting_callbacks(ctx, &ecount); set_counting_callbacks(sttc, &ecount); @@ -440,7 +439,9 @@ void test_keypair(void) { memset(&keypair, 0, sizeof(keypair)); CHECK(secp256k1_keypair_sec(ctx, sk_tmp, &keypair) == 1); CHECK(secp256k1_memcmp_var(zeros96, sk_tmp, sizeof(sk_tmp)) == 0); - secp256k1_context_destroy(sttc); + + secp256k1_context_set_error_callback(sttc, NULL, NULL); + secp256k1_context_set_illegal_callback(sttc, NULL, NULL); } void test_keypair_add(void) { diff --git a/src/modules/recovery/tests_impl.h b/src/modules/recovery/tests_impl.h index 0ff9294e3..0769b961b 100644 --- a/src/modules/recovery/tests_impl.h +++ b/src/modules/recovery/tests_impl.h @@ -30,7 +30,6 @@ static int recovery_test_nonce_function(unsigned char *nonce32, const unsigned c void test_ecdsa_recovery_api(void) { /* Setup contexts that just count errors */ - secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static); secp256k1_pubkey pubkey; secp256k1_pubkey recpubkey; secp256k1_ecdsa_signature normal_sig; @@ -124,7 +123,8 @@ void test_ecdsa_recovery_api(void) { CHECK(ecount == 7); /* cleanup */ - secp256k1_context_destroy(sttc); + secp256k1_context_set_error_callback(sttc, NULL, NULL); + secp256k1_context_set_illegal_callback(sttc, NULL, NULL); } void test_ecdsa_recovery_end_to_end(void) { diff --git a/src/modules/schnorrsig/tests_impl.h b/src/modules/schnorrsig/tests_impl.h index 06cc097cc..f79d7aa0f 100644 --- a/src/modules/schnorrsig/tests_impl.h +++ b/src/modules/schnorrsig/tests_impl.h @@ -128,8 +128,7 @@ void test_schnorrsig_api(void) { secp256k1_schnorrsig_extraparams invalid_extraparams = {{ 0 }, NULL, NULL}; /** setup **/ - secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static); - int ecount; + int ecount = 0; secp256k1_context_set_error_callback(ctx, counting_illegal_callback_fn, &ecount); secp256k1_context_set_illegal_callback(ctx, counting_illegal_callback_fn, &ecount); @@ -198,7 +197,8 @@ void test_schnorrsig_api(void) { CHECK(secp256k1_schnorrsig_verify(ctx, sig, msg, sizeof(msg), &zero_pk) == 0); CHECK(ecount == 4); - secp256k1_context_destroy(sttc); + secp256k1_context_set_error_callback(sttc, NULL, NULL); + secp256k1_context_set_illegal_callback(sttc, NULL, NULL); } /* Checks that hash initialized by secp256k1_schnorrsig_sha256_tagged has the diff --git a/src/tests.c b/src/tests.c index 7aa15c946..0a09c0d96 100644 --- a/src/tests.c +++ b/src/tests.c @@ -29,6 +29,7 @@ static int count = 64; static secp256k1_context *ctx = NULL; +static secp256k1_context *sttc = NULL; static void counting_illegal_callback_fn(const char* str, void* data) { /* Dummy callback function that just counts. */ @@ -180,9 +181,7 @@ void run_context_tests(int use_prealloc) { unsigned char ctmp[32]; int32_t ecount; int32_t ecount2; - secp256k1_context *sttc; void *ctx_prealloc = NULL; - void *sttc_prealloc = NULL; secp256k1_gej pubj; secp256k1_ge pub; @@ -196,11 +195,7 @@ void run_context_tests(int use_prealloc) { ctx_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE)); CHECK(ctx_prealloc != NULL); ctx = secp256k1_context_preallocated_create(ctx_prealloc, SECP256K1_CONTEXT_NONE); - sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_static)); - CHECK(sttc_prealloc != NULL); - sttc = secp256k1_context_preallocated_clone(secp256k1_context_static, sttc_prealloc); } else { - sttc = secp256k1_context_clone(secp256k1_context_static); ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE); } @@ -312,12 +307,9 @@ void run_context_tests(int use_prealloc) { /* cleanup */ if (use_prealloc) { secp256k1_context_preallocated_destroy(ctx); - secp256k1_context_preallocated_destroy(sttc); free(ctx_prealloc); - free(sttc_prealloc); } else { secp256k1_context_destroy(ctx); - secp256k1_context_destroy(sttc); } /* Defined as no-op. */ secp256k1_context_destroy(NULL); @@ -7357,6 +7349,15 @@ int main(int argc, char **argv) { secp256k1_testrand_init(argc > 2 ? argv[2] : NULL); /* initialize */ + /* Make a writable copy of secp256k1_context_static in order to test the effect of API functions + that write to the context. The API does not support cloning the static context, so we use + memcpy instead. The user is not supposed to copy a context but we should still ensure that + the API functions handle copies of the static context gracefully. */ + sttc = malloc(sizeof(*secp256k1_context_static)); + CHECK(sttc != NULL); + memcpy(sttc, secp256k1_context_static, sizeof(secp256k1_context)); + CHECK(!secp256k1_context_is_proper(sttc)); + run_selftest_tests(); run_context_tests(0); run_context_tests(1); @@ -7463,6 +7464,7 @@ int main(int argc, char **argv) { secp256k1_testrand_finish(); /* shutdown */ + free(sttc); secp256k1_context_destroy(ctx); printf("no problems found\n");