[API BREAK] Use a nonce-generation function instead of a nonce #162

pull sipa wants to merge 1 commits into bitcoin-core:master from sipa:noncefp changing 5 files +112 −46

sipa commented at 5:12 PM on December 12, 2014: contributor

No description provided.
sipa force-pushed on Dec 12, 2014

in include/secp256k1.h:None in 82c9b3e6d0 outdated

  91 | +typedef int (*secp256k1_nonce_function_t)(
  92 | +  unsigned char *nonce32,
  93 | +  const unsigned char *msg32,
  94 | +  const unsigned char *key32,
  95 | +  unsigned int attempt,
  96 | +  const void* data

aalness commented at 5:27 PM on December 12, 2014:

nit: inconsistency in * placement everywhere for the callback data.

sipa commented at 5:29 PM on December 12, 2014:

Fixed.

sipa force-pushed on Dec 12, 2014
in src/bench_sign.c:None in cc60a8df2b outdated
```
   7 | @@ -8,17 +8,26 @@
   8 |  #include "util.h"
   9 |  #include "bench.h"
  10 |  
  11 | +#include <string.h>
```
aalness commented at 5:31 PM on December 12, 2014:

nit: don't see where this dependency was introduced

sipa commented at 5:32 PM on December 12, 2014:

Fixed, It was introduced in an earlier edit that was changed again.
sipa force-pushed on Dec 12, 2014

in include/secp256k1.h:None in 2f87366bad outdated

  76 | @@ -77,40 +77,63 @@ SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify(
  77 |    int pubkeylen
  78 |  ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(4);
  79 |  
  80 | +/** A pointer to a function to deterministically generate a nonce.
  81 | + * Returns: 1 if a nonce was succesfully generated. 0 will cause signing to fail.
  82 | + * In:      msg32:     the 32-byte message hash being verified (will not be NULL)
  83 | + *          key32:     pointer to a 32-byte secret key (will not be NULL)
  84 | + *          attempt:   how many iterations we have tried to find a nonce.

aalness commented at 5:40 PM on December 12, 2014:

Maybe there should be an explicit mention that if attempt is non-zero it implies a different nonce is required.

sipa commented at 5:41 PM on December 12, 2014:

Maybe there should be an explicit mention that if attempt is non-zero it implies a different nonce is required.

Done.

aalness commented at 5:41 PM on December 12, 2014: none

Change looks reasonable to me. Seems safer in any case.
sipa force-pushed on Dec 12, 2014
aalness cross-referenced this on Dec 12, 2014 from issue Initial commit to add libsecp256k1 support by aalness
gmaxwell commented at 11:30 PM on December 12, 2014: contributor

Perhaps the comment for the nonce function should say "The resulting output MUST be uniformly distributed. Even small biases can compromises the security of the private key."
peterdettman commented at 8:56 AM on December 13, 2014: contributor

Would it be better to bundle the callback args into a struct?
sipa commented at 3:14 PM on December 13, 2014: contributor

@peterdettman Which args?
sipa force-pushed on Dec 13, 2014
sipa commented at 3:18 PM on December 13, 2014: contributor

@gmaxwell Added a comment.
sipa cross-referenced this on Dec 13, 2014 from issue [API BREAK] Use rfc6979 as default nonce-generation function by sipa
peterdettman commented at 4:27 PM on December 14, 2014: contributor

I mean that secp256k1_nonce_function_t could just have a single struct* parameter, which struct would contain nonce32, msg32, key32, attempt, data. I'm just thinking that if later we discover we want to pass more parameters, they could be added to the struct without a breaking change to the API.
sipa commented at 10:38 PM on December 16, 2014: contributor

I don't consider the API stable at this point anyway.
sipa force-pushed on Dec 18, 2014
[API BREAK] Use a nonce-generation function instead of a nonce c6e7f4e8d8
sipa force-pushed on Dec 20, 2014
sipa merged this on Dec 23, 2014
sipa closed this on Dec 23, 2014

Contributors

Linked (view graph)

#163 [API BREAK] Use rfc6979 as default nonce-generation function