Skip to content

ecdsa: implement RFC6979 ephemeral scalar generation#133

Merged
tarcieri merged 1 commit into
masterfrom
ecdsa/rfc6979
Aug 31, 2020
Merged

ecdsa: implement RFC6979 ephemeral scalar generation#133
tarcieri merged 1 commit into
masterfrom
ecdsa/rfc6979

Conversation

@tarcieri

@tarcieri tarcieri commented Aug 26, 2020

Copy link
Copy Markdown
Member

Adds a deterministic signing mode based on RFC6979. Closes #124.

This could be further extended to bolster the security when using RandomizedSigner using the method described in Section 3.6.

Edit: went ahead and made RandomizedDigestSigner use an RFC6979-style derivation but supplying some additional data from the RNG as input. It was easy enough.

@tarcieri tarcieri marked this pull request as draft August 26, 2020 22:29
@tarcieri tarcieri requested a review from newpavlov August 26, 2020 22:29
/// Internal implementation of `HMAC_DRBG` as described in NIST SP800-90A:
/// <https://csrc.nist.gov/publications/detail/sp/800-90a/rev-1/final>
///
/// This is a HMAC-based deterministic random bit generator used internally
/// to compute a deterministic ECDSA ephemeral scalar `k`.
// TODO(tarcieri): generalize and extract this into the `hmac` crate?
struct HmacDrbg<D>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@newpavlov it might be interesting to extract or reimplement this in the hmac crate (possibly as a CryptoRng?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be better to implement it in a separate crate and it would fit nicely into the currently empty CSRNGs repository. And it looks like @sorpaas has already published such crate (although without implementing the rand_core traits).

@sorpaas
Would you be interested in transferring hmac-drbg crate to this organization?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh neat! Unfortunately it's hmac dependency is a version behind or otherwise it looks like what I need.

@tarcieri tarcieri Aug 31, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened a PR to bump the hmac dependency here: sorpaas/rust-hmac-drbg#3

@codecov-commenter

codecov-commenter commented Aug 26, 2020

Copy link
Copy Markdown

Codecov Report

Merging #133 into master will decrease coverage by 5.77%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #133      +/-   ##
==========================================
- Coverage   43.37%   37.60%   -5.78%     
==========================================
  Files           6        7       +1     
  Lines         219      250      +31     
==========================================
- Hits           95       94       -1     
- Misses        124      156      +32     
Impacted Files Coverage Δ
ecdsa/src/signer.rs 0.00% <0.00%> (ø)
ecdsa/src/signer/rfc6979.rs 0.00% <0.00%> (ø)
ecdsa/src/asn1.rs 70.47% <0.00%> (-0.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 352631b...2e5ec4e. Read the comment docs.

Comment thread ecdsa/src/signer/rfc6979.rs Outdated
Comment on lines +37 to +40
// TODO(tarcieri): don't panic (i.e. unwrap)! add/use trait for reducing digests mod q
let h1: ElementBytes<C> = C::Scalar::from_bytes(&msg_digest.finalize())
.unwrap()
.into();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Need a Scalar::from_digest-style trait here.

Adds a deterministic signing mode based on RFC6979.

Modifies the existing `RandomizedDigestSigner` and `RandomizedSigner`
impls to also use an RFC6979-style derivation, but supplying added
entropy derived from a provided RNG, per an RFC6979 variant described in
Section 3.6.
@tarcieri tarcieri changed the title [WIP] ecdsa: implement RFC6979 ephemeral scalar generation ecdsa: implement RFC6979 ephemeral scalar generation Aug 31, 2020
@tarcieri tarcieri marked this pull request as ready for review August 31, 2020 18:53
@tarcieri

Copy link
Copy Markdown
Member Author

Removed draft and WIP.

This still doesn't have tests, but due to the nature of RFC6979 it's a bit tricky to test in isolation. It would require implementing the rudiments of a particular elliptic curve and writing tests against it.

It will be easier to test in conjunction with particular elliptic curve implementations.

@tarcieri tarcieri merged commit 66c9393 into master Aug 31, 2020
@tarcieri tarcieri deleted the ecdsa/rfc6979 branch August 31, 2020 19:19
tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Sep 1, 2020
Adds support for RFC6979 deterministic ECDSA ephemeral scalars (`k`)
using the generic implementation added to the `ecdsa` crate in
RustCrypto/signatures#133.
tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Sep 1, 2020
Adds support for RFC6979 deterministic ECDSA ephemeral scalars (`k`)
using the generic implementation added to the `ecdsa` crate in
RustCrypto/signatures#133.
tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Sep 1, 2020
Adds support for RFC6979 deterministic ECDSA ephemeral scalars (`k`)
using the generic implementation added to the `ecdsa` crate in
RustCrypto/signatures#133.
@tarcieri tarcieri mentioned this pull request Sep 11, 2020
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.

Deterministic ECDSA support (RFC 6979 and more)

3 participants