From b30fc85c9eb24dfbfa2c21175a42aa0a98319d76 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Mon, 19 Oct 2015 23:35:29 +0000 Subject: [PATCH] Avoid nonce_function_rfc6979 algo16 argument emulation. This avoids data=NULL and data = zeros to producing the same nonce. Previously the code tried to avoid the case where some data inputs aliased algo16 inputs by always padding out the data. But because algo16 and data are different lengths they cannot emulate each other, and the padding would match a data value of all zeros. --- src/secp256k1.c | 11 ++++++----- src/tests.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/secp256k1.c b/src/secp256k1.c index a9e5a6d0..42b8df73 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -300,8 +300,10 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m /* We feed a byte array to the PRNG as input, consisting of: * - the private key (32 bytes) and message (32 bytes), see RFC 6979 3.2d. * - optionally 32 extra bytes of data, see RFC 6979 3.6 Additional Data. - * - optionally 16 extra bytes with the algorithm name (the extra data bytes - * are set to zeroes when not present, while the algorithm name is). + * - optionally 16 extra bytes with the algorithm name. + * Because the arguments have distinct fixed lengths it is not possible for + * different argument mixtures to emulate each other and result in the same + * nonces. */ memcpy(keydata, key32, 32); memcpy(keydata + 32, msg32, 32); @@ -310,9 +312,8 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m keylen = 96; } if (algo16 != NULL) { - memset(keydata + keylen, 0, 96 - keylen); - memcpy(keydata + 96, algo16, 16); - keylen = 112; + memcpy(keydata + keylen, algo16, 16); + keylen += 16; } secp256k1_rfc6979_hmac_sha256_initialize(&rng, keydata, keylen); memset(keydata, 0, sizeof(keydata)); diff --git a/src/tests.c b/src/tests.c index 0e047173..2d8de12d 100644 --- a/src/tests.c +++ b/src/tests.c @@ -2960,6 +2960,34 @@ void test_ecdsa_edge_cases(void) { key[0] = 0; } + { + /* Check that optional nonce arguments do not have equivilent effect. */ + const unsigned char zeros[32] = {0}; + unsigned char nonce[32]; + unsigned char nonce2[32]; + unsigned char nonce3[32]; + unsigned char nonce4[32]; + VG_UNDEF(nonce,32); + VG_UNDEF(nonce2,32); + VG_UNDEF(nonce3,32); + VG_UNDEF(nonce4,32); + CHECK(nonce_function_rfc6979(nonce, zeros, zeros, NULL, NULL, 0) == 1); + VG_CHECK(nonce,32); + CHECK(nonce_function_rfc6979(nonce2, zeros, zeros, zeros, NULL, 0) == 1); + VG_CHECK(nonce2,32); + CHECK(nonce_function_rfc6979(nonce3, zeros, zeros, NULL, (void *)zeros, 0) == 1); + VG_CHECK(nonce3,32); + CHECK(nonce_function_rfc6979(nonce4, zeros, zeros, zeros, (void *)zeros, 0) == 1); + VG_CHECK(nonce4,32); + CHECK(memcmp(nonce, nonce2, 32) != 0); + CHECK(memcmp(nonce, nonce3, 32) != 0); + CHECK(memcmp(nonce, nonce4, 32) != 0); + CHECK(memcmp(nonce2, nonce3, 32) != 0); + CHECK(memcmp(nonce2, nonce4, 32) != 0); + CHECK(memcmp(nonce3, nonce4, 32) != 0); + } + + /* Privkey export where pubkey is the point at infinity. */ { unsigned char privkey[300];