Skip to content

Commit 8030d7c

Browse files
sipasipa
committed
Improve signing API documentation & specification
1 parent 7b2fc1c commit 8030d7c

File tree

3 files changed

+56
-9
lines changed

3 files changed

+56
-9
lines changed

‎include/secp256k1.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,16 @@ extern const secp256k1_nonce_function_t secp256k1_nonce_function_default;
106106

107107
/** Create an ECDSA signature.
108108
* Returns: 1: signature created
109-
* 0: the nonce generation function failed
109+
* 0: the nonce generation function failed, the private key was invalid, or there is not
110+
* enough space in the signature (as indicated by siglen).
110111
* In: msg32: the 32-byte message hash being signed (cannot be NULL)
111-
* seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid)
112+
* seckey: pointer to a 32-byte secret key (cannot be NULL)
112113
* noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used
113114
* ndata: pointer to arbitrary data used by the nonce generation function (can be NULL)
114115
* Out: sig: pointer to an array where the signature will be placed (cannot be NULL)
115116
* In/Out: siglen: pointer to an int with the length of sig, which will be updated
116-
* to contain the actual signature length (<=72).
117+
* to contain the actual signature length (<=72). If 0 is returned, this will be
118+
* set to zero.
117119
* Requires starting using SECP256K1_START_SIGN.
118120
*
119121
* The sig always has an s value in the lower half of the range (From 0x1
@@ -153,12 +155,13 @@ int secp256k1_ecdsa_sign(
153155

154156
/** Create a compact ECDSA signature (64 byte + recovery id).
155157
* Returns: 1: signature created
156-
* 0: the nonce generation function failed
158+
* 0: the nonce generation function failed, or the secret key was invalid.
157159
* In: msg32: the 32-byte message hash being signed (cannot be NULL)
158-
* seckey: pointer to a 32-byte secret key (cannot be NULL, assumed to be valid)
160+
* seckey: pointer to a 32-byte secret key (cannot be NULL)
159161
* noncefp:pointer to a nonce generation function. If NULL, secp256k1_nonce_function_default is used
160162
* ndata: pointer to arbitrary data used by the nonce generation function (can be NULL)
161163
* Out: sig: pointer to a 64-byte array where the signature will be placed (cannot be NULL)
164+
* In case 0 is returned, the returned signature length will be zero.
162165
* recid: pointer to an int, which will be updated to contain the recovery id (can be NULL)
163166
* Requires starting using SECP256K1_START_SIGN.
164167
*/

‎src/secp256k1.c‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ int secp256k1_ecdsa_sign(const unsigned char *msg32, unsigned char *signature, i
8787
noncefp = secp256k1_nonce_function_default;
8888
}
8989

90-
secp256k1_scalar_set_b32(&sec, seckey, NULL);
90+
secp256k1_scalar_set_b32(&sec, seckey, &overflow);
91+
if (overflow || secp256k1_scalar_is_zero(&sec)) {
92+
*signaturelen = 0;
93+
return 0;
94+
}
9195
secp256k1_scalar_set_b32(&msg, msg32, NULL);
9296
while (1) {
9397
unsigned char nonce32[32];
@@ -107,6 +111,9 @@ int secp256k1_ecdsa_sign(const unsigned char *msg32, unsigned char *signature, i
107111
if (ret) {
108112
ret = secp256k1_ecdsa_sig_serialize(signature, signaturelen, &sig);
109113
}
114+
if (!ret) {
115+
*signaturelen = 0;
116+
}
110117
secp256k1_scalar_clear(&msg);
111118
secp256k1_scalar_clear(&non);
112119
secp256k1_scalar_clear(&sec);
@@ -127,7 +134,11 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg32, unsigned char *sig6
127134
noncefp = secp256k1_nonce_function_default;
128135
}
129136

130-
secp256k1_scalar_set_b32(&sec, seckey, NULL);
137+
secp256k1_scalar_set_b32(&sec, seckey, &overflow);
138+
if (overflow || secp256k1_scalar_is_zero(&sec)) {
139+
memset(sig64, 0, 64);
140+
return 0;
141+
}
131142
secp256k1_scalar_set_b32(&msg, msg32, NULL);
132143
while (1) {
133144
unsigned char nonce32[32];
@@ -148,6 +159,9 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg32, unsigned char *sig6
148159
secp256k1_scalar_get_b32(sig64, &sig.r);
149160
secp256k1_scalar_get_b32(sig64 + 32, &sig.s);
150161
}
162+
if (!ret) {
163+
memset(sig64, 0, 64);
164+
}
151165
secp256k1_scalar_clear(&msg);
152166
secp256k1_scalar_clear(&non);
153167
secp256k1_scalar_clear(&sec);

‎src/tests.c‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,6 +1229,11 @@ static int nonce_function_test_retry(unsigned char *nonce32, const unsigned char
12291229
return nonce_function_rfc6979(nonce32, msg32, key32, counter - 5, data);
12301230
}
12311231

1232+
int is_empty_compact_signature(const unsigned char *sig64) {
1233+
static const unsigned char res[64] = {0};
1234+
return memcmp(sig64, res, 64) == 0;
1235+
}
1236+
12321237
void test_ecdsa_end_to_end(void) {
12331238
unsigned char privkey[32];
12341239
unsigned char message[32];
@@ -1300,6 +1305,7 @@ void test_ecdsa_end_to_end(void) {
13001305

13011306
/* Sign. */
13021307
CHECK(secp256k1_ecdsa_sign(message, signature, &signaturelen, privkey, NULL, NULL) == 1);
1308+
CHECK(signaturelen > 0);
13031309
/* Verify. */
13041310
CHECK(secp256k1_ecdsa_verify(message, signature, signaturelen, pubkey, pubkeylen) == 1);
13051311
/* Destroy signature and verify again. */
@@ -1308,6 +1314,7 @@ void test_ecdsa_end_to_end(void) {
13081314

13091315
/* Compact sign. */
13101316
CHECK(secp256k1_ecdsa_sign_compact(message, csignature, privkey, NULL, NULL, &recid) == 1);
1317+
CHECK(!is_empty_compact_signature(csignature));
13111318
/* Recover. */
13121319
CHECK(secp256k1_ecdsa_recover_compact(message, csignature, recpubkey, &recpubkeylen, pubkeylen == 33, recid) == 1);
13131320
CHECK(recpubkeylen == pubkeylen);
@@ -1588,13 +1595,18 @@ void test_ecdsa_edge_cases(void) {
15881595
unsigned char sig[72];
15891596
int siglen = 72;
15901597
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) == 0);
1598+
CHECK(siglen == 0);
15911599
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce2) == 0);
1600+
CHECK(siglen == 0);
15921601
msg[31] = 0xaa;
15931602
siglen = 72;
15941603
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) == 1);
1604+
CHECK(siglen > 0);
15951605
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce2) == 1);
1606+
CHECK(siglen > 0);
15961607
siglen = 10;
15971608
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, precomputed_nonce_function, nonce) != 1);
1609+
CHECK(siglen == 0);
15981610
}
15991611

16001612
/* Nonce function corner cases. */
@@ -1608,36 +1620,54 @@ void test_ecdsa_edge_cases(void) {
16081620
int siglen = 72;
16091621
int siglen2 = 72;
16101622
int recid2;
1611-
memset(key, 0, 32);
16121623
memset(msg, 0, 32);
1613-
key[31] = 1;
16141624
msg[31] = 1;
1625+
/* High key results in signature failure. */
1626+
memset(key, 0xFF, 32);
1627+
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 0);
1628+
CHECK(siglen == 0);
1629+
/* Zero key results in signature failure. */
1630+
memset(key, 0, 32);
1631+
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 0);
1632+
CHECK(siglen == 0);
16151633
/* Nonce function failure results in signature failure. */
1634+
key[31] = 1;
16161635
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, nonce_function_test_fail, NULL) == 0);
1636+
CHECK(siglen == 0);
16171637
CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, nonce_function_test_fail, NULL, &recid) == 0);
1638+
CHECK(is_empty_compact_signature(sig));
16181639
/* The retry loop successfully makes its way to the first good value. */
16191640
siglen = 72;
16201641
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, nonce_function_test_retry, NULL) == 1);
1642+
CHECK(siglen > 0);
16211643
CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, nonce_function_rfc6979, NULL) == 1);
1644+
CHECK(siglen > 0);
16221645
CHECK((siglen == siglen2) && (memcmp(sig, sig2, siglen) == 0));
16231646
CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, nonce_function_test_retry, NULL, &recid) == 1);
1647+
CHECK(!is_empty_compact_signature(sig));
16241648
CHECK(secp256k1_ecdsa_sign_compact(msg, sig2, key, nonce_function_rfc6979, NULL, &recid2) == 1);
1649+
CHECK(!is_empty_compact_signature(sig2));
16251650
CHECK((recid == recid2) && (memcmp(sig, sig2, 64) == 0));
16261651
/* The default nonce function is determinstic. */
16271652
siglen = 72;
16281653
siglen2 = 72;
16291654
CHECK(secp256k1_ecdsa_sign(msg, sig, &siglen, key, NULL, NULL) == 1);
1655+
CHECK(siglen > 0);
16301656
CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, NULL, NULL) == 1);
1657+
CHECK(siglen2 > 0);
16311658
CHECK((siglen == siglen2) && (memcmp(sig, sig2, siglen) == 0));
16321659
CHECK(secp256k1_ecdsa_sign_compact(msg, sig, key, NULL, NULL, &recid) == 1);
1660+
CHECK(!is_empty_compact_signature(sig));
16331661
CHECK(secp256k1_ecdsa_sign_compact(msg, sig2, key, NULL, NULL, &recid2) == 1);
1662+
CHECK(!is_empty_compact_signature(sig));
16341663
CHECK((recid == recid2) && (memcmp(sig, sig2, 64) == 0));
16351664
/* The default nonce function changes output with different messages. */
16361665
for(i = 0; i < 256; i++) {
16371666
int j;
16381667
siglen2 = 72;
16391668
msg[0] = i;
16401669
CHECK(secp256k1_ecdsa_sign(msg, sig2, &siglen2, key, NULL, NULL) == 1);
1670+
CHECK(!is_empty_compact_signature(sig));
16411671
CHECK(secp256k1_ecdsa_sig_parse(&s[i], sig2, siglen2));
16421672
for (j = 0; j < i; j++) {
16431673
CHECK(!secp256k1_scalar_eq(&s[i].r, &s[j].r));

0 commit comments

Comments
 (0)