Merge #354: [ECDH API change] Support custom hash function

c8fbc3c [ECDH API change] Allow pass arbitrary data to hash function (Kirill Fomichev)
b00be65 [ECDH API change] Support custom hash function (Kirill Fomichev)

Pull request description:

  Solve #352

Tree-SHA512: f5985874d03e976cdb3d59036af7720636ad1488da40fd3bd7881b1fb71b05036a952013d519baa84c4ce4b558bdef25c4ce76b384b297e4d0aece9e37e78a01

include/secp256k1_ecdh.h

@@ -7,21 +7,45 @@
 extern "C" {
 #endif
 
+/** A pointer to a function that applies hash function to a point
+ *
+ *  Returns: 1 if a point was successfully hashed. 0 will cause ecdh to fail
+ *  Out:    output:     pointer to an array to be filled by the function
+ *  In:     x:          pointer to a 32-byte x coordinate
+ *          y:          pointer to a 32-byte y coordinate
+ *          data:       Arbitrary data pointer that is passed through
+ */
+typedef int (*secp256k1_ecdh_hash_function)(
+  unsigned char *output,
+  const unsigned char *x,
+  const unsigned char *y,
+  void *data
+);
+
+/** An implementation of SHA256 hash function that applies to compressed public key. */
+SECP256K1_API extern const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256;
+
+/** A default ecdh hash function (currently equal to secp256k1_ecdh_hash_function_sha256). */
+SECP256K1_API extern const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default;
+
 /** Compute an EC Diffie-Hellman secret in constant time
  *  Returns: 1: exponentiation was successful
  *           0: scalar was invalid (zero or overflow)
  *  Args:    ctx:        pointer to a context object (cannot be NULL)
- *  Out:     result:     a 32-byte array which will be populated by an ECDH
+ *  Out:     output:     pointer to an array to be filled by the function
- *                       secret computed from the point and scalar
  *  In:      pubkey:     a pointer to a secp256k1_pubkey containing an
  *                       initialized public key
  *           privkey:    a 32-byte scalar with which to multiply the point
+ *           hashfp:     pointer to a hash function. If NULL, secp256k1_ecdh_hash_function_sha256 is used
+ *           data:       Arbitrary data pointer that is passed through
  */
 SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdh(
   const secp256k1_context* ctx,
-  unsigned char *result,
+  unsigned char *output,
   const secp256k1_pubkey *pubkey,
-  const unsigned char *privkey
+  const unsigned char *privkey,
+  secp256k1_ecdh_hash_function hashfp,
+  void *data
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);
 
 #ifdef __cplusplus

src/bench_ecdh.c

@@ -42,7 +42,7 @@ static void bench_ecdh(void* arg) {
     bench_ecdh_data *data = (bench_ecdh_data*)arg;
 
     for (i = 0; i < 20000; i++) {
-        CHECK(secp256k1_ecdh(data->ctx, res, &data->point, data->scalar) == 1);
+        CHECK(secp256k1_ecdh(data->ctx, res, &data->point, data->scalar, NULL, NULL) == 1);
     }
 }
 

src/java/org_bitcoin_NativeSecp256k1.c

@@ -83,7 +83,7 @@ SECP256K1_API jobjectArray JNICALL Java_org_bitcoin_NativeSecp256k1_secp256k1_1e
 
   secp256k1_ecdsa_signature sig[72];
 
-  int ret = secp256k1_ecdsa_sign(ctx, sig, data, secKey, NULL, NULL );
+  int ret = secp256k1_ecdsa_sign(ctx, sig, data, secKey, NULL, NULL);
 
   unsigned char outputSer[72];
   size_t outputLen = 72;
@@ -353,7 +353,9 @@ SECP256K1_API jobjectArray JNICALL Java_org_bitcoin_NativeSecp256k1_secp256k1_1e
       ctx,
       nonce_res,
       &pubkey,
-      secdata
+      secdata,
+      NULL,
+      NULL
     );
   }
 

src/modules/ecdh/main_impl.h

@@ -10,16 +10,35 @@
 #include "include/secp256k1_ecdh.h"
 #include "ecmult_const_impl.h"
 
-int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *result, const secp256k1_pubkey *point, const unsigned char *scalar) {
+static int ecdh_hash_function_sha256(unsigned char *output, const unsigned char *x, const unsigned char *y, void *data) {
+    unsigned char version = (y[31] & 0x01) | 0x02;
+    secp256k1_sha256 sha;
+    (void)data;
+
+    secp256k1_sha256_initialize(&sha);
+    secp256k1_sha256_write(&sha, &version, 1);
+    secp256k1_sha256_write(&sha, x, 32);
+    secp256k1_sha256_finalize(&sha, output);
+
+    return 1;
+}
+
+const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256 = ecdh_hash_function_sha256;
+const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default = ecdh_hash_function_sha256;
+
+int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const secp256k1_pubkey *point, const unsigned char *scalar, secp256k1_ecdh_hash_function hashfp, void *data) {
     int ret = 0;
     int overflow = 0;
     secp256k1_gej res;
     secp256k1_ge pt;
     secp256k1_scalar s;
     VERIFY_CHECK(ctx != NULL);
-    ARG_CHECK(result != NULL);
+    ARG_CHECK(output != NULL);
     ARG_CHECK(point != NULL);
     ARG_CHECK(scalar != NULL);
+    if (hashfp == NULL) {
+        hashfp = secp256k1_ecdh_hash_function_default;
+    }
 
     secp256k1_pubkey_load(ctx, &pt, point);
     secp256k1_scalar_set_b32(&s, scalar, &overflow);
@@ -27,24 +46,18 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *result, const se
         ret = 0;
     } else {
         unsigned char x[32];
-        unsigned char y[1];
+        unsigned char y[32];
-        secp256k1_sha256 sha;
 
         secp256k1_ecmult_const(&res, &pt, &s, 256);
         secp256k1_ge_set_gej(&pt, &res);
-        /* Compute a hash of the point in compressed form
+
-         * Note we cannot use secp256k1_eckey_pubkey_serialize here since it does not
+        /* Compute a hash of the point */
-         * expect its output to be secret and has a timing sidechannel. */
         secp256k1_fe_normalize(&pt.x);
         secp256k1_fe_normalize(&pt.y);
         secp256k1_fe_get_b32(x, &pt.x);
-        y[0] = 0x02 | secp256k1_fe_is_odd(&pt.y);
+        secp256k1_fe_get_b32(y, &pt.y);
 
-        secp256k1_sha256_initialize(&sha);
+        ret = hashfp(output, x, y, data);
-        secp256k1_sha256_write(&sha, y, sizeof(y));
-        secp256k1_sha256_write(&sha, x, sizeof(x));
-        secp256k1_sha256_finalize(&sha, result);
-        ret = 1;
     }
 
     secp256k1_scalar_clear(&s);

src/modules/ecdh/tests_impl.h

@@ -7,6 +7,23 @@
 #ifndef SECP256K1_MODULE_ECDH_TESTS_H
 #define SECP256K1_MODULE_ECDH_TESTS_H
 
+int ecdh_hash_function_test_fail(unsigned char *output, const unsigned char *x, const unsigned char *y, void *data) {
+    (void)output;
+    (void)x;
+    (void)y;
+    (void)data;
+    return 0;
+}
+
+int ecdh_hash_function_custom(unsigned char *output, const unsigned char *x, const unsigned char *y, void *data) {
+    (void)data;
+    /* Save x and y as uncompressed public key */
+    output[0] = 0x04;
+    memcpy(output + 1, x, 32);
+    memcpy(output + 33, y, 32);
+    return 1;
+}
+
 void test_ecdh_api(void) {
     /* Setup context that just counts errors */
     secp256k1_context *tctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
@@ -21,15 +38,15 @@ void test_ecdh_api(void) {
     CHECK(secp256k1_ec_pubkey_create(tctx, &point, s_one) == 1);
 
     /* Check all NULLs are detected */
-    CHECK(secp256k1_ecdh(tctx, res, &point, s_one) == 1);
+    CHECK(secp256k1_ecdh(tctx, res, &point, s_one, NULL, NULL) == 1);
     CHECK(ecount == 0);
-    CHECK(secp256k1_ecdh(tctx, NULL, &point, s_one) == 0);
+    CHECK(secp256k1_ecdh(tctx, NULL, &point, s_one, NULL, NULL) == 0);
     CHECK(ecount == 1);
-    CHECK(secp256k1_ecdh(tctx, res, NULL, s_one) == 0);
+    CHECK(secp256k1_ecdh(tctx, res, NULL, s_one, NULL, NULL) == 0);
     CHECK(ecount == 2);
-    CHECK(secp256k1_ecdh(tctx, res, &point, NULL) == 0);
+    CHECK(secp256k1_ecdh(tctx, res, &point, NULL, NULL, NULL) == 0);
     CHECK(ecount == 3);
-    CHECK(secp256k1_ecdh(tctx, res, &point, s_one) == 1);
+    CHECK(secp256k1_ecdh(tctx, res, &point, s_one, NULL, NULL) == 1);
     CHECK(ecount == 3);
 
     /* Cleanup */
@@ -46,27 +63,34 @@ void test_ecdh_generator_basepoint(void) {
     for (i = 0; i < 100; ++i) {
         secp256k1_sha256 sha;
         unsigned char s_b32[32];
-        unsigned char output_ecdh[32];
+        unsigned char output_ecdh[65];
         unsigned char output_ser[32];
-        unsigned char point_ser[33];
+        unsigned char point_ser[65];
         size_t point_ser_len = sizeof(point_ser);
         secp256k1_scalar s;
 
         random_scalar_order(&s);
         secp256k1_scalar_get_b32(s_b32, &s);
 
-        /* compute using ECDH function */
         CHECK(secp256k1_ec_pubkey_create(ctx, &point[0], s_one) == 1);
-        CHECK(secp256k1_ecdh(ctx, output_ecdh, &point[0], s_b32) == 1);
-        /* compute "explicitly" */
         CHECK(secp256k1_ec_pubkey_create(ctx, &point[1], s_b32) == 1);
+
+        /* compute using ECDH function with custom hash function */
+        CHECK(secp256k1_ecdh(ctx, output_ecdh, &point[0], s_b32, ecdh_hash_function_custom, NULL) == 1);
+        /* compute "explicitly" */
+        CHECK(secp256k1_ec_pubkey_serialize(ctx, point_ser, &point_ser_len, &point[1], SECP256K1_EC_UNCOMPRESSED) == 1);
+        /* compare */
+        CHECK(memcmp(output_ecdh, point_ser, 65) == 0);
+
+        /* compute using ECDH function with default hash function */
+        CHECK(secp256k1_ecdh(ctx, output_ecdh, &point[0], s_b32, NULL, NULL) == 1);
+        /* compute "explicitly" */
         CHECK(secp256k1_ec_pubkey_serialize(ctx, point_ser, &point_ser_len, &point[1], SECP256K1_EC_COMPRESSED) == 1);
-        CHECK(point_ser_len == sizeof(point_ser));
         secp256k1_sha256_initialize(&sha);
         secp256k1_sha256_write(&sha, point_ser, point_ser_len);
         secp256k1_sha256_finalize(&sha, output_ser);
         /* compare */
-        CHECK(memcmp(output_ecdh, output_ser, sizeof(output_ser)) == 0);
+        CHECK(memcmp(output_ecdh, output_ser, 32) == 0);
     }
 }
 
@@ -89,11 +113,14 @@ void test_bad_scalar(void) {
     CHECK(secp256k1_ec_pubkey_create(ctx, &point, s_rand) == 1);
 
     /* Try to multiply it by bad values */
-    CHECK(secp256k1_ecdh(ctx, output, &point, s_zero) == 0);
+    CHECK(secp256k1_ecdh(ctx, output, &point, s_zero, NULL, NULL) == 0);
-    CHECK(secp256k1_ecdh(ctx, output, &point, s_overflow) == 0);
+    CHECK(secp256k1_ecdh(ctx, output, &point, s_overflow, NULL, NULL) == 0);
     /* ...and a good one */
     s_overflow[31] -= 1;
-    CHECK(secp256k1_ecdh(ctx, output, &point, s_overflow) == 1);
+    CHECK(secp256k1_ecdh(ctx, output, &point, s_overflow, NULL, NULL) == 1);
+
+    /* Hash function failure results in ecdh failure */
+    CHECK(secp256k1_ecdh(ctx, output, &point, s_overflow, ecdh_hash_function_test_fail, NULL) == 0);
 }
 
 void run_ecdh_tests(void) {