From b19806122e9065c6f434fc6160cd0c57fa3fea8c Mon Sep 17 00:00:00 2001
From: Tim Ruffing <crypto@timruffing.de>
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 8030aedad6391..dd535b9ad2dbd 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 0ff9294e38ad6..0769b961ba9c5 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 06cc097cc1b47..f79d7aa0fad10 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 7aa15c9467438..0a09c0d9697f4 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");