diff --git a/src/common/wallet.h b/src/common/wallet.h index e5058480..eb360c58 100644 --- a/src/common/wallet.h +++ b/src/common/wallet.h @@ -290,7 +290,7 @@ typedef struct policy_node_ext_info_s { DEFINE_REL_PTR(uint16, uint16_t) typedef struct { - int16_t n; // number of key indexes + uint16_t n; // number of key indexes rptr_uint16_t key_indexes; // pointer to an array of exactly n key indexes } musig_aggr_key_info_t; @@ -323,14 +323,14 @@ typedef struct { union { // type == 0 struct { - int16_t key_index; // index of the key (common between V1 and V2) + uint16_t key_index; // index of the key (common between V1 and V2) } k; // type == 1 struct { rptr_musig_aggr_key_info_t musig_info; // only used in V2 } m; }; - int16_t + uint16_t keyexpr_index; // index of the key expression in the descriptor template, in parsing order } policy_node_keyexpr_t; @@ -377,8 +377,8 @@ typedef struct { // 12 bytes typedef struct { struct policy_node_s base; // type is TOKEN_MULTI or TOKEN_SORTEDMULTI - int16_t k; // threshold - int16_t n; // number of keys + uint16_t k; // threshold + uint16_t n; // number of keys rptr_policy_node_keyexpr_t keys; // pointer to array of exactly n key expressions } policy_node_multisig_t; @@ -395,8 +395,8 @@ typedef struct policy_node_scriptlist_s { // 12 bytes, (+ 8 bytes for every script) typedef struct { struct policy_node_s base; // type is TOKEN_THRESH - int16_t k; // threshold - int16_t n; // number of child scripts + uint16_t k; // threshold + uint16_t n; // number of child scripts rptr_policy_node_scriptlist_t scriptlist; // pointer to array of exactly n pointers to child scripts } policy_node_thresh_t; diff --git a/src/handler/lib/policy.c b/src/handler/lib/policy.c index ad348cc9..895b536e 100644 --- a/src/handler/lib/policy.c +++ b/src/handler/lib/policy.c @@ -489,7 +489,9 @@ __attribute__((warn_unused_result)) static int get_derived_pubkey( qsort(keys, musig_info->n, sizeof(plain_pk_t), compare_plain_pk); musig_keyagg_context_t musig_ctx; - musig_key_agg(keys, musig_info->n, &musig_ctx); + if (0 > musig_key_agg(keys, musig_info->n, &musig_ctx)) { + return -1; + } // compute the aggregated extended pubkey memset(&ext_pubkey, 0, sizeof(ext_pubkey)); diff --git a/src/handler/sign_psbt.c b/src/handler/sign_psbt.c index 622cc97c..84b45974 100644 --- a/src/handler/sign_psbt.c +++ b/src/handler/sign_psbt.c @@ -659,7 +659,9 @@ static bool fill_keyexpr_info_if_internal(dispatcher_context_t *dc, qsort(keys, musig_info->n, sizeof(plain_pk_t), compare_plain_pk); musig_keyagg_context_t musig_ctx; - musig_key_agg(keys, musig_info->n, &musig_ctx); + if (0 > musig_key_agg(keys, musig_info->n, &musig_ctx)) { + return false; + } // compute the aggregated extended pubkey memset(&keyexpr_info->pubkey, 0, sizeof(keyexpr_info->pubkey)); diff --git a/src/handler/sign_psbt/musig_signing.c b/src/handler/sign_psbt/musig_signing.c index 61a00cc1..4ae706e3 100644 --- a/src/handler/sign_psbt/musig_signing.c +++ b/src/handler/sign_psbt/musig_signing.c @@ -270,6 +270,7 @@ bool produce_and_yield_pubnonce(dispatcher_context_t *dc, musig_secnonce_t secnonce; musig_pubnonce_t pubnonce; if (0 > musig_nonce_gen(rand_i_j, + sizeof(rand_i_j), keyexpr_info->internal_pubkey.compressed_pubkey, musig_per_input_info.agg_key_tweaked.compressed_pubkey + 1, &secnonce, @@ -401,6 +402,7 @@ bool __attribute__((noinline)) sign_sighash_musig_and_yield(dispatcher_context_t musig_pubnonce_t pubnonce; if (0 > musig_nonce_gen(rand_i_j, + sizeof(rand_i_j), keyexpr_info->internal_pubkey.compressed_pubkey, musig_per_input_info.agg_key_tweaked.compressed_pubkey + 1, &secnonce, diff --git a/src/musig/musig.c b/src/musig/musig.c index 752e54e4..4785d2ef 100644 --- a/src/musig/musig.c +++ b/src/musig/musig.c @@ -73,9 +73,9 @@ static int point_negate(const point_t *P, point_t *out) { set_point_infinite(out); return 0; } - memmove(out->x, P->x, 32); + memmove(out->x, P->x, sizeof(out->x)); - if (CX_OK != cx_math_sub_no_throw(out->y, secp256k1_p, P->y, 32)) return -1; + if (CX_OK != cx_math_sub_no_throw(out->y, secp256k1_p, P->y, sizeof(out->y))) return -1; out->prefix = 4; return 0; @@ -114,9 +114,9 @@ static bool is_array_zero(const uint8_t buffer[], size_t buffer_len) { return acc == 0; } -int cpoint_ext(const uint8_t x[static 33], point_t *out) { +static int cpoint_ext(const uint8_t x[static sizeof(plain_pk_t)], point_t *out) { // Check if the point is at infinity (all bytes zero) - if (is_array_zero(x, 33)) { + if (is_array_zero(x, sizeof(plain_pk_t))) { set_point_infinite(out); return 0; } @@ -135,7 +135,9 @@ static void musig_get_second_key(const plain_pk_t pubkeys[], size_t n_keys, plai memset(out, 0, sizeof(plain_pk_t)); } -static void musig_hash_keys(const plain_pk_t pubkeys[], size_t n_keys, uint8_t out[static 32]) { +static void musig_hash_keys(const plain_pk_t pubkeys[], + size_t n_keys, + uint8_t out[static CX_SHA256_SIZE]) { cx_sha256_t hash_context; crypto_tr_tagged_hash_init(&hash_context, BIP0327_keyagg_list_tag, @@ -143,7 +145,7 @@ static void musig_hash_keys(const plain_pk_t pubkeys[], size_t n_keys, uint8_t o for (size_t i = 0; i < n_keys; i++) { crypto_hash_update(&hash_context.header, pubkeys[i], sizeof(plain_pk_t)); } - crypto_hash_digest(&hash_context.header, out, 32); + crypto_hash_digest(&hash_context.header, out, CX_SHA256_SIZE); } static void musig_key_agg_coeff_internal(const plain_pk_t pubkeys[], @@ -207,6 +209,12 @@ int musig_key_agg(const plain_pk_t pubkeys[], size_t n_keys, musig_keyagg_contex point_add(&ctx->Q, &P, &ctx->Q); } + + if (is_point_infinite(&ctx->Q)) { + PRINTF("Musig key aggregation resulted in an infinite point\n"); + return -1; + } + memset(ctx->tacc, 0, sizeof(ctx->tacc)); memset(ctx->gacc, 0, sizeof(ctx->gacc)); ctx->gacc[31] = 1; @@ -214,6 +222,7 @@ int musig_key_agg(const plain_pk_t pubkeys[], size_t n_keys, musig_keyagg_contex } static void musig_nonce_hash(const uint8_t *rand, + size_t rand_len, const plain_pk_t pk, const xonly_pk_t aggpk, uint8_t i, @@ -226,7 +235,7 @@ static void musig_nonce_hash(const uint8_t *rand, crypto_tr_tagged_hash_init(&hash_context, BIP0327_nonce_tag, sizeof(BIP0327_nonce_tag)); // rand - crypto_hash_update(&hash_context.header, rand, 32); + crypto_hash_update(&hash_context.header, rand, rand_len); // len(pk) + pk crypto_hash_update_u8(&hash_context.header, sizeof(plain_pk_t)); @@ -252,19 +261,26 @@ static void musig_nonce_hash(const uint8_t *rand, // same as nonce_gen_internal from the reference, removing the optional arguments sk, msg and // extra_in, and making aggpk compulsory -int musig_nonce_gen(const uint8_t rand[32], +int musig_nonce_gen(const uint8_t *rand, + size_t rand_len, const plain_pk_t pk, const xonly_pk_t aggpk, musig_secnonce_t *secnonce, musig_pubnonce_t *pubnonce) { uint8_t msg[] = {0x00}; - musig_nonce_hash(rand, pk, aggpk, 0, msg, 1, NULL, 0, secnonce->k_1); + musig_nonce_hash(rand, rand_len, pk, aggpk, 0, msg, 1, NULL, 0, secnonce->k_1); if (CX_OK != cx_math_modm_no_throw(secnonce->k_1, 32, secp256k1_n, 32)) return -1; - musig_nonce_hash(rand, pk, aggpk, 1, msg, 1, NULL, 0, secnonce->k_2); + musig_nonce_hash(rand, rand_len, pk, aggpk, 1, msg, 1, NULL, 0, secnonce->k_2); if (CX_OK != cx_math_modm_no_throw(secnonce->k_2, 32, secp256k1_n, 32)) return -1; - memcpy(secnonce->pk, pk, 33); + if (is_array_zero(secnonce->k_1, sizeof(secnonce->k_1)) || + is_array_zero(secnonce->k_2, sizeof(secnonce->k_2))) { + // this can only happen with negligible probability + return -1; + } + + memcpy(secnonce->pk, pk, sizeof(secnonce->pk)); point_t R_s1, R_s2; @@ -283,7 +299,7 @@ int musig_nonce_agg(const musig_pubnonce_t pubnonces[], size_t n_keys, musig_pub set_point_infinite(&R_j); for (size_t i = 0; i < n_keys; i++) { point_t R_ij; - if (0 > cpoint(&pubnonces[i].raw[(j - 1) * 33], &R_ij)) { + if (0 > cpoint(&pubnonces[i].raw[(j - 1) * sizeof(plain_pk_t)], &R_ij)) { PRINTF("Musig2 nonce aggregation: invalid contribution from cosigner %d\n", i); return -i - 1; } @@ -291,9 +307,9 @@ int musig_nonce_agg(const musig_pubnonce_t pubnonces[], size_t n_keys, musig_pub } if (is_point_infinite(&R_j)) { - memset(&out->raw[(j - 1) * 33], 0, 33); + memset(&out->raw[(j - 1) * sizeof(plain_pk_t)], 0, sizeof(plain_pk_t)); } else { - crypto_get_compressed_pubkey(R_j.raw, &out->raw[(j - 1) * 33]); + crypto_get_compressed_pubkey(R_j.raw, &out->raw[(j - 1) * sizeof(plain_pk_t)]); } } return 0; @@ -362,14 +378,18 @@ static int musig_get_session_values(const musig_session_context_t *session_ctx, point_t *Q, uint8_t gacc[static 32], uint8_t tacc[static 32], - uint8_t b[static 32], + uint8_t b[static CX_SHA256_SIZE], point_t *R, - uint8_t e[static 32]) { + uint8_t e[static CX_SHA256_SIZE]) { cx_sha256_t hash_context; // Perform key aggregation and tweaking musig_keyagg_context_t keyagg_ctx; - musig_key_agg(session_ctx->pubkeys, session_ctx->n_keys, &keyagg_ctx); + + if (0 > musig_key_agg(session_ctx->pubkeys, session_ctx->n_keys, &keyagg_ctx)) { + return -1; + } + for (size_t i = 0; i < session_ctx->n_tweaks; i++) { if (0 > apply_tweak(&keyagg_ctx, session_ctx->tweaks[i], session_ctx->is_xonly[i])) { return -1; @@ -383,10 +403,12 @@ static int musig_get_session_values(const musig_session_context_t *session_ctx, // Calculate b crypto_tr_tagged_hash_init(&hash_context, BIP0327_noncecoef_tag, sizeof(BIP0327_noncecoef_tag)); - crypto_hash_update(&hash_context.header, session_ctx->aggnonce->raw, 66); - crypto_hash_update(&hash_context.header, Q->x, 32); + crypto_hash_update(&hash_context.header, + session_ctx->aggnonce->raw, + sizeof(session_ctx->aggnonce->raw)); + crypto_hash_update(&hash_context.header, Q->x, sizeof(Q->x)); crypto_hash_update(&hash_context.header, session_ctx->msg, session_ctx->msg_len); - crypto_hash_digest(&hash_context.header, b, 32); + crypto_hash_digest(&hash_context.header, b, CX_SHA256_SIZE); // Calculate R point_t R_1, R_2; @@ -411,10 +433,10 @@ static int musig_get_session_values(const musig_session_context_t *session_ctx, // Calculate e crypto_tr_tagged_hash_init(&hash_context, BIP0340_challenge_tag, sizeof(BIP0340_challenge_tag)); - crypto_hash_update(&hash_context.header, R->x, 32); - crypto_hash_update(&hash_context.header, Q->x, 32); + crypto_hash_update(&hash_context.header, R->x, sizeof(R->x)); + crypto_hash_update(&hash_context.header, Q->x, sizeof(Q->x)); crypto_hash_update(&hash_context.header, session_ctx->msg, session_ctx->msg_len); - crypto_hash_digest(&hash_context.header, e, 32); + crypto_hash_digest(&hash_context.header, e, CX_SHA256_SIZE); return 0; } @@ -448,9 +470,9 @@ int musig_sign(musig_secnonce_t *secnonce, point_t Q; uint8_t gacc[32]; uint8_t tacc[32]; - uint8_t b[32]; + uint8_t b[CX_SHA256_SIZE]; point_t R; - uint8_t e[32]; + uint8_t e[CX_SHA256_SIZE]; int diff; @@ -460,8 +482,8 @@ int musig_sign(musig_secnonce_t *secnonce, uint8_t k_1[32]; uint8_t k_2[32]; - memcpy(k_1, secnonce->k_1, 32); - memcpy(k_2, secnonce->k_2, 32); + memcpy(k_1, secnonce->k_1, sizeof(k_1)); + memcpy(k_2, secnonce->k_2, sizeof(k_2)); // paranoia: since reusing nonces is catastrophic, we make sure that they are zeroed out and // work with a local copy instead @@ -522,7 +544,7 @@ int musig_sign(musig_secnonce_t *secnonce, plain_pk_t pk; crypto_get_compressed_pubkey(secrets.P.raw, pk); - if (memcmp(pk, secnonce->pk, 33) != 0) { + if (memcmp(pk, secnonce->pk, sizeof(pk)) != 0) { err = true; PRINTF("Public key does not match nonce_gen argument\n"); break; diff --git a/src/musig/musig.h b/src/musig/musig.h index b914f2c7..9227561f 100644 --- a/src/musig/musig.h +++ b/src/musig/musig.h @@ -14,15 +14,13 @@ typedef uint8_t xonly_pk_t[32]; // An uncompressed pubkey, encoded as 04||x||y, where x and y are 32-byte big-endian coordinates. // If the first byte (prefix) is 0, encodes the point at infinity. -typedef struct { - union { - uint8_t raw[65]; - struct { - uint8_t prefix; // 0 for the point at infinity, otherwise 4. - uint8_t x[32]; - uint8_t y[32]; - }; - }; +typedef union { + uint8_t raw[65]; + struct { + uint8_t prefix; // 0 for the point at infinity, otherwise 4. + uint8_t x[32]; + uint8_t y[32]; + } __attribute__((packed)); } point_t; typedef struct musig_keyagg_context_s { @@ -37,14 +35,12 @@ typedef struct musig_secnonce_s { uint8_t pk[33]; } musig_secnonce_t; -typedef struct musig_pubnonce_s { - union { - struct { - uint8_t R_s1[33]; - uint8_t R_s2[33]; - }; - uint8_t raw[66]; - }; +typedef union { + struct { + uint8_t R_s1[33]; + uint8_t R_s2[33]; + } __attribute__((packed)); + uint8_t raw[66]; } musig_pubnonce_t; typedef struct musig_session_context_s { @@ -83,6 +79,8 @@ int musig_key_agg(const plain_pk_t pubkeys[], size_t n_keys, musig_keyagg_contex * * @param[in] rand * The randomness to use. + * @param[in] rand_len + * The length of the randomness. * @param[in] pk * The 33-byte public key of the signer. * @param[in] aggpk @@ -94,7 +92,8 @@ int musig_key_agg(const plain_pk_t pubkeys[], size_t n_keys, musig_keyagg_contex * * @return 0 on success, a negative number in case of error. */ -int musig_nonce_gen(const uint8_t rand[32], +int musig_nonce_gen(const uint8_t *rand, + size_t rand_len, const plain_pk_t pk, const xonly_pk_t aggpk, musig_secnonce_t *secnonce, diff --git a/src/musig/musig_sessions.c b/src/musig/musig_sessions.c index 60bd10fb..b92f6269 100644 --- a/src/musig/musig_sessions.c +++ b/src/musig/musig_sessions.c @@ -132,12 +132,12 @@ const musig_psbt_session_t *musigsession_round2_initialize( void musigsession_commit(musig_signing_state_t *musig_signing_state) { uint8_t acc = 0; - for (size_t i = 0; i < sizeof(musig_signing_state->_round1); i++) { + for (size_t i = 0; i < sizeof(musig_signing_state->_round1._id); i++) { acc |= musig_signing_state->_round1._id[i]; } // If round 1 was not executed, then there is nothing to store. - // This assumes that musigsession_initialize_signing_state, therefore the field is zeroed out - // if it wasn't used. + // This assumes that musigsession_initialize_signing_state zeroes the id, therefore the field is + // zeroed out if and only if it wasn't used. if (acc != 0) { musigsession_store(musig_signing_state->_round1._id, &musig_signing_state->_round1); }