From 9ab2cbe0ebf8dfafd5499ea7a79c35f0d0cfe5e2 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 15:32:00 +0000 Subject: [PATCH 01/10] Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key --- src/scalar.h | 4 ++++ src/scalar_impl.h | 6 ++++++ src/tests.c | 26 ++++++++++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/src/scalar.h b/src/scalar.h index da711788c60b3..6dc7574ca82bc 100644 --- a/src/scalar.h +++ b/src/scalar.h @@ -39,6 +39,10 @@ static unsigned int secp256k1_scalar_get_bits_var(const secp256k1_scalar *a, uns */ static void secp256k1_scalar_set_b32(secp256k1_scalar *r, const unsigned char *bin, int *overflow); +/** Set a scalar from a big endian byte array and returns 1 if it is a valid + * seckey and 0 otherwise. */ +static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin); + /** Set a scalar to an unsigned integer. */ static void secp256k1_scalar_set_int(secp256k1_scalar *r, unsigned int v); diff --git a/src/scalar_impl.h b/src/scalar_impl.h index c9b38f3c7c447..70cd73db062af 100644 --- a/src/scalar_impl.h +++ b/src/scalar_impl.h @@ -55,6 +55,12 @@ static void secp256k1_scalar_order_get_num(secp256k1_num *r) { } #endif +static int secp256k1_scalar_set_b32_seckey(secp256k1_scalar *r, const unsigned char *bin) { + int overflow; + secp256k1_scalar_set_b32(r, bin, &overflow); + return (!overflow) & (!secp256k1_scalar_is_zero(r)); +} + static void secp256k1_scalar_inverse(secp256k1_scalar *r, const secp256k1_scalar *x) { #if defined(EXHAUSTIVE_TEST_ORDER) int i; diff --git a/src/tests.c b/src/tests.c index 2f2cb71539049..0a0cb5268a2fb 100644 --- a/src/tests.c +++ b/src/tests.c @@ -140,6 +140,12 @@ void random_scalar_order(secp256k1_scalar *num) { } while(1); } +void random_scalar_order_b32(unsigned char *b32) { + secp256k1_scalar num; + random_scalar_order(&num); + secp256k1_scalar_get_b32(b32, &num); +} + void run_context_tests(int use_prealloc) { secp256k1_pubkey pubkey; secp256k1_pubkey zero_pubkey; @@ -1077,11 +1083,31 @@ void scalar_test(void) { } +void run_scalar_set_b32_seckey_tests(void) { + unsigned char b32[32]; + secp256k1_scalar s1; + secp256k1_scalar s2; + + /* Usually set_b32 and set_b32_seckey give the same result */ + random_scalar_order_b32(b32); + secp256k1_scalar_set_b32(&s1, b32, NULL); + CHECK(secp256k1_scalar_set_b32_seckey(&s2, b32) == 1); + CHECK(secp256k1_scalar_eq(&s1, &s2) == 1); + + memset(b32, 0, sizeof(b32)); + CHECK(secp256k1_scalar_set_b32_seckey(&s2, b32) == 0); + memset(b32, 0xFF, sizeof(b32)); + CHECK(secp256k1_scalar_set_b32_seckey(&s2, b32) == 0); +} + void run_scalar_tests(void) { int i; for (i = 0; i < 128 * count; i++) { scalar_test(); } + for (i = 0; i < count; i++) { + run_scalar_set_b32_seckey_tests(); + } { /* (-1)+1 should be zero. */ From 3fec9826086aa45ebbac1ff6fc3bb7b25ca78b1d Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 15:56:09 +0000 Subject: [PATCH 02/10] Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify --- src/secp256k1.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/secp256k1.c b/src/secp256k1.c index c10ac4cda546a..20cf3c7e05b57 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -471,7 +471,7 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature secp256k1_scalar r, s; secp256k1_scalar sec, non, msg; int ret = 0; - int overflow = 0; + int is_sec_valid; unsigned char nonce32[32]; unsigned int count = 0; VERIFY_CHECK(ctx != NULL); @@ -483,22 +483,20 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature noncefp = secp256k1_nonce_function_default; } - secp256k1_scalar_set_b32(&sec, seckey, &overflow); /* Fail if the secret key is invalid. */ - overflow |= secp256k1_scalar_is_zero(&sec); - secp256k1_scalar_cmov(&sec, &secp256k1_scalar_one, overflow); + is_sec_valid = secp256k1_scalar_set_b32_seckey(&sec, seckey); + secp256k1_scalar_cmov(&sec, &secp256k1_scalar_one, !is_sec_valid); secp256k1_scalar_set_b32(&msg, msg32, NULL); while (1) { - int koverflow; - ret = noncefp(nonce32, msg32, seckey, NULL, (void*)noncedata, count); + int is_nonce_valid; + ret = !!noncefp(nonce32, msg32, seckey, NULL, (void*)noncedata, count); if (!ret) { break; } - secp256k1_scalar_set_b32(&non, nonce32, &koverflow); - koverflow |= secp256k1_scalar_is_zero(&non); - /* The nonce is still secret here, but it overflowing or being zero is is less likely than 1:2^255. */ - secp256k1_declassify(ctx, &koverflow, sizeof(koverflow)); - if (!koverflow) { + is_nonce_valid = secp256k1_scalar_set_b32_seckey(&non, nonce32); + /* The nonce is still secret here, but it being invalid is is less likely than 1:2^255. */ + secp256k1_declassify(ctx, &is_nonce_valid, sizeof(is_nonce_valid)); + if (is_nonce_valid) { ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL); /* The final signature is no longer a secret, nor is the fact that we were successful or not. */ secp256k1_declassify(ctx, &ret, sizeof(ret)); @@ -508,25 +506,27 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature } count++; } + /* We don't want to declassify is_sec_valid and therefore the range of + * seckey. As a result is_sec_valid is included in ret only after ret was + * used as a branching variable. */ + ret &= is_sec_valid; memset(nonce32, 0, 32); secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); - secp256k1_scalar_cmov(&r, &secp256k1_scalar_zero, (!ret) | overflow); - secp256k1_scalar_cmov(&s, &secp256k1_scalar_zero, (!ret) | overflow); + secp256k1_scalar_cmov(&r, &secp256k1_scalar_zero, !ret); + secp256k1_scalar_cmov(&s, &secp256k1_scalar_zero, !ret); secp256k1_ecdsa_signature_save(signature, &r, &s); - return !!ret & !overflow; + return ret; } int secp256k1_ec_seckey_verify(const secp256k1_context* ctx, const unsigned char *seckey) { secp256k1_scalar sec; int ret; - int overflow; VERIFY_CHECK(ctx != NULL); ARG_CHECK(seckey != NULL); - secp256k1_scalar_set_b32(&sec, seckey, &overflow); - ret = !overflow & !secp256k1_scalar_is_zero(&sec); + ret = secp256k1_scalar_set_b32_seckey(&sec, seckey); secp256k1_scalar_clear(&sec); return ret; } @@ -535,7 +535,6 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p secp256k1_gej pj; secp256k1_ge p; secp256k1_scalar sec; - int overflow; int ret = 0; VERIFY_CHECK(ctx != NULL); ARG_CHECK(pubkey != NULL); @@ -543,8 +542,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); ARG_CHECK(seckey != NULL); - secp256k1_scalar_set_b32(&sec, seckey, &overflow); - ret = !overflow & !secp256k1_scalar_is_zero(&sec); + ret = secp256k1_scalar_set_b32_seckey(&sec, seckey); secp256k1_scalar_cmov(&sec, &secp256k1_scalar_one, !ret); secp256k1_ecmult_gen(&ctx->ecmult_gen_ctx, &pj, &sec); From 8f814cddb94f4018646569143d10ebbb98daa454 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 8 Oct 2019 09:11:16 +0000 Subject: [PATCH 03/10] Add test for boundary conditions of scalar_set_b32 with respect to overflows --- src/tests.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/tests.c b/src/tests.c index 0a0cb5268a2fb..ef166e4afecd1 100644 --- a/src/tests.c +++ b/src/tests.c @@ -1123,16 +1123,43 @@ void run_scalar_tests(void) { #ifndef USE_NUM_NONE { - /* A scalar with value of the curve order should be 0. */ + /* Test secp256k1_scalar_set_b32 boundary conditions */ secp256k1_num order; - secp256k1_scalar zero; + secp256k1_scalar scalar; unsigned char bin[32]; + unsigned char bin_tmp[32]; int overflow = 0; + /* 2^256-1 - order */ + static const secp256k1_scalar all_ones_minus_order = SECP256K1_SCALAR_CONST( + 0x00000000UL, 0x00000000UL, 0x00000000UL, 0x00000001UL, + 0x45512319UL, 0x50B75FC4UL, 0x402DA173UL, 0x2FC9BEBEUL + ); + + /* A scalar set to 0s should be 0. */ + memset(bin, 0, 32); + secp256k1_scalar_set_b32(&scalar, bin, &overflow); + CHECK(overflow == 0); + CHECK(secp256k1_scalar_is_zero(&scalar)); + + /* A scalar with value of the curve order should be 0. */ secp256k1_scalar_order_get_num(&order); secp256k1_num_get_bin(bin, 32, &order); - secp256k1_scalar_set_b32(&zero, bin, &overflow); + secp256k1_scalar_set_b32(&scalar, bin, &overflow); + CHECK(overflow == 1); + CHECK(secp256k1_scalar_is_zero(&scalar)); + + /* A scalar with value of the curve order minus one should not overflow. */ + bin[31] -= 1; + secp256k1_scalar_set_b32(&scalar, bin, &overflow); + CHECK(overflow == 0); + secp256k1_scalar_get_b32(bin_tmp, &scalar); + CHECK(memcmp(bin, bin_tmp, 32) == 0); + + /* A scalar set to all 1s should overflow. */ + memset(bin, 0xFF, 32); + secp256k1_scalar_set_b32(&scalar, bin, &overflow); CHECK(overflow == 1); - CHECK(secp256k1_scalar_is_zero(&zero)); + CHECK(secp256k1_scalar_eq(&scalar, &all_ones_minus_order)); } #endif From 5894e1f1df74b23435cfd1e9a8b25f22ac631ebe Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 16:52:07 +0000 Subject: [PATCH 04/10] Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul --- include/secp256k1.h | 21 ++++++++++------- src/eckey_impl.h | 5 +--- src/secp256k1.c | 14 ++++++----- src/tests.c | 57 +++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 77 insertions(+), 20 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 1678406610e68..816374a4674fd 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -579,9 +579,12 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( /** Negates a private key in place. * - * Returns: 1 always + * Returns: 0 if the given private key is invalid according to + * secp256k1_ec_seckey_verify. 1 otherwise * Args: ctx: pointer to a context object - * In/Out: seckey: pointer to the 32-byte private key to be negated (cannot be NULL) + * In/Out: seckey: pointer to the 32-byte private key to be negated. The private + * key should be valid according to secp256k1_ec_seckey_verify + * (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_negate( const secp256k1_context* ctx, @@ -601,9 +604,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( /** Tweak a private key by adding tweak to it. * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or if the resulting private key - * would be invalid (only when the tweak is the complement of the - * private key). 1 otherwise. + * uniformly random 32-byte arrays, or if the given private key is + * invalid according to secp256k1_ec_seckey_verify, or if the resulting + * private key would be invalid (only when the tweak is the complement + * of the private key). 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). * In/Out: seckey: pointer to a 32-byte private key. * In: tweak: pointer to a 32-byte tweak. @@ -616,9 +620,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( /** Tweak a public key by adding tweak times the generator to it. * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or if the resulting public key - * would be invalid (only when the tweak is the complement of the - * corresponding private key). 1 otherwise. + * uniformly random 32-byte arrays, or if the given private key is + * invalid according to secp256k1_ec_seckey_verify, or if the resulting + * public key would be invalid (only when the tweak is the complement + * of the corresponding private key). 1 otherwise. * Args: ctx: pointer to a context object initialized for validation * (cannot be NULL). * In/Out: pubkey: pointer to a public key object. diff --git a/src/eckey_impl.h b/src/eckey_impl.h index 00e4a86fc5bd7..e2e72d93039a7 100644 --- a/src/eckey_impl.h +++ b/src/eckey_impl.h @@ -54,10 +54,7 @@ static int secp256k1_eckey_pubkey_serialize(secp256k1_ge *elem, unsigned char *p static int secp256k1_eckey_privkey_tweak_add(secp256k1_scalar *key, const secp256k1_scalar *tweak) { secp256k1_scalar_add(key, key, tweak); - if (secp256k1_scalar_is_zero(key)) { - return 0; - } - return 1; + return !secp256k1_scalar_is_zero(key); } static int secp256k1_eckey_pubkey_tweak_add(const secp256k1_ecmult_context *ctx, secp256k1_ge *key, const secp256k1_scalar *tweak) { diff --git a/src/secp256k1.c b/src/secp256k1.c index 20cf3c7e05b57..d205e3072e508 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -556,15 +556,17 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p int secp256k1_ec_privkey_negate(const secp256k1_context* ctx, unsigned char *seckey) { secp256k1_scalar sec; + int ret = 0; VERIFY_CHECK(ctx != NULL); ARG_CHECK(seckey != NULL); - secp256k1_scalar_set_b32(&sec, seckey, NULL); + ret = secp256k1_scalar_set_b32_seckey(&sec, seckey); + secp256k1_scalar_cmov(&sec, &secp256k1_scalar_zero, !ret); secp256k1_scalar_negate(&sec, &sec); secp256k1_scalar_get_b32(seckey, &sec); secp256k1_scalar_clear(&sec); - return 1; + return ret; } int secp256k1_ec_pubkey_negate(const secp256k1_context* ctx, secp256k1_pubkey *pubkey) { @@ -592,9 +594,9 @@ int secp256k1_ec_privkey_tweak_add(const secp256k1_context* ctx, unsigned char * ARG_CHECK(tweak != NULL); secp256k1_scalar_set_b32(&term, tweak, &overflow); - secp256k1_scalar_set_b32(&sec, seckey, NULL); + ret = secp256k1_scalar_set_b32_seckey(&sec, seckey); - ret = (!overflow) & secp256k1_eckey_privkey_tweak_add(&sec, &term); + ret &= (!overflow) & secp256k1_eckey_privkey_tweak_add(&sec, &term); secp256k1_scalar_cmov(&sec, &secp256k1_scalar_zero, !ret); secp256k1_scalar_get_b32(seckey, &sec); @@ -637,8 +639,8 @@ int secp256k1_ec_privkey_tweak_mul(const secp256k1_context* ctx, unsigned char * ARG_CHECK(tweak != NULL); secp256k1_scalar_set_b32(&factor, tweak, &overflow); - secp256k1_scalar_set_b32(&sec, seckey, NULL); - ret = (!overflow) & secp256k1_eckey_privkey_tweak_mul(&sec, &factor); + ret = secp256k1_scalar_set_b32_seckey(&sec, seckey); + ret &= (!overflow) & secp256k1_eckey_privkey_tweak_mul(&sec, &factor); secp256k1_scalar_cmov(&sec, &secp256k1_scalar_zero, !ret); secp256k1_scalar_get_b32(seckey, &sec); diff --git a/src/tests.c b/src/tests.c index ef166e4afecd1..534f5bbfd6290 100644 --- a/src/tests.c +++ b/src/tests.c @@ -4000,7 +4000,20 @@ void run_eckey_edge_case_test(void) { CHECK(secp256k1_ec_pubkey_tweak_mul(ctx, &pubkey, ctmp2) == 0); CHECK(memcmp(&pubkey, zeros, sizeof(pubkey)) == 0); memcpy(&pubkey, &pubkey2, sizeof(pubkey)); - /* Overflowing key tweak zeroizes. */ + /* If seckey_tweak_add or seckey_tweak_mul are called with an overflowing + seckey, the seckey is zeroized. */ + memcpy(ctmp, orderc, 32); + memset(ctmp2, 0, 32); + ctmp2[31] = 0x01; + CHECK(secp256k1_ec_seckey_verify(ctx, ctmp2) == 1); + CHECK(secp256k1_ec_seckey_verify(ctx, ctmp) == 0); + CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 0); + CHECK(memcmp(zeros, ctmp, 32) == 0); + memcpy(ctmp, orderc, 32); + CHECK(secp256k1_ec_privkey_tweak_mul(ctx, ctmp, ctmp2) == 0); + CHECK(memcmp(zeros, ctmp, 32) == 0); + /* If seckey_tweak_add or seckey_tweak_mul are called with an overflowing + tweak, the seckey is zeroized. */ memcpy(ctmp, orderc, 32); ctmp[31] = 0x40; CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, orderc) == 0); @@ -4011,13 +4024,20 @@ void run_eckey_edge_case_test(void) { CHECK(memcmp(zeros, ctmp, 32) == 0); memcpy(ctmp, orderc, 32); ctmp[31] = 0x40; + /* If pubkey_tweak_add or pubkey_tweak_mul are called with an overflowing + tweak, the pubkey is zeroized. */ CHECK(secp256k1_ec_pubkey_tweak_add(ctx, &pubkey, orderc) == 0); CHECK(memcmp(&pubkey, zeros, sizeof(pubkey)) == 0); memcpy(&pubkey, &pubkey2, sizeof(pubkey)); CHECK(secp256k1_ec_pubkey_tweak_mul(ctx, &pubkey, orderc) == 0); CHECK(memcmp(&pubkey, zeros, sizeof(pubkey)) == 0); memcpy(&pubkey, &pubkey2, sizeof(pubkey)); - /* Private key tweaks results in a key of zero. */ + /* If the resulting key in secp256k1_ec_seckey_tweak_add and + * secp256k1_ec_pubkey_tweak_add is 0 the functions fail and in the latter + * case the pubkey is zeroized. */ + memcpy(ctmp, orderc, 32); + ctmp[31] = 0x40; + memset(ctmp2, 0, 32); ctmp2[31] = 1; CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp2, ctmp) == 0); CHECK(memcmp(zeros, ctmp2, 32) == 0); @@ -4157,6 +4177,36 @@ void run_eckey_edge_case_test(void) { secp256k1_context_set_illegal_callback(ctx, NULL, NULL); } +void run_eckey_negate_test(void) { + unsigned char seckey[32]; + unsigned char seckey_tmp[32]; + + random_scalar_order_b32(seckey); + memcpy(seckey_tmp, seckey, 32); + + /* Verify negation changes the key and changes it back */ + CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 1); + CHECK(memcmp(seckey, seckey_tmp, 32) != 0); + CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 1); + CHECK(memcmp(seckey, seckey_tmp, 32) == 0); + + /* Negating all 0s fails */ + memset(seckey, 0, 32); + memset(seckey_tmp, 0, 32); + CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 0); + /* Check that seckey is not modified */ + CHECK(memcmp(seckey, seckey_tmp, 32) == 0); + + /* Negating an overflowing seckey fails and the seckey is zeroed. In this + * test, the seckey has 16 random bytes to ensure that ec_privkey_negate + * doesn't just set seckey to a constant value in case of failure. */ + random_scalar_order_b32(seckey); + memset(seckey, 0xFF, 16); + memset(seckey_tmp, 0, 32); + CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 0); + CHECK(memcmp(seckey, seckey_tmp, 32) == 0); +} + void random_sign(secp256k1_scalar *sigr, secp256k1_scalar *sigs, const secp256k1_scalar *key, const secp256k1_scalar *msg, int *recid) { secp256k1_scalar nonce; do { @@ -5347,6 +5397,9 @@ int main(int argc, char **argv) { /* EC key edge cases */ run_eckey_edge_case_test(); + /* EC key arithmetic test */ + run_eckey_negate_test(); + #ifdef ENABLE_MODULE_ECDH /* ecdh tests */ run_ecdh_tests(); From f03df0e6d7d272808c9d27df40bb92716b341d03 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 17:05:42 +0000 Subject: [PATCH 05/10] Define valid ECDSA keys in the documentation of seckey_verify --- include/secp256k1.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/secp256k1.h b/include/secp256k1.h index 816374a4674fd..785525b67c2e9 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -552,6 +552,11 @@ SECP256K1_API int secp256k1_ecdsa_sign( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); /** Verify an ECDSA secret key. + * + * A secret key is valid if it is not 0 and less than the secp256k1 curve order + * when interpreted as an integer (most significant byte first). The + * probability of choosing a 32-byte string uniformly at random which is an + * invalid secret key is negligible. * * Returns: 1: secret key is valid * 0: secret key is invalid From 5a73f14d6cfab043f94b2032d6958e15f84065fe Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 17:06:03 +0000 Subject: [PATCH 06/10] Mention that value is unspecified for In/Out parameters if the function returns 0 --- include/secp256k1.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 785525b67c2e9..ca4164ebe5bf3 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -588,7 +588,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( * secp256k1_ec_seckey_verify. 1 otherwise * Args: ctx: pointer to a context object * In/Out: seckey: pointer to the 32-byte private key to be negated. The private - * key should be valid according to secp256k1_ec_seckey_verify + * key should be valid according to secp256k1_ec_seckey_verify. + * Value becomes unspecified if this function returns 0. * (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_negate( @@ -614,7 +615,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( * private key would be invalid (only when the tweak is the complement * of the private key). 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte private key. + * In/Out: seckey: pointer to a 32-byte private key. Value becomes unspecified if this + * function returns 0. (cannot be NULL) * In: tweak: pointer to a 32-byte tweak. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( @@ -631,7 +633,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( * of the corresponding private key). 1 otherwise. * Args: ctx: pointer to a context object initialized for validation * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. + * In/Out: pubkey: pointer to a public key object. Value becomes unspecified if this + * function returns 0. (cannot be NULL). * In: tweak: pointer to a 32-byte tweak. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( @@ -644,7 +647,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for * uniformly random 32-byte arrays, or equal to zero. 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte private key. + * In/Out: seckey: pointer to a 32-byte private key. Value becomes unspecified if this + * function returns 0. (cannot be NULL). * In: tweak: pointer to a 32-byte tweak. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( @@ -658,7 +662,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( * uniformly random 32-byte arrays, or equal to zero. 1 otherwise. * Args: ctx: pointer to a context object initialized for validation * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. + * In/Out: pubkey: pointer to a public key object. Value becomes unspecified if this + * function returns 0. (cannot be NULL). * In: tweak: pointer to a 32-byte tweak. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( From 22911ee6da7197c45226ab4bcd84b8c2773baf9f Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Tue, 17 Dec 2019 17:10:11 +0000 Subject: [PATCH 07/10] Rename private key to secret key in public API (with the exception of function names) --- include/secp256k1.h | 80 +++++++++++++++++++----------------- include/secp256k1_ecdh.h | 4 +- include/secp256k1_recovery.h | 2 +- 3 files changed, 46 insertions(+), 40 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index ca4164ebe5bf3..076a7962d3f5a 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -14,7 +14,7 @@ extern "C" { * 2. Array lengths always immediately the follow the argument whose length * they describe, even if this violates rule 1. * 3. Within the OUT/OUTIN/IN groups, pointers to data that is typically generated - * later go first. This means: signatures, public nonces, private nonces, + * later go first. This means: signatures, public nonces, secret nonces, * messages, public keys, secret keys, tweaks. * 4. Arguments that are not data pointers go last, from more complex to less * complex: function pointers, algorithm names, messages, void pointers, @@ -531,7 +531,7 @@ SECP256K1_API extern const secp256k1_nonce_function secp256k1_nonce_function_def /** Create an ECDSA signature. * * Returns: 1: signature created - * 0: the nonce generation function failed, or the private key was invalid. + * 0: the nonce generation function failed, or the secret key was invalid. * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) * Out: sig: pointer to an array where the signature will be placed (cannot be NULL) * In: msg32: the 32-byte message hash being signed (cannot be NULL) @@ -574,7 +574,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_verify( * 0: secret was invalid, try again * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) * Out: pubkey: pointer to the created public key (cannot be NULL) - * In: seckey: pointer to a 32-byte private key (cannot be NULL) + * In: seckey: pointer to a 32-byte secret key (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( const secp256k1_context* ctx, @@ -582,15 +582,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( const unsigned char *seckey ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); -/** Negates a private key in place. +/** Negates a secret key in place. * - * Returns: 0 if the given private key is invalid according to + * Returns: 0 if the given secret key is invalid according to * secp256k1_ec_seckey_verify. 1 otherwise * Args: ctx: pointer to a context object - * In/Out: seckey: pointer to the 32-byte private key to be negated. The private + * In/Out: seckey: pointer to the 32-byte secret key to be negated. The secret * key should be valid according to secp256k1_ec_seckey_verify. - * Value becomes unspecified if this function returns 0. - * (cannot be NULL) + * If this function returns 0, seckey will be some + * unspecified value. (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_negate( const secp256k1_context* ctx, @@ -608,16 +608,18 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( secp256k1_pubkey *pubkey ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); -/** Tweak a private key by adding tweak to it. - * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or if the given private key is - * invalid according to secp256k1_ec_seckey_verify, or if the resulting - * private key would be invalid (only when the tweak is the complement - * of the private key). 1 otherwise. +/** Tweak a secret key by adding tweak to it. + * Returns: 0 if the resulting secret key would be invalid (only when the tweak + * is the negation of the secret key). 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte private key. Value becomes unspecified if this - * function returns 0. (cannot be NULL) - * In: tweak: pointer to a 32-byte tweak. + * In/Out: seckey: pointer to a 32-byte secret key. The secret key should be + * valid according to secp256k1_ec_seckey_verify. If this + * function returns 0, seckey will be some unspecified + * value. (cannot be NULL) + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( const secp256k1_context* ctx, @@ -626,16 +628,16 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Tweak a public key by adding tweak times the generator to it. - * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or if the given private key is - * invalid according to secp256k1_ec_seckey_verify, or if the resulting - * public key would be invalid (only when the tweak is the complement - * of the corresponding private key). 1 otherwise. + * Returns: 0 if the resulting public key would be invalid (only when the tweak + * is the negation of the corresponding secret key). 1 otherwise. * Args: ctx: pointer to a context object initialized for validation * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. Value becomes unspecified if this - * function returns 0. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. + * In/Out: pubkey: pointer to a public key object. If this function returns 0, + * pubkey will be invalid. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const secp256k1_context* ctx, @@ -643,13 +645,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const unsigned char *tweak ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); -/** Tweak a private key by multiplying it by a tweak. - * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or equal to zero. 1 otherwise. +/** Tweak a secret key by multiplying it by a tweak. + * Returns: 0 if the arguments are invalid.. 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte private key. Value becomes unspecified if this - * function returns 0. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. + * In/Out: seckey: pointer to a 32-byte secret key. If this function returns 0, + * seckey will be some unspecified value. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( const secp256k1_context* ctx, @@ -658,13 +662,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Tweak a public key by multiplying it by a tweak value. - * Returns: 0 if the tweak was out of range (chance of around 1 in 2^128 for - * uniformly random 32-byte arrays, or equal to zero. 1 otherwise. + * Returns: 0 if the arguments are invalid. 1 otherwise. * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. Value becomes unspecified if this - * function returns 0. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. + * (cannot be NULL). + * In/Out: pubkey: pointer to a public key object. If this function returns 0, + * pubkey will be invalid. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( const secp256k1_context* ctx, diff --git a/include/secp256k1_ecdh.h b/include/secp256k1_ecdh.h index 59b74091f75bd..4058e9c0436dd 100644 --- a/include/secp256k1_ecdh.h +++ b/include/secp256k1_ecdh.h @@ -41,7 +41,7 @@ SECP256K1_API extern const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_func * Out: output: pointer to an array to be filled by hashfp * In: pubkey: a pointer to a secp256k1_pubkey containing an * initialized public key - * privkey: a 32-byte scalar with which to multiply the point + * seckey: 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 * (in which case, 32 bytes will be written to output) * data: arbitrary data pointer that is passed through to hashfp @@ -50,7 +50,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdh( const secp256k1_context* ctx, unsigned char *output, const secp256k1_pubkey *pubkey, - const unsigned char *privkey, + const unsigned char *seckey, secp256k1_ecdh_hash_function hashfp, void *data ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); diff --git a/include/secp256k1_recovery.h b/include/secp256k1_recovery.h index cf6c5ed7f5e3d..f8ccaecd3dfb1 100644 --- a/include/secp256k1_recovery.h +++ b/include/secp256k1_recovery.h @@ -70,7 +70,7 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact( /** Create a recoverable ECDSA signature. * * Returns: 1: signature created - * 0: the nonce generation function failed, or the private key was invalid. + * 0: the nonce generation function failed, or the secret key was invalid. * Args: ctx: pointer to a context object, initialized for signing (cannot be NULL) * Out: sig: pointer to an array where the signature will be placed (cannot be NULL) * In: msg32: the 32-byte message hash being signed (cannot be NULL) From 41fc78560223aa035d9fe2cbeedb3ee632c740e2 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Thu, 19 Dec 2019 15:02:29 +0000 Subject: [PATCH 08/10] Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul --- include/secp256k1.h | 23 +++++++++++++++ src/secp256k1.c | 18 ++++++++++-- src/tests.c | 61 +++++++++++++++++++++++++-------------- src/valgrind_ctime_test.c | 6 ++-- 4 files changed, 81 insertions(+), 27 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 076a7962d3f5a..36051b540eed5 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -592,6 +592,13 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( * If this function returns 0, seckey will be some * unspecified value. (cannot be NULL) */ +SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_negate( + const secp256k1_context* ctx, + unsigned char *seckey +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); + +/** Same as secp256k1_ec_seckey_negate, but DEPRECATED. Will be removed in + * future versions. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_negate( const secp256k1_context* ctx, unsigned char *seckey @@ -621,6 +628,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( * 32-byte arrays the chance of being out of range is * negligible (around 1 in 2^128). (cannot be NULL) */ +SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add( + const secp256k1_context* ctx, + unsigned char *seckey, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); + +/** Same as secp256k1_ec_seckey_tweak_add, but DEPRECATED. Will be removed in + * future versions. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( const secp256k1_context* ctx, unsigned char *seckey, @@ -655,6 +670,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( * 32-byte arrays the chance of being out of range is * negligible (around 1 in 2^128). (cannot be NULL) */ +SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_mul( + const secp256k1_context* ctx, + unsigned char *seckey, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); + +/** Same as secp256k1_ec_seckey_tweak_mul, but DEPRECATED. Will be removed in + * future versions. */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( const secp256k1_context* ctx, unsigned char *seckey, diff --git a/src/secp256k1.c b/src/secp256k1.c index d205e3072e508..de66be5785ab5 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -554,7 +554,7 @@ int secp256k1_ec_pubkey_create(const secp256k1_context* ctx, secp256k1_pubkey *p return ret; } -int secp256k1_ec_privkey_negate(const secp256k1_context* ctx, unsigned char *seckey) { +int secp256k1_ec_seckey_negate(const secp256k1_context* ctx, unsigned char *seckey) { secp256k1_scalar sec; int ret = 0; VERIFY_CHECK(ctx != NULL); @@ -569,6 +569,10 @@ int secp256k1_ec_privkey_negate(const secp256k1_context* ctx, unsigned char *sec return ret; } +int secp256k1_ec_privkey_negate(const secp256k1_context* ctx, unsigned char *seckey) { + return secp256k1_ec_seckey_negate(ctx, seckey); +} + int secp256k1_ec_pubkey_negate(const secp256k1_context* ctx, secp256k1_pubkey *pubkey) { int ret = 0; secp256k1_ge p; @@ -584,7 +588,7 @@ int secp256k1_ec_pubkey_negate(const secp256k1_context* ctx, secp256k1_pubkey *p return ret; } -int secp256k1_ec_privkey_tweak_add(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { +int secp256k1_ec_seckey_tweak_add(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { secp256k1_scalar term; secp256k1_scalar sec; int ret = 0; @@ -605,6 +609,10 @@ int secp256k1_ec_privkey_tweak_add(const secp256k1_context* ctx, unsigned char * return ret; } +int secp256k1_ec_privkey_tweak_add(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { + return secp256k1_ec_seckey_tweak_add(ctx, seckey, tweak); +} + int secp256k1_ec_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey *pubkey, const unsigned char *tweak) { secp256k1_ge p; secp256k1_scalar term; @@ -629,7 +637,7 @@ int secp256k1_ec_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey return ret; } -int secp256k1_ec_privkey_tweak_mul(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { +int secp256k1_ec_seckey_tweak_mul(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { secp256k1_scalar factor; secp256k1_scalar sec; int ret = 0; @@ -649,6 +657,10 @@ int secp256k1_ec_privkey_tweak_mul(const secp256k1_context* ctx, unsigned char * return ret; } +int secp256k1_ec_privkey_tweak_mul(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak) { + return secp256k1_ec_seckey_tweak_mul(ctx, seckey, tweak); +} + int secp256k1_ec_pubkey_tweak_mul(const secp256k1_context* ctx, secp256k1_pubkey *pubkey, const unsigned char *tweak) { secp256k1_ge p; secp256k1_scalar factor; diff --git a/src/tests.c b/src/tests.c index 534f5bbfd6290..324b60cf1e79f 100644 --- a/src/tests.c +++ b/src/tests.c @@ -3989,13 +3989,13 @@ void run_eckey_edge_case_test(void) { pubkey_negone = pubkey; /* Tweak of zero leaves the value unchanged. */ memset(ctmp2, 0, 32); - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 1); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp, ctmp2) == 1); CHECK(memcmp(orderc, ctmp, 31) == 0 && ctmp[31] == 0x40); memcpy(&pubkey2, &pubkey, sizeof(pubkey)); CHECK(secp256k1_ec_pubkey_tweak_add(ctx, &pubkey, ctmp2) == 1); CHECK(memcmp(&pubkey, &pubkey2, sizeof(pubkey)) == 0); /* Multiply tweak of zero zeroizes the output. */ - CHECK(secp256k1_ec_privkey_tweak_mul(ctx, ctmp, ctmp2) == 0); + CHECK(secp256k1_ec_seckey_tweak_mul(ctx, ctmp, ctmp2) == 0); CHECK(memcmp(zeros, ctmp, 32) == 0); CHECK(secp256k1_ec_pubkey_tweak_mul(ctx, &pubkey, ctmp2) == 0); CHECK(memcmp(&pubkey, zeros, sizeof(pubkey)) == 0); @@ -4007,20 +4007,20 @@ void run_eckey_edge_case_test(void) { ctmp2[31] = 0x01; CHECK(secp256k1_ec_seckey_verify(ctx, ctmp2) == 1); CHECK(secp256k1_ec_seckey_verify(ctx, ctmp) == 0); - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, ctmp2) == 0); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp, ctmp2) == 0); CHECK(memcmp(zeros, ctmp, 32) == 0); memcpy(ctmp, orderc, 32); - CHECK(secp256k1_ec_privkey_tweak_mul(ctx, ctmp, ctmp2) == 0); + CHECK(secp256k1_ec_seckey_tweak_mul(ctx, ctmp, ctmp2) == 0); CHECK(memcmp(zeros, ctmp, 32) == 0); /* If seckey_tweak_add or seckey_tweak_mul are called with an overflowing tweak, the seckey is zeroized. */ memcpy(ctmp, orderc, 32); ctmp[31] = 0x40; - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, orderc) == 0); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp, orderc) == 0); CHECK(memcmp(zeros, ctmp, 32) == 0); memcpy(ctmp, orderc, 32); ctmp[31] = 0x40; - CHECK(secp256k1_ec_privkey_tweak_mul(ctx, ctmp, orderc) == 0); + CHECK(secp256k1_ec_seckey_tweak_mul(ctx, ctmp, orderc) == 0); CHECK(memcmp(zeros, ctmp, 32) == 0); memcpy(ctmp, orderc, 32); ctmp[31] = 0x40; @@ -4039,7 +4039,7 @@ void run_eckey_edge_case_test(void) { ctmp[31] = 0x40; memset(ctmp2, 0, 32); ctmp2[31] = 1; - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp2, ctmp) == 0); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp2, ctmp) == 0); CHECK(memcmp(zeros, ctmp2, 32) == 0); ctmp2[31] = 1; CHECK(secp256k1_ec_pubkey_tweak_add(ctx, &pubkey, ctmp2) == 0); @@ -4047,7 +4047,7 @@ void run_eckey_edge_case_test(void) { memcpy(&pubkey, &pubkey2, sizeof(pubkey)); /* Tweak computation wraps and results in a key of 1. */ ctmp2[31] = 2; - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp2, ctmp) == 1); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp2, ctmp) == 1); CHECK(memcmp(ctmp2, zeros, 31) == 0 && ctmp2[31] == 1); ctmp2[31] = 2; CHECK(secp256k1_ec_pubkey_tweak_add(ctx, &pubkey, ctmp2) == 1); @@ -4095,16 +4095,16 @@ void run_eckey_edge_case_test(void) { CHECK(ecount == 2); ecount = 0; memset(ctmp2, 0, 32); - CHECK(secp256k1_ec_privkey_tweak_add(ctx, NULL, ctmp2) == 0); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, NULL, ctmp2) == 0); CHECK(ecount == 1); - CHECK(secp256k1_ec_privkey_tweak_add(ctx, ctmp, NULL) == 0); + CHECK(secp256k1_ec_seckey_tweak_add(ctx, ctmp, NULL) == 0); CHECK(ecount == 2); ecount = 0; memset(ctmp2, 0, 32); ctmp2[31] = 1; - CHECK(secp256k1_ec_privkey_tweak_mul(ctx, NULL, ctmp2) == 0); + CHECK(secp256k1_ec_seckey_tweak_mul(ctx, NULL, ctmp2) == 0); CHECK(ecount == 1); - CHECK(secp256k1_ec_privkey_tweak_mul(ctx, ctmp, NULL) == 0); + CHECK(secp256k1_ec_seckey_tweak_mul(ctx, ctmp, NULL) == 0); CHECK(ecount == 2); ecount = 0; CHECK(secp256k1_ec_pubkey_create(ctx, NULL, ctmp) == 0); @@ -4184,26 +4184,31 @@ void run_eckey_negate_test(void) { random_scalar_order_b32(seckey); memcpy(seckey_tmp, seckey, 32); - /* Verify negation changes the key and changes it back */ - CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 1); + /* Verify negation changes the key and changes it back */ + CHECK(secp256k1_ec_seckey_negate(ctx, seckey) == 1); CHECK(memcmp(seckey, seckey_tmp, 32) != 0); - CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 1); + CHECK(secp256k1_ec_seckey_negate(ctx, seckey) == 1); + CHECK(memcmp(seckey, seckey_tmp, 32) == 0); + + /* Check that privkey alias gives same result */ + CHECK(secp256k1_ec_seckey_negate(ctx, seckey) == 1); + CHECK(secp256k1_ec_privkey_negate(ctx, seckey_tmp) == 1); CHECK(memcmp(seckey, seckey_tmp, 32) == 0); - /* Negating all 0s fails */ + /* Negating all 0s fails */ memset(seckey, 0, 32); memset(seckey_tmp, 0, 32); - CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 0); + CHECK(secp256k1_ec_seckey_negate(ctx, seckey) == 0); /* Check that seckey is not modified */ CHECK(memcmp(seckey, seckey_tmp, 32) == 0); /* Negating an overflowing seckey fails and the seckey is zeroed. In this - * test, the seckey has 16 random bytes to ensure that ec_privkey_negate + * test, the seckey has 16 random bytes to ensure that ec_seckey_negate * doesn't just set seckey to a constant value in case of failure. */ random_scalar_order_b32(seckey); memset(seckey, 0xFF, 16); memset(seckey_tmp, 0, 32); - CHECK(secp256k1_ec_privkey_negate(ctx, seckey) == 0); + CHECK(secp256k1_ec_seckey_negate(ctx, seckey) == 0); CHECK(memcmp(seckey, seckey_tmp, 32) == 0); } @@ -4346,15 +4351,22 @@ void test_ecdsa_end_to_end(void) { if (secp256k1_rand_int(3) == 0) { int ret1; int ret2; + int ret3; unsigned char rnd[32]; + unsigned char privkey_tmp[32]; secp256k1_pubkey pubkey2; secp256k1_rand256_test(rnd); - ret1 = secp256k1_ec_privkey_tweak_add(ctx, privkey, rnd); + memcpy(privkey_tmp, privkey, 32); + ret1 = secp256k1_ec_seckey_tweak_add(ctx, privkey, rnd); ret2 = secp256k1_ec_pubkey_tweak_add(ctx, &pubkey, rnd); + /* Check that privkey alias gives same result */ + ret3 = secp256k1_ec_privkey_tweak_add(ctx, privkey_tmp, rnd); CHECK(ret1 == ret2); + CHECK(ret2 == ret3); if (ret1 == 0) { return; } + CHECK(memcmp(privkey, privkey_tmp, 32) == 0); CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey2, privkey) == 1); CHECK(memcmp(&pubkey, &pubkey2, sizeof(pubkey)) == 0); } @@ -4363,15 +4375,22 @@ void test_ecdsa_end_to_end(void) { if (secp256k1_rand_int(3) == 0) { int ret1; int ret2; + int ret3; unsigned char rnd[32]; + unsigned char privkey_tmp[32]; secp256k1_pubkey pubkey2; secp256k1_rand256_test(rnd); - ret1 = secp256k1_ec_privkey_tweak_mul(ctx, privkey, rnd); + memcpy(privkey_tmp, privkey, 32); + ret1 = secp256k1_ec_seckey_tweak_mul(ctx, privkey, rnd); ret2 = secp256k1_ec_pubkey_tweak_mul(ctx, &pubkey, rnd); + /* Check that privkey alias gives same result */ + ret3 = secp256k1_ec_privkey_tweak_mul(ctx, privkey_tmp, rnd); CHECK(ret1 == ret2); + CHECK(ret2 == ret3); if (ret1 == 0) { return; } + CHECK(memcmp(privkey, privkey_tmp, 32) == 0); CHECK(secp256k1_ec_pubkey_create(ctx, &pubkey2, privkey) == 1); CHECK(memcmp(&pubkey, &pubkey2, sizeof(pubkey)) == 0); } diff --git a/src/valgrind_ctime_test.c b/src/valgrind_ctime_test.c index 04c06d498fe58..5d26244dba0a4 100644 --- a/src/valgrind_ctime_test.c +++ b/src/valgrind_ctime_test.c @@ -73,19 +73,19 @@ int main(void) { CHECK(ret == 1); VALGRIND_MAKE_MEM_UNDEFINED(key, 32); - ret = secp256k1_ec_privkey_negate(ctx, key); + ret = secp256k1_ec_seckey_negate(ctx, key); VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); CHECK(ret == 1); VALGRIND_MAKE_MEM_UNDEFINED(key, 32); VALGRIND_MAKE_MEM_UNDEFINED(msg, 32); - ret = secp256k1_ec_privkey_tweak_add(ctx, key, msg); + ret = secp256k1_ec_seckey_tweak_add(ctx, key, msg); VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); CHECK(ret == 1); VALGRIND_MAKE_MEM_UNDEFINED(key, 32); VALGRIND_MAKE_MEM_UNDEFINED(msg, 32); - ret = secp256k1_ec_privkey_tweak_mul(ctx, key, msg); + ret = secp256k1_ec_seckey_tweak_mul(ctx, key, msg); VALGRIND_MAKE_MEM_DEFINED(&ret, sizeof(ret)); CHECK(ret == 1); From 89853a0f2e2b324bd32e78641aeaad6d1e427e81 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Thu, 19 Mar 2020 21:52:37 +0000 Subject: [PATCH 09/10] Make tweak function documentation more consistent. Do this by adding a newline after the first sentence and aligning the rest. --- include/secp256k1.h | 81 ++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 36051b540eed5..c3be2e71c6271 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -616,17 +616,18 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); /** Tweak a secret key by adding tweak to it. - * Returns: 0 if the resulting secret key would be invalid (only when the tweak - * is the negation of the secret key). 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte secret key. The secret key should be - * valid according to secp256k1_ec_seckey_verify. If this - * function returns 0, seckey will be some unspecified - * value. (cannot be NULL) - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * + * Returns: 0 if the resulting secret key would be invalid (only when the tweak + * is the negation of the secret key). 1 otherwise. + * Args: ctx: pointer to a context object (cannot be NULL). + * In/Out: seckey: pointer to a 32-byte secret key. The secret key should be + * valid according to secp256k1_ec_seckey_verify. If this + * function returns 0, seckey will be some unspecified + * value. (cannot be NULL) + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add( const secp256k1_context* ctx, @@ -643,16 +644,17 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Tweak a public key by adding tweak times the generator to it. - * Returns: 0 if the resulting public key would be invalid (only when the tweak - * is the negation of the corresponding secret key). 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. If this function returns 0, - * pubkey will be invalid. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * + * Returns: 0 if the resulting public key would be invalid (only when the tweak + * is the negation of the corresponding secret key). 1 otherwise. + * Args: ctx: pointer to a context object initialized for validation + * (cannot be NULL). + * In/Out: pubkey: pointer to a public key object. If this function returns 0, + * pubkey will be invalid. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const secp256k1_context* ctx, @@ -661,14 +663,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Tweak a secret key by multiplying it by a tweak. - * Returns: 0 if the arguments are invalid.. 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte secret key. If this function returns 0, - * seckey will be some unspecified value. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * + * Returns: 0 if the arguments are invalid.. 1 otherwise. + * Args: ctx: pointer to a context object (cannot be NULL). + * In/Out: seckey: pointer to a 32-byte secret key. If this function returns 0, + * seckey will be some unspecified value. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_mul( const secp256k1_context* ctx, @@ -685,15 +688,16 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Tweak a public key by multiplying it by a tweak value. - * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. If this function returns 0, - * pubkey will be invalid. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * + * Returns: 0 if the arguments are invalid. 1 otherwise. + * Args: ctx: pointer to a context object initialized for validation + * (cannot be NULL). + * In/Out: pubkey: pointer to a public key object. If this function returns 0, + * pubkey will be invalid. (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret + * keys (see secp256k1_ec_seckey_verify). For uniformly random + * 32-byte arrays the chance of being out of range is + * negligible (around 1 in 2^128). (cannot be NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( const secp256k1_context* ctx, @@ -732,6 +736,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize( ) SECP256K1_ARG_NONNULL(1); /** Add a number of public keys together. + * * Returns: 1: the sum of the public keys is valid. * 0: the sum of the public keys is not valid. * Args: ctx: pointer to a context object From 7e3952ae82af2a98619de74614f2d3f0ff072941 Mon Sep 17 00:00:00 2001 From: Jonas Nick Date: Fri, 20 Mar 2020 14:14:11 +0000 Subject: [PATCH 10/10] Clarify documentation of tweak functions. In particular, mention that the functions return 0 if seckey or tweak are invalid (as opposed to saying "should" or "must" be valid). --- include/secp256k1.h | 87 ++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index c3be2e71c6271..2ba2dca388150 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -586,11 +586,12 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( * * Returns: 0 if the given secret key is invalid according to * secp256k1_ec_seckey_verify. 1 otherwise - * Args: ctx: pointer to a context object - * In/Out: seckey: pointer to the 32-byte secret key to be negated. The secret - * key should be valid according to secp256k1_ec_seckey_verify. - * If this function returns 0, seckey will be some - * unspecified value. (cannot be NULL) + * Args: ctx: pointer to a context object + * In/Out: seckey: pointer to the 32-byte secret key to be negated. If the + * secret key is invalid according to + * secp256k1_ec_seckey_verify, this function returns 0 and + * seckey will be set to some unspecified value. (cannot be + * NULL) */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_negate( const secp256k1_context* ctx, @@ -617,17 +618,18 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( /** Tweak a secret key by adding tweak to it. * - * Returns: 0 if the resulting secret key would be invalid (only when the tweak - * is the negation of the secret key). 1 otherwise. - * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte secret key. The secret key should be - * valid according to secp256k1_ec_seckey_verify. If this - * function returns 0, seckey will be some unspecified - * value. (cannot be NULL) - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * Returns: 0 if the arguments are invalid or the resulting secret key would be + * invalid (only when the tweak is the negation of the secret key). 1 + * otherwise. + * Args: ctx: pointer to a context object (cannot be NULL). + * In/Out: seckey: pointer to a 32-byte secret key. If the secret key is + * invalid according to secp256k1_ec_seckey_verify, this + * function returns 0. seckey will be set to some unspecified + * value if this function returns 0. (cannot be NULL) + * In: tweak: pointer to a 32-byte tweak. If the tweak is invalid according to + * secp256k1_ec_seckey_verify, this function returns 0. For + * uniformly random 32-byte arrays the chance of being invalid + * is negligible (around 1 in 2^128) (cannot be NULL). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add( const secp256k1_context* ctx, @@ -645,16 +647,17 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( /** Tweak a public key by adding tweak times the generator to it. * - * Returns: 0 if the resulting public key would be invalid (only when the tweak - * is the negation of the corresponding secret key). 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. If this function returns 0, - * pubkey will be invalid. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * Returns: 0 if the arguments are invalid or the resulting public key would be + * invalid (only when the tweak is the negation of the corresponding + * secret key). 1 otherwise. + * Args: ctx: pointer to a context object initialized for validation + * (cannot be NULL). + * In/Out: pubkey: pointer to a public key object. pubkey will be set to an + * invalid value if this function returns 0 (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. If the tweak is invalid according to + * secp256k1_ec_seckey_verify, this function returns 0. For + * uniformly random 32-byte arrays the chance of being invalid + * is negligible (around 1 in 2^128) (cannot be NULL). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const secp256k1_context* ctx, @@ -664,14 +667,16 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( /** Tweak a secret key by multiplying it by a tweak. * - * Returns: 0 if the arguments are invalid.. 1 otherwise. + * Returns: 0 if the arguments are invalid. 1 otherwise. * Args: ctx: pointer to a context object (cannot be NULL). - * In/Out: seckey: pointer to a 32-byte secret key. If this function returns 0, - * seckey will be some unspecified value. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * In/Out: seckey: pointer to a 32-byte secret key. If the secret key is + * invalid according to secp256k1_ec_seckey_verify, this + * function returns 0. seckey will be set to some unspecified + * value if this function returns 0. (cannot be NULL) + * In: tweak: pointer to a 32-byte tweak. If the tweak is invalid according to + * secp256k1_ec_seckey_verify, this function returns 0. For + * uniformly random 32-byte arrays the chance of being invalid + * is negligible (around 1 in 2^128) (cannot be NULL). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_mul( const secp256k1_context* ctx, @@ -690,14 +695,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( /** Tweak a public key by multiplying it by a tweak value. * * Returns: 0 if the arguments are invalid. 1 otherwise. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. If this function returns 0, - * pubkey will be invalid. (cannot be NULL). - * In: tweak: pointer to a 32-byte tweak. Must be in the same range as secret - * keys (see secp256k1_ec_seckey_verify). For uniformly random - * 32-byte arrays the chance of being out of range is - * negligible (around 1 in 2^128). (cannot be NULL) + * Args: ctx: pointer to a context object initialized for validation + * (cannot be NULL). + * In/Out: pubkey: pointer to a public key object. pubkey will be set to an + * invalid value if this function returns 0 (cannot be NULL). + * In: tweak: pointer to a 32-byte tweak. If the tweak is invalid according to + * secp256k1_ec_seckey_verify, this function returns 0. For + * uniformly random 32-byte arrays the chance of being invalid + * is negligible (around 1 in 2^128) (cannot be NULL). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( const secp256k1_context* ctx,