diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -14,7 +14,7 @@ * 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 @@ /** 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) @@ -552,6 +552,11 @@ ) 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 @@ -569,7 +574,7 @@ * 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, @@ -577,12 +582,24 @@ 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: 1 always - * Args: ctx: pointer to a context object - * In/Out: seckey: pointer to the 32-byte private key to be negated (cannot be NULL) + * 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. 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, + 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 @@ -599,15 +616,29 @@ 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 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. - */ +/** Tweak a secret key by adding tweak to it. + * + * 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, + 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, @@ -615,14 +646,18 @@ ) 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 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 + * + * 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. - * In: tweak: pointer to a 32-byte tweak. + * 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, @@ -630,13 +665,27 @@ 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. - * 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. +/** 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 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, + 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, @@ -644,12 +693,16 @@ ) 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. - * Args: ctx: pointer to a context object initialized for validation - * (cannot be NULL). - * In/Out: pubkey: pointer to a public key object. - * In: tweak: pointer to a 32-byte tweak. + * + * 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. 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, @@ -688,6 +741,7 @@ ) 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 diff --git a/src/secp256k1/include/secp256k1_ecdh.h b/src/secp256k1/include/secp256k1_ecdh.h --- a/src/secp256k1/include/secp256k1_ecdh.h +++ b/src/secp256k1/include/secp256k1_ecdh.h @@ -41,7 +41,7 @@ * 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 @@ 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/src/secp256k1/include/secp256k1_recovery.h b/src/secp256k1/include/secp256k1_recovery.h --- a/src/secp256k1/include/secp256k1_recovery.h +++ b/src/secp256k1/include/secp256k1_recovery.h @@ -70,7 +70,7 @@ /** 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) diff --git a/src/secp256k1/src/eckey_impl.h b/src/secp256k1/src/eckey_impl.h --- a/src/secp256k1/src/eckey_impl.h +++ b/src/secp256k1/src/eckey_impl.h @@ -54,10 +54,7 @@ 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/src/scalar.h b/src/secp256k1/src/scalar.h --- a/src/secp256k1/src/scalar.h +++ b/src/secp256k1/src/scalar.h @@ -39,6 +39,10 @@ */ 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/secp256k1/src/scalar_impl.h b/src/secp256k1/src/scalar_impl.h --- a/src/secp256k1/src/scalar_impl.h +++ b/src/secp256k1/src/scalar_impl.h @@ -55,6 +55,12 @@ } #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/secp256k1/src/secp256k1.c b/src/secp256k1/src/secp256k1.c --- a/src/secp256k1/src/secp256k1.c +++ b/src/secp256k1/src/secp256k1.c @@ -471,7 +471,7 @@ 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; const unsigned char secp256k1_ecdsa_der_algo16[17] = "ECDSA+DER "; @@ -484,22 +484,20 @@ 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, secp256k1_ecdsa_der_algo16, (void*)noncedata, count); + int is_nonce_valid; + ret = !!noncefp(nonce32, msg32, seckey, secp256k1_ecdsa_der_algo16, (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)); @@ -509,25 +507,27 @@ } 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; } @@ -536,7 +536,6 @@ secp256k1_gej pj; secp256k1_ge p; secp256k1_scalar sec; - int overflow; int ret = 0; VERIFY_CHECK(ctx != NULL); ARG_CHECK(pubkey != NULL); @@ -544,8 +543,7 @@ 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); @@ -557,17 +555,23 @@ 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); 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_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) { @@ -585,7 +589,7 @@ 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; @@ -595,9 +599,9 @@ 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); @@ -606,6 +610,10 @@ 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; @@ -630,7 +638,7 @@ 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; @@ -640,8 +648,8 @@ 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); @@ -650,6 +658,10 @@ 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/secp256k1/src/tests.c b/src/secp256k1/src/tests.c --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -140,6 +140,12 @@ } 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 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. */ @@ -1097,16 +1123,43 @@ #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(&zero)); + 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_eq(&scalar, &all_ones_minus_order)); } #endif @@ -3935,37 +3988,57 @@ 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); 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_seckey_tweak_add(ctx, ctmp, ctmp2) == 0); + CHECK(memcmp(zeros, ctmp, 32) == 0); + memcpy(ctmp, orderc, 32); + 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; + /* 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(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); @@ -3973,7 +4046,7 @@ 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); @@ -4021,16 +4094,16 @@ 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); @@ -4103,6 +4176,41 @@ 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_seckey_negate(ctx, seckey) == 1); + CHECK(memcmp(seckey, seckey_tmp, 32) != 0); + 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 */ + memset(seckey, 0, 32); + memset(seckey_tmp, 0, 32); + 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_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_seckey_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 { @@ -4242,15 +4350,22 @@ 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); } @@ -4259,15 +4374,22 @@ 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); } @@ -5301,6 +5423,9 @@ /* 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(); diff --git a/src/secp256k1/src/valgrind_ctime_test.c b/src/secp256k1/src/valgrind_ctime_test.c --- a/src/secp256k1/src/valgrind_ctime_test.c +++ b/src/secp256k1/src/valgrind_ctime_test.c @@ -85,19 +85,19 @@ 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);