diff --git a/include/secp256k1_musig.h b/include/secp256k1_musig.h index 8d82fb18..942e325c 100644 --- a/include/secp256k1_musig.h +++ b/include/secp256k1_musig.h @@ -22,7 +22,7 @@ extern "C" { /** Data structure containing auxiliary data generated in `pubkey_combine` and * required for `session_*_initialize`. * Fields: - * magic: Set during initialization in `pubkey_combine` in order to allow + * magic: Set during initialization in `pubkey_combine` to allow * detecting an uninitialized object. * pk_hash: The 32-byte hash of the original public keys * is_negated: Whether the MuSig-aggregated point was negated when @@ -45,6 +45,8 @@ typedef struct { * structure. * * Fields: + * magic: Set in `musig_session_initialize` to allow detecting an + * uninitialized object. * round: Current round of the session * pre_session: Auxiliary data created in `pubkey_combine` * combined_pk: MuSig-computed combined xonly public key @@ -64,6 +66,7 @@ typedef struct { * coordinate is even. */ typedef struct { + uint64_t magic; int round; secp256k1_musig_pre_session pre_session; secp256k1_xonly_pubkey combined_pk; diff --git a/src/modules/musig/main_impl.h b/src/modules/musig/main_impl.h index fd180569..8f9ae305 100644 --- a/src/modules/musig/main_impl.h +++ b/src/modules/musig/main_impl.h @@ -87,7 +87,6 @@ static int secp256k1_musig_pubkey_combine_callback(secp256k1_scalar *sc, secp256 return secp256k1_xonly_pubkey_load(ctx->ctx, pt, &ctx->pks[idx]); } - static void secp256k1_musig_signers_init(secp256k1_musig_session_signer_data *signers, uint32_t n_signers) { uint32_t i; for (i = 0; i < n_signers; i++) { @@ -132,6 +131,8 @@ int secp256k1_musig_pubkey_combine(const secp256k1_context* ctx, secp256k1_scrat return 1; } +static const uint64_t session_magic = 0xd92e6fc1ee41b4cbUL; + int secp256k1_musig_session_initialize(const secp256k1_context* ctx, secp256k1_musig_session *session, secp256k1_musig_session_signer_data *signers, unsigned char *nonce_commitment32, const unsigned char *session_id32, const unsigned char *msg32, const secp256k1_xonly_pubkey *combined_pk, const secp256k1_musig_pre_session *pre_session, size_t n_signers, size_t my_index, const unsigned char *seckey) { unsigned char combined_ser[32]; int overflow; @@ -156,6 +157,7 @@ int secp256k1_musig_session_initialize(const secp256k1_context* ctx, secp256k1_m memset(session, 0, sizeof(*session)); + session->magic = session_magic; if (msg32 != NULL) { memcpy(session->msg, msg32, 32); session->msg_is_set = 1; @@ -244,6 +246,7 @@ int secp256k1_musig_session_get_public_nonce(const secp256k1_context* ctx, secp2 ARG_CHECK(signers != NULL); ARG_CHECK(nonce != NULL); ARG_CHECK(commitments != NULL); + ARG_CHECK(session->magic == session_magic); ARG_CHECK(session->round == 0); /* If the message was not set during initialization it must be set now. */ ARG_CHECK(!(!session->msg_is_set && msg32 == NULL)); @@ -296,6 +299,7 @@ int secp256k1_musig_session_initialize_verifier(const secp256k1_context* ctx, se memset(session, 0, sizeof(*session)); + session->magic = session_magic; memcpy(&session->combined_pk, combined_pk, sizeof(*combined_pk)); session->pre_session = *pre_session; if (n_signers == 0) { @@ -349,6 +353,7 @@ int secp256k1_musig_session_combine_nonces(const secp256k1_context* ctx, secp256 VERIFY_CHECK(ctx != NULL); ARG_CHECK(session != NULL); ARG_CHECK(signers != NULL); + ARG_CHECK(session->magic == session_magic); ARG_CHECK(session->round == 1); if (n_signers != session->n_signers) { @@ -441,6 +446,7 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, const secp256k1_m VERIFY_CHECK(ctx != NULL); ARG_CHECK(partial_sig != NULL); ARG_CHECK(session != NULL); + ARG_CHECK(session->magic == session_magic); ARG_CHECK(session->round == 2); if (!session->has_secret_data) { @@ -489,6 +495,7 @@ int secp256k1_musig_partial_sig_combine(const secp256k1_context* ctx, const secp ARG_CHECK(sig64 != NULL); ARG_CHECK(partial_sigs != NULL); ARG_CHECK(session != NULL); + ARG_CHECK(session->magic == session_magic); ARG_CHECK(session->round == 2); if (n_sigs != session->n_signers) { @@ -532,6 +539,7 @@ int secp256k1_musig_partial_sig_verify(const secp256k1_context* ctx, const secp2 ARG_CHECK(signer != NULL); ARG_CHECK(partial_sig != NULL); ARG_CHECK(pubkey != NULL); + ARG_CHECK(session->magic == session_magic); ARG_CHECK(session->round == 2); if (!signer->present) { diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h index 820b8534..cc15bcec 100644 --- a/src/modules/musig/tests_impl.h +++ b/src/modules/musig/tests_impl.h @@ -75,6 +75,7 @@ void musig_simple_test(secp256k1_scratch_space *scratch) { void musig_api_tests(secp256k1_scratch_space *scratch) { secp256k1_scratch_space *scratch_small; secp256k1_musig_session session[2]; + secp256k1_musig_session session_uninitialized; secp256k1_musig_session verifier_session; secp256k1_musig_session_signer_data signer0[2]; secp256k1_musig_session_signer_data signer1[2]; @@ -117,10 +118,11 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { secp256k1_context_set_illegal_callback(vrfy, counting_illegal_callback_fn, &ecount); memset(ones, 0xff, 32); - /* Simulate pre_session being uninitialized by setting it to 0s. Actually providing - * an unitialized pre_session object to a initialize_*_session would be undefined - * behavior */ + /* Simulate structs being uninitialized by setting it to 0s. We don't want + * to produce undefined behavior by actually providing uninitialized + * structs. */ memset(&pre_session_uninitialized, 0, sizeof(pre_session_uninitialized)); + memset(&session_uninitialized, 0, sizeof(session_uninitialized)); secp256k1_testrand256(session_id[0]); secp256k1_testrand256(session_id[1]); @@ -260,15 +262,18 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { memcpy(&session_0_tmp, &session[0], sizeof(session_0_tmp)); CHECK(secp256k1_musig_session_get_public_nonce(none, NULL, signer0, &public_nonce[0], ncs, 2, NULL) == 0); CHECK(ecount == 1); - CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, NULL, &public_nonce[0], ncs, 2, NULL) == 0); + /* uninitialized session */ + CHECK(secp256k1_musig_session_get_public_nonce(none, &session_uninitialized, signer0, &public_nonce[0], ncs, 2, NULL) == 0); CHECK(ecount == 2); - CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, signer0, NULL, ncs, 2, NULL) == 0); + CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, NULL, &public_nonce[0], ncs, 2, NULL) == 0); CHECK(ecount == 3); - CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, signer0, &public_nonce[0], NULL, 2, NULL) == 0); + CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, signer0, NULL, ncs, 2, NULL) == 0); CHECK(ecount == 4); + CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, signer0, &public_nonce[0], NULL, 2, NULL) == 0); + CHECK(ecount == 5); /* Number of commitments and number of signers are different */ CHECK(secp256k1_musig_session_get_public_nonce(none, &session_0_tmp, signer0, &public_nonce[0], ncs, 1, NULL) == 0); - CHECK(ecount == 4); + CHECK(ecount == 5); CHECK(secp256k1_musig_session_get_public_nonce(none, &session[0], signer0, &public_nonce[0], ncs, 2, NULL) == 1); CHECK(secp256k1_musig_session_get_public_nonce(none, &session[1], signer1, &public_nonce[1], ncs, 2, NULL) == 1); @@ -277,12 +282,12 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(secp256k1_musig_set_nonce(none, &signer0[1], &public_nonce[0]) == 0); CHECK(secp256k1_musig_set_nonce(none, &signer0[1], &public_nonce[1]) == 1); CHECK(secp256k1_musig_set_nonce(none, &signer0[1], &public_nonce[1]) == 1); - CHECK(ecount == 4); + CHECK(ecount == 5); CHECK(secp256k1_musig_set_nonce(none, NULL, &public_nonce[0]) == 0); - CHECK(ecount == 5); - CHECK(secp256k1_musig_set_nonce(none, &signer1[0], NULL) == 0); CHECK(ecount == 6); + CHECK(secp256k1_musig_set_nonce(none, &signer1[0], NULL) == 0); + CHECK(ecount == 7); CHECK(secp256k1_musig_set_nonce(none, &signer1[0], &public_nonce[0]) == 1); CHECK(secp256k1_musig_set_nonce(none, &signer1[1], &public_nonce[1]) == 1); @@ -295,13 +300,16 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { memcpy(&session_0_tmp, &session[0], sizeof(session_0_tmp)); CHECK(secp256k1_musig_session_combine_nonces(none, NULL, signer0, 2, &nonce_is_negated, &adaptor) == 0); CHECK(ecount == 1); - CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, NULL, 2, &nonce_is_negated, &adaptor) == 0); + /* Uninitialized session */ + CHECK(secp256k1_musig_session_combine_nonces(none, &session_uninitialized, signer0, 2, &nonce_is_negated, &adaptor) == 0); CHECK(ecount == 2); + CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, NULL, 2, &nonce_is_negated, &adaptor) == 0); + CHECK(ecount == 3); /* Number of signers differs from number during intialization */ CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, signer0, 1, &nonce_is_negated, &adaptor) == 0); - CHECK(ecount == 2); + CHECK(ecount == 3); CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, signer0, 2, NULL, &adaptor) == 1); - CHECK(ecount == 2); + CHECK(ecount == 3); memcpy(&session_0_tmp, &session[0], sizeof(session_0_tmp)); CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, signer0, 2, &nonce_is_negated, NULL) == 1); @@ -316,14 +324,17 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(ecount == 0); CHECK(secp256k1_musig_partial_sign(none, NULL, &partial_sig[0]) == 0); CHECK(ecount == 1); - CHECK(secp256k1_musig_partial_sign(none, &session[0], NULL) == 0); + /* Uninitialized session */ + CHECK(secp256k1_musig_partial_sign(none, &session_uninitialized, &partial_sig[0]) == 0); CHECK(ecount == 2); + CHECK(secp256k1_musig_partial_sign(none, &session[0], NULL) == 0); + CHECK(ecount == 3); CHECK(secp256k1_musig_partial_sign(none, &session[0], &partial_sig[0]) == 1); CHECK(secp256k1_musig_partial_sign(none, &session[1], &partial_sig[1]) == 1); /* observer can't sign */ CHECK(secp256k1_musig_partial_sign(none, &verifier_session, &partial_sig[2]) == 0); - CHECK(ecount == 2); + CHECK(ecount == 3); ecount = 0; CHECK(secp256k1_musig_partial_signature_serialize(none, buf, &partial_sig[0]) == 1); @@ -350,14 +361,17 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(ecount == 2); CHECK(secp256k1_musig_partial_sig_verify(vrfy, NULL, &signer0[0], &partial_sig[0], &pk[0]) == 0); CHECK(ecount == 3); - CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], NULL, &partial_sig[0], &pk[0]) == 0); + /* Unitialized session */ + CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session_uninitialized, &signer0[0], &partial_sig[0], &pk[0]) == 0); CHECK(ecount == 4); + CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], NULL, &partial_sig[0], &pk[0]) == 0); + CHECK(ecount == 5); CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], NULL, &pk[0]) == 0); - CHECK(ecount == 5); - CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], &partial_sig_overflow, &pk[0]) == 0); - CHECK(ecount == 5); - CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], &partial_sig[0], NULL) == 0); CHECK(ecount == 6); + CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], &partial_sig_overflow, &pk[0]) == 0); + CHECK(ecount == 6); + CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], &partial_sig[0], NULL) == 0); + CHECK(ecount == 7); CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[0], &signer0[0], &partial_sig[0], &pk[0]) == 1); CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[1], &signer1[0], &partial_sig[0], &pk[0]) == 1); @@ -365,7 +379,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(secp256k1_musig_partial_sig_verify(vrfy, &session[1], &signer1[1], &partial_sig[1], &pk[1]) == 1); CHECK(secp256k1_musig_partial_sig_verify(vrfy, &verifier_session, &verifier_signer_data[0], &partial_sig[0], &pk[0]) == 1); CHECK(secp256k1_musig_partial_sig_verify(vrfy, &verifier_session, &verifier_signer_data[1], &partial_sig[1], &pk[1]) == 1); - CHECK(ecount == 6); + CHECK(ecount == 7); /** Adaptor signature verification */ memcpy(&partial_sig_adapted[1], &partial_sig[1], sizeof(partial_sig_adapted[1])); @@ -392,22 +406,25 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(secp256k1_musig_partial_sig_combine(none, NULL, final_sig, partial_sig_adapted, 2) == 0); CHECK(ecount == 1); - CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], NULL, partial_sig_adapted, 2) == 0); + /* Unitialized session */ + CHECK(secp256k1_musig_partial_sig_combine(none, &session_uninitialized, final_sig, partial_sig_adapted, 2) == 0); CHECK(ecount == 2); - CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], final_sig, NULL, 2) == 0); + CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], NULL, partial_sig_adapted, 2) == 0); CHECK(ecount == 3); + CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], final_sig, NULL, 2) == 0); + CHECK(ecount == 4); { secp256k1_musig_partial_signature partial_sig_tmp[2]; partial_sig_tmp[0] = partial_sig_adapted[0]; partial_sig_tmp[1] = partial_sig_overflow; CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], final_sig, partial_sig_tmp, 2) == 0); } - CHECK(ecount == 3); + CHECK(ecount == 4); /* Wrong number of partial sigs */ CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], final_sig, partial_sig_adapted, 1) == 0); - CHECK(ecount == 3); + CHECK(ecount == 4); CHECK(secp256k1_musig_partial_sig_combine(none, &session[0], final_sig, partial_sig_adapted, 2) == 1); - CHECK(ecount == 3); + CHECK(ecount == 4); CHECK(secp256k1_schnorrsig_verify(vrfy, final_sig, msg, &combined_pk) == 1);