golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.72k stars 17.5k forks source link

crypto/ecdsa: generate RFC 6979 signatures if rand is nil #64802

Open FiloSottile opened 8 months ago

FiloSottile commented 8 months ago

crypto/ecdsa currently generates "hedged" signatures, by drawing the random nonce from an AES-CTR CSPRNG keyed by SHA2-512(priv.D || entropy || hash)[:32]. This is great, as it provides the best of both worlds: fault injection tolerance and RNG failure resistance.

However, sometimes you do need deterministic signatures, not because they are more secure (as hedged signatures are even better) but because you actually need the signature not to change over time. I propose that when the rand parameter is nil (which currently panics), we produce RFC 6979 deterministic signatures.

Morevoer, Section 3.6 of RFC 6979 specifies how to add additional data to the RNG input, so we can replace our AES-CTR construction with that. The advantage of this would be that although the RFC doesn't have test vectors for this variant, we can generate some and share them with other implementations.

o  Additional data may be added to the input of HMAC, concatenated
      after bits2octets(H(m)):

         K = HMAC_K(V || 0x00 || int2octets(x) || bits2octets(h1) || k')

      A use case may be a protocol that requires a non-deterministic
      signature algorithm on a system that does not have access to a
      high-quality random source.  It suffices that the additional data
      k' is non-repeating (e.g., a signature counter or a monotonic
      clock) to ensure "random-looking" signatures are
      indistinguishable, in a cryptographic way, from plain (EC)DSA
      signatures.  In [SP800-90A] terminology, k' is the "additional
      input" that can be set as a parameter when generating pseudorandom
      bits.  This variant can be thought of as a "strengthening" of the
      randomness of the source of the additional data k'.

(The CFRG has a forever-stalled draft on "deterministic with noise" ECDSA signatures, draft-irtf-cfrg-det-sigs-with-noise, but it's unclear to me why we'd want that instead of the existing RFC 6979 mechanism.)

The properties of signatures generated by current applications wouldn't change: they would still be hendged, randomized, and unpredictable (because we use MaybeReadByte, so we can change the internals, yay!).

/cc @golang/security

FiloSottile commented 8 months ago

Unfortunately, for RFC 6979 we need to instantiate HMAC with the hash that was used to hash the message. In Sign and SignASN1, we don't have this information, only in PrivateKey.Sign.

We could implement this only for PrivateKey.Sign, or take a pragmatic route and map hash lengths to SHA-256/SHA-512 in SignASN1. (We'd document that, and still do the right thing in PrivateKey.Sign.)

Another wrinkle is that opts is currently ignored in PrivateKey.Sign, so we need to be prepared for current applications setting it wrong.

gopherbot commented 8 months ago

Change https://go.dev/cl/552215 mentions this issue: crypto/ecdsa: implement RFC6979

FiloSottile commented 8 months ago

Ok, since we can't do this properly in Sign and SignASN1, I propose

  1. when the rand parameter to (*PrivateKey).Sign) is nil we produce RFC 6979 deterministic signatures

  2. we add SignDeterministic(priv *PrivateKey, digest []byte, hash crypto.Hash) ([]byte, error)

FiloSottile commented 4 months ago

Actually, the top level functions might be worth rethinking in a v2 anyway, especially if we adopt some standard hedged signature scheme such as draft-irtf-cfrg-det-sigs-with-noise, maybe while doing FIPS work.

What I'm proposing here then is just that when the rand parameter to (*PrivateKey).Sign is nil we produce RFC 6979 deterministic signatures. Currently, the function panics if rand is nil.

Deterministic signatures are enough of an uncommon use case that it's ok if they are not super easy to access.

rsc commented 4 months ago

Based on the discussion above, this proposal seems like a likely accept. — rsc for the proposal review group

The proposal is that ecdsa.PrivateKey.Sign start accepting rand==nil and in that case return a deterministic RFC 6979 signature. There is no new API, only the redefinition of that failure mode.

A new GODEBUG setting will control the behavior. GODEBUG=ecdsarfc6979=1 (the new default for go 1.23+ main modules) means enable these deterministic signatures when rand==nil; GODEBUG=ecdsarfc6979=0 means don’t.

FiloSottile commented 4 months ago

Given rand==nil currently panics, do we need the GODEBUG? I can't imagine an application relying on a panic here.

rsc commented 3 months ago

No change in consensus, so accepted. 🎉 This issue now tracks the work of implementing the proposal. — rsc for the proposal review group

The proposal is that ecdsa.PrivateKey.Sign start accepting rand==nil and in that case return a deterministic RFC 6979 signature. There is no new API, only the redefinition of that failure mode.

A new GODEBUG setting will control the behavior. GODEBUG=ecdsarfc6979=1 (the new default for go 1.23+ main modules) means enable these deterministic signatures when rand==nil; GODEBUG=ecdsarfc6979=0 means don’t.

mateusz834 commented 3 months ago

@rsc Was this https://github.com/golang/go/issues/64802#issuecomment-2101504892 addressed? Is there a need for this GODEBUG?

rolandshoemaker commented 3 months ago

The previous behavior here (rand == nil) would cause a panic. This change will "fix" otherwise broken old code, by causing usage of deterministic signatures, rather than panicking, but that seems reasonable. I don't think a GODEBUG is strictly necessary here.

rsc commented 3 months ago

Since we missed Go 1.23, let's land this at the start of Go 1.24 and wait for feedback before adding the GODEBUG. We will have the whole cycle for someone to point out a convoluted sequence that makes this a breaking change.

FiloSottile commented 3 months ago

Sounds good, will target early Go 1.24 for this. (I chose not to land it in Go 1.23 because it turned out to be a larger change than expected, touching very delicate parts of the code, and I didn't feel like we had enough review bandwidth or brewing time to do it safely.)

gopherbot commented 1 month ago

This issue is currently labeled as early-in-cycle for Go 1.24. That time is now, so a friendly reminder to look at it again.