From 2876af4f8da952e39c06bc229d68cd4892ea2c85 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Wed, 27 May 2020 00:37:59 +0300 Subject: [PATCH] Split ecdsa_sign logic into a new function and use it from ecdsa_sign and recovery --- src/modules/recovery/main_impl.h | 39 +++----------------------------- src/secp256k1.c | 38 ++++++++++++++++++++++--------- src/util.h | 14 ++++++++++++ 3 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/modules/recovery/main_impl.h b/src/modules/recovery/main_impl.h index ed356e53a5..e2576aa953 100644 --- a/src/modules/recovery/main_impl.h +++ b/src/modules/recovery/main_impl.h @@ -122,48 +122,15 @@ static int secp256k1_ecdsa_sig_recover(const secp256k1_ecmult_context *ctx, cons int secp256k1_ecdsa_sign_recoverable(const secp256k1_context* ctx, secp256k1_ecdsa_recoverable_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata) { secp256k1_scalar r, s; - secp256k1_scalar sec, non, msg; - int recid; - int ret = 0; - int overflow = 0; + int ret, recid; VERIFY_CHECK(ctx != NULL); ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); ARG_CHECK(msg32 != NULL); ARG_CHECK(signature != NULL); ARG_CHECK(seckey != NULL); - if (noncefp == NULL) { - noncefp = secp256k1_nonce_function_default; - } - secp256k1_scalar_set_b32(&sec, seckey, &overflow); - /* Fail if the secret key is invalid. */ - if (!overflow && !secp256k1_scalar_is_zero(&sec)) { - unsigned char nonce32[32]; - unsigned int count = 0; - secp256k1_scalar_set_b32(&msg, msg32, NULL); - while (1) { - ret = noncefp(nonce32, msg32, seckey, NULL, (void*)noncedata, count); - if (!ret) { - break; - } - secp256k1_scalar_set_b32(&non, nonce32, &overflow); - if (!overflow && !secp256k1_scalar_is_zero(&non)) { - if (secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, &recid)) { - break; - } - } - count++; - } - memset(nonce32, 0, 32); - secp256k1_scalar_clear(&msg); - secp256k1_scalar_clear(&non); - secp256k1_scalar_clear(&sec); - } - if (ret) { - secp256k1_ecdsa_recoverable_signature_save(signature, &r, &s, recid); - } else { - memset(signature, 0, sizeof(*signature)); - } + ret = secp256k1_ecdsa_sign_inner(ctx, &r, &s, &recid, msg32, seckey, noncefp, noncedata); + secp256k1_ecdsa_recoverable_signature_save(signature, &r, &s, recid); return ret; } diff --git a/src/secp256k1.c b/src/secp256k1.c index 2b2238b8f8..b03a6e6345 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -467,19 +467,18 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m const secp256k1_nonce_function secp256k1_nonce_function_rfc6979 = nonce_function_rfc6979; const secp256k1_nonce_function secp256k1_nonce_function_default = nonce_function_rfc6979; -int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata) { - /* Default initialization here is important so we won't pass uninit values to the cmov in the end */ - secp256k1_scalar r = secp256k1_scalar_zero, s = secp256k1_scalar_zero; +static int secp256k1_ecdsa_sign_inner(const secp256k1_context* ctx, secp256k1_scalar* r, secp256k1_scalar* s, int* recid, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata) { secp256k1_scalar sec, non, msg; int ret = 0; int is_sec_valid; unsigned char nonce32[32]; unsigned int count = 0; - VERIFY_CHECK(ctx != NULL); - ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); - ARG_CHECK(msg32 != NULL); - ARG_CHECK(signature != NULL); - ARG_CHECK(seckey != NULL); + /* Default initialization here is important so we won't pass uninit values to the cmov in the end */ + *r = secp256k1_scalar_zero; + *s = secp256k1_scalar_zero; + if (recid) { + *recid = 0; + } if (noncefp == NULL) { noncefp = secp256k1_nonce_function_default; } @@ -498,7 +497,7 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature /* The nonce is still secret here, but it being invalid is is less likely than 1:2^255. */ secp256k1_declassify(ctx, &is_nonce_valid, sizeof(is_nonce_valid)); if (is_nonce_valid) { - ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, &r, &s, &sec, &msg, &non, NULL); + ret = secp256k1_ecdsa_sig_sign(&ctx->ecmult_gen_ctx, r, s, &sec, &msg, &non, recid); /* The final signature is no longer a secret, nor is the fact that we were successful or not. */ secp256k1_declassify(ctx, &ret, sizeof(ret)); if (ret) { @@ -515,8 +514,25 @@ int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); - secp256k1_scalar_cmov(&r, &secp256k1_scalar_zero, !ret); - secp256k1_scalar_cmov(&s, &secp256k1_scalar_zero, !ret); + secp256k1_scalar_cmov(r, &secp256k1_scalar_zero, !ret); + secp256k1_scalar_cmov(s, &secp256k1_scalar_zero, !ret); + if (recid) { + const int zero = 0; + secp256k1_int_cmov(recid, &zero, !ret); + } + return ret; +} + +int secp256k1_ecdsa_sign(const secp256k1_context* ctx, secp256k1_ecdsa_signature *signature, const unsigned char *msg32, const unsigned char *seckey, secp256k1_nonce_function noncefp, const void* noncedata) { + secp256k1_scalar r, s; + int ret; + VERIFY_CHECK(ctx != NULL); + ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); + ARG_CHECK(msg32 != NULL); + ARG_CHECK(signature != NULL); + ARG_CHECK(seckey != NULL); + + ret = secp256k1_ecdsa_sign_inner(ctx, &r, &s, NULL, msg32, seckey, noncefp, noncedata); secp256k1_ecdsa_signature_save(signature, &r, &s); return ret; } diff --git a/src/util.h b/src/util.h index a93ffb182b..8289e23e0c 100644 --- a/src/util.h +++ b/src/util.h @@ -194,4 +194,18 @@ static SECP256K1_INLINE void memczero(void *s, size_t len, int flag) { } } +/** If flag is true, set *r equal to *a; otherwise leave it. Constant-time. Both *r and *a must be initialized and non-negative.*/ +static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag) { + unsigned int mask0, mask1, r_masked, a_masked; + /* Casting a negative int to unsigned and back to int is implementation defined behavior */ + VERIFY_CHECK(*r >= 0 && *a >= 0); + + mask0 = (unsigned int)flag + ~0u; + mask1 = ~mask0; + r_masked = ((unsigned int)*r & mask0); + a_masked = ((unsigned int)*a & mask1); + + *r = (int)(r_masked | a_masked); +} + #endif /* SECP256K1_UTIL_H */