Skip to content

Fix #1191: Naming convention for secret function arguments #1875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kallal79 wants to merge 1 commit into
base: master
Choose a base branch
from
+41 −40
Open

Fix #1191: Naming convention for secret function arguments #1875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ In addition, libsecp256k1 tries to maintain the following coding conventions:
* Arguments of the publicly-facing API must have a specific order defined in [include/secp256k1.h](include/secp256k1.h).
* User-facing comment lines in headers should be limited to 80 chars if possible.
* All identifiers in file scope should start with `secp256k1_`.
* Function arguments that are considered secret by the function (with respect to side channels, i.e., processed in constant time) should explicitly denote their secret nature using the `sec` prefix (e.g., `seckey`, `secnonce`, `sectweak32`, `secrand32`).
* Avoid trailing whitespace.
* Use the constants `EXIT_SUCCESS`/`EXIT_FAILURE` (defined in `stdlib.h`) to indicate program execution status for examples and other binaries.

Expand Down
16 changes: 8 additions & 8 deletions include/secp256k1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ typedef struct secp256k1_ecdsa_signature {
/** A pointer to a function to deterministically generate a nonce.
*
* Returns: 1 if a nonce was successfully generated. 0 will cause signing to fail.
* Out: nonce32: pointer to a 32-byte array to be filled by the function.
* Out: secnonce32: pointer to a 32-byte array to be filled by the function.
* In: msg32: the 32-byte message hash being verified (will not be NULL)
* key32: pointer to a 32-byte secret key (will not be NULL)
* seckey32: pointer to a 32-byte secret key (will not be NULL)
* algo16: pointer to a 16-byte array describing the signature
* algorithm (will be NULL for ECDSA for compatibility).
* data: Arbitrary data pointer that is passed through.
Expand All @@ -93,9 +93,9 @@ typedef struct secp256k1_ecdsa_signature {
* the message, the algorithm, the key and the attempt.
*/
typedef int (*secp256k1_nonce_function)(
unsigned char *nonce32,
unsigned char *secnonce32,
const unsigned char *msg32,
const unsigned char *key32,
const unsigned char *seckey32,
const unsigned char *algo16,
void *data,
unsigned int attempt
Expand Down Expand Up @@ -778,15 +778,15 @@ SECP256K1_API int secp256k1_ec_pubkey_negate(
* invalid according to secp256k1_ec_seckey_verify, this
* function returns 0. seckey will be set to some unspecified
* value if this function returns 0.
* In: tweak32: pointer to a 32-byte tweak, which must be valid according to
* In: sectweak32: pointer to a 32-byte tweak, which must be valid according to
* secp256k1_ec_seckey_verify or 32 zero bytes. For uniformly
* random 32-byte tweaks, the chance of being invalid is
* negligible (around 1 in 2^128).
*/
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add(
const secp256k1_context *ctx,
unsigned char *seckey,
const unsigned char *tweak32
const unsigned char *sectweak32
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);

/** Tweak a public key by adding tweak times the generator to it.
Expand Down Expand Up @@ -816,15 +816,15 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add(
* invalid according to secp256k1_ec_seckey_verify, this
* function returns 0. seckey will be set to some unspecified
* value if this function returns 0.
* In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to
* In: sectweak32: 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).
*/
SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_mul(
const secp256k1_context *ctx,
unsigned char *seckey,
const unsigned char *tweak32
const unsigned char *sectweak32
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3);

/** Tweak a public key by multiplying it by a tweak value.
Expand Down
20 changes: 10 additions & 10 deletions include/secp256k1_schnorrsig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ extern "C" {
*
* Returns: 1 if a nonce was successfully generated. 0 will cause signing to
* return an error.
* Out: nonce32: pointer to a 32-byte array to be filled by the function
* Out: secnonce32: pointer to a 32-byte array to be filled by the function
* In: msg: the message being verified. Is NULL if and only if msglen
* is 0.
* msglen: the length of the message
* key32: pointer to a 32-byte secret key (will not be NULL)
* xonly_pk32: the 32-byte serialized xonly pubkey corresponding to key32
* seckey32: pointer to a 32-byte secret key (will not be NULL)
* xonly_pk32: the 32-byte serialized xonly pubkey corresponding to seckey32
* (will not be NULL)
* algo: pointer to an array describing the signature
* algorithm (will not be NULL)
Expand All @@ -39,10 +39,10 @@ extern "C" {
* the message, the key, the pubkey, the algorithm description, and data.
*/
typedef int (*secp256k1_nonce_function_hardened)(
unsigned char *nonce32,
unsigned char *secnonce32,
const unsigned char *msg,
size_t msglen,
const unsigned char *key32,
const unsigned char *seckey32,
const unsigned char *xonly_pk32,
const unsigned char *algo,
size_t algolen,
Expand Down Expand Up @@ -110,7 +110,7 @@ typedef struct secp256k1_schnorrsig_extraparams {
* Out: sig64: pointer to a 64-byte array to store the serialized signature.
* In: msg32: the 32-byte message being signed.
* keypair: pointer to an initialized keypair.
* aux_rand32: 32 bytes of fresh randomness. While recommended to provide
* aux_secrand32: 32 bytes of fresh randomness. While recommended to provide
* this, it is only supplemental to security and can be NULL. A
* NULL argument is treated the same as an all-zero one. See
* BIP-340 "Default Signing" for a full explanation of this
Expand All @@ -121,7 +121,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign32(
unsigned char *sig64,
const unsigned char *msg32,
const secp256k1_keypair *keypair,
const unsigned char *aux_rand32
const unsigned char *aux_secrand32
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4);

/** Same as secp256k1_schnorrsig_sign32, but DEPRECATED. Will be removed in
Expand All @@ -131,7 +131,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign(
unsigned char *sig64,
const unsigned char *msg32,
const secp256k1_keypair *keypair,
const unsigned char *aux_rand32
const unsigned char *aux_secrand32
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4)
SECP256K1_DEPRECATED("Use secp256k1_schnorrsig_sign32 instead");

Expand All @@ -141,11 +141,11 @@ SECP256K1_API int secp256k1_schnorrsig_sign(
* variable length messages and accepts a pointer to an extraparams object that
* allows customizing signing by passing additional arguments.
*
* Equivalent to secp256k1_schnorrsig_sign32(..., aux_rand32) if msglen is 32
* Equivalent to secp256k1_schnorrsig_sign32(..., aux_secrand32) if msglen is 32
* and extraparams is initialized as follows:
* ```
* secp256k1_schnorrsig_extraparams extraparams = SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT;
* extraparams.ndata = (unsigned char*)aux_rand32;
* extraparams.ndata = (unsigned char*)aux_secrand32;
* ```
*
* Returns 1 on success, 0 on failure.
Expand Down
22 changes: 11 additions & 11 deletions src/modules/schnorrsig/main_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static const unsigned char bip340_algo[] = {'B', 'I', 'P', '0', '3', '4', '0', '

static const unsigned char schnorrsig_extraparams_magic[4] = SECP256K1_SCHNORRSIG_EXTRAPARAMS_MAGIC;

static int nonce_function_bip340_impl(const secp256k1_hash_ctx *hash_ctx, unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) {
static int nonce_function_bip340_impl(const secp256k1_hash_ctx *hash_ctx, unsigned char *secnonce32, const unsigned char *msg, size_t msglen, const unsigned char *seckey32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) {
secp256k1_sha256 sha;
unsigned char masked_key[32];
int i;
Expand All @@ -51,7 +51,7 @@ static int nonce_function_bip340_impl(const secp256k1_hash_ctx *hash_ctx, unsign
secp256k1_sha256_write(hash_ctx, &sha, data, 32);
secp256k1_sha256_finalize(hash_ctx, &sha, masked_key);
for (i = 0; i < 32; i++) {
masked_key[i] ^= key32[i];
masked_key[i] ^= seckey32[i];
}
} else {
/* Precomputed TaggedHash("BIP0340/aux", 0x0000...00); */
Expand All @@ -62,7 +62,7 @@ static int nonce_function_bip340_impl(const secp256k1_hash_ctx *hash_ctx, unsign
170, 247, 175, 105, 39, 10, 165, 20
};
for (i = 0; i < 32; i++) {
masked_key[i] = key32[i] ^ ZERO_MASK[i];
masked_key[i] = seckey32[i] ^ ZERO_MASK[i];
}
}

Expand All @@ -80,15 +80,15 @@ static int nonce_function_bip340_impl(const secp256k1_hash_ctx *hash_ctx, unsign
secp256k1_sha256_write(hash_ctx, &sha, masked_key, 32);
secp256k1_sha256_write(hash_ctx, &sha, xonly_pk32, 32);
secp256k1_sha256_write(hash_ctx, &sha, msg, msglen);
secp256k1_sha256_finalize(hash_ctx, &sha, nonce32);
secp256k1_sha256_finalize(hash_ctx, &sha, secnonce32);
secp256k1_sha256_clear(&sha);
secp256k1_memclear_explicit(masked_key, sizeof(masked_key));

return 1;
}

static int nonce_function_bip340(unsigned char *nonce32, const unsigned char *msg, size_t msglen, const unsigned char *key32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) {
return nonce_function_bip340_impl(secp256k1_get_hash_context(secp256k1_context_static), nonce32, msg, msglen, key32, xonly_pk32, algo, algolen, data);
static int nonce_function_bip340(unsigned char *secnonce32, const unsigned char *msg, size_t msglen, const unsigned char *seckey32, const unsigned char *xonly_pk32, const unsigned char *algo, size_t algolen, void *data) {
return nonce_function_bip340_impl(secp256k1_get_hash_context(secp256k1_context_static), secnonce32, msg, msglen, seckey32, xonly_pk32, algo, algolen, data);
}

const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340 = nonce_function_bip340;
Expand Down Expand Up @@ -185,13 +185,13 @@ static int secp256k1_schnorrsig_sign_internal(const secp256k1_context* ctx, unsi
return ret;
}

int secp256k1_schnorrsig_sign32(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, const unsigned char *aux_rand32) {
/* We cast away const from the passed aux_rand32 argument since we know the default nonce function does not modify it. */
return secp256k1_schnorrsig_sign_internal(ctx, sig64, msg32, 32, keypair, secp256k1_nonce_function_bip340, (unsigned char*)aux_rand32);
int secp256k1_schnorrsig_sign32(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, const unsigned char *aux_secrand32) {
/* We cast away const from the passed aux_secrand32 argument since we know the default nonce function does not modify it. */
return secp256k1_schnorrsig_sign_internal(ctx, sig64, msg32, 32, keypair, secp256k1_nonce_function_bip340, (unsigned char*)aux_secrand32);
}

int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, const unsigned char *aux_rand32) {
return secp256k1_schnorrsig_sign32(ctx, sig64, msg32, keypair, aux_rand32);
int secp256k1_schnorrsig_sign(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg32, const secp256k1_keypair *keypair, const unsigned char *aux_secrand32) {
return secp256k1_schnorrsig_sign32(ctx, sig64, msg32, keypair, aux_secrand32);
}

int secp256k1_schnorrsig_sign_custom(const secp256k1_context* ctx, unsigned char *sig64, const unsigned char *msg, size_t msglen, const secp256k1_keypair *keypair, secp256k1_schnorrsig_extraparams *extraparams) {
Expand Down
22 changes: 11 additions & 11 deletions src/secp256k1.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ static SECP256K1_INLINE void buffer_append(unsigned char *buf, unsigned int *off
*offset += len;
}

static int nonce_function_rfc6979_impl(const secp256k1_hash_ctx *hash_ctx, unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *algo16, void *data, unsigned int counter) {
static int nonce_function_rfc6979_impl(const secp256k1_hash_ctx *hash_ctx, unsigned char *secnonce32, const unsigned char *msg32, const unsigned char *seckey32, const unsigned char *algo16, void *data, unsigned int counter) {
unsigned char keydata[112];
unsigned int offset = 0;
secp256k1_rfc6979_hmac_sha256 rng;
Expand All @@ -512,7 +512,7 @@ static int nonce_function_rfc6979_impl(const secp256k1_hash_ctx *hash_ctx, unsig
* different argument mixtures to emulate each other and result in the same
* nonces.
*/
buffer_append(keydata, &offset, key32, 32);
buffer_append(keydata, &offset, seckey32, 32);
buffer_append(keydata, &offset, msgmod32, 32);
if (data != NULL) {
buffer_append(keydata, &offset, data, 32);
Expand All @@ -522,7 +522,7 @@ static int nonce_function_rfc6979_impl(const secp256k1_hash_ctx *hash_ctx, unsig
}
secp256k1_rfc6979_hmac_sha256_initialize(hash_ctx, &rng, keydata, offset);
for (i = 0; i <= counter; i++) {
secp256k1_rfc6979_hmac_sha256_generate(hash_ctx, &rng, nonce32, 32);
secp256k1_rfc6979_hmac_sha256_generate(hash_ctx, &rng, secnonce32, 32);
}
secp256k1_rfc6979_hmac_sha256_finalize(&rng);

Expand All @@ -531,8 +531,8 @@ static int nonce_function_rfc6979_impl(const secp256k1_hash_ctx *hash_ctx, unsig
return 1;
}

static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *msg32, const unsigned char *key32, const unsigned char *algo16, void *data, unsigned int counter) {
return nonce_function_rfc6979_impl(secp256k1_get_hash_context(secp256k1_context_static), nonce32, msg32, key32, algo16, data, counter);
static int nonce_function_rfc6979(unsigned char *secnonce32, const unsigned char *msg32, const unsigned char *seckey32, const unsigned char *algo16, void *data, unsigned int counter) {
return nonce_function_rfc6979_impl(secp256k1_get_hash_context(secp256k1_context_static), secnonce32, msg32, seckey32, algo16, data, counter);
}

const secp256k1_nonce_function secp256k1_nonce_function_rfc6979 = nonce_function_rfc6979;
Expand Down Expand Up @@ -693,15 +693,15 @@ static int secp256k1_ec_seckey_tweak_add_helper(secp256k1_scalar *sec, const uns
return ret;
}

int secp256k1_ec_seckey_tweak_add(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak32) {
int secp256k1_ec_seckey_tweak_add(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *sectweak32) {
secp256k1_scalar sec;
int ret = 0;
VERIFY_CHECK(ctx != NULL);
ARG_CHECK(seckey != NULL);
ARG_CHECK(tweak32 != NULL);
ARG_CHECK(sectweak32 != NULL);

ret = secp256k1_scalar_set_b32_seckey(&sec, seckey);
ret &= secp256k1_ec_seckey_tweak_add_helper(&sec, tweak32);
ret &= secp256k1_ec_seckey_tweak_add_helper(&sec, sectweak32);
secp256k1_scalar_cmov(&sec, &secp256k1_scalar_zero, !ret);
secp256k1_scalar_get_b32(seckey, &sec);

Expand Down Expand Up @@ -733,16 +733,16 @@ int secp256k1_ec_pubkey_tweak_add(const secp256k1_context* ctx, secp256k1_pubkey
return ret;
}

int secp256k1_ec_seckey_tweak_mul(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *tweak32) {
int secp256k1_ec_seckey_tweak_mul(const secp256k1_context* ctx, unsigned char *seckey, const unsigned char *sectweak32) {
secp256k1_scalar factor;
secp256k1_scalar sec;
int ret = 0;
int overflow = 0;
VERIFY_CHECK(ctx != NULL);
ARG_CHECK(seckey != NULL);
ARG_CHECK(tweak32 != NULL);
ARG_CHECK(sectweak32 != NULL);

secp256k1_scalar_set_b32(&factor, tweak32, &overflow);
secp256k1_scalar_set_b32(&factor, sectweak32, &overflow);
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);
Expand Down
Loading