From 4ff6e4274d49cb95ab246b599b274104baf83f9f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 26 Jul 2022 17:09:36 +0000 Subject: [PATCH 1/3] surjectionproof: add test for existing behavior on input=output proofs --- src/modules/surjection/tests_impl.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/modules/surjection/tests_impl.h b/src/modules/surjection/tests_impl.h index 89ba4e1a..70955760 100644 --- a/src/modules/surjection/tests_impl.h +++ b/src/modules/surjection/tests_impl.h @@ -524,6 +524,33 @@ void test_bad_parse(void) { CHECK(secp256k1_surjectionproof_parse(ctx, &proof, serialized_proof2, sizeof(serialized_proof2)) == 0); } +void test_input_eq_output(void) { + secp256k1_surjectionproof proof; + secp256k1_fixed_asset_tag fixed_tag; + secp256k1_generator ephemeral_tag; + unsigned char blinding_key[32]; + unsigned char entropy[32]; + size_t input_index; + + secp256k1_testrand256(fixed_tag.data); + secp256k1_testrand256(blinding_key); + secp256k1_testrand256(entropy); + + CHECK(secp256k1_surjectionproof_initialize(ctx, &proof, &input_index, &fixed_tag, 1, 1, &fixed_tag, 100, entropy) == 1); + CHECK(input_index == 0); + + /* Generation should fail */ + CHECK(secp256k1_generator_generate_blinded(ctx, &ephemeral_tag, fixed_tag.data, blinding_key)); + CHECK(!secp256k1_surjectionproof_generate(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag, input_index, blinding_key, blinding_key)); + + /* It succeeds when the blinding factor is 0... (will fix this in the next commit) */ + memset(blinding_key, 0, 32); + CHECK(secp256k1_generator_generate_blinded(ctx, &ephemeral_tag, fixed_tag.data, blinding_key)); + CHECK(secp256k1_surjectionproof_generate(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag, input_index, blinding_key, blinding_key)); + /* ...but verification doesn't */ + CHECK(!secp256k1_surjectionproof_verify(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag)); +} + void test_fixed_vectors(void) { const unsigned char tag0_ser[] = { 0x0a, @@ -672,6 +699,7 @@ void test_fixed_vectors(void) { void run_surjection_tests(void) { test_surjectionproof_api(); + test_input_eq_output(); test_fixed_vectors(); test_input_selection(0); From bf18ff5a8c6295cb7db6e2989aefd6a78df7720f Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Tue, 26 Jul 2022 17:14:49 +0000 Subject: [PATCH 2/3] surjectionproof: fix generation to fail when any input == the output Verification will fail in this case, so don't "succeed" in generating a bad proof. --- src/modules/surjection/main_impl.h | 12 ++++++++---- src/modules/surjection/tests_impl.h | 6 ++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/modules/surjection/main_impl.h b/src/modules/surjection/main_impl.h index b8f145e5..343b86ae 100644 --- a/src/modules/surjection/main_impl.h +++ b/src/modules/surjection/main_impl.h @@ -307,10 +307,14 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s if (overflow) { return 0; } - /* The only time the input may equal the output is if neither one was blinded in the first place, - * i.e. both blinding keys are zero. Otherwise this is a privacy leak. */ - if (secp256k1_scalar_eq(&tmps, &blinding_key) && !secp256k1_scalar_is_zero(&blinding_key)) { - return 0; + /* If any input tag is equal to an output tag, verification will fail, because our ring + * signature logic would receive a zero-key, which is illegal. This is unfortunate but + * it is deployed on Liquid and cannot be fixed without a hardfork. We should review + * this at the same time that we relax the max-256-inputs rule. */ + for (i = 0; i < n_ephemeral_input_tags; i++) { + if (memcmp(ephemeral_input_tags[i].data, ephemeral_output_tag->data, sizeof(ephemeral_output_tag->data)) == 0) { + return 0; + } } secp256k1_scalar_negate(&tmps, &tmps); secp256k1_scalar_add(&blinding_key, &blinding_key, &tmps); diff --git a/src/modules/surjection/tests_impl.h b/src/modules/surjection/tests_impl.h index 70955760..ac7c269b 100644 --- a/src/modules/surjection/tests_impl.h +++ b/src/modules/surjection/tests_impl.h @@ -543,12 +543,10 @@ void test_input_eq_output(void) { CHECK(secp256k1_generator_generate_blinded(ctx, &ephemeral_tag, fixed_tag.data, blinding_key)); CHECK(!secp256k1_surjectionproof_generate(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag, input_index, blinding_key, blinding_key)); - /* It succeeds when the blinding factor is 0... (will fix this in the next commit) */ + /* ...even when the blinding key is zero */ memset(blinding_key, 0, 32); CHECK(secp256k1_generator_generate_blinded(ctx, &ephemeral_tag, fixed_tag.data, blinding_key)); - CHECK(secp256k1_surjectionproof_generate(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag, input_index, blinding_key, blinding_key)); - /* ...but verification doesn't */ - CHECK(!secp256k1_surjectionproof_verify(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag)); + CHECK(!secp256k1_surjectionproof_generate(ctx, &proof, &ephemeral_tag, 1, &ephemeral_tag, input_index, blinding_key, blinding_key)); } void test_fixed_vectors(void) { From d1175d265d514bd0c22faaf262d7df362f33af89 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Fri, 29 Jul 2022 21:04:04 +0000 Subject: [PATCH 3/3] surjectionproof: use secp256k1_memcmp_var rather than bare memcmp Co-authored-by: Tim Ruffing --- src/modules/surjection/main_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/surjection/main_impl.h b/src/modules/surjection/main_impl.h index 343b86ae..d74f2a48 100644 --- a/src/modules/surjection/main_impl.h +++ b/src/modules/surjection/main_impl.h @@ -312,7 +312,7 @@ int secp256k1_surjectionproof_generate(const secp256k1_context* ctx, secp256k1_s * it is deployed on Liquid and cannot be fixed without a hardfork. We should review * this at the same time that we relax the max-256-inputs rule. */ for (i = 0; i < n_ephemeral_input_tags; i++) { - if (memcmp(ephemeral_input_tags[i].data, ephemeral_output_tag->data, sizeof(ephemeral_output_tag->data)) == 0) { + if (secp256k1_memcmp_var(ephemeral_input_tags[i].data, ephemeral_output_tag->data, sizeof(ephemeral_output_tag->data)) == 0) { return 0; } }