Skip to content

Commit

Permalink
Merge bitcoin#595: Allow to use external default callbacks
Browse files Browse the repository at this point in the history 
e49f799 Add missing #(un)defines to base-config.h (Tim Ruffing)
77defd2 Add secp256k1_ prefix to default callback functions (Tim Ruffing)
908bdce Include stdio.h and stdlib.h explicitly in secp256k1.c (Tim Ruffing)
5db782e Allow usage of external default callbacks (Tim Ruffing)
6095a86 Replace CHECKs for no_precomp ctx by ARG_CHECKs without a return (Tim Ruffing)

Pull request description:

  This is intended for environments without implementations for `abort()`, `fprintf()`, and `stderr`. e.g., embedded systems. Those can provide their own implementations of `default_illegal_callback_fn` and `default_error_callback_fn` at compile time.

  If you want to use your own default callback, things will be somewhat inconsistent unfortunately: We cannot make the callback data `extern` too, because then the initialization lists for `default_illegal_callback` won't contain only constants. (`const` variables are not compile-time constants). So you cannot take callback data in your own default callback function.

  As a more drastic/breaking alternative I suggest to remove the callback data entirely. I don't think it's a big loss and I would be surprised if anyone uses it. Additionally, we could even remove the possibility to set the callback function at runtime after this PR. This will simplify things a lot, and again I don't think it's a big loss.

  Note that `abort()`, `fprintf()`, and `stderr` are also used in `CHECK`, which is still used in production code if we rely on gmp for scalar and field inversions (e.g.,  https://github.com/bitcoin-core/secp256k1/blob/master/src/scalar_impl.h#L240). This is not an issue for embedded system which probably don't want to use gmp anyway, but it is probably an issue for the reasons explained in bitcoin-core/secp256k1#566 (comment).

  (related downstream: rust-bitcoin/rust-secp256k1#100 @elichai)

ACKs for commit e49f79:

Tree-SHA512: 4dec0821eef4156cbe162bd8cdf0531c1fae8c98cd9db8438170ff1aa0e59b199739eeab293695bb582246812bea5309959f02f1fb74bb57872da54ebc52313f
  • Loading branch information
@gmaxwell
gmaxwell committed May 27, 2019
2 parents 6c36de7 + e49f799 commit 143dc6e
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 37 deletions.
42 changes: 26 additions & 16 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ AC_ARG_ENABLE(module_recovery,
[enable_module_recovery=$enableval],
[enable_module_recovery=no])

AC_ARG_ENABLE(external_default_callbacks,
AS_HELP_STRING([--enable-external-default-callbacks],[enable external default callback functions (default is no)]),
[use_external_default_callbacks=$enableval],
[use_external_default_callbacks=no])

AC_ARG_ENABLE(jni,
AS_HELP_STRING([--enable-jni],[enable libsecp256k1_jni [default=no]]),
[use_jni=$enableval],
Expand Down Expand Up @@ -493,6 +498,10 @@ if test x"$use_external_asm" = x"yes"; then
AC_DEFINE(USE_EXTERNAL_ASM, 1, [Define this symbol if an external (non-inline) assembly implementation is used])
fi

if test x"$use_external_default_callbacks" = x"yes"; then
AC_DEFINE(USE_EXTERNAL_DEFAULT_CALLBACKS, 1, [Define this symbol if an external implementation of the default callbacks is used])
fi

if test x"$enable_experimental" = x"yes"; then
AC_MSG_NOTICE([******])
AC_MSG_NOTICE([WARNING: experimental build])
Expand Down Expand Up @@ -535,22 +544,23 @@ AC_OUTPUT

echo
echo "Build Options:"
echo " with endomorphism = $use_endomorphism"
echo " with ecmult precomp = $set_precomp"
echo " with jni = $use_jni"
echo " with benchmarks = $use_benchmark"
echo " with coverage = $enable_coverage"
echo " module ecdh = $enable_module_ecdh"
echo " module recovery = $enable_module_recovery"
echo " with endomorphism = $use_endomorphism"
echo " with ecmult precomp = $set_precomp"
echo " with external callbacks = $use_external_default_callbacks"
echo " with jni = $use_jni"
echo " with benchmarks = $use_benchmark"
echo " with coverage = $enable_coverage"
echo " module ecdh = $enable_module_ecdh"
echo " module recovery = $enable_module_recovery"
echo
echo " asm = $set_asm"
echo " bignum = $set_bignum"
echo " field = $set_field"
echo " scalar = $set_scalar"
echo " ecmult window size = $set_ecmult_window"
echo " asm = $set_asm"
echo " bignum = $set_bignum"
echo " field = $set_field"
echo " scalar = $set_scalar"
echo " ecmult window size = $set_ecmult_window"
echo
echo " CC = $CC"
echo " CFLAGS = $CFLAGS"
echo " CPPFLAGS = $CPPFLAGS"
echo " LDFLAGS = $LDFLAGS"
echo " CC = $CC"
echo " CFLAGS = $CFLAGS"
echo " CPPFLAGS = $CPPFLAGS"
echo " LDFLAGS = $LDFLAGS"
echo
28 changes: 24 additions & 4 deletions include/secp256k1.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,28 @@ SECP256K1_API void secp256k1_context_destroy(
* to cause a crash, though its return value and output arguments are
* undefined.
*
* When this function has not been called (or called with fn==NULL), then the
* default handler will be used. The library provides a default handler which
* writes the message to stderr and calls abort. This default handler can be
* replaced at link time if the preprocessor macro
* USE_EXTERNAL_DEFAULT_CALLBACKS is defined, which is the case if the build
* has been configured with --enable-external-default-callbacks. Then the
* following two symbols must be provided to link against:
* - void secp256k1_default_illegal_callback_fn(const char* message, void* data);
* - void secp256k1_default_error_callback_fn(const char* message, void* data);
* The library can call these default handlers even before a proper callback data
* pointer could have been set using secp256k1_context_set_illegal_callback or
* secp256k1_context_set_illegal_callback, e.g., when the creation of a context
* fails. In this case, the corresponding default handler will be called with
* the data pointer argument set to NULL.
*
* Args: ctx: an existing context object (cannot be NULL)
* In: fun: a pointer to a function to call when an illegal argument is
* passed to the API, taking a message and an opaque pointer
* (NULL restores a default handler that calls abort).
* passed to the API, taking a message and an opaque pointer.
* (NULL restores the default handler.)
* data: the opaque pointer to pass to fun above.
*
* See also secp256k1_context_set_error_callback.
*/
SECP256K1_API void secp256k1_context_set_illegal_callback(
secp256k1_context* ctx,
Expand All @@ -271,9 +288,12 @@ SECP256K1_API void secp256k1_context_set_illegal_callback(
*
* Args: ctx: an existing context object (cannot be NULL)
* In: fun: a pointer to a function to call when an internal error occurs,
* taking a message and an opaque pointer (NULL restores a default
* handler that calls abort).
* taking a message and an opaque pointer (NULL restores the
* default handler, see secp256k1_context_set_illegal_callback
* for details).
* data: the opaque pointer to pass to fun above.
*
* See also secp256k1_context_set_illegal_callback.
*/
SECP256K1_API void secp256k1_context_set_error_callback(
secp256k1_context* ctx,
Expand Down
2 changes: 2 additions & 0 deletions src/basic-config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#undef USE_ASM_X86_64
#undef USE_ECMULT_STATIC_PRECOMPUTATION
#undef USE_ENDOMORPHISM
#undef USE_EXTERNAL_ASM
#undef USE_EXTERNAL_DEFAULT_CALLBACKS
#undef USE_FIELD_10X26
#undef USE_FIELD_5X52
#undef USE_FIELD_INV_BUILTIN
Expand Down
45 changes: 28 additions & 17 deletions src/secp256k1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,39 @@
} \
} while(0)

static void default_illegal_callback_fn(const char* str, void* data) {
#define ARG_CHECK_NO_RETURN(cond) do { \
if (EXPECT(!(cond), 0)) { \
secp256k1_callback_call(&ctx->illegal_callback, #cond); \
} \
} while(0)

#ifndef USE_EXTERNAL_DEFAULT_CALLBACKS
#include <stdlib.h>
#include <stdio.h>
static void secp256k1_default_illegal_callback_fn(const char* str, void* data) {
(void)data;
fprintf(stderr, "[libsecp256k1] illegal argument: %s\n", str);
abort();
}

static const secp256k1_callback default_illegal_callback = {
default_illegal_callback_fn,
NULL
};

static void default_error_callback_fn(const char* str, void* data) {
static void secp256k1_default_error_callback_fn(const char* str, void* data) {
(void)data;
fprintf(stderr, "[libsecp256k1] internal consistency check failed: %s\n", str);
abort();
}
#else
void secp256k1_default_illegal_callback_fn(const char* str, void* data);
void secp256k1_default_error_callback_fn(const char* str, void* data);
#endif

static const secp256k1_callback default_error_callback = {
default_error_callback_fn,
static const secp256k1_callback default_illegal_callback = {
secp256k1_default_illegal_callback_fn,
NULL
};

static const secp256k1_callback default_error_callback = {
secp256k1_default_error_callback_fn,
NULL
};

struct secp256k1_context_struct {
secp256k1_ecmult_context ecmult_ctx;
Expand All @@ -60,8 +71,8 @@ struct secp256k1_context_struct {
static const secp256k1_context secp256k1_context_no_precomp_ = {
{ 0 },
{ 0 },
{ default_illegal_callback_fn, 0 },
{ default_error_callback_fn, 0 }
{ secp256k1_default_illegal_callback_fn, 0 },
{ secp256k1_default_error_callback_fn, 0 }
};
const secp256k1_context *secp256k1_context_no_precomp = &secp256k1_context_no_precomp_;

Expand Down Expand Up @@ -162,7 +173,7 @@ secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) {
}

void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) {
CHECK(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
if (ctx != NULL) {
secp256k1_ecmult_context_clear(&ctx->ecmult_ctx);
secp256k1_ecmult_gen_context_clear(&ctx->ecmult_gen_ctx);
Expand All @@ -177,18 +188,18 @@ 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) {
CHECK(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
if (fun == NULL) {
fun = default_illegal_callback_fn;
fun = secp256k1_default_illegal_callback_fn;
}
ctx->illegal_callback.fn = fun;
ctx->illegal_callback.data = data;
}

void secp256k1_context_set_error_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) {
CHECK(ctx != secp256k1_context_no_precomp);
ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp);
if (fun == NULL) {
fun = default_error_callback_fn;
fun = secp256k1_default_error_callback_fn;
}
ctx->error_callback.fn = fun;
ctx->error_callback.data = data;
Expand Down

0 comments on commit 143dc6e

Please sign in to comment.