Skip to content

Commit

Permalink
Merge #196: surjectionproof: fail to generate proofs when an input eq…
Browse files Browse the repository at this point in the history
…uals the output

d1175d2 surjectionproof: use secp256k1_memcmp_var rather than bare memcmp (Andrew Poelstra)
bf18ff5 surjectionproof: fix generation to fail when any input == the output (Andrew Poelstra)
4ff6e42 surjectionproof: add test for existing behavior on input=output proofs (Andrew Poelstra)

Pull request description:

  If any ephemeral input tag equals the ephemeral output tag (i.e. an input asset is exactly equal to the output asset), verification will fail due to an unexpected interaction between our surjectionproof logic and the underlying borromean ring siganture logic. However, our generation code still allows creating proofs like this, "succeeding" in creating bad proofs.

  Since we cannot fix the verification side without hardforking Liquid, fix the generation side to fail in this situation.

ACKs for top commit:
  real-or-random:
    utACK d1175d2

Tree-SHA512: c15e130de028d6c1f705543fe2774ec23016c71f9d6b38ef0708820a517d156e2126f8369e94f16f9fd1855c29cd907d406f6ea26c95499a9ae1ce0dd92f77b2
  • Loading branch information
real-or-random committed Aug 1, 2022
2 parents 71a206f + d1175d2 commit d1d6e47
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
12 changes: 8 additions & 4 deletions src/modules/surjection/main_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 (secp256k1_memcmp_var(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);
Expand Down
26 changes: 26 additions & 0 deletions src/modules/surjection/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,31 @@ 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));

/* ...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));
}

void test_fixed_vectors(void) {
const unsigned char tag0_ser[] = {
0x0a,
Expand Down Expand Up @@ -672,6 +697,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);
Expand Down

0 comments on commit d1d6e47

Please sign in to comment.