Merge bitcoin#845: Extract the secret key from a keypair

33cb3c2 Add secret key extraction from keypair to constant time tests (Elichai Turkel)
36d9dc1 Add seckey extraction from keypair to the extrakeys tests (Elichai Turkel)
fc96aa7 Add a function to extract the secretkey from a keypair (Elichai Turkel)

Pull request description:

  With schnorrsig if you need to tweak the secret key (for BIP32) you must use the keypair API to get compatible secret/public keys which you do by calling `secp256k1_keypair_xonly_tweak_add()`, but after that there's no currently a way to extract the secret key back for storage.
  so I added a `secp256k1_keypair_seckey` function to extract the key

ACKs for top commit:
  jonasnick:
    ACK 33cb3c2
  real-or-random:
    ACK 33cb3c2 code inspection, tests pass

Tree-SHA512: 11212db38c8b87a87e2dc35c4d6993716867b45215b94b20522b1b3164ca63d4c6bf5192a6bff0e9267b333779cc8164844c56669a94e9be72df9ef025ffcfd4

include/secp256k1_extrakeys.h

@@ -165,6 +165,19 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_create(
     const unsigned char *seckey
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
 
+/** Get the secret key from a keypair.
+ *
+ *  Returns: 0 if the arguments are invalid. 1 otherwise.
+ *  Args:   ctx: pointer to a context object (cannot be NULL)
+ *  Out: seckey: pointer to a 32-byte buffer for the secret key (cannot be NULL)
+ *  In: keypair: pointer to a keypair (cannot be NULL)
+ */
+SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_sec(
+    const secp256k1_context* ctx,
+    unsigned char *seckey,
+    const secp256k1_keypair *keypair
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);
+
 /** Get the public key from a keypair.
  *
  *  Returns: 0 if the arguments are invalid. 1 otherwise.

src/modules/extrakeys/main_impl.h

@@ -186,6 +186,16 @@ int secp256k1_keypair_create(const secp256k1_context* ctx, secp256k1_keypair *ke
     return ret;
 }
 
+int secp256k1_keypair_sec(const secp256k1_context* ctx, unsigned char *seckey, const secp256k1_keypair *keypair) {
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(seckey != NULL);
+    memset(seckey, 0, 32);
+    ARG_CHECK(keypair != NULL);
+
+    memcpy(seckey, &keypair->data[0], 32);
+    return 1;
+}
+
 int secp256k1_keypair_pub(const secp256k1_context* ctx, secp256k1_pubkey *pubkey, const secp256k1_keypair *keypair) {
     VERIFY_CHECK(ctx != NULL);
     ARG_CHECK(pubkey != NULL);

src/modules/extrakeys/tests_impl.h

@@ -311,6 +311,7 @@ void test_xonly_pubkey_tweak_recursive(void) {
 
 void test_keypair(void) {
     unsigned char sk[32];
+    unsigned char sk_tmp[32];
     unsigned char zeros96[96] = { 0 };
     unsigned char overflows[32];
     secp256k1_keypair keypair;
@@ -396,6 +397,28 @@ void test_keypair(void) {
     CHECK(secp256k1_memcmp_var(&xonly_pk, &xonly_pk_tmp, sizeof(pk)) == 0);
     CHECK(pk_parity == pk_parity_tmp);
 
+    /* Test keypair_seckey */
+    ecount = 0;
+    secp256k1_testrand256(sk);
+    CHECK(secp256k1_keypair_create(ctx, &keypair, sk) == 1);
+    CHECK(secp256k1_keypair_sec(none, sk_tmp, &keypair) == 1);
+    CHECK(secp256k1_keypair_sec(none, NULL, &keypair) == 0);
+    CHECK(ecount == 1);
+    CHECK(secp256k1_keypair_sec(none, sk_tmp, NULL) == 0);
+    CHECK(ecount == 2);
+    CHECK(secp256k1_memcmp_var(zeros96, sk_tmp, sizeof(sk_tmp)) == 0);
+
+    /* keypair returns the same seckey it got */
+    CHECK(secp256k1_keypair_create(sign, &keypair, sk) == 1);
+    CHECK(secp256k1_keypair_sec(none, sk_tmp, &keypair) == 1);
+    CHECK(secp256k1_memcmp_var(sk, sk_tmp, sizeof(sk_tmp)) == 0);
+
+
+    /* Using an invalid keypair is fine for keypair_seckey */
+    memset(&keypair, 0, sizeof(keypair));
+    CHECK(secp256k1_keypair_sec(none, sk_tmp, &keypair) == 1);
+    CHECK(secp256k1_memcmp_var(zeros96, sk_tmp, sizeof(sk_tmp)) == 0);
+
     secp256k1_context_destroy(none);
     secp256k1_context_destroy(sign);
     secp256k1_context_destroy(verify);
@@ -484,6 +507,7 @@ void test_keypair_add(void) {
         secp256k1_pubkey output_pk_xy;
         secp256k1_pubkey output_pk_expected;
         unsigned char pk32[32];
+        unsigned char sk32[32];
         int pk_parity;
 
         secp256k1_testrand256(tweak);
@@ -501,7 +525,8 @@ void test_keypair_add(void) {
         CHECK(secp256k1_memcmp_var(&output_pk_xy, &output_pk_expected, sizeof(output_pk_xy)) == 0);
 
         /* Check that the secret key in the keypair is tweaked correctly */
-        CHECK(secp256k1_ec_pubkey_create(ctx, &output_pk_expected, &keypair.data[0]) == 1);
+        CHECK(secp256k1_keypair_sec(none, sk32, &keypair) == 1);
+        CHECK(secp256k1_ec_pubkey_create(ctx, &output_pk_expected, sk32) == 1);
         CHECK(secp256k1_memcmp_var(&output_pk_xy, &output_pk_expected, sizeof(output_pk_xy)) == 0);
     }
     secp256k1_context_destroy(none);

src/valgrind_ctime_test.c

@@ -140,6 +140,12 @@ int main(void) {
     ret = secp256k1_keypair_xonly_tweak_add(ctx, &keypair, msg);
     VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret));
     CHECK(ret == 1);
+
+    VALGRIND_MAKE_MEM_UNDEFINED(key, 32);
+    VALGRIND_MAKE_MEM_UNDEFINED(&keypair, sizeof(keypair));
+    ret = secp256k1_keypair_sec(ctx, key, &keypair);
+    VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret));
+    CHECK(ret == 1);
 #endif
 
 #ifdef ENABLE_MODULE_SCHNORRSIG