From 8030d7c0e52047ca14c7758f4e44a1c784176393 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 12 Feb 2015 18:00:50 -0800 Subject: [PATCH] Improve signing API documentation & specification --- include/secp256k1.h | 13 ++++++++----- src/secp256k1.c | 18 ++++++++++++++++-- src/tests.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 5b219c301a..79477fa403 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -106,14 +106,16 @@ extern const secp256k1_nonce_function_t secp256k1_nonce_function_default; /** Create an ECDSA signature. * Returns: 1: signature created - * 0: the nonce generation function failed + * 0: the nonce generation function failed, the private key was invalid, or there is not + * enough space in the signature (as indicated by siglen). * In: msg32: the 32-byte message hash being signed (cannot be NULL) - * seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid) + * seckey: pointer to a 32-byte secret key (cannot be NULL) * noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used * ndata: pointer to arbitrary data used by the nonce generation function (can be NULL) * Out: sig: pointer to an array where the signature will be placed (cannot be NULL) * In/Out: siglen: pointer to an int with the length of sig, which will be updated - * to contain the actual signature length (<=72). + * to contain the actual signature length (<=72). If 0 is returned, this will be + * set to zero. * Requires starting using SECP256K1_START_SIGN. * * The sig always has an s value in the lower half of the range (From 0x1 @@ -153,12 +155,13 @@ int secp256k1_ecdsa_sign( /** Create a compact ECDSA signature (64 byte + recovery id). * Returns: 1: signature created - * 0: the nonce generation function failed + * 0: the nonce generation function failed, or the secret key was invalid. * In: msg32: the 32-byte message hash being signed (cannot be NULL) - * seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid) + * seckey: pointer to a 32-byte secret key (cannot be NULL) * noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used * ndata: pointer to arbitrary data used by the nonce generation function (can be NULL) * Out: sig: pointer to a 64-byte array where the signature will be placed (cannot be NULL) + * In case 0 is returned, the returned signature length will be zero. * recid: pointer to an int, which will be updated to contain the recovery id (can be NULL) * Requires starting using SECP256K1_START_SIGN. */ diff --git a/src/secp256k1.c b/src/secp256k1.c index 1b07b8a717..280dee7022 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -87,7 +87,11 @@ int secp256k1_ecdsa_sign(const unsigned char *msg32, unsigned char *signature, i noncefp = secp256k1_nonce_function_default; } - secp256k1_scalar_set_b32(&sec, seckey, NULL); + secp256k1_scalar_set_b32(&sec, seckey, &overflow); + if (overflow || secp256k1_scalar_is_zero(&sec)) { + *signaturelen = 0; + return 0; + } secp256k1_scalar_set_b32(&msg, msg32, NULL); while (1) { unsigned char nonce32[32]; @@ -107,6 +111,9 @@ int secp256k1_ecdsa_sign(const unsigned char *msg32, unsigned char *signature, i if (ret) { ret = secp256k1_ecdsa_sig_serialize(signature, signaturelen, &sig); } + if (!ret) { + *signaturelen = 0; + } secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); @@ -127,7 +134,11 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg32, unsigned char *sig6 noncefp = secp256k1_nonce_function_default; } - secp256k1_scalar_set_b32(&sec, seckey, NULL); + secp256k1_scalar_set_b32(&sec, seckey, &overflow); + if (overflow || secp256k1_scalar_is_zero(&sec)) { + memset(sig64, 0, 64); + return 0; + } secp256k1_scalar_set_b32(&msg, msg32, NULL); while (1) { unsigned char nonce32[32]; @@ -148,6 +159,9 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg32, unsigned char *sig6 secp256k1_scalar_get_b32(sig64, &sig.r); secp256k1_scalar_get_b32(sig64 + 32, &sig.s); } + if (!ret) { + memset(sig64, 0, 64); + } secp256k1_scalar_clear(&msg); secp256k1_scalar_clear(&non); secp256k1_scalar_clear(&sec); diff --git a/src/tests.c b/src/tests.c index 32bb73aa6a..313c0cc795 100644 --- a/src/tests.c +++ b/src/tests.c @@ -1229,6 +1229,11 @@ static int nonce_function_test_retry(unsigned char *nonce32, const unsigned char return nonce_function_rfc6979(nonce32, msg32, key32, counter - 5, data); } +int is_empty_compact_signature(const unsigned char *sig64) { + static const unsigned char res[64] = {0}; + return memcmp(sig64, res, 64) == 0; +} + void test_ecdsa_end_to_end(void) { unsigned char privkey[32]; unsigned char message[32]; @@ -1300,6 +1305,7 @@ void test_ecdsa_end_to_end(void) { /* Sign. */ CHECK(secp256k1_ecdsa_sign(message, signature, &signaturelen, privkey, NULL, NULL) == 1); + CHECK(signaturelen > 0); /* Verify. */ CHECK(secp256k1_ecdsa_verify(message, signature, signaturelen, pubkey, pubkeylen) == 1); /* Destroy signature and verify again. */ @@ -1308,6 +1314,7 @@ void test_ecdsa_end_to_end(void) { /* Compact sign. */ CHECK(secp256k1_ecdsa_sign_compact(message, csignature, privkey, NULL, NULL, &recid) == 1); + CHECK(!is_empty_compact_signature(csignature)); /* Recover. */ CHECK(secp256k1_ecdsa_recover_compact(message, csignature, recpubkey, &recpubkeylen, pubkeylen == 33, recid) == 1); CHECK(recpubkeylen == pubkeylen); @@ -1588,13 +1595,18 @@ void test_ecdsa_edge_cases(void) { unsigned char sig[72]; int siglen = 72; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) == 0); + CHECK(siglen == 0); CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce2) == 0); + CHECK(siglen == 0); msg[31] = 0xaa; siglen = 72; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) == 1); + CHECK(siglen > 0); CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce2) == 1); + CHECK(siglen > 0); siglen = 10; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) != 1); + CHECK(siglen == 0); } /* Nonce function corner cases. */ @@ -1608,29 +1620,46 @@ void test_ecdsa_edge_cases(void) { int siglen = 72; int siglen2 = 72; int recid2; - memset(key, 0, 32); memset(msg, 0, 32); - key[31] = 1; msg[31] = 1; + /* High key results in signature failure. */ + memset(key, 0xFF, 32); + CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 0); + CHECK(siglen == 0); + /* Zero key results in signature failure. */ + memset(key, 0, 32); + CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 0); + CHECK(siglen == 0); /* Nonce function failure results in signature failure. */ + key[31] = 1; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, nonce_function_test_fail, NULL) == 0); + CHECK(siglen == 0); CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, nonce_function_test_fail, NULL, &recid) == 0); + CHECK(is_empty_compact_signature(sig)); /* The retry loop successfully makes its way to the first good value. */ siglen = 72; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, nonce_function_test_retry, NULL) == 1); + CHECK(siglen > 0); CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, nonce_function_rfc6979, NULL) == 1); + CHECK(siglen > 0); CHECK((siglen == siglen2) && (memcmp(sig, sig2, siglen) == 0)); CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, nonce_function_test_retry, NULL, &recid) == 1); + CHECK(!is_empty_compact_signature(sig)); CHECK(secp256k1_ecdsa_sign_compact(msg, sig2, key, nonce_function_rfc6979, NULL, &recid2) == 1); + CHECK(!is_empty_compact_signature(sig2)); CHECK((recid == recid2) && (memcmp(sig, sig2, 64) == 0)); /* The default nonce function is determinstic. */ siglen = 72; siglen2 = 72; CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 1); + CHECK(siglen > 0); CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, NULL, NULL) == 1); + CHECK(siglen2 > 0); CHECK((siglen == siglen2) && (memcmp(sig, sig2, siglen) == 0)); CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, NULL, NULL, &recid) == 1); + CHECK(!is_empty_compact_signature(sig)); CHECK(secp256k1_ecdsa_sign_compact(msg, sig2, key, NULL, NULL, &recid2) == 1); + CHECK(!is_empty_compact_signature(sig)); CHECK((recid == recid2) && (memcmp(sig, sig2, 64) == 0)); /* The default nonce function changes output with different messages. */ for(i = 0; i < 256; i++) { @@ -1638,6 +1667,7 @@ void test_ecdsa_edge_cases(void) { siglen2 = 72; msg[0] = i; CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, NULL, NULL) == 1); + CHECK(!is_empty_compact_signature(sig)); CHECK(secp256k1_ecdsa_sig_parse(&s[i], sig2, siglen2)); for (j = 0; j < i; j++) { CHECK(!secp256k1_scalar_eq(&s[i].r, &s[j].r));