Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change RFC6979 implementation to be a generic PRNG #269

Merged
merged 1 commit into from
Jul 24, 2015

Conversation

sipa
Copy link
Contributor

@sipa sipa commented Jul 8, 2015

Instead of making it take key, message, and extra data separate, just pass in a byte array of seed data, turning it into just a non-EC specific PRNG.

The usage inside the blinding code and the tests is also adapted to make use of this in a more natural way.

@sipa
Copy link
Contributor Author

sipa commented Jul 16, 2015

@gmaxwell Want to review?

@dcousens
Copy link
Contributor

@sipa maybe its taken care by secp256k1_rfc6979_hmac_sha256_initialize, but RFC6979 specifies:

If that value of k is within the [1,q-1] range, and is suitable for DSA or ECDSA (i.e., it results in an r value that is not 0; see Section 3.4), then the generation of k is finished. The obtained value of k is used in DSA or ECDSA.

Is this handled externally [to that function] as part of the libraries signing process?
In which case, I assume you feed T back in somehow?
If you don't feed it back, and just restart, then technically this isn't RFC6979 compliant.

@sipa
Copy link
Contributor Author

sipa commented Jul 18, 2015

@dcousens Right, secp256k1_rfc6979_hmac_sha256 only implements the PRNG side of the construction. The specific use as defined by RFC6979 is done inside the signing code.

@apoelstra
Copy link
Contributor

With benchmarks on

src/bench_internal.c: In function ‘bench_rfc6979_hmac_sha256’:
src/bench_internal.c:268:9: error: too many arguments to function ‘secp256k1_rfc6979_hmac_sha256_initialize’
         secp256k1_rfc6979_hmac_sha256_initialize(&rng, data->data, 32, data->data, 32, NULL, 0);
         ^
In file included from src/bench_internal.c:11:0:
src/hash_impl.h:205:13: note: declared here
 static void secp256k1_rfc6979_hmac_sha256_initialize(secp256k1_rfc6979_hmac_sha256_t *rng, const unsigned char *key, size_t keylen) {
             ^
make: *** [src/bench_internal-bench_internal.o] Error 1

@sipa
Copy link
Contributor Author

sipa commented Jul 24, 2015

Rebased and fixed bug found by @apoelstra.

@gmaxwell
Copy link
Contributor

ACK. (but see nit)

@apoelstra
Copy link
Contributor

tested ACK

@sipa
Copy link
Contributor Author

sipa commented Jul 24, 2015

Addressed nit.

@sipa sipa merged commit 3e6f1e2 into bitcoin-core:master Jul 24, 2015
sipa added a commit that referenced this pull request Jul 24, 2015
3e6f1e2 Change rfc6979 implementation to be a generic PRNG (Pieter Wuille)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants