Skip to content

Commit

Permalink
Merge bitcoin-core#1126: API cleanup with respect to contexts
Browse files Browse the repository at this point in the history
4386a23 examples: Switch to NONE contexts (Tim Ruffing)
7289b51 docs: Use doxygen style if and only if comment is user-facing (Tim Ruffing)
e7d0185 docs: Get rid of "initialized for signing" terminology (Tim Ruffing)
0612636 docs: Tidy and improve docs about contexts and randomization (Tim Ruffing)
e02d686 selftest: Expose in public API (Tim Ruffing)
e383fbf selftest: Rename internal function to make name available for API (Tim Ruffing)
d2c6d48 tests: Use new name of static context (Tim Ruffing)
53796d2 contexts: Rename static context (Tim Ruffing)
72fedf8 docs: Improve docs for static context (Tim Ruffing)
316ac76 contexts: Deprecate all context flags except SECP256K1_CONTEXT_NONE (Tim Ruffing)
1a553ee docs: Change signature "validation" to "verification" (Tim Ruffing)
ee7341f docs: Never require a verification context (Tim Ruffing)

Pull request description:

ACKs for top commit:
  sipa:
    utACK 4386a23
  jonasnick:
    ACK 4386a23

Tree-SHA512: 7bf07dfae0ecbf7de1418de64ef743a23dc5f244aeba2c1cf3ecbdc117d6ac12bb6c8f17f739605566074a9b901765ee4a32288b6edc6f9a0040a70cb472f6ee
  • Loading branch information
jonasnick committed Dec 6, 2022
2 parents 477f02c + 4386a23 commit e3f8477
Show file tree
Hide file tree
Showing 16 changed files with 174 additions and 99 deletions.
3 changes: 1 addition & 2 deletions contrib/lax_der_privatekey_parsing.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ extern "C" {
/** Export a private key in DER format.
*
* Returns: 1 if the private key was valid.
* Args: ctx: pointer to a context object, initialized for signing (cannot
* be NULL)
* Args: ctx: pointer to a context object (not secp256k1_context_static).
* Out: privkey: pointer to an array for storing the private key in BER.
* Should have space for 279 bytes, and cannot be NULL.
* privkeylen: Pointer to an int where the length of the private key in
Expand Down
7 changes: 7 additions & 0 deletions doc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ Each change falls into one of the following categories: Added, Changed, Deprecat
### Changed
- Enable modules schnorrsig, extrakeys and ECDH by default in ./configure

### Deprecated
- Deprecated context flags `SECP256K1_CONTEXT_VERIFY` and `SECP256K1_CONTEXT_SIGN`. Use `SECP256K1_CONTEXT_NONE` instead.
- Renamed `secp256k1_context_no_precomp` to `secp256k1_context_static`.

### Added
- Added `secp256k1_selftest`, to be used in conjunction with `secp256k1_context_static`.

## [MAJOR.MINOR.PATCH] - YYYY-MM-DD

### Added/Changed/Deprecated/Removed/Fixed/Security
Expand Down
8 changes: 2 additions & 6 deletions examples/ecdh.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ int main(void) {
secp256k1_pubkey pubkey1;
secp256k1_pubkey pubkey2;

/* The specification in secp256k1.h states that `secp256k1_ec_pubkey_create`
* needs a context object initialized for signing, which is why we create
* a context with the SECP256K1_CONTEXT_SIGN flag.
* (The docs for `secp256k1_ecdh` don't require any special context, just
* some initialized context) */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
/* Before we can call actual API functions, we need to create a "context". */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
if (!fill_random(randomize, sizeof(randomize))) {
printf("Failed to generate randomness\n");
return 1;
Expand Down
8 changes: 2 additions & 6 deletions examples/ecdsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,8 @@ int main(void) {
int return_val;
secp256k1_pubkey pubkey;
secp256k1_ecdsa_signature sig;
/* The specification in secp256k1.h states that `secp256k1_ec_pubkey_create` needs
* a context object initialized for signing and `secp256k1_ecdsa_verify` needs
* a context initialized for verification, which is why we create a context
* for both signing and verification with the SECP256K1_CONTEXT_SIGN and
* SECP256K1_CONTEXT_VERIFY flags. */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
/* Before we can call actual API functions, we need to create a "context". */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
if (!fill_random(randomize, sizeof(randomize))) {
printf("Failed to generate randomness\n");
return 1;
Expand Down
8 changes: 2 additions & 6 deletions examples/schnorr.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ int main(void) {
int return_val;
secp256k1_xonly_pubkey pubkey;
secp256k1_keypair keypair;
/* The specification in secp256k1_extrakeys.h states that `secp256k1_keypair_create`
* needs a context object initialized for signing. And in secp256k1_schnorrsig.h
* they state that `secp256k1_schnorrsig_verify` needs a context initialized for
* verification, which is why we create a context for both signing and verification
* with the SECP256K1_CONTEXT_SIGN and SECP256K1_CONTEXT_VERIFY flags. */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
/* Before we can call actual API functions, we need to create a "context". */
secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
if (!fill_random(randomize, sizeof(randomize))) {
printf("Failed to generate randomness\n");
return 1;
Expand Down
175 changes: 120 additions & 55 deletions include/secp256k1.h

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions include/secp256k1_extrakeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_from_pubke
* invalid (only when the tweak is the negation of the corresponding
* secret key). 1 otherwise.
*
* Args: ctx: pointer to a context object initialized for verification.
* Args: ctx: pointer to a context object.
* Out: output_pubkey: pointer to a public key to store the result. Will be set
* to an invalid value if this function returns 0.
* In: internal_pubkey: pointer to an x-only pubkey to apply the tweak to.
Expand Down Expand Up @@ -137,7 +137,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add(
*
* Returns: 0 if the arguments are invalid or the tweaked pubkey is not the
* result of tweaking the internal_pubkey with tweak32. 1 otherwise.
* Args: ctx: pointer to a context object initialized for verification.
* Args: ctx: pointer to a context object.
* In: tweaked_pubkey32: pointer to a serialized xonly_pubkey.
* tweaked_pk_parity: the parity of the tweaked pubkey (whose serialization
* is passed in as tweaked_pubkey32). This must match the
Expand All @@ -159,7 +159,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add_
*
* Returns: 1: secret was valid, keypair is ready to use
* 0: secret was invalid, try again with a different secret
* Args: ctx: pointer to a context object, initialized for signing.
* Args: ctx: pointer to a context object (not secp256k1_context_static).
* Out: keypair: pointer to the created keypair.
* In: seckey: pointer to a 32-byte secret key.
*/
Expand Down Expand Up @@ -228,7 +228,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_pub(
* invalid (only when the tweak is the negation of the keypair's
* secret key). 1 otherwise.
*
* Args: ctx: pointer to a context object initialized for verification.
* Args: ctx: pointer to a context object.
* In/Out: keypair: pointer to a keypair to apply the tweak to. Will be set to
* an invalid value if this function returns 0.
* In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according
Expand Down
2 changes: 2 additions & 0 deletions include/secp256k1_preallocated.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ SECP256K1_API size_t secp256k1_context_preallocated_size(
* bytes, as detailed above.
* flags: which parts of the context to initialize.
*
* See secp256k1_context_create (in secp256k1.h) for further details.
*
* See also secp256k1_context_randomize (in secp256k1.h)
* and secp256k1_context_preallocated_destroy.
*/
Expand Down
4 changes: 2 additions & 2 deletions include/secp256k1_recovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact(
*
* Returns: 1: signature created
* 0: the nonce generation function failed, or the secret key was invalid.
* Args: ctx: pointer to a context object, initialized for signing.
* Args: ctx: pointer to a context object (not secp256k1_context_static).
* Out: sig: pointer to an array where the signature will be placed.
* In: msghash32: the 32-byte message hash being signed.
* seckey: pointer to a 32-byte secret key.
Expand All @@ -94,7 +94,7 @@ SECP256K1_API int secp256k1_ecdsa_sign_recoverable(
*
* Returns: 1: public key successfully recovered (which guarantees a correct signature).
* 0: otherwise.
* Args: ctx: pointer to a context object, initialized for verification.
* Args: ctx: pointer to a context object.
* Out: pubkey: pointer to the recovered public key.
* In: sig: pointer to initialized signature that supports pubkey recovery.
* msghash32: the 32-byte message hash assumed to be signed.
Expand Down
4 changes: 2 additions & 2 deletions include/secp256k1_schnorrsig.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ typedef struct {
* signatures from being valid in multiple contexts by accident.
*
* Returns 1 on success, 0 on failure.
* Args: ctx: pointer to a context object, initialized for signing.
* Args: ctx: pointer to a context object (not secp256k1_context_static).
* 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.
Expand Down Expand Up @@ -161,7 +161,7 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom(
*
* Returns: 1: correct signature
* 0: incorrect signature
* Args: ctx: a secp256k1 context object, initialized for verification.
* Args: ctx: a secp256k1 context object.
* In: sig64: pointer to the 64-byte signature to verify.
* msg: the message being verified. Can only be NULL if msglen is 0.
* msglen: length of the message
Expand Down
2 changes: 1 addition & 1 deletion src/modules/extrakeys/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ void test_keypair(void) {
secp256k1_context *none = api_test_context(SECP256K1_CONTEXT_NONE, &ecount);
secp256k1_context *sign = api_test_context(SECP256K1_CONTEXT_SIGN, &ecount);
secp256k1_context *verify = api_test_context(SECP256K1_CONTEXT_VERIFY, &ecount);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
secp256k1_context_set_error_callback(sttc, counting_illegal_callback_fn, &ecount);
secp256k1_context_set_illegal_callback(sttc, counting_illegal_callback_fn, &ecount);

Expand Down
2 changes: 1 addition & 1 deletion src/modules/recovery/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void test_ecdsa_recovery_api(void) {
secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
secp256k1_pubkey pubkey;
secp256k1_pubkey recpubkey;
secp256k1_ecdsa_signature normal_sig;
Expand Down
2 changes: 1 addition & 1 deletion src/modules/schnorrsig/tests_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ void test_schnorrsig_api(void) {
secp256k1_context *sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
secp256k1_context *vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
secp256k1_context *both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_static);
int ecount;

secp256k1_context_set_error_callback(none, counting_illegal_callback_fn, &ecount);
Expand Down
21 changes: 13 additions & 8 deletions src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,20 @@ struct secp256k1_context_struct {
int declassify;
};

static const secp256k1_context secp256k1_context_no_precomp_ = {
static const secp256k1_context secp256k1_context_static_ = {
{ 0 },
{ secp256k1_default_illegal_callback_fn, 0 },
{ secp256k1_default_error_callback_fn, 0 },
0
};
const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_precomp_;
const secp256k1_context *secp256k1_context_static = &secp256k1_context_static_;
const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_static_;

void secp256k1_selftest(void) {
if (!secp256k1_selftest_passes()) {
secp256k1_callback_call(&default_error_callback, "self test failed");
}
}

size_t secp256k1_context_preallocated_size(unsigned int flags) {
size_t ret = sizeof(secp256k1_context);
Expand All @@ -96,9 +103,7 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne
size_t prealloc_size;
secp256k1_context* ret;

if (!secp256k1_selftest()) {
secp256k1_callback_call(&default_error_callback, "self test failed");
}
secp256k1_selftest();

prealloc_size = secp256k1_context_preallocated_size(flags);
if (prealloc_size == 0) {
Expand Down Expand Up @@ -150,7 +155,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) {
}

void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) {
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
if (ctx != NULL) {
secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx);
}
Expand All @@ -164,7 +169,7 @@ void secp256k1_context_destroy(secp256k1_context* ctx) {
}

void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
if (fun == NULL) {
fun = secp256k1_default_illegal_callback_fn;
}
Expand All @@ -173,7 +178,7 @@ void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(
}

void secp256k1_context_set_error_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_static);
if (fun == NULL) {
fun = secp256k1_default_error_callback_fn;
}
Expand Down
2 changes: 1 addition & 1 deletion src/selftest.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ static int secp256k1_selftest_sha256(void) {
return secp256k1_memcmp_var(out, output32, 32) == 0;
}

static int secp256k1_selftest(void) {
static int secp256k1_selftest_passes(void) {
return secp256k1_selftest_sha256();
}

Expand Down
17 changes: 13 additions & 4 deletions src/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ void random_scalar_order_b32(unsigned char *b32) {
secp256k1_scalar_get_b32(b32, &num);
}

void run_selftest_tests(void) {
/* Test public API */
secp256k1_selftest();
}

void run_context_tests(int use_prealloc) {
secp256k1_pubkey pubkey;
secp256k1_pubkey zero_pubkey;
Expand All @@ -164,12 +169,15 @@ void run_context_tests(int use_prealloc) {
secp256k1_scalar msg, key, nonce;
secp256k1_scalar sigr, sigs;

/* Check that deprecated secp256k1_context_no_precomp is an alias to secp256k1_context_static. */
CHECK(secp256k1_context_no_precomp == secp256k1_context_static);

if (use_prealloc) {
none_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_NONE));
sign_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN));
vrfy_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_VERIFY));
both_prealloc = malloc(secp256k1_context_preallocated_size(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY));
sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_no_precomp));
sttc_prealloc = malloc(secp256k1_context_preallocated_clone_size(secp256k1_context_static));
CHECK(none_prealloc != NULL);
CHECK(sign_prealloc != NULL);
CHECK(vrfy_prealloc != NULL);
Expand All @@ -179,13 +187,13 @@ void run_context_tests(int use_prealloc) {
sign = secp256k1_context_preallocated_create(sign_prealloc, SECP256K1_CONTEXT_SIGN);
vrfy = secp256k1_context_preallocated_create(vrfy_prealloc, SECP256K1_CONTEXT_VERIFY);
both = secp256k1_context_preallocated_create(both_prealloc, SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
sttc = secp256k1_context_preallocated_clone(secp256k1_context_no_precomp, sttc_prealloc);
sttc = secp256k1_context_preallocated_clone(secp256k1_context_static, sttc_prealloc);
} else {
none = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
sign = secp256k1_context_create(SECP256K1_CONTEXT_SIGN);
vrfy = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
both = secp256k1_context_create(SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY);
sttc = secp256k1_context_clone(secp256k1_context_no_precomp);
sttc = secp256k1_context_clone(secp256k1_context_static);
}

memset(&zero_pubkey, 0, sizeof(zero_pubkey));
Expand Down Expand Up @@ -5799,7 +5807,7 @@ void run_ec_pubkey_parse_test(void) {
ecount = 0;
VG_UNDEF(&pubkey, sizeof(pubkey));
CHECK(secp256k1_ec_pubkey_parse(ctx, &pubkey, pubkeyc, 65) == 1);
CHECK(secp256k1_ec_pubkey_parse(secp256k1_context_no_precomp, &pubkey, pubkeyc, 65) == 1);
CHECK(secp256k1_ec_pubkey_parse(secp256k1_context_static, &pubkey, pubkeyc, 65) == 1);
VG_CHECK(&pubkey, sizeof(pubkey));
CHECK(ecount == 0);
VG_UNDEF(&ge, sizeof(ge));
Expand Down Expand Up @@ -7385,6 +7393,7 @@ int main(int argc, char **argv) {
secp256k1_testrand_init(argc > 2 ? argv[2] : NULL);

/* initialize */
run_selftest_tests();
run_context_tests(0);
run_context_tests(1);
run_scratch_tests();
Expand Down

0 comments on commit e3f8477

Please sign in to comment.