address Jonas' feedback

Author: benma

include/secp256k1.h

@@ -534,10 +534,10 @@ SECP256K1_API extern const secp256k1_nonce_function secp256k1_nonce_function_def
  *    `secp256k1_ecdsa_anti_nonce_sidechan_client_commit`. The client sends the resulting commitment
  *   `R1` to the host.
  * 3. The host replies with `k2` generated in step 1.
- * 4. The client checks that the host commitment from step 1 commits to `k2` from step 3 and signs
- *    with `secp256k1_ecdsa_sign_nonce_tweak_add`, using the `k2` as the tweak and the host
- *    commitment as additional noncedata and sends the signature to the host.
- * 5. The host reconstructs the curve point `R` from the signature and verifies that `R = R1 + k2*G`.
+ * 4. The client signs with `secp256k1_ecdsa_sign_to_contract`, using the `k2` as `s2c_data` and
+ *    sends the signature to the host.
+ * 5. The host reconstructs the curve point `R` from the signature and verifies that
+ *    `R = R1 + H(R1, k2)*G`.
  *
  *  Returns 1 on success, 0 on failure.
  *  Args:           ctx: pointer to a context object (cannot be NULL)
@@ -553,9 +553,8 @@ SECP256K1_API int secp256k1_ecdsa_anti_nonce_sidechan_client_commit(
     secp256k1_pubkey *client_commit,
     const unsigned char *msg32,
     const unsigned char *seckey32,
-    secp256k1_nonce_function noncefp,
     unsigned char *rand_commitment32
-) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(6);
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5);
 
 /** Create an ECDSA signature.
  *
@@ -580,16 +579,17 @@ SECP256K1_API int secp256k1_ecdsa_sign(
     const void *ndata
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
 
-/** Same as secp256k1_ecdsa_sign, but nonce_tweak32 is added to the nonce generated by noncefp.
+/** Same as secp256k1_ecdsa_sign, but s2c_data32 is committed to by adding `hash(R1, s2c_data32)` to
+    the nonce generated by noncefp, with `ndata=hash(s2c_data32, ndata)`.
  */
-SECP256K1_API int secp256k1_ecdsa_sign_nonce_tweak_add(
+SECP256K1_API int secp256k1_ecdsa_sign_to_contract(
     const secp256k1_context* ctx,
     secp256k1_ecdsa_signature *sig,
     const unsigned char *msg32,
     const unsigned char *seckey,
     secp256k1_nonce_function noncefp,
     const void *ndata,
-    const unsigned char* nonce_tweak32
+    const unsigned char* s2c_data32
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
 
 /** Verify an ECDSA secret key.

src/secp256k1.c

@@ -451,7 +451,6 @@ int secp256k1_ecdsa_anti_nonce_sidechan_client_commit(
     secp256k1_pubkey *client_commit,
     const unsigned char *msg32,
     const unsigned char *seckey32,
-    secp256k1_nonce_function noncefp,
     unsigned char *rand_commitment32
 ) {
     unsigned char nonce32[32];
@@ -464,13 +463,10 @@ int secp256k1_ecdsa_anti_nonce_sidechan_client_commit(
     ARG_CHECK(client_commit != NULL);
     ARG_CHECK(msg32 != NULL);
     ARG_CHECK(seckey32 != NULL);
-    if (noncefp == NULL) {
-        noncefp = secp256k1_nonce_function_default;
-    }
     ARG_CHECK(rand_commitment32 != NULL);
 
 
-    if (!noncefp(nonce32, msg32, seckey32, NULL, rand_commitment32, 0)) {
+    if (!secp256k1_nonce_function_default(nonce32, msg32, seckey32, NULL, rand_commitment32, 0)) {
         return 0;
     }
 
@@ -486,14 +482,56 @@ int secp256k1_ecdsa_anti_nonce_sidechan_client_commit(
 }
 
 int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata) {
-    return secp256k1_ecdsa_sign_nonce_tweak_add(ctx, signature, msg32, seckey, noncefp, noncedata, NULL);
+    return secp256k1_ecdsa_sign_to_contract(ctx, signature, msg32, seckey, noncefp, noncedata, NULL);
 }
 
-int secp256k1_ecdsa_sign_nonce_tweak_add(const secp256k1_context* ctx, secp256k1_ecdsa_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata, const unsigned char* nonce_tweak32) {
+/* Compute an ec commitment tweak as hash(pubkey, data). */
+static int secp256k1_ec_commit_tweak(const secp256k1_context *ctx, unsigned char *tweak32, const secp256k1_pubkey *pubkey, const unsigned char *data, size_t data_size) {
+    secp256k1_ge p;
+    unsigned char rbuf[33];
+    size_t rbuf_size = sizeof(rbuf);
+    secp256k1_sha256 sha;
+
+    if (data_size == 0) {
+        /* That's probably not what the caller wanted */
+        return 0;
+    }
+    if(!secp256k1_pubkey_load(ctx, &p, pubkey)) {
+        return 0;
+    }
+    secp256k1_eckey_pubkey_serialize(&p, rbuf, &rbuf_size, 1);
+
+    secp256k1_sha256_initialize(&sha);
+    secp256k1_sha256_write(&sha, rbuf, rbuf_size);
+    secp256k1_sha256_write(&sha, data, data_size);
+    secp256k1_sha256_finalize(&sha, tweak32);
+    return 1;
+}
+
+/* Compute an ec commitment tweak as hash(pubkey, data). */
+static int secp256k1_ec_commit_tweak_from_seckey(
+    const secp256k1_context *ctx,
+    unsigned char *tweak32,
+    const secp256k1_scalar *seckey,
+    const unsigned char *data,
+    size_t data_size) {
+    secp256k1_gej rp;
+    secp256k1_ge r;
+    secp256k1_pubkey pubkey_tmp;
+    secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &rp, seckey);
+    secp256k1_ge_set_gej(&r, &rp);
+    secp256k1_pubkey_save(&pubkey_tmp, &r);
+    return secp256k1_ec_commit_tweak(ctx, tweak32, &pubkey_tmp, data, data_size);
+}
+
+int secp256k1_ecdsa_sign_to_contract(const secp256k1_context* ctx, secp256k1_ecdsa_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata, const unsigned char* s2c_data32) {
     secp256k1_scalar r, s;
-    secp256k1_scalar sec, non, msg, nonce_tweak;
+    secp256k1_scalar sec, non, msg;
+    secp256k1_sha256 sha;
     int ret = 0;
     int overflow = 0;
+    int is_zero = 0;
+    unsigned char ndata[32];
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx));
     ARG_CHECK(msg32 != NULL);
@@ -502,12 +540,27 @@ int secp256k1_ecdsa_sign_nonce_tweak_add(const secp256k1_context* ctx, secp256k1
     if (noncefp == NULL) {
         noncefp = secp256k1_nonce_function_default;
     }
-
-    if (nonce_tweak32 != NULL) {
-        secp256k1_scalar_set_b32(&nonce_tweak, nonce_tweak32, &overflow);
-        if (overflow || secp256k1_scalar_is_zero(&nonce_tweak)) {
-            return 0;
+    /* sign-to-contract commitments only work with the default nonce function,
+     * because we need to ensure that s2c_data is actually hashed into the nonce and
+     * not just ignored. */
+    ARG_CHECK(s2c_data32 == NULL || noncefp == secp256k1_nonce_function_default);
+
+    if(s2c_data32 != NULL) {
+        /* Provide s2c_data32 and ndata (if not NULL) to the the nonce function
+         * as additional data to derive the nonce from. If both pointers are
+         * not NULL, they need to be hashed to get the nonce data 32 bytes.
+         * Even if only s2c_data32 is not NULL, it's hashed because it should
+         * be possible to derive nonces even if only a SHA256 commitment to the
+         * data is known.  This is for example important in the
+         * anti-nonce-sidechannel protocol.
+         */
+        secp256k1_sha256_initialize(&sha);
+        secp256k1_sha256_write(&sha, s2c_data32, 32);
+        if (noncedata != NULL) {
+            secp256k1_sha256_write(&sha, noncedata, 32);
         }
+        secp256k1_sha256_finalize(&sha, ndata);
+        noncedata = &ndata;
     }
 
     secp256k1_scalar_set_b32(&sec, seckey, &overflow);
@@ -522,12 +575,28 @@ int secp256k1_ecdsa_sign_nonce_tweak_add(const secp256k1_context* ctx, secp256k1
                 break;
             }
             secp256k1_scalar_set_b32(&non, nonce32, &overflow);
-            if (nonce_tweak32 != NULL) {
-                secp256k1_scalar_add(&non, &non, &nonce_tweak);
-            }
-            if (!overflow && !secp256k1_scalar_is_zero(&non)) {
-                if (secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL)) {
-                    break;
+            is_zero = secp256k1_scalar_is_zero(&non);
+            if (!overflow && !is_zero) {
+                if (s2c_data32 != NULL) {
+                    secp256k1_scalar nonce_tweak;
+                    unsigned char nonce_tweak32[32];
+                    if (!secp256k1_ec_commit_tweak_from_seckey(ctx, nonce_tweak32, &non, s2c_data32, 32)) {
+                        return 0;
+                    }
+
+                    secp256k1_scalar_set_b32(&nonce_tweak, nonce_tweak32, &overflow);
+                    if (overflow || secp256k1_scalar_is_zero(&nonce_tweak)) {
+                        return 0;
+                    }
+
+                    secp256k1_scalar_add(&non, &non, &nonce_tweak);
+                    is_zero = secp256k1_scalar_is_zero(&non);
+                }
+
+                if (!is_zero) {
+                    if (secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL)) {
+                        break;
+                    }
                 }
             }
             count++;