Skip to content

Commit

Permalink
Merge bitcoin#701: Make ec_ arithmetic more consistent and add docume…
Browse files Browse the repository at this point in the history 
…ntation

7e3952a Clarify documentation of tweak functions. (Jonas Nick)
89853a0 Make tweak function documentation more consistent. (Jonas Nick)
41fc785 Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul (Jonas Nick)
22911ee Rename private key to secret key in public API (with the exception of function names) (Jonas Nick)
5a73f14 Mention that value is unspecified for In/Out parameters if the function returns 0 (Jonas Nick)
f03df0e Define valid ECDSA keys in the documentation of seckey_verify (Jonas Nick)
5894e1f Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul (Jonas Nick)
8f814cd Add test for boundary conditions of scalar_set_b32 with respect to overflows (Jonas Nick)
3fec982 Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify (Jonas Nick)
9ab2cbe Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key (Jonas Nick)

Pull request description:

  Fixes bitcoin#671. Supersedes bitcoin#668.

  This PR unifies handling of invalid secret keys by introducing a new function `scalar_set_b32_secret` which returns false if the b32 overflows or is 0. By using this in `privkey_{negate, tweak_add, tweak_mul}` these function will now return 0 if the secret key is invalid which matches the behavior of `ecdsa_sign` and `pubkey_create`.

  Instead of deciding whether to zeroize the secret key on failure, I only added documentation for now that the value is undefined on failure.

ACKs for top commit:
  real-or-random:
    ACK 7e3952a I read the diff carefully and tested the changes
  apoelstra:
    ACK 7e3952a

Tree-SHA512: 8e9a66799cd3b6ec1c3acb731d6778035417e3dca9300d840e2437346ff0ac94f0c9be4de20aa2fac9bb4ae2f8a36d4e6a34795a640b9cfbfee8311decb102f0
  • Loading branch information
@real-or-random
real-or-random committed Apr 30, 2020
2 parents 39198a0 + 7e3952a commit f39f99b
Show file tree
Hide file tree
Showing 9 changed files with 290 additions and 92 deletions.
124 changes: 89 additions & 35 deletions include/secp256k1.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -569,20 +574,32 @@ 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,
secp256k1_pubkey *pubkey,
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
Expand All @@ -599,57 +616,93 @@ 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 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,
const unsigned char *tweak
) 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,
secp256k1_pubkey *pubkey,
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,
const unsigned char *tweak
) 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,
Expand Down Expand Up @@ -688,6 +741,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
Expand Down
4 changes: 2 additions & 2 deletions include/secp256k1_ecdh.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion include/secp256k1_recovery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions src/eckey_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions src/scalar.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
6 changes: 6 additions & 0 deletions src/scalar_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit f39f99b

Please sign in to comment.