odeke-em
commented
6 years ago /cc @agl @FiloSottile @rsc
Open AnomalRoil opened 6 years ago
odeke-em
commented
6 years ago /cc @agl @FiloSottile @rsc
FiloSottile
commented
4 years ago We should refuse to Sign and Verify with invalid keys, as long as the checks are not too expensive compared to the rest of the operation. I'd take a PR with some benchstat for this.
ericlagergren
commented
2 years ago Adding a very simple check to Sign to see if D is in [1, N-1].
name old time/op new time/op delta
Sign/P256-8 19.3µs ± 1% 19.3µs ± 1% ~ (p=0.371 n=8+10)
Sign/P224-8 162µs ± 0% 161µs ± 0% -0.64% (p=0.000 n=9+10)
Sign/P384-8 547µs ± 1% 546µs ± 0% ~ (p=0.962 n=10+7)
Sign/P521-8 1.63ms ± 1% 1.63ms ± 1% ~ (p=0.052 n=10+10)
name old alloc/op new alloc/op delta
Sign/P256-8 2.94kB ± 0% 2.94kB ± 0% ~ (all equal)
Sign/P224-8 4.97kB ± 0% 4.97kB ± 0% ~ (all equal)
Sign/P384-8 6.18kB ± 0% 6.18kB ± 0% ~ (all equal)
Sign/P521-8 7.79kB ± 0% 7.79kB ± 0% ~ (all equal)
name old allocs/op new allocs/op delta
Sign/P256-8 43.0 ± 0% 43.0 ± 0% ~ (all equal)
Sign/P224-8 70.0 ± 0% 70.0 ± 0% ~ (all equal)
Sign/P384-8 71.0 ± 0% 71.0 ± 0% ~ (all equal)
Sign/P521-8 72.0 ± 0% 72.0 ± 0% ~ (all equal)Checking the public key, too:
name old time/op new time/op delta
Sign/P256-8 19.3µs ± 1% 20.0µs ± 1% +3.70% (p=0.000 n=8+10)
Sign/P224-8 162µs ± 0% 162µs ± 0% -0.19% (p=0.004 n=9+9)
Sign/P384-8 547µs ± 1% 546µs ± 0% ~ (p=0.631 n=10+10)
Sign/P521-8 1.63ms ± 1% 1.64ms ± 1% ~ (p=0.315 n=10+10)
name old alloc/op new alloc/op delta
Sign/P256-8 2.94kB ± 0% 3.56kB ± 0% +21.25% (p=0.000 n=10+10)
Sign/P224-8 4.97kB ± 0% 5.15kB ± 0% +3.71% (p=0.000 n=8+10)
Sign/P384-8 6.18kB ± 0% 6.46kB ± 0% +4.54% (p=0.000 n=8+10)
Sign/P521-8 7.79kB ± 0% 8.20kB ± 0% +5.24% (p=0.000 n=8+10)
name old allocs/op new allocs/op delta
Sign/P256-8 43.0 ± 0% 50.0 ± 0% +16.28% (p=0.000 n=10+10)
Sign/P224-8 70.0 ± 0% 75.0 ± 0% +7.14% (p=0.000 n=10+10)
Sign/P384-8 71.0 ± 0% 76.0 ± 0% +7.04% (p=0.000 n=10+10)
Sign/P521-8 72.0 ± 0% 77.0 ± 0% +6.94% (p=0.000 n=10+10)Checking [d]G=Q, too:
name old time/op new time/op delta
Sign/P256-8 19.3µs ± 1% 31.2µs ± 0% +62.01% (p=0.000 n=8+9)
Sign/P224-8 162µs ± 0% 306µs ± 0% +88.42% (p=0.000 n=9+8)
Sign/P384-8 547µs ± 1% 1047µs ± 1% +91.50% (p=0.000 n=10+9)
Sign/P521-8 1.63ms ± 1% 3.17ms ± 1% +94.10% (p=0.000 n=10+10)
name old alloc/op new alloc/op delta
Sign/P256-8 2.94kB ± 0% 3.85kB ± 0% +31.05% (p=0.000 n=10+10)
Sign/P224-8 4.97kB ± 0% 5.56kB ± 0% +11.92% (p=0.000 n=8+10)
Sign/P384-8 6.18kB ± 0% 7.01kB ± 0% +13.49% (p=0.000 n=8+10)
Sign/P521-8 7.79kB ± 0% 8.97kB ± 0% +15.17% (p=0.000 n=8+9)
name old allocs/op new allocs/op delta
Sign/P256-8 43.0 ± 0% 56.0 ± 0% +30.23% (p=0.000 n=10+10)
Sign/P224-8 70.0 ± 0% 85.0 ± 0% +21.43% (p=0.000 n=10+10)
Sign/P384-8 71.0 ± 0% 86.0 ± 0% +21.13% (p=0.000 n=10+10)
Sign/P521-8 72.0 ± 0% 87.0 ± 0% +20.83% (p=0.000 n=10+10)
What version of Go are you using (
go version)?go1.9.1 linux/amd64What operating system and processor architecture are you using (
go env)?GOHOSTARCH="amd64" (intel) GOHOSTOS="linux"
What did you do?
Playground link: https://play.golang.org/p/Ldd8fvrP5m
I've been playing around with Go's ECDSA package and have noticed a few problems, none of practical security relevance, since those problems have premises unlikely to happen in real use cases. (I actually discussed those on Go's security mailing list a long time ago.)
The problems are all about parameters validation in ECDSA.
What did you expect to see?
The Go's implementation should conform to the DSS standard, notably by performing verification that the point used are actually on the curve at hand. But if the checks I discuss here are considered not worthy, a workaround like what has been done for DSA might be good to completely avoid any risk of infinite loops.
I would expect a method to check the validity of the provided parameters, as per FIPS 186-4 sections 3.1 and 3.3.
What did you see instead?
No check is performed on the curve's points used to see whether they are on the curve or not, so it is possible to use a point not even on the curve. (Whether this is dangerous for ECDSA has not been studied.)
Note that checks that points are on the curve are performed in the TLS package (where they are important, because of the invalid curve attack) thanks to the elliptic.Unmarshal method.
I also see that no check is performed on the private parameter "d" to verify if it's within well defined bounds. So it is possible to sign using a 0 value, which is not among the correct boundaries which are [1, n-1] (even if the value 1 does not make much sense from a security point of view).
There is an example on the playground linked where this causes an infinite loop when signing an null hash with a null private integer.
The value 0 is simply invalid as a private integer and should be rejected.
There are no "validation" methods for keys, this could be a good way to avoid performing such checks at each signature/verification: by providing a method users are supposed to use to validate the keys.
Such checks are important in my opinion and are part of the so-called "defense in depth" we want to generally ensure in a crypto library.
What are your opinions on these topics?