fixup! split counter mode from musig_nonce_gen

Author: jonasnick

examples/musig.c

@@ -102,19 +102,19 @@ static int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secr
 
     for (i = 0; i < N_SIGNERS; i++) {
         unsigned char seckey[32];
-        unsigned char session_id[32];
+        unsigned char session_secrand[32];
         /* Create random session ID. It is absolutely necessary that the session ID
          * is unique for every call of secp256k1_musig_nonce_gen. Otherwise
          * it's trivial for an attacker to extract the secret key! */
-        if (!fill_random(session_id, sizeof(session_id))) {
+        if (!fill_random(session_secrand, sizeof(session_secrand))) {
             return 0;
         }
         if (!secp256k1_keypair_sec(ctx, seckey, &signer_secrets[i].keypair)) {
             return 0;
         }
         /* Initialize session and create secret nonce for signing and public
          * nonce to send to the other signers. */
-        if (!secp256k1_musig_nonce_gen(ctx, &signer_secrets[i].secnonce, &signer[i].pubnonce, session_id, seckey, &signer[i].pubkey, msg32, NULL, NULL)) {
+        if (!secp256k1_musig_nonce_gen(ctx, &signer_secrets[i].secnonce, &signer[i].pubnonce, session_secrand, seckey, &signer[i].pubkey, msg32, NULL, NULL)) {
             return 0;
         }
         pubnonces[i] = &signer[i].pubnonce;
@@ -132,7 +132,7 @@ static int sign(const secp256k1_context* ctx, struct signer_secrets *signer_secr
             return 0;
         }
         /* partial_sign will clear the secnonce by setting it to 0. That's because
-         * you must _never_ reuse the secnonce (or use the same session_id to
+         * you must _never_ reuse the secnonce (or use the same session_secrand to
          * create a secnonce). If you do, you effectively reuse the nonce and
          * leak the secret key. */
         if (!secp256k1_musig_partial_sign(ctx, &signer[i].partial_sig, &signer_secrets[i].secnonce, &signer_secrets[i].keypair, cache, &session)) {

include/secp256k1_musig.h

@@ -317,12 +317,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_xonly_twea
  *  MuSig differs from regular Schnorr signing in that implementers _must_ take
  *  special care to not reuse a nonce. This can be ensured by following these rules:
  *
- *  1. Each call to this function must have a UNIQUE session_id32 that must NOT BE
+ *  1. Each call to this function must have a UNIQUE session_secrand32 that must NOT BE
  *     REUSED in subsequent calls to this function.
- *     If you do not provide a seckey, session_id32 _must_ be UNIFORMLY RANDOM
- *     AND KEPT SECRET (even from other signers). If you do provide a seckey,
- *     session_id32 can instead be a counter (that must never repeat!). However,
- *     it is recommended to always choose session_id32 uniformly at random.
+ *     If you do not provide a seckey, session_secrand32 _must_ be UNIFORMLY RANDOM
+ *     AND KEPT SECRET (even from other signers).
  *  2. If you already know the seckey, message or aggregate public key
  *     cache, they can be optionally provided to derive the nonce and increase
  *     misuse-resistance. The extra_input32 argument can be used to provide
@@ -338,9 +336,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_musig_pubkey_xonly_twea
  *  Args:         ctx: pointer to a context object (not secp256k1_context_static)
  *  Out:     secnonce: pointer to a structure to store the secret nonce
  *           pubnonce: pointer to a structure to store the public nonce
- *  In:  session_id32: a 32-byte session_id32 as explained above. Must be unique to this
- *                     call to secp256k1_musig_nonce_gen and must be uniformly random
- *                     unless you really know what you are doing.
+ *  In:
+ *  session_secrand32: a 32-byte session_secrand32 as explained above. Must be unique to this
+ *                     call to secp256k1_musig_nonce_gen and must be uniformly random.
  *             seckey: the 32-byte secret key that will later be used for signing, if
  *                     already known (can be NULL)
  *             pubkey: public key of the signer creating the nonce. The secnonce
@@ -358,14 +356,72 @@ SECP256K1_API int secp256k1_musig_nonce_gen(
     const secp256k1_context *ctx,
     secp256k1_musig_secnonce *secnonce,
     secp256k1_musig_pubnonce *pubnonce,
-    const unsigned char *session_id32,
+    const unsigned char *session_secrand32,
     const unsigned char *seckey,
     const secp256k1_pubkey *pubkey,
     const unsigned char *msg32,
     const secp256k1_musig_keyagg_cache *keyagg_cache,
     const unsigned char *extra_input32
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(6);
 
+
+/** Alternative way to generate a nonce and start a signing session
+ *
+ *  This function outputs a secret nonce that will be required for signing and a
+ *  corresponding public nonce that is intended to be sent to other signers.
+ *
+ *  This function differs from `secp256k1_musig_nonce_gen` by accepting a
+ *  non-repeating counter value instead of a secret random value. This requires
+ *  the seckey argument to be mandatory.
+ *
+ *  MuSig differs from regular Schnorr signing in that implementers _must_ take
+ *  special care to not reuse a nonce. This can be ensured by following these rules:
+ *
+ *  1. The nonrepeating_cnt argument must be a counter value that never
+ *     repeats, i.e., you must never call `secp256k1_musig_nonce_gen_counter`
+ *     twice with the same seckey and nonrepeating_cnt value.
+ *  2. If you already know the seckey, message or aggregate public key
+ *     cache, they can be optionally provided to derive the nonce and increase
+ *     misuse-resistance. The extra_input32 argument can be used to provide
+ *     additional data that does not repeat in normal scenarios, such as the
+ *     current time.
+ *  3. Avoid copying (or serializing) the secnonce. This reduces the possibility
+ *     that it is used more than once for signing.
+ *
+ *  Remember that nonce reuse will leak the secret key!
+ *  Note that using the same seckey for multiple MuSig sessions is fine.
+ *
+ *  Returns: 0 if the arguments are invalid and 1 otherwise
+ *  Args:         ctx: pointer to a context object (not secp256k1_context_static)
+ *  Out:     secnonce: pointer to a structure to store the secret nonce
+ *           pubnonce: pointer to a structure to store the public nonce
+ *  In:
+ *   nonrepeating_cnt: the value of a counter as explained above. Must be
+ *                     unique to this call to secp256k1_musig_nonce_gen.
+ *             seckey: the 32-byte secret key that will later be used for signing
+ *             pubkey: public key of the signer creating the nonce. The secnonce
+ *                     output of this function cannot be used to sign for any
+ *                     other public key.
+ *              msg32: the 32-byte message that will later be signed, if already known
+ *                     (can be NULL)
+ *       keyagg_cache: pointer to the keyagg_cache that was used to create the aggregate
+ *                     (and potentially tweaked) public key if already known
+ *                     (can be NULL)
+ *      extra_input32: an optional 32-byte array that is input to the nonce
+ *                     derivation function (can be NULL)
+ */
+SECP256K1_API int secp256k1_musig_nonce_gen_counter(
+    const secp256k1_context *ctx,
+    secp256k1_musig_secnonce *secnonce,
+    secp256k1_musig_pubnonce *pubnonce,
+    uint64_t nonrepeating_cnt,
+    const unsigned char *seckey,
+    const secp256k1_pubkey *pubkey,
+    const unsigned char *msg32,
+    const secp256k1_musig_keyagg_cache *keyagg_cache,
+    const unsigned char *extra_input32
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6);
+
 /** Aggregates the nonces of all signers into a single nonce
  *
  *  This can be done by an untrusted party to reduce the communication

src/ctime_tests.c

@@ -190,7 +190,8 @@ static void run_tests(secp256k1_context *ctx, unsigned char *key) {
         secp256k1_pubkey pk;
         const secp256k1_pubkey *pk_ptr[1];
         secp256k1_xonly_pubkey agg_pk;
-        unsigned char session_id[32];
+        unsigned char session_secrand[32];
+        uint64_t nonrepeating_cnt = 0;
         secp256k1_musig_secnonce secnonce;
         secp256k1_musig_pubnonce pubnonce;
         const secp256k1_musig_pubnonce *pubnonce_ptr[1];
@@ -203,20 +204,25 @@ static void run_tests(secp256k1_context *ctx, unsigned char *key) {
         pk_ptr[0] = &pk;
         pubnonce_ptr[0] = &pubnonce;
         SECP256K1_CHECKMEM_DEFINE(key, 32);
-        memcpy(session_id, key, sizeof(session_id));
-        session_id[0] = session_id[0] + 1;
+        memcpy(session_secrand, key, sizeof(session_secrand));
+        session_secrand[0] = session_secrand[0] + 1;
         memcpy(extra_input, key, sizeof(extra_input));
         extra_input[0] = extra_input[0] + 2;
 
         CHECK(secp256k1_keypair_create(ctx, &keypair, key));
         CHECK(secp256k1_keypair_pub(ctx, &pk, &keypair));
         CHECK(secp256k1_musig_pubkey_agg(ctx, &agg_pk, &cache, pk_ptr, 1));
+
         SECP256K1_CHECKMEM_UNDEFINE(key, 32);
-        SECP256K1_CHECKMEM_UNDEFINE(session_id, sizeof(session_id));
+        SECP256K1_CHECKMEM_UNDEFINE(session_secrand, sizeof(session_secrand));
         SECP256K1_CHECKMEM_UNDEFINE(extra_input, sizeof(extra_input));
-        ret = secp256k1_musig_nonce_gen(ctx, &secnonce, &pubnonce, session_id, key, &pk, msg, &cache, extra_input);
+        ret = secp256k1_musig_nonce_gen(ctx, &secnonce, &pubnonce, session_secrand, key, &pk, msg, &cache, extra_input);
+        SECP256K1_CHECKMEM_DEFINE(&ret, sizeof(ret));
+        CHECK(ret == 1);
+        ret = secp256k1_musig_nonce_gen_counter(ctx, &secnonce, &pubnonce, nonrepeating_cnt, key, &pk, msg, &cache, extra_input);
         SECP256K1_CHECKMEM_DEFINE(&ret, sizeof(ret));
         CHECK(ret == 1);
+
         CHECK(secp256k1_musig_nonce_agg(ctx, &aggnonce, pubnonce_ptr, 1));
         /* Make sure that previous tests don't undefine msg. It's not used as a secret here. */
         SECP256K1_CHECKMEM_DEFINE(msg, sizeof(msg));

src/modules/musig/session_impl.h

@@ -341,21 +341,21 @@ static void secp256k1_nonce_function_musig_sha256_tagged(secp256k1_sha256 *sha)
     sha->bytes = 64;
 }
 
-static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned char *session_id, const unsigned char *msg32, const unsigned char *seckey32, const unsigned char *pk33, const unsigned char *agg_pk32, const unsigned char *extra_input32) {
+static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned char *session_secrand, const unsigned char *msg32, const unsigned char *seckey32, const unsigned char *pk33, const unsigned char *agg_pk32, const unsigned char *extra_input32) {
     secp256k1_sha256 sha;
     unsigned char rand[32];
     unsigned char i;
     unsigned char msg_present;
 
     if (seckey32 != NULL) {
         secp256k1_nonce_function_musig_sha256_tagged_aux(&sha);
-        secp256k1_sha256_write(&sha, session_id, 32);
+        secp256k1_sha256_write(&sha, session_secrand, 32);
         secp256k1_sha256_finalize(&sha, rand);
         for (i = 0; i < 32; i++) {
             rand[i] ^= seckey32[i];
         }
     } else {
-        memcpy(rand, session_id, sizeof(rand));
+        memcpy(rand, session_secrand, sizeof(rand));
     }
 
     /* Subtract one from `sizeof` to avoid hashing the implicit null byte */
@@ -379,7 +379,7 @@ static void secp256k1_nonce_function_musig(secp256k1_scalar *k, const unsigned c
     }
 }
 
-int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *session_id32, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
+int secp256k1_musig_nonce_gen_internal(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *input_nonce, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
     secp256k1_keyagg_cache_internal cache_i;
     secp256k1_scalar k[2];
     secp256k1_ge nonce_pt[2];
@@ -392,24 +392,12 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn
     int pk_serialize_success;
     int ret = 1;
 
-    VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(secnonce != NULL);
     memset(secnonce, 0, sizeof(*secnonce));
     ARG_CHECK(pubnonce != NULL);
     memset(pubnonce, 0, sizeof(*pubnonce));
-    ARG_CHECK(session_id32 != NULL);
     ARG_CHECK(pubkey != 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. */
-        unsigned char acc = 0;
-        for (i = 0; i < 32; i++) {
-            acc |= session_id32[i];
-        }
-        ret &= !!acc;
-        memset(&acc, 0, sizeof(acc));
-    }
 
     /* Check that the seckey is valid to be able to sign for it later. */
     if (seckey != NULL) {
@@ -439,7 +427,7 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn
     (void) pk_serialize_success;
 #endif
 
-    secp256k1_nonce_function_musig(k, session_id32, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32);
+    secp256k1_nonce_function_musig(k, input_nonce, msg32, seckey, pk_ser, aggpk_ser_ptr, extra_input32);
     VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[0]));
     VERIFY_CHECK(!secp256k1_scalar_is_zero(&k[1]));
     VERIFY_CHECK(!secp256k1_scalar_eq(&k[0], &k[1]));
@@ -458,6 +446,47 @@ int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secn
     return ret;
 }
 
+int secp256k1_musig_nonce_gen(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, const unsigned char *session_secrand32, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
+    int ret = 1;
+    unsigned char acc = 0;
+    int i;
+
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(session_secrand32 != NULL);
+
+    /* Check in constant time that the session_secrand32 is not 0 as a
+     * defense-in-depth measure that may protect against a faulty RNG. */
+    for (i = 0; i < 32; i++) {
+        acc |= session_secrand32[i];
+    }
+    ret &= !!acc;
+    memset(&acc, 0, sizeof(acc));
+
+    /* We can declassify because branching on ret is only relevant when this
+     * function called with an invalid session_secrand32 argument */
+    secp256k1_declassify(ctx, &ret, sizeof(ret));
+    if (ret == 0) {
+        secp256k1_musig_secnonce_invalidate(ctx, secnonce, 1);
+        return 0;
+    }
+
+    return secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, session_secrand32, seckey, pubkey, msg32, keyagg_cache, extra_input32);
+}
+
+int secp256k1_musig_nonce_gen_counter(const secp256k1_context* ctx, secp256k1_musig_secnonce *secnonce, secp256k1_musig_pubnonce *pubnonce, uint64_t nonrepeating_cnt, const unsigned char *seckey, const secp256k1_pubkey *pubkey, const unsigned char *msg32, const secp256k1_musig_keyagg_cache *keyagg_cache, const unsigned char *extra_input32) {
+    unsigned char buf[32] = { 0 };
+    int i;
+
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK((seckey != NULL));
+
+    for (i = 0; i < 8; ++i) {
+        buf[7 - i] = (nonrepeating_cnt >> (i * 8)) & 0xFF;
+    }
+
+    return secp256k1_musig_nonce_gen_internal(ctx, secnonce, pubnonce, buf, seckey, pubkey, msg32, keyagg_cache, extra_input32);
+}
+
 static int secp256k1_musig_sum_nonces(const secp256k1_context* ctx, secp256k1_gej *summed_nonces, const secp256k1_musig_pubnonce * const* pubnonces, size_t n_pubnonces) {
     size_t i;
     int j;