musig: add ARG_CHECKs to functions to help debuggability

This commit is contained in:
Jonas Nick 2019-11-22 13:58:40 +00:00
parent ac2d0e6697
commit ebc31f1f9d
3 changed files with 44 additions and 59 deletions

View File

@ -142,7 +142,7 @@ typedef struct {
* In: pubkeys: input array of public keys to combine. The order is important;
* a different order will result in a different combined public
* key (cannot be NULL)
* n_pubkeys: length of pubkeys array
* n_pubkeys: length of pubkeys array. Must be greater than 0.
*/
SECP256K1_API int secp256k1_musig_pubkey_combine(
const secp256k1_context* ctx,
@ -176,7 +176,8 @@ SECP256K1_API int secp256k1_musig_pubkey_combine(
* `musig_pubkey_combine` (cannot be NULL)
* n_signers: length of signers array. Number of signers participating in
* the MuSig. Must be greater than 0 and at most 2^32 - 1.
* my_index: index of this signer in the signers array
* my_index: index of this signer in the signers array. Must be less
* than `n_signers`.
* seckey: the signer's 32-byte secret key (cannot be NULL)
*/
SECP256K1_API int secp256k1_musig_session_initialize(
@ -193,7 +194,10 @@ SECP256K1_API int secp256k1_musig_session_initialize(
const unsigned char *seckey
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(7) SECP256K1_ARG_NONNULL(8) SECP256K1_ARG_NONNULL(11);
/** Gets the signer's public nonce given a list of all signers' data with commitments
/** Gets the signer's public nonce given a list of all signers' data with
* commitments. Called by participating signers after
* `secp256k1_musig_session_initialize` and after all nonce commitments have
* been collected
*
* Returns: 1: public nonce is written in nonce
* 0: signer data is missing commitments or session isn't initialized
@ -204,7 +208,7 @@ SECP256K1_API int secp256k1_musig_session_initialize(
* `musig_session_initialize`. Array length must equal to
* `n_commitments` (cannot be NULL)
* Out: nonce: the nonce (cannot be NULL)
* In: commitments: array of 32-byte nonce commitments (cannot be NULL)
* In: commitments: array of pointers to 32-byte nonce commitments (cannot be NULL)
* n_commitments: the length of commitments and signers array. Must be the total
* number of signers participating in the MuSig.
* msg32: the 32-byte message to be signed. Must be NULL if already
@ -234,8 +238,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_session_get_publi
* pre_session: pointer to a musig_pre_session struct from
* `musig_pubkey_combine` (cannot be NULL)
* pk_hash32: the 32-byte hash of the signers' individual keys (cannot be NULL)
* commitments: array of 32-byte nonce commitments. Array length must equal to
* `n_signers` (cannot be NULL)
* commitments: array of pointers to 32-byte nonce commitments. Array
* length must equal to `n_signers` (cannot be NULL)
* n_signers: length of signers and commitments array. Number of signers
* participating in the MuSig. Must be greater than 0 and at most
* 2^32 - 1.
@ -369,7 +373,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_partial_sig_verif
*
* Returns: 1: all partial signatures have values in range. Does NOT mean the
* resulting signature verifies.
* 0: some partial signature had s/r out of range
* 0: some partial signature are missing or had s or r out of range
* Args: ctx: pointer to a context object (cannot be NULL)
* session: initialized session for which the combined nonce has been
* computed (cannot be NULL)

View File

@ -155,6 +155,10 @@ int secp256k1_musig_session_initialize(const secp256k1_context* ctx, secp256k1_m
ARG_CHECK(pre_session->magic == pre_session_magic);
ARG_CHECK(seckey != NULL);
ARG_CHECK(n_signers > 0);
ARG_CHECK(n_signers <= UINT32_MAX);
ARG_CHECK(my_index < n_signers);
memset(session, 0, sizeof(*session));
session->magic = session_magic;
@ -167,12 +171,6 @@ int secp256k1_musig_session_initialize(const secp256k1_context* ctx, secp256k1_m
memcpy(&session->combined_pk, combined_pk, sizeof(*combined_pk));
session->pre_session = *pre_session;
session->has_secret_data = 1;
if (n_signers == 0 || my_index >= n_signers) {
return 0;
}
if (n_signers > UINT32_MAX) {
return 0;
}
session->n_signers = (uint32_t) n_signers;
secp256k1_musig_signers_init(signers, session->n_signers);
@ -243,19 +241,18 @@ int secp256k1_musig_session_get_public_nonce(const secp256k1_context* ctx, secp2
VERIFY_CHECK(ctx != NULL);
ARG_CHECK(session != NULL);
ARG_CHECK(session->magic == session_magic);
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));
/* The message can only be set once. */
ARG_CHECK(!(session->msg_is_set && msg32 != NULL));
if (!session->has_secret_data || n_commitments != session->n_signers) {
return 0;
}
ARG_CHECK(session->has_secret_data);
ARG_CHECK(n_commitments == session->n_signers);
for (i = 0; i < n_commitments; i++) {
ARG_CHECK(commitments[i] != NULL);
}
@ -289,9 +286,8 @@ int secp256k1_musig_session_initialize_verifier(const secp256k1_context* ctx, se
ARG_CHECK(commitments != NULL);
/* Check n_signers before checking commitments to allow testing the case where
* n_signers is big without allocating the space. */
if (n_signers > UINT32_MAX) {
return 0;
}
ARG_CHECK(n_signers > 0);
ARG_CHECK(n_signers <= UINT32_MAX);
for (i = 0; i < n_signers; i++) {
ARG_CHECK(commitments[i] != NULL);
}
@ -302,9 +298,6 @@ int secp256k1_musig_session_initialize_verifier(const secp256k1_context* ctx, se
session->magic = session_magic;
memcpy(&session->combined_pk, combined_pk, sizeof(*combined_pk));
session->pre_session = *pre_session;
if (n_signers == 0) {
return 0;
}
session->n_signers = (uint32_t) n_signers;
secp256k1_musig_signers_init(signers, session->n_signers);
@ -355,10 +348,8 @@ int secp256k1_musig_session_combine_nonces(const secp256k1_context* ctx, secp256
ARG_CHECK(signers != NULL);
ARG_CHECK(session->magic == session_magic);
ARG_CHECK(session->round == 1);
ARG_CHECK(n_signers == session->n_signers);
if (n_signers != session->n_signers) {
return 0;
}
secp256k1_sha256_initialize(&sha);
secp256k1_gej_set_infinity(&combined_noncej);
for (i = 0; i < n_signers; i++) {
@ -418,7 +409,7 @@ int secp256k1_musig_partial_signature_parse(const secp256k1_context* ctx, secp25
}
/* Compute msghash = SHA256(combined_nonce, combined_pk, msg) */
static int secp256k1_musig_compute_messagehash(const secp256k1_context *ctx, unsigned char *msghash, const secp256k1_musig_session *session) {
static void secp256k1_musig_compute_messagehash(const secp256k1_context *ctx, unsigned char *msghash, const secp256k1_musig_session *session) {
unsigned char buf[32];
secp256k1_ge rp;
secp256k1_sha256 sha;
@ -434,7 +425,6 @@ static int secp256k1_musig_compute_messagehash(const secp256k1_context *ctx, uns
secp256k1_sha256_write(&sha, buf, 32);
secp256k1_sha256_write(&sha, session->msg, 32);
secp256k1_sha256_finalize(&sha, msghash);
return 1;
}
int secp256k1_musig_partial_sign(const secp256k1_context* ctx, const secp256k1_musig_session *session, secp256k1_musig_partial_signature *partial_sig) {
@ -448,15 +438,10 @@ int secp256k1_musig_partial_sign(const secp256k1_context* ctx, const secp256k1_m
ARG_CHECK(session != NULL);
ARG_CHECK(session->magic == session_magic);
ARG_CHECK(session->round == 2);
if (!session->has_secret_data) {
return 0;
}
ARG_CHECK(session->has_secret_data);
/* build message hash */
if (!secp256k1_musig_compute_messagehash(ctx, msghash, session)) {
return 0;
}
secp256k1_musig_compute_messagehash(ctx, msghash, session);
secp256k1_scalar_set_b32(&e, msghash, NULL);
secp256k1_scalar_set_b32(&sk, session->seckey, &overflow);
@ -541,17 +526,13 @@ int secp256k1_musig_partial_sig_verify(const secp256k1_context* ctx, const secp2
ARG_CHECK(pubkey != NULL);
ARG_CHECK(session->magic == session_magic);
ARG_CHECK(session->round == 2);
ARG_CHECK(signer->present);
if (!signer->present) {
return 0;
}
secp256k1_scalar_set_b32(&s, partial_sig->data, &overflow);
if (overflow) {
return 0;
}
if (!secp256k1_musig_compute_messagehash(ctx, msghash, session)) {
return 0;
}
secp256k1_musig_compute_messagehash(ctx, msghash, session);
secp256k1_scalar_set_b32(&e, msghash, NULL);
/* Multiplying the messagehash by the musig coefficient is equivalent

View File

@ -196,18 +196,18 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session_uninitialized, 2, 0, sk[0]) == 0);
CHECK(ecount == 9);
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session, 0, 0, sk[0]) == 0);
CHECK(ecount == 9);
CHECK(ecount == 10);
/* If more than UINT32_MAX fits in a size_t, test that session_initialize
* rejects n_signers that high. */
if (SIZE_MAX > UINT32_MAX) {
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session, ((size_t) UINT32_MAX) + 2, 0, sk[0]) == 0);
}
CHECK(ecount == 9);
CHECK(ecount == 11);
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session, 2, 0, NULL) == 0);
CHECK(ecount == 10);
CHECK(ecount == 12);
/* secret key overflows */
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session, 2, 0, ones) == 0);
CHECK(ecount == 10);
CHECK(ecount == 12);
CHECK(secp256k1_musig_session_initialize(sign, &session[0], signer0, nonce_commitment[0], session_id[0], msg, &combined_pk, &pre_session, 2, 0, sk[0]) == 1);
CHECK(secp256k1_musig_session_initialize(sign, &session[1], signer1, nonce_commitment[1], session_id[1], msg, &combined_pk, &pre_session, 2, 1, sk[1]) == 1);
@ -228,11 +228,11 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
CHECK(secp256k1_musig_session_initialize_verifier(none, &verifier_session, verifier_signer_data, msg, &combined_pk, &pre_session, NULL, 2) == 0);
CHECK(ecount == 5);
CHECK(secp256k1_musig_session_initialize_verifier(none, &verifier_session, verifier_signer_data, msg, &combined_pk, &pre_session, ncs, 0) == 0);
CHECK(ecount == 5);
CHECK(ecount == 6);
if (SIZE_MAX > UINT32_MAX) {
CHECK(secp256k1_musig_session_initialize_verifier(none, &verifier_session, verifier_signer_data, msg, &combined_pk, &pre_session, ncs, ((size_t) UINT32_MAX) + 2) == 0);
}
CHECK(ecount == 5);
CHECK(ecount == 7);
CHECK(secp256k1_musig_session_initialize_verifier(none, &verifier_session, verifier_signer_data, msg, &combined_pk, &pre_session, ncs, 2) == 1);
/** Signing step 0 -- exchange nonce commitments */
@ -273,7 +273,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
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 == 5);
CHECK(ecount == 6);
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);
@ -282,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 == 5);
CHECK(ecount == 6);
CHECK(secp256k1_musig_set_nonce(none, NULL, &public_nonce[0]) == 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], NULL) == 0);
CHECK(ecount == 8);
CHECK(secp256k1_musig_set_nonce(none, &signer1[0], &public_nonce[0]) == 1);
CHECK(secp256k1_musig_set_nonce(none, &signer1[1], &public_nonce[1]) == 1);
@ -307,9 +307,9 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
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 == 3);
CHECK(ecount == 4);
CHECK(secp256k1_musig_session_combine_nonces(none, &session_0_tmp, signer0, 2, NULL, &adaptor) == 1);
CHECK(ecount == 3);
CHECK(ecount == 4);
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);
@ -334,7 +334,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
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 == 3);
CHECK(ecount == 4);
ecount = 0;
CHECK(secp256k1_musig_partial_signature_serialize(none, buf, &partial_sig[0]) == 1);
@ -469,7 +469,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) {
* ones and return the resulting messagehash. This should not result in a different
* messagehash because the public keys of the signers are only used during session
* initialization. */
int musig_state_machine_diff_signer_msghash_test(unsigned char *msghash, secp256k1_xonly_pubkey *pks, secp256k1_xonly_pubkey *combined_pk, secp256k1_musig_pre_session *pre_session, const unsigned char * const *nonce_commitments, unsigned char *msg, secp256k1_pubkey *nonce_other, unsigned char *sk, unsigned char *session_id) {
void musig_state_machine_diff_signer_msghash_test(unsigned char *msghash, secp256k1_xonly_pubkey *pks, secp256k1_xonly_pubkey *combined_pk, secp256k1_musig_pre_session *pre_session, const unsigned char * const *nonce_commitments, unsigned char *msg, secp256k1_pubkey *nonce_other, unsigned char *sk, unsigned char *session_id) {
secp256k1_musig_session session;
secp256k1_musig_session session_tmp;
unsigned char nonce_commitment[32];
@ -498,7 +498,7 @@ int musig_state_machine_diff_signer_msghash_test(unsigned char *msghash, secp256
CHECK(secp256k1_musig_set_nonce(ctx, &signers[1], &nonce) == 1);
CHECK(secp256k1_musig_session_combine_nonces(ctx, &session, signers, 2, NULL, NULL) == 1);
return secp256k1_musig_compute_messagehash(ctx, msghash, &session);
secp256k1_musig_compute_messagehash(ctx, msghash, &session);
}
/* Creates a new session (with a different session id) and tries to use that session
@ -664,8 +664,8 @@ void musig_state_machine_tests(secp256k1_scratch_space *scratch) {
/* messagehash should be the same as a session whose get_public_nonce was called
* with different signers (i.e. they diff in public keys). This is because the
* public keys of the signers is set in stone when initializing the session. */
CHECK(secp256k1_musig_compute_messagehash(ctx, msghash1, &session[1]) == 1);
CHECK(musig_state_machine_diff_signer_msghash_test(msghash2, pk, &combined_pk, &pre_session, ncs, msg, &nonce[0], sk[1], session_id[1]) == 1);
secp256k1_musig_compute_messagehash(ctx, msghash1, &session[1]);
musig_state_machine_diff_signer_msghash_test(msghash2, pk, &combined_pk, &pre_session, ncs, msg, &nonce[0], sk[1], session_id[1]);
CHECK(memcmp(msghash1, msghash2, 32) == 0);
CHECK(secp256k1_musig_partial_sign(ctx, &session[1], &partial_sig[1]) == 1);