From e0e4c0ec1fc479527ab896dbc1dc55d66e151009 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Mon, 7 Jul 2025 23:12:12 +1200 Subject: [PATCH 1/4] ccan: call mbedtls_sha256_free in sha256_done --- src/ccan/ccan/crypto/sha256/sha256.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ccan/ccan/crypto/sha256/sha256.c b/src/ccan/ccan/crypto/sha256/sha256.c index 6f3c5478c..582b40f16 100644 --- a/src/ccan/ccan/crypto/sha256/sha256.c +++ b/src/ccan/ccan/crypto/sha256/sha256.c @@ -59,6 +59,7 @@ inline void sha256_update(struct sha256_ctx *ctx, const void *p, size_t size) inline void sha256_done(struct sha256_ctx *ctx, struct sha256* res) { mbedtls_sha256_finish(&ctx->c, res->u.u8); + mbedtls_sha256_free(&ctx->c); } void sha256_optimize(void) { From f5fc513ac353c6a53080616b20275b5ac1bad6c1 Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Mon, 7 Jul 2025 23:31:38 +1200 Subject: [PATCH 2/4] ccan: call mbedtls_sha512_free in sha512_done Rearrange the code slightly to match the sha256 impl. --- src/ccan/ccan/crypto/sha512/sha512.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/ccan/ccan/crypto/sha512/sha512.c b/src/ccan/ccan/crypto/sha512/sha512.c index 3811357fc..a8629bc99 100644 --- a/src/ccan/ccan/crypto/sha512/sha512.c +++ b/src/ccan/ccan/crypto/sha512/sha512.c @@ -15,28 +15,19 @@ #endif #include +#ifdef CCAN_CRYPTO_SHA512_USE_OPENSSL static void invalidate_sha512(struct sha512_ctx *ctx) { -#ifdef CCAN_CRYPTO_SHA512_USE_OPENSSL ctx->c.md_len = 0; -#elif defined(CCAN_CRYPTO_SHA512_USE_MBEDTLS) -#else - ctx->bytes = (size_t)-1; -#endif } static void check_sha512(struct sha512_ctx *ctx UNUSED) { #if 0 -#ifdef CCAN_CRYPTO_SHA512_USE_OPENSSL assert(ctx->c.md_len != 0); -#else - assert(ctx->bytes != (size_t)-1); -#endif #endif } -#ifdef CCAN_CRYPTO_SHA512_USE_OPENSSL void sha512_init(struct sha512_ctx *ctx) { SHA512_Init(&ctx->c); @@ -62,16 +53,27 @@ inline void sha512_init(struct sha512_ctx *ctx) inline void sha512_update(struct sha512_ctx *ctx, const void *p, size_t size) { - check_sha512(ctx); mbedtls_sha512_update(&ctx->c, p, size); } inline void sha512_done(struct sha512_ctx *ctx, struct sha512* res) { mbedtls_sha512_finish(&ctx->c, res->u.u8); - invalidate_sha512(ctx); + mbedtls_sha512_free(&ctx->c); } #else +static void invalidate_sha512(struct sha512_ctx *ctx) +{ + ctx->bytes = (size_t)-1; +} + +static void check_sha512(struct sha512_ctx *ctx UNUSED) +{ +#if 0 + assert(ctx->bytes != (size_t)-1); +#endif +} + static uint64_t Ch(uint64_t x, uint64_t y, uint64_t z) { return z ^ (x & (y ^ z)); From abbe82a57ed4d83a8ae2d9882f445d4918571c3a Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Wed, 9 Jul 2025 16:58:30 +1200 Subject: [PATCH 3/4] ccan: expose ALIGNED for future manual structure alignment --- src/ccan/ccan/compiler/compiler.h | 12 ++++++++++++ src/ccan/ccan/crypto/sha256/sha256_sse4.c | 3 --- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/ccan/ccan/compiler/compiler.h b/src/ccan/ccan/compiler/compiler.h index bce4f25a1..4342fe18b 100644 --- a/src/ccan/ccan/compiler/compiler.h +++ b/src/ccan/ccan/compiler/compiler.h @@ -228,4 +228,16 @@ #define WARN_UNUSED_RESULT #endif #endif + +/* ALIGNED - ensure a structure/variable is aligned to a given number of bytes + * + */ +#ifndef ALIGNED +#if (defined(__clang__) || defined(__GNUC__)) +#define ALIGNED(N) __attribute__((aligned(N))) +#else +#define ALIGNED(N) +#endif +#endif /* ALIGNED */ + #endif /* CCAN_COMPILER_H */ diff --git a/src/ccan/ccan/crypto/sha256/sha256_sse4.c b/src/ccan/ccan/crypto/sha256/sha256_sse4.c index 8ead1dbf6..7eb32fca9 100644 --- a/src/ccan/ccan/crypto/sha256/sha256_sse4.c +++ b/src/ccan/ccan/crypto/sha256/sha256_sse4.c @@ -9,9 +9,6 @@ #include #if defined(__x86_64__) || defined(__amd64__) -/* TODO: Support alignment in compiler.h */ -#define ALIGNED(N) __attribute__((aligned(N))) - void TransformSSE4(uint32_t* s, const uint32_t* chunk, size_t blocks) { static const uint32_t K256[] ALIGNED(16) = { From 8fc41b9d2b152868953176176cff9890aa268c8d Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Wed, 16 Jul 2025 19:17:41 +1200 Subject: [PATCH 4/4] tx_io: avoid caching sha256 context for external sha256 impls For example, mbedtls hardware hashing requires (at least) semaphore initialization/aquisition performed by sha256_init() to be done before the context is considered valid. Openssl with HW hashers is likely the same for at least some configs. so, for non-standard setups like these, avoid caching the initial bip340 tagged hash context. Many thanks to github users @advorzhak and @hazrulnizam for helping to reproduce the issue! --- src/tx_io.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/tx_io.c b/src/tx_io.c index aea93e988..fb170a0fb 100644 --- a/src/tx_io.c +++ b/src/tx_io.c @@ -10,6 +10,16 @@ #define SIGTYPE_ALL (WALLY_SIGTYPE_PRE_SW | WALLY_SIGTYPE_SW_V0 | WALLY_SIGTYPE_SW_V1) +#if defined(CCAN_CRYPTO_SHA256_USE_OPENSSL) || defined(CCAN_CRYPTO_SHA256_USE_MBEDTLS) +/* For external sha256 implementations, we cannot cache the sha256 context as + * they require extra setup before use that only sha256_init() provides. + */ +#define TXIO_CTX_CACHEABLE 0 +#else +/* For our built-in sha256 implementation we can cache and use the context */ +#define TXIO_CTX_CACHEABLE 1 +#endif + /* Cache keys for data that is constant while signing a given tx. * We also cache other data keyed by their binary value directly. */ @@ -761,12 +771,14 @@ static int bip143_signature_hash( static void txio_bip341_init(cursor_io *io, const unsigned char *genesis_blockhash, size_t genesis_blockhash_len) { - const struct wally_map_item *item; - item = io->cache ? wally_map_get_integer(io->cache, TXIO_SHA_TAPSIGHASH_CTX) : NULL; - if (item) { - /* Note we hash the intial sha256_ctx itself here and so memcpy it */ - memcpy(&io->ctx, item->value, item->value_len); - return; + if (TXIO_CTX_CACHEABLE && io->cache) { + const struct wally_map_item *item = NULL; + item = wally_map_get_integer(io->cache, TXIO_SHA_TAPSIGHASH_CTX); + if (item) { + /* Note we cached the intial sha256_ctx itself here and so memcpy it */ + memcpy(&io->ctx, item->value, item->value_len); + return; + } } tagged_hash_init(&io->ctx, TAPSIGHASH_SHA256(genesis_blockhash != NULL), SHA256_LEN); @@ -774,7 +786,7 @@ static void txio_bip341_init(cursor_io *io, hash_bytes(&io->ctx, genesis_blockhash, genesis_blockhash_len); hash_bytes(&io->ctx, genesis_blockhash, genesis_blockhash_len); } - if (io->cache) + if (TXIO_CTX_CACHEABLE && io->cache) wally_map_add_integer(io->cache, TXIO_SHA_TAPSIGHASH_CTX, (const unsigned char*)&io->ctx, sizeof(io->ctx)); }