From c59aa0b1de82d35347dae202d094a6915902bdb6 Mon Sep 17 00:00:00 2001 From: Elliott Jin Date: Thu, 23 Dec 2021 21:39:03 -0800 Subject: [PATCH] Reject weak entropy in nonce_gen when seckey isn't provided --- src/modules/musig/session_impl.h | 10 +++++++--- src/modules/musig/tests_impl.h | 13 +++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/modules/musig/session_impl.h b/src/modules/musig/session_impl.h index 7cdcebe3e..3a0cd9a6f 100644 --- a/src/modules/musig/session_impl.h +++ b/src/modules/musig/session_impl.h @@ -283,10 +283,14 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn ARG_CHECK(session_id32 != NULL); ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); if (seckey == NULL) { - /* Check in constant time that the session_id is not 0 as a - * defense-in-depth measure that may protect against a faulty RNG. */ + /* Check in constant time that if seckey isn't provided, then the + * middle 16 bytes of session_id are not all 0. + * + * This is a defense-in-depth measure that may protect against a + * faulty RNG, as well invalid use of a 64-bit counter (in any + * endian-ness) instead of a secure RNG. */ unsigned char acc = 0; - for (i = 0; i < 32; i++) { + for (i = 8; i < 24; i++) { acc |= session_id32[i]; } ret &= !!acc; diff --git a/src/modules/musig/tests_impl.h b/src/modules/musig/tests_impl.h index 383b4161e..93a494897 100644 --- a/src/modules/musig/tests_impl.h +++ b/src/modules/musig/tests_impl.h @@ -126,6 +126,7 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { unsigned char max64[64]; unsigned char zeros68[68] = { 0 }; unsigned char session_id[2][32]; + unsigned char invalid_session_id[32]; secp256k1_musig_secnonce secnonce[2]; secp256k1_musig_secnonce secnonce_tmp; secp256k1_musig_secnonce invalid_secnonce; @@ -293,6 +294,18 @@ void musig_api_tests(secp256k1_scratch_space *scratch) { CHECK(secp256k1_musig_nonce_gen(sign, &secnonce[0], &pubnonce[0], zeros68, NULL, msg, &keyagg_cache, max64) == 0); CHECK(ecount == 5); CHECK(memcmp_and_randomize(secnonce[0].data, zeros68, sizeof(secnonce[0].data)) == 0); + /* no seckey and session_id looks like a counter (little-endian) */ + memset(invalid_session_id, 0x55, 8); + memset(invalid_session_id + 8, 0, 24); + CHECK(secp256k1_musig_nonce_gen(sign, &secnonce[0], &pubnonce[0], invalid_session_id, NULL, msg, &keyagg_cache, max64) == 0); + CHECK(ecount == 5); + CHECK(memcmp_and_randomize(secnonce[0].data, zeros68, sizeof(secnonce[0].data)) == 0); + /* no seckey and session_id looks like a counter (big-endian) */ + memset(invalid_session_id, 0, 24); + memset(invalid_session_id + 24, 0x55, 8); + CHECK(secp256k1_musig_nonce_gen(sign, &secnonce[0], &pubnonce[0], invalid_session_id, NULL, msg, &keyagg_cache, max64) == 0); + CHECK(ecount == 5); + CHECK(memcmp_and_randomize(secnonce[0].data, zeros68, sizeof(secnonce[0].data)) == 0); /* session_id 0 is fine when a seckey is provided */ CHECK(secp256k1_musig_nonce_gen(sign, &secnonce[0], &pubnonce[0], zeros68, sk[0], msg, &keyagg_cache, max64) == 1); CHECK(secp256k1_musig_nonce_gen(sign, &secnonce[0], &pubnonce[0], session_id[0], NULL, msg, &keyagg_cache, max64) == 1);