Skip to content

Make SHA256 compression pluggable #1777

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
furszy wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make SHA256 compression pluggable #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
75ccf99
Add ability to provide SHA256 compression at compile-time
furszy Nov 24, 2025
7fefa9c
Add API to override SHA256 transform at runtime
furszy Nov 24, 2025
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 CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ message(" extrakeys ........................... ${SECP256K1_ENABLE_MODULE_EXTRA
message(" schnorrsig .......................... ${SECP256K1_ENABLE_MODULE_SCHNORRSIG}")
message(" musig ............................... ${SECP256K1_ENABLE_MODULE_MUSIG}")
message(" ElligatorSwift ...................... ${SECP256K1_ENABLE_MODULE_ELLSWIFT}")
message(" External SHA256 ..................... ${SECP256K1_EXTERNAL_SHA256_PATH-NONE}")
message("Parameters:")
message(" ecmult window size .................. ${SECP256K1_ECMULT_WINDOW_SIZE}")
message(" ecmult gen table size ............... ${SECP256K1_ECMULT_GEN_KB} KiB")
Expand Down
6 changes: 6 additions & 0 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ noinst_HEADERS += src/scratch_impl.h
noinst_HEADERS += src/selftest.h
noinst_HEADERS += src/testrand.h
noinst_HEADERS += src/testrand_impl.h
if !SECP256K1_EXTERNAL_SHA256
noinst_HEADERS += src/modules/sha/main_impl.h
endif
noinst_HEADERS += src/hash.h
noinst_HEADERS += src/hash_impl.h
noinst_HEADERS += src/field.h
Expand Down Expand Up @@ -314,3 +317,6 @@ endif
if ENABLE_MODULE_ELLSWIFT
include src/modules/ellswift/Makefile.am.include
endif

# Always include SHA module files, but they conditionally compile
include src/modules/sha/Makefile.am.include
14 changes: 14 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)
### Define config arguments
###

AC_ARG_WITH([external-sha256],
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this one should also be an enable-style configure option instead of a with-style configure options; with is for compiling with external packages e.g., with-libxyz. And what the user provides here is not a package.

AS_HELP_STRING([--with-external-sha256=PATH], [use external SHA256 compression implementation]),
[external_sha256_path="$withval"],
[external_sha256_path=""]
)
Comment on lines +137 to +141
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the build stuff for the build-time override: We already have the possibility to have build-time overrides for the error callbacks, e.g., see AC_ARG_ENABLE(external_default_callbacks, below.

When it comes to the implementation of how the new override is configured and performed, I think it's similar to the existing overrides for the callback. (Except that these don't allow configuring the header to be included, which makes them rather useless... #1461)

So this will need a rework anyway [1]. This belongs to a different PR, of course, but I think the SHA override here is a good opportunity to think about a better design of a compile-time override, so we may want to iterate on this a bit more than expected. No matter how it's designed, I think we should, in the long run, have only one (non-deprecated) mechanism for a build-time override of a function.

[1] I don't think what I did for the error callbacks great, and I guess it would be okay to change it (I doubt that many users rely on this). Or even better, we could deprecate it and simply provide ways to provide build-time overrides for standard library symbols like printf, stderr, and abort. See for example the approach taken by mbedTLS (in the past ?) which is built with embedded systems in mind: https://github.com/Mbed-TLS/mbedtls/blob/550a18d4d6b29e62b6824201d8a49b8224e61c97/tf-psa-crypto%2Fdrivers%2Fbuiltin%2Finclude%2Fmbedtls%2Fplatform.h They provide macros for overriding standard library functions. So you have full flexibility as a user: if you've implemented a function that aborts, but it's not called abort() (perhaps because that is a reserved name) then you can link it to the library also with a different name.


# In dev mode, we enable all binaries and modules by default but individual options can still be overridden explicitly.
# Check for dev mode first because SECP_SET_DEFAULT needs enable_dev_mode set.
AC_ARG_ENABLE(dev_mode, [], [],
Expand Down Expand Up @@ -397,6 +403,12 @@ SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS"

# Processing must be done in a reverse topological sorting of the dependency graph
# (dependent module first).

if test "x$external_sha256_path" != "x"; then
SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DSECP256K1_EXTERNAL_SHA256=1"
SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DSECP256K1_EXTERNAL_SHA256_HEADER='\"$external_sha256_path\"'"
fi

if test x"$enable_module_ellswift" = x"yes"; then
SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ELLSWIFT=1"
fi
Expand Down Expand Up @@ -470,6 +482,7 @@ AM_CONDITIONAL([ENABLE_MODULE_EXTRAKEYS], [test x"$enable_module_extrakeys" = x"
AM_CONDITIONAL([ENABLE_MODULE_SCHNORRSIG], [test x"$enable_module_schnorrsig" = x"yes"])
AM_CONDITIONAL([ENABLE_MODULE_MUSIG], [test x"$enable_module_musig" = x"yes"])
AM_CONDITIONAL([ENABLE_MODULE_ELLSWIFT], [test x"$enable_module_ellswift" = x"yes"])
AM_CONDITIONAL([SECP256K1_EXTERNAL_SHA256], [test "x$SHA256_EXTERNAL_SRC" != "x"])
AM_CONDITIONAL([USE_EXTERNAL_ASM], [test x"$enable_external_asm" = x"yes"])
AM_CONDITIONAL([USE_ASM_ARM], [test x"$set_asm" = x"arm32"])
AM_CONDITIONAL([BUILD_WINDOWS], [test "$build_windows" = "yes"])
Expand All @@ -494,6 +507,7 @@ echo " module extrakeys = $enable_module_extrakeys"
echo " module schnorrsig = $enable_module_schnorrsig"
echo " module musig = $enable_module_musig"
echo " module ellswift = $enable_module_ellswift"
echo " external sha256 = ${external_sha256_path:-none}"
echo
echo " asm = $set_asm"
echo " ecmult window size = $set_ecmult_window"
Expand Down
25 changes: 25 additions & 0 deletions include/secp256k1.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ extern "C" {
#endif

#include <stddef.h>
#include <stdint.h>

/** Unless explicitly stated all pointer arguments must not be NULL.
*
Expand Down Expand Up @@ -92,6 +93,7 @@ typedef struct secp256k1_ecdsa_signature {
* the message, the algorithm, the key and the attempt.
*/
typedef int (*secp256k1_nonce_function)(
const secp256k1_context *ctx,
Comment on lines 95 to +96
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argh, that's a breaking change. I didn't see that coming.

But I think it can be avoided. If the user wants to pass a custom nonce function, they anyway have no way of calling our internal function (whether it uses a custom SHA256 transform or not).

So we can keep the struct here and add the context arg only to our built-in nonce function. Our ECDSA signing function (and Schnorr signing, and ECDH, etc.) will need to special-case the built-in nonce function because it has a different function signature. Not elegant but it will do the job.

Maybe we can come up with something nicer if we (ever) add a modern ECDSA module and deprecate the ECDSA stuff in the main secp256k1.h file. edit: Probably no because this affects not just ECDSA.

unsigned char *nonce32,
const unsigned char *msg32,
const unsigned char *key32,
Expand Down Expand Up @@ -403,6 +405,29 @@ SECP256K1_API void secp256k1_context_set_error_callback(
const void *data
) SECP256K1_ARG_NONNULL(1);

/**
* Set a callback function to override the internal SHA-256 transform.
*
* This installs a function to replace the built-in block-compression
* step used by the library's internal SHA-256 implementation.
* The provided callback must be functionally identical (bit-for-bit)
* to the default transform. Any deviation will cause incorrect results
* and undefined behaviour.
*
* This API exists to support environments that wish to route the
* SHA-256 compression step through a hardware-accelerated or otherwise
* specialized implementation. It is not meant for modifying the
* semantics of SHA-256.
*
* Args: ctx: pointer to a context object.
* In: callback: pointer to a function implementing the transform step.
* (passing NULL restores the default implementation)
*/
SECP256K1_API void secp256k1_context_set_sha256_transform_callback(
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming: I wonder whether it makes sense to avoid the word callback here. Sure, it's a callback, but whenever we said callback in the past 10 years of this library, it was about error handling. We even have type secp256k1_callback (which probably should have been called secp256k1_error_callback but it's too late now.)

Maybe we can just call it "function " and "function pointer", like we do in the bring-your-own-nonce-function interfaces.

secp256k1_context *ctx,
void (*sha256_transform_callback)(uint32_t *state, const unsigned char *block, size_t rounds)
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering whether it makes sense to allow for a real return value here to make it possible for the callback to indicate some kind of failure (e.g., couldn't access external hardware, or malloc failed). We could then fall back to our implementation or simply call the error callback in order to crash.

My current conclusion is no: I can't imagine this being useful in practice. (If you call external hardware, this will be super slow anyway. If you use malloc for SHA256, you're doing it wrong.) Even if there's some failure, the callback will still have the possibility to crash the process.

Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for the rounds arg? (Do you think we'll ever call this with any other value than 1?)

) SECP256K1_ARG_NONNULL(1);

/** Parse a variable-length public key into the pubkey object.
*
* Returns: 1 if the public key was fully valid.
Expand Down
1 change: 1 addition & 0 deletions include/secp256k1_ecdh.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ extern "C" {
* data: arbitrary data pointer that is passed through
*/
typedef int (*secp256k1_ecdh_hash_function)(
const secp256k1_context *ctx,
unsigned char *output,
const unsigned char *x32,
const unsigned char *y32,
Expand Down
1 change: 1 addition & 0 deletions include/secp256k1_ellswift.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ extern "C" {
* data: arbitrary data pointer that is passed through
*/
typedef int (*secp256k1_ellswift_xdh_hash_function)(
const secp256k1_context *ctx,
unsigned char *output,
const unsigned char *x32,
const unsigned char *ell_a64,
Expand Down
1 change: 1 addition & 0 deletions include/secp256k1_schnorrsig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ extern "C" {
* the message, the key, the pubkey, the algorithm description, and data.
*/
typedef int (*secp256k1_nonce_function_hardened)(
const secp256k1_context *ctx,
unsigned char *nonce32,
const unsigned char *msg,
size_t msglen,
Expand Down
39 changes: 39 additions & 0 deletions include/secp256k1_sha.h
View file
Open in desktop
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation for exposing the SHA256 compression?

Unless I'm missing something, then if anything, I think we'd want to expose the high-level SHA256 function. I'm not sure how useful it is, and we tried to keep the library focused on ECC. On the other hand, we'll have it anyway in the code base, and it's required for Schnorr sigs (and we anyway expose a tagged hash for Schnorr sigs). So if you ask me, it's fine to expose SHA256 as long as we add a comment that says that users shouldn't expect a highly performant implementation.

But perhaps it's better to debate the exposing in a separate PR.

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#ifndef SECP256K1_SHA_H
#define SECP256K1_SHA_H

#include "secp256k1.h"

#include <stdint.h>

#ifdef __cplusplus
extern "C" {
#endif

/**
* SHA-256 block compression function.
*
* Performs the SHA-256 transform step on a single 64-byte message block,
* updating the 8-word `state` in place. This is the raw block-level primitive:
* no padding, no message scheduling across blocks, and no length encoding.
* Only the compression function is applied.
*
* If `rounds` is greater than 1, the same 64-byte block is re-compressed
* repeatedly onto the updated state.
*
* The caller must supply a fully-formed, 64-byte, block-aligned message block.
*
* @param state Current hash state (8 x 32-bit words), updated in place.
* @param block Pointer to a 64-byte message block.
* @param rounds Number of times to apply the compression to this block.
*/
SECP256K1_API void secp256k1_sha256_transform(
uint32_t *state,
const unsigned char *block,
size_t rounds
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2);

#ifdef __cplusplus
}
#endif

#endif /* SECP256K1_SHA_H */
16 changes: 16 additions & 0 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,26 @@ add_library(secp256k1)
set_property(TARGET secp256k1 PROPERTY PUBLIC_HEADER
${PROJECT_SOURCE_DIR}/include/secp256k1.h
${PROJECT_SOURCE_DIR}/include/secp256k1_preallocated.h
${PROJECT_SOURCE_DIR}/include/secp256k1_sha.h
)

# Processing must be done in a topological sorting of the dependency graph
# (dependent module first).

if(SECP256K1_EXTERNAL_SHA256_PATH)
if(EXISTS "${SECP256K1_EXTERNAL_SHA256_PATH}")
add_compile_definitions(SECP256K1_EXTERNAL_SHA256=1)
add_compile_definitions(SECP256K1_EXTERNAL_SHA256_HEADER="${SECP256K1_EXTERNAL_SHA256_PATH}")
else()
message(FATAL_ERROR "SECP256K1_EXTERNAL_SHA256_PATH does not exist: ${SECP256K1_EXTERNAL_SHA256_PATH}")
endif()
else()
# No external override. Compile internal implementation
target_sources(secp256k1 PRIVATE
${PROJECT_SOURCE_DIR}/src/modules/sha/main_impl.h
)
endif()

if(SECP256K1_ENABLE_MODULE_ELLSWIFT)
add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1)
set_property(TARGET secp256k1 APPEND PROPERTY PUBLIC_HEADER ${PROJECT_SOURCE_DIR}/include/secp256k1_ellswift.h)
Expand Down
2 changes: 1 addition & 1 deletion src/bench_ecmult.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ static void generate_scalar(uint32_t num, secp256k1_scalar* scalar) {
c[7] = num >> 8;
c[8] = num >> 16;
c[9] = num >> 24;
secp256k1_sha256_initialize(&sha256);
secp256k1_sha256_initialize(&sha256, /*fn_transform=*/NULL);
secp256k1_sha256_write(&sha256, c, sizeof(c));
secp256k1_sha256_finalize(&sha256, buf);
secp256k1_scalar_set_b32(scalar, buf, &overflow);
Expand Down
12 changes: 8 additions & 4 deletions src/bench_internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static void help(int default_iters) {
}

typedef struct {
const secp256k1_context* ctx;
secp256k1_scalar scalar[2];
secp256k1_fe fe[4];
secp256k1_ge ge[2];
Expand Down Expand Up @@ -82,6 +83,9 @@ static void bench_setup(void* arg) {
}
};

/* Customize context if needed */
data->ctx = secp256k1_context_static;

secp256k1_scalar_set_b32(&data->scalar[0], init[0], NULL);
secp256k1_scalar_set_b32(&data->scalar[1], init[1], NULL);
secp256k1_fe_set_b32_limit(&data->fe[0], init[0]);
Expand Down Expand Up @@ -346,7 +350,7 @@ static void bench_sha256(void* arg, int iters) {
secp256k1_sha256 sha;

for (i = 0; i < iters; i++) {
secp256k1_sha256_initialize(&sha);
secp256k1_sha256_initialize(&sha, data->ctx->hash_context.fn_sha256_transform);
secp256k1_sha256_write(&sha, data->data, 32);
secp256k1_sha256_finalize(&sha, data->data);
}
Expand All @@ -358,7 +362,7 @@ static void bench_hmac_sha256(void* arg, int iters) {
secp256k1_hmac_sha256 hmac;

for (i = 0; i < iters; i++) {
secp256k1_hmac_sha256_initialize(&hmac, data->data, 32);
secp256k1_hmac_sha256_initialize(&hmac, data->data, 32, data->ctx->hash_context.fn_sha256_transform);
secp256k1_hmac_sha256_write(&hmac, data->data, 32);
secp256k1_hmac_sha256_finalize(&hmac, data->data);
}
Expand All @@ -370,8 +374,8 @@ static void bench_rfc6979_hmac_sha256(void* arg, int iters) {
secp256k1_rfc6979_hmac_sha256 rng;

for (i = 0; i < iters; i++) {
secp256k1_rfc6979_hmac_sha256_initialize(&rng, data->data, 64);
secp256k1_rfc6979_hmac_sha256_generate(&rng, data->data, 32);
secp256k1_rfc6979_hmac_sha256_initialize(&rng, data->data, 64, data->ctx->hash_context.fn_sha256_transform);
secp256k1_rfc6979_hmac_sha256_generate(&rng, data->data, 32, data->ctx->hash_context.fn_sha256_transform);
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/ecmult_gen_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,9 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
secp256k1_rfc6979_hmac_sha256 rng;
unsigned char keydata[64];

/* future: use context callback */
const sha256_transform_callback fn_sha256_transform = NULL;

/* Compute the (2^COMB_BITS - 1)/2 term once. */
secp256k1_ecmult_gen_scalar_diff(&diff);

Expand All @@ -309,17 +312,17 @@ static void secp256k1_ecmult_gen_blind(secp256k1_ecmult_gen_context *ctx, const
*/
VERIFY_CHECK(seed32 != NULL);
memcpy(keydata + 32, seed32, 32);
secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64);
secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, 64, fn_sha256_transform);
secp256k1_memclear_explicit(keydata, sizeof(keydata));

/* Compute projective blinding factor (cannot be 0). */
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32, fn_sha256_transform);
secp256k1_fe_set_b32_mod(&f, nonce32);
secp256k1_fe_cmov(&f, &secp256k1_fe_one, secp256k1_fe_normalizes_to_zero(&f));
ctx->proj_blind = f;

/* For a random blinding value b, set scalar_offset=diff-b, ge_offset=bG */
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32);
secp256k1_rfc6979_hmac_sha256_generate(&rng, nonce32, 32, fn_sha256_transform);
secp256k1_scalar_set_b32(&b, nonce32, NULL);
/* The blinding value cannot be zero, as that would mean ge_offset = infinity,
* which secp256k1_gej_add_ge cannot handle. */
Expand Down
15 changes: 11 additions & 4 deletions src/hash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,20 @@
#include <stdlib.h>
#include <stdint.h>

typedef void (*sha256_transform_callback)(uint32_t *state, const unsigned char *block, size_t rounds);

/* Validate user-supplied SHA-256 transform by comparing its output against
* the library's linked implementation */
static int secp256k1_sha256_check_transform(sha256_transform_callback fn_transform);
Comment on lines +15 to +17
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this function given that there's already secp256k1_selftest_sha256(void), which is run when a context is created? The existing one seems a bit simpler than your function and it's already there, so using that will make the diff smaller (but it will need to be modified to obtain an optional context). And perhaps the existing one is a bit slower than yours. I'd say either function is fine but keeping both seems overkill.

(The existing selftest is currently in selftest.h. That's already wrong; it should have been put in a selftest_impl.h. But your approach is better: the SHA256 test function should be exposed from the internal hash module and then called from selftest.)

edit: Okay, I see now. It's annoying than I thought. We expose secp256k1_selftest(void) which doesn't get a context object... The reason it's exposed is that you can use it when using the static context.

So I believe it makes sense to check the SHA256 at two places:

  • In secp256k1_selftest(void) because this will catch bugs in the built-in SHA256. This is particularly important with this PR given that it makes possible overriding the built-in implementation it at build time.
  • When setting a custom SHA256 transform.

This means we may perform two checks in the worst case, but that won't be the end of the world. You get the overhead only at library initialization time. But the question remains: How should the test look like? And I still think what I said above is true: It's okay to have a single secp256k1_sha256_selftest function in the hash module, and this can be called from the selftest module and from secp256k1_context_set_sha256_transform_callback.


typedef struct {
uint32_t s[8];
unsigned char buf[64];
uint64_t bytes;
sha256_transform_callback fn_transform;
} secp256k1_sha256;

static void secp256k1_sha256_initialize(secp256k1_sha256 *hash);
static void secp256k1_sha256_initialize(secp256k1_sha256 *hash, sha256_transform_callback fn_transform);
static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *data, size_t size);
static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out32);
Comment on lines -19 to 28
Copy link
Contributor

@real-or-random real-or-random Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A potential problem with storing the callback in the secp256k1_sha256 struct is that you could have this series of events:

  • User sets callback to cb1.
  • User calls API function that calls secp256k1_sha256_initialize, which outputs some opaque object that contains the secp256k1_sha256 struct.
  • User sets callback go cb2.
  • Users passes the opaque object to another API function, which calls secp256k1_sha256_write, which calls cb1. UB.

(I believe) this cannot happen in the current code because there's no pair of API functions that behaves in this way, but it's a potential footgun for the future.

My suggestion is just passing the context object to every internal function that needs the SHA256 callback. (Another angle: State is annoying. We have state already in the context, so let's try to keep it there.)

static void secp256k1_sha256_clear(secp256k1_sha256 *hash);
Expand All @@ -25,7 +32,7 @@ typedef struct {
secp256k1_sha256 inner, outer;
} secp256k1_hmac_sha256;

static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t size);
static void secp256k1_hmac_sha256_initialize(secp256k1_hmac_sha256 *hash, const unsigned char *key, size_t size, sha256_transform_callback fn_sha256_transform);
static void secp256k1_hmac_sha256_write(secp256k1_hmac_sha256 *hash, const unsigned char *data, size_t size);
static void secp256k1_hmac_sha256_finalize(secp256k1_hmac_sha256 *hash, unsigned char *out32);
static void secp256k1_hmac_sha256_clear(secp256k1_hmac_sha256 *hash);
Expand All @@ -36,8 +43,8 @@ typedef struct {
int retry;
} secp256k1_rfc6979_hmac_sha256;

static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen);
static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 *rng, unsigned char *out, size_t outlen);
static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256 *rng, const unsigned char *key, size_t keylen, sha256_transform_callback fn_sha256_transform);
static void secp256k1_rfc6979_hmac_sha256_generate(secp256k1_rfc6979_hmac_sha256 *rng, unsigned char *out, size_t outlen, sha256_transform_callback fn_sha256_transform);
static void secp256k1_rfc6979_hmac_sha256_finalize(secp256k1_rfc6979_hmac_sha256 *rng);
static void secp256k1_rfc6979_hmac_sha256_clear(secp256k1_rfc6979_hmac_sha256 *rng);

Expand Down
Loading
Loading