Commit Graph

916 Commits

Author SHA1 Message Date
Roland Shoemaker 2e210b41ea crypto/x509: remove ios build tag restriction
Fixes #49435

Change-Id: I77ce12f447e727e7dc3b23de947357c27a268bd2
Reviewed-on: https://go-review.googlesource.com/c/go/+/362294
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-11-08 17:46:34 +00:00
Roland Shoemaker 3544082f75 crypto/x509: verification with system and custom roots
Make system cert pools special, such that when one has extra roots
added to it we run verifications twice, once using the platform
verifier, if available, and once using the Go verifier, merging the
results.

This change re-enables SystemCertPool on Windows, but explicitly does
not return anything from CertPool.Subjects (which matches the behavior
of macOS). CertPool.Subjects is also marked deprecated.

Fixes #46287
Fixes #16736

Change-Id: Idc1843f715ae2b2d0108e55ab942c287181a340a
Reviewed-on: https://go-review.googlesource.com/c/go/+/353589
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
2021-11-06 16:43:43 +00:00
Tobias Klauser f19e400180 all: remove more leftover // +build lines
CL 344955 and CL 359476 removed almost all // +build lines, but leaving
some assembly files and generating scripts. Also, some files were added
with // +build lines after CL 359476 was merged. Remove these or rename
files where more appropriate.

For #41184

Change-Id: I7eb85a498ed9788b42a636e775f261d755504ffa
Reviewed-on: https://go-review.googlesource.com/c/go/+/361480
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-11-06 10:24:44 +00:00
Roland Shoemaker b74f2efc47 crypto/x509: use the platform verifier on iOS
Use the same certificate verification APIs on iOS as on macOS (they
share the same APIs, so we should be able to transparently use them
on both.)

Updates #46287
Fixes #38843

Change-Id: If70f99b0823dd5fa747c42ff4f20c3b625605327
Reviewed-on: https://go-review.googlesource.com/c/go/+/353403
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
2021-11-06 00:29:44 +00:00
Roland Shoemaker feb024f415 crypto/x509: use platform verifier on darwin
When VerifyOptions.Roots is nil, default to using the platform X.509
certificate verification APIs on darwin, rather than using the Go
verifier. Since our oldest supported version of macOS is 10.12, we are
able to use the modern verification APIs, and don't need to resort to
the complex chain building trickery employed by chromium et al.

Unfortunately there is not a clean way to programmatically add test
roots to the system trust store that the builders would tolerate. The
most obvious solution, using 'security add-trusted-cert' requires human
interaction for authorization. We could also manually add anchors to
the constructed SecTrustRef, but that would require adding a whole
bunch of plumbing for test functionality, and would mean we weren't
really testing the actual non-test path. The path I've chosen here is
to just utilize existing valid, and purposefully invalid, trusted
chains, from google.com and the badssl.com test suite. This requires
external network access, but most accurately reflects real world
contexts.

This change removes the x509.SystemCertPool() functionality, which will
be ammended in a follow-up change which supports the suggested hybrid
pool approach described in #46287.

Updates #46287
Fixes #42414
Fixes #38888
Fixes #35631
Fixes #19561

Change-Id: I17f0d6c5cb3ef8a1f2731ce3296478b28d30df46
Reviewed-on: https://go-review.googlesource.com/c/go/+/353132
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-11-05 22:28:32 +00:00
Filippo Valsorda 035963c7f5 crypto/tls: set default minimum client version to TLS 1.2
Updates #45428

Change-Id: I5d70066d4091196ec6f8bfc2edf3d78fdc0520c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/359779
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-11-05 22:03:24 +00:00
Filippo Valsorda 6b223e872a crypto/x509: disable SHA-1 signature verification
Updates #41682

Change-Id: Ib766d2587d54dd3aeff8ecab389741df5e8af7cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/359777
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-11-05 21:48:25 +00:00
Lynn Boger c353f1b41d crypt/aes: update formatting of ppc64le asm comments
This does not change any code, just reformats the comments in
the asm code.

Change-Id: I70fbfa77db164898d25b59b589d3e85b8399b0fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/361694
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
2021-11-05 19:48:29 +00:00
Filippo Valsorda 93bab8a2f9 crypto/elliptic: port P-224 and P-384 to fiat-crypto
Also, adopt addchain code generation for field inversion, and switch
P-521 to Montgomery multiplication, which is significantly slower but
allows us to reuse the P-224/P-256/P-384 wrapper code. No one uses P-521
anyway, and it's still faster than it was in Go 1.16.

Removed a portion of tests that ran the P-224 vectors against P-256,
for some reason.

Sadly, fiat-crypto is not fast enough to replace the generic 32-bit
P-256 implementation (just yet?).

A change in visible behavior is that we literally can't internally
operate on invalid curve points anymore (yay!) but the crypto/elliptic
API locked us into accepting any pair of integers for
Add/Double/ScalarMult and return no error (sigh), although of course
that's undefined behavior. Panics are always regretted. Returning nil
leads to panics. A fixed point might be exploited. The most reasonable
solution felt to return a made up random point, which is not that
different from an off-curve point but leaks less.

name                                  old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P224-8                    573µs ± 0%     146µs ± 0%   -74.56%  (p=0.000 n=7+9)
ScalarMult/P224-8                        574µs ± 0%     152µs ± 5%   -73.58%  (p=0.000 n=7+10)
MarshalUnmarshal/P224/Uncompressed-8     664ns ± 0%     481ns ± 1%   -27.64%  (p=0.000 n=8+10)
MarshalUnmarshal/P224/Compressed-8       666ns ± 1%     480ns ± 0%   -27.92%  (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P224-8                              597µs ± 0%     169µs ± 2%   -71.71%  (p=0.000 n=10+9)
Verify/P224-8                           1.18ms ± 1%    0.32ms ± 5%   -72.81%  (p=0.000 n=10+10)
GenerateKey/P224-8                       577µs ± 0%     147µs ± 0%   -74.51%  (p=0.000 n=8+8)

name                                  old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P384-8                   2.01ms ± 2%    0.50ms ± 0%  -75.00%  (p=0.000 n=10+8)
ScalarMult/P384-8                       2.02ms ± 3%    0.51ms ± 3%  -74.64%  (p=0.000 n=10+10)
MarshalUnmarshal/P384/Uncompressed-8    1.09µs ± 1%    0.76µs ± 0%  -30.27%  (p=0.000 n=10+9)
MarshalUnmarshal/P384/Compressed-8      1.08µs ± 0%    0.76µs ± 1%  -29.86%  (p=0.000 n=8+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P384-8                             2.06ms ± 1%    0.56ms ± 2%  -72.76%  (p=0.000 n=10+10)
Verify/P384-8                           4.06ms ± 2%    1.08ms ± 0%  -73.49%  (p=0.000 n=10+8)
GenerateKey/P384-8                      2.01ms ± 1%    0.51ms ± 3%  -74.65%  (p=0.000 n=10+10)

name                                  old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P521-8                    715µs ± 6%    1525µs ± 4%  +113.39%  (p=0.000 n=10+10)
ScalarMult/P521-8                        698µs ± 1%    1543µs ± 1%  +120.99%  (p=0.000 n=9+9)
MarshalUnmarshal/P521/Uncompressed-8     797ns ± 0%    1296ns ± 0%   +62.65%  (p=0.000 n=10+9)
MarshalUnmarshal/P521/Compressed-8       798ns ± 0%    1299ns ± 1%   +62.82%  (p=0.000 n=8+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P521-8                              810µs ± 3%    1645µs ± 0%  +103.03%  (p=0.000 n=10+10)
Verify/P521-8                           1.42ms ± 1%    3.19ms ± 1%  +125.28%  (p=0.000 n=10+8)
GenerateKey/P521-8                       698µs ± 1%    1549µs ± 0%  +121.87%  (p=0.000 n=10+7)

Updates #40171

Change-Id: I34edf5002b5e9fad0ebb6c1e2119fb123ea6d18f
Reviewed-on: https://go-review.googlesource.com/c/go/+/360014
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-11-05 19:01:13 +00:00
Filippo Valsorda 0a5ca2422f crypto/ecdsa: draw a fixed amount of entropy while signing
The current code, introduced in CL 2422, mixes K bits of entropy with
the private key and message digest to generate the signature nonce,
where K is half the bit size of the curve. While the ECDLP complexity
(and hence security level) of a curve is half its bit size, the birthday
bound on K bits is only K/2. For P-224, this means we should expect a
collision after 2^56 signatures over the same message with the same key.

A collision, which is unlikely, would still not be a major practical
concern, because the scheme would fall back to a secure deterministic
signature scheme, and simply leak the fact that the two signed messages
are the same (which is presumably already public).

Still, we can simplify the code and remove the eventuality by always
drawing 256 bits of entropy.

Change-Id: I58097bd3cfc9283503e38751c924c53d271af92b
Reviewed-on: https://go-review.googlesource.com/c/go/+/352530
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-11-05 04:20:33 +00:00
Filippo Valsorda 978e39e9e6 crypto/elliptic: tolerate large inputs to IsOnCurve methods
The behavior of all Curve methods and package functions when provided an
off-curve point is undefined, except for IsOnCurve which should really
always return false, not panic.

Change-Id: I52f65df25c5af0314fef2c63d0778db72c0f1313
Reviewed-on: https://go-review.googlesource.com/c/go/+/361402
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-11-04 20:31:02 +00:00
Brad Fitzpatrick a59e33224e net/netip: add new IP address package
Co-authored-by: Alex Willmer <alex@moreati.org.uk> (GitHub @moreati)
Co-authored-by: Alexander Yastrebov <yastrebov.alex@gmail.com>
Co-authored-by: David Anderson <dave@natulte.net> (Tailscale CLA)
Co-authored-by: David Crawshaw <crawshaw@tailscale.com> (Tailscale CLA)
Co-authored-by: Dmytro Shynkevych <dmytro@tailscale.com> (Tailscale CLA)
Co-authored-by: Elias Naur <mail@eliasnaur.com>
Co-authored-by: Joe Tsai <joetsai@digital-static.net> (Tailscale CLA)
Co-authored-by: Jonathan Yu <jawnsy@cpan.org> (GitHub @jawnsy)
Co-authored-by: Josh Bleecher Snyder <josharian@gmail.com> (Tailscale CLA)
Co-authored-by: Maisem Ali <maisem@tailscale.com> (Tailscale CLA)
Co-authored-by: Manuel Mendez (Go AUTHORS mmendez534@...)
Co-authored-by: Matt Layher <mdlayher@gmail.com>
Co-authored-by: Noah Treuhaft <noah.treuhaft@gmail.com> (GitHub @nwt)
Co-authored-by: Stefan Majer <stefan.majer@gmail.com>
Co-authored-by: Terin Stock <terinjokes@gmail.com> (Cloudflare CLA)
Co-authored-by: Tobias Klauser <tklauser@distanz.ch>

Fixes #46518

Change-Id: I0041f9e1115d61fa6e95fcf32b01d9faee708712
Reviewed-on: https://go-review.googlesource.com/c/go/+/339309
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
2021-11-02 01:28:01 +00:00
Filippo Valsorda 30b5d6385e crypto/elliptic: move P-521 group logic to internal/nistec
This abstracts the clunky and not constant time math/big elliptic.Curve
compatibility layer away from the pure fiat-backed group logic.

Change-Id: I3b7a7495034d0c569b21c442ae36958763b8b2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/320074
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
2021-10-30 16:47:17 +00:00
Filippo Valsorda d1dceafc29 crypto/elliptic: use a 4-bit sliding window for P-521 ScalarMult
name                    old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:amd64
ScalarBaseMult/P521-16    1.63ms ± 4%    1.00ms ± 1%  -38.69%  (p=0.000 n=10+8)
ScalarMult/P521-16        1.65ms ± 4%    0.99ms ± 2%  -40.15%  (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:amd64
Sign/P521-16              1.67ms ± 1%    1.12ms ± 2%  -32.82%  (p=0.000 n=8+10)
Verify/P521-16            3.10ms ± 2%    2.00ms ± 2%  -35.54%  (p=0.000 n=9+10)
GenerateKey/P521-16       1.53ms ± 1%    0.98ms ± 2%  -35.81%  (p=0.000 n=9+10)

Change-Id: I109e821399d71330a77d105496e227746cc3ea0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/320072
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
2021-10-30 16:46:47 +00:00
Filippo Valsorda e39b854a67 crypto/elliptic: use complete addition formulas for P-521
Complete formulas don't have exceptions for P = Q or P = 0, which makes
them significantly simpler and safer to implement. Notice how the
constant time IsZero checks are gone.

It's not free, but still well within the performance gains of CL 315271.

name                    old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:amd64
ScalarBaseMult/P521-16    1.34ms ± 3%    1.63ms ± 4%  +21.78%  (p=0.000 n=10+10)
ScalarMult/P521-16        1.35ms ± 3%    1.65ms ± 4%  +22.58%  (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:amd64
Sign/P521-16              1.45ms ± 2%    1.67ms ± 1%  +15.00%  (p=0.000 n=10+8)
Verify/P521-16            2.68ms ± 1%    3.10ms ± 2%  +16.02%  (p=0.000 n=10+9)
GenerateKey/P521-16       1.31ms ± 4%    1.53ms ± 1%  +16.89%  (p=0.000 n=10+9)

Change-Id: Ibd9a961e9865df68a1250aba739c190caf9a54de
Reviewed-on: https://go-review.googlesource.com/c/go/+/320071
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
2021-10-30 16:45:25 +00:00
Filippo Valsorda c76893d0da crypto/elliptic: refactor P-224 field implementation
Improved readability, replaced constant time bit masked operations with
named functions, added comments. The behavior of every function should
be unchanged.

The largest change is the logic that in p224Contract checks if the value
is greater than or equal to p. Instead of a lot of error-prone masking,
we run a throwaway subtraction chain and look at the final borrow bit.

We could also not throw away the subtraction chain output and do a
constant time select instead of another masked subtraction, but we'd
still have to fix any underflows (because these are unsaturated limbs
and they underflow at 2^32 instead of 2^28). That's similar but
different from the carry-down chain we do elsewhere in that function
(which does undeflow fixing and borrow at the same time). I thought
having both variations in the same function would be confusing. Here's
how it would look like.

	var b uint32
	var outMinusP p224FieldElement
	for i := 0; i < len(out); i++ {
		outMinusP[i], b = bits.Sub32(out[i], p224P[i], b)
	}
	for i := 0; i < 3; i++ {
		mask := maskIfNegative(outMinusP[i])
		outMinusP[i] += (1 << 28) & mask
		// Note we DON'T borrow here, because it happened above.
	}
	for i := 0; i < len(out); i++ {
		out[i] = select32(b, out[i], outMinusP[i])
	}

Change-Id: I00932e8f171eff7f441b45666dccfd219ecbbc50
Reviewed-on: https://go-review.googlesource.com/c/go/+/326311
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
2021-10-29 22:01:56 +00:00
Russ Cox af05d8be3d all: manual fixups for //go:build vs // +build
Update many generators, also handle files that were not part of the
standard build during 'go fix' in CL 344955.

Fixes #41184.

Change-Id: I1edc684e8101882dcd11f75c6745c266fccfe9e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/359476
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-10-28 22:38:00 +00:00
Russ Cox f229e7031a all: go fix -fix=buildtag std cmd (except for bootstrap deps, vendor)
When these packages are released as part of Go 1.18,
Go 1.16 will no longer be supported, so we can remove
the +build tags in these files.

Ran go fix -fix=buildtag std cmd and then reverted the bootstrapDirs
as defined in src/cmd/dist/buildtool.go, which need to continue
to build with Go 1.4 for now.

Also reverted src/vendor and src/cmd/vendor, which will need
to be updated in their own repos first.

Manual changes in runtime/pprof/mprof_test.go to adjust line numbers.

For #41184.

Change-Id: Ic0f93f7091295b6abc76ed5cd6e6746e1280861e
Reviewed-on: https://go-review.googlesource.com/c/go/+/344955
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
2021-10-28 18:17:57 +00:00
Josh Bleecher Snyder e5c512520b crypto/elliptic: use a const string for precomputed P256 table
Const strings can be marked readonly. This is particularly
important for this relatively large table (88kb).
This is a follow-up to CL 315189.

The generation script is a bit awkward. It needs access to crypto/elliptic
internals, but also needs to be package main. Work around this by
exporting those internals with the "tablegen" build tag.

This requires changing the function signature at the Go-asm bridge.
As long as we're here, shrink the point argument type as well;
the net result is three fewer words of params.

Performance impact is probably noise.

name                   old time/op    new time/op    delta
ScalarBaseMult/P256-8    11.4µs ± 2%    11.3µs ± 1%  -1.32%  (p=0.000 n=19+16)
ScalarBaseMult/P224-8     579µs ± 1%     577µs ± 0%  -0.30%  (p=0.024 n=19+20)
ScalarBaseMult/P384-8    2.31ms ± 4%    2.34ms ± 4%  +1.25%  (p=0.033 n=20+20)
ScalarBaseMult/P521-8    1.33ms ± 0%    1.33ms ± 1%    ~     (p=0.173 n=18+17)
ScalarMult/P256-8        42.7µs ± 0%    42.7µs ± 2%    ~     (p=0.989 n=20+20)
ScalarMult/P224-8         579µs ± 0%     579µs ± 0%    ~     (p=0.538 n=19+18)
ScalarMult/P384-8        2.32ms ± 3%    2.34ms ± 5%    ~     (p=0.235 n=19+20)
ScalarMult/P521-8        1.33ms ± 1%    1.34ms ± 2%    ~     (p=0.141 n=17+20)

Change-Id: I3bee56df34ae61ca8829791d2e67e058ecc8ddbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/339591
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-10-26 23:06:10 +00:00
Josh Bleecher Snyder 80be4a4f90 crypto/x509: generate new-style build tags for iOS
Make the input match gofmt's output,
to make our lives easier as we phase out
old style build tags.

Change-Id: I95dc5a77058bf17cb02e289703f60784616db006
Reviewed-on: https://go-review.googlesource.com/c/go/+/358934
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-10-26 22:05:53 +00:00
Agniva De Sarker f47335e653 crypto/tls: add Conn.NetConn method
NetConn method gives us access to the underlying net.Conn
value.

Fixes #29257

Change-Id: I68b2a92ed9dab4be9900807c94184f8c0aeb4f72
Reviewed-on: https://go-review.googlesource.com/c/go/+/325250
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Trust: Agniva De Sarker <agniva.quicksilver@gmail.com>
Trust: Katie Hockman <katie@golang.org>
2021-10-25 18:46:45 +00:00
Russ Cox 4d8db00641 all: use bytes.Cut, strings.Cut
Many uses of Index/IndexByte/IndexRune/Split/SplitN
can be written more clearly using the new Cut functions.
Do that. Also rewrite to other functions if that's clearer.

For #46336.

Change-Id: I68d024716ace41a57a8bf74455c62279bde0f448
Reviewed-on: https://go-review.googlesource.com/c/go/+/351711
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-10-06 15:53:04 +00:00
Tobias Klauser d55009c594 crypto/rand: document additional getrandom/getentropy support in Reader
CL 269999 added support for getrandom on Dragonfly.
CL 299134 added support for getrandom on Solaris.
CL 302489 added support for getentropy on macOS.

Update the godoc for Reader accordingly.

Change-Id: Ice39e5e62f052f21b664db6abbfd97f03944586e
Reviewed-on: https://go-review.googlesource.com/c/go/+/353190
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-10-05 07:32:41 +00:00
Filippo Valsorda cc5e3de593 crypto/tls: use cryptobyte.NewFixedBuilder
Change-Id: Ia2a9465680e766336dae34f5d2b3cb412185bf1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/318131
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-10-04 17:17:11 +00:00
Filippo Valsorda 7162c4c7ca crypto: document the extended key interfaces
Change-Id: Iaff3f77b0a168e8bde981c791035a6451b3a49ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/353049
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
2021-09-30 15:20:44 +00:00
Joe Tsai 5961134fa5 crypto: avoid escaping Hash.Sum on generic architectures
For architectures without a specialized implementation (e.g. arm),
the generic implementation allocates because it does:

	var block = blockGeneric

which causes the compiler to give up trying to analyze block
even though it is technically only ever one implementation.
Instead of a variable, declare a function that wraps blockGeneric.

We apply this fix to md5, sha1, and sha256,
while sha512 already had the equivalent change.
We add a test to all hashing packages to ensure no allocations.

Credit goes to Cuong Manh Le for more specifically identifying
the problem and Keith Randal for suggesting a concrete solution.

Fixes #48055

Change-Id: I1a6a2e028038e051c83fd72b10a8bf4d210df57d
Reviewed-on: https://go-review.googlesource.com/c/go/+/346209
Trust: Joe Tsai <joetsai@digital-static.net>
Run-TryBot: Joe Tsai <joetsai@digital-static.net>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-09-23 16:54:46 +00:00
Filippo Valsorda 39eb1cc3f4 crypto/x509: drop compatibility hack for expired COMODO intermediates
The hack was there for a couple intermediates with only SGC EKUs that
issued severAuth certificates. They now all expired, so we can drop it.

https://crt.sh/?id=10066
https://crt.sh/?id=213

Change-Id: I46820024892b2f9918ce125bafbbaf9e6c5c58b3
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/781225
Reviewed-on: https://go-review.googlesource.com/c/go/+/327809
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-08-27 13:05:08 +00:00
vinckr 770df2e18d crypto/tls: fix typo in PreferServerCipherSuites comment
Fixing a typo, Deprected -> Deprecated.

Change-Id: Ie0ccc9a57ae6a935b4f67154ac097dba4c3832ec
GitHub-Last-Rev: 57337cc1bf
GitHub-Pull-Request: golang/go#47745
Reviewed-on: https://go-review.googlesource.com/c/go/+/342791
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2021-08-26 12:19:23 +00:00
Tobias Klauser 7007431374 crypto/rand, internal/syscall/unix: don't use getentropy on iOS
CL 302489 switched crypto/rand to use getentropy on darwin, however this
function is not available on iOS. Enable getentropy only on macOS and
disable it on iOS.

Fixes #47812

Change-Id: Ib7ba5d77346aee87904bb93d60cacc845f5c0089
Reviewed-on: https://go-review.googlesource.com/c/go/+/343609
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-08-20 16:44:10 +00:00
Yasuhiro Matsumoto 4012fea822 all: fix typos
Change-Id: I83180c472db8795803c1b9be3a33f35959e4dcc2
Reviewed-on: https://go-review.googlesource.com/c/go/+/336889
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
2021-08-17 13:54:10 +00:00
Brad Fitzpatrick 57c115e1f6 crypto/sha{256,512}: unname result parameters for consistency
Sum224 and Sum256 didn't look the same at:

    https://golang.org/pkg/crypto/sha256/

Now they match. Likewise with sha512's funcs.

Per:
https://github.com/golang/go/wiki/CodeReviewComments#named-result-parameters

Change-Id: I6b88c8ef15141c78a6cddeb0960b3ad52db34244
Reviewed-on: https://go-review.googlesource.com/c/go/+/322329
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-08-16 03:24:20 +00:00
Josh Bleecher Snyder b8ca6e59ed all: gofmt
Change-Id: Icfafcfb62a389d9fd2e7a4d17809486ed91f15c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/338629
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
2021-07-31 23:59:40 +00:00
Dmitri Shuralyov 1868f8296e crypto/x509: update iOS bundled roots to version 55188.120.1.0.1
Updates #38843.

Change-Id: I6e003ed03cd13d8ecf86ce05ab0e11c47e271c0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/337329
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2021-07-26 14:24:27 +00:00
Roland Shoemaker a98589711d crypto/tls: test key type when casting
When casting the certificate public key in generateClientKeyExchange,
check the type is appropriate. This prevents a panic when a server
agrees to a RSA based key exchange, but then sends an ECDSA (or
other) certificate.

Fixes #47143
Fixes CVE-2021-34558

Thanks to Imre Rad for reporting this issue.

Change-Id: Iabccacca6052769a605cccefa1216a9f7b7f6aea
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1116723
Reviewed-by: Filippo Valsorda <valsorda@google.com>
Reviewed-by: Katie Hockman <katiehockman@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/334031
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2021-07-12 20:58:00 +00:00
Roland Shoemaker c45e800e0c crypto/x509: don't fail on optional auth key id fields
If a certificate contains an AuthorityKeyIdentifier extension that
lacks the keyIdentifier field, but contains the authorityCertIssuer
and/or the authorityCertSerialNumber fields, don't return an error and
continue parsing.

Fixes #46854

Change-Id: I82739b415441f639a722755cc1f449d73078adfc
Reviewed-on: https://go-review.googlesource.com/c/go/+/331689
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2021-06-30 01:29:49 +00:00
Filippo Valsorda dc00dc6c6b crypto/tls: let HTTP/1.1 clients connect to servers with NextProtos "h2"
Fixes #46310

Change-Id: Idd5e30f05c439f736ae6f3904cbb9cc2ba772315
Reviewed-on: https://go-review.googlesource.com/c/go/+/325432
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
2021-06-10 12:41:37 +00:00
Filippo Valsorda 8212707871 crypto/elliptic: update P-521 docs to say it's constant-time
This is true since CL 315274.

Also adjust the P-256 note, since Add, Double, and IsOnCurve use the
generic, non-constant-time implementation.

Change-Id: I4b3b340f65bce91dcca30bcf86456cc8ce4dd4bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/325650
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
2021-06-07 15:19:57 +00:00
Filippo Valsorda e3176bbc3e crypto/tls: fix typo in Config.NextProtos docs
Change-Id: I916df584859595067e5e86c35607869397dbbd8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/325651
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
2021-06-07 15:16:23 +00:00
Filippo Valsorda 6ff0ae2aa4 crypto/elliptic: fix typo in p521Point type name
Change-Id: I6cab3624c875d9a70441a560e84f91c9b2df17b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/320070
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-27 11:17:57 +00:00
Lapo Luchini ce9a3b79d5 crypto/x509: add new FreeBSD 12.2+ trusted certificate folder
Up to FreeBSD 12.1 the package ca_root_nss was needed in order to have
certificates under /usr/local/share/certs as the base system didn't have
a system trusted certificate store.

This has been fixed in FreeBSD 12.2 using /etc/ssl/certs:
https://svnweb.freebsd.org/base?view=revision&revision=357082

Fixes #46284

Change-Id: I912b1bacc30cdf20d19e3ef9d09b69bb8055ff49
GitHub-Last-Rev: 0fa5542ea3
GitHub-Pull-Request: golang/go#46276
Reviewed-on: https://go-review.googlesource.com/c/go/+/321190
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
2021-05-20 17:06:05 +00:00
Roland Shoemaker 048cb4ceee crypto/x509: remove duplicate import
Change-Id: I86742ae7aa4ff49a38f8e3bc1d64fb223feae73e
Reviewed-on: https://go-review.googlesource.com/c/go/+/318409
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-18 20:00:18 +00:00
Tobias Klauser 2c76a6f7f8 all: add //go:build lines to assembly files
Don't add them to files in vendor and cmd/vendor though. These will be
pulled in by updating the respective dependencies.

For #41184

Change-Id: Icc57458c9b3033c347124323f33084c85b224c70
Reviewed-on: https://go-review.googlesource.com/c/go/+/319389
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
2021-05-13 09:12:17 +00:00
Roland Shoemaker dc50683bf7 crypto/elliptic: upgrade from generic curve impl to specific if available
This change alters the CurveParam methods to upgrade from the generic
curve implementation to the specific P224 or P256 implementations when
called on the embedded CurveParams. This removes the trap of using
elliptic.P224().Params() instead of elliptic.P224(), for example, which
results in using the generic implementation instead of the optimized
constant time one. For P224 this is done for all of the CurveParams
methods, except Params, as the optimized implementation covers all
these methods. For P256 this is only done for ScalarMult and
ScalarBaseMult, as despite having implementations of addition and
doubling they aren't exposed and instead the generic implementation is
used. For P256 an additional check that there actually is a specific
implementation is added, as unlike the P224 implementation the P256 one
is only available on certain platforms.

This change takes the simple, fast approach to checking this, it simply
compares pointers. This removes the most obvious class of mistakes
people make, but still allows edge cases where the embedded CurveParams
pointer has been dereferenced (as seen in the unit tests) or when someone
has manually constructed their own CurveParams that matches one of the
standard curves. A more complex approach could be taken to also address
these cases, but it would require directly comparing all of the
CurveParam fields which would, in the worst case, require comparing
against two standard CurveParam sets in the ScalarMult and
ScalarBaseMult paths, which are likely to be the hottest already.

Updates #34648

Change-Id: I82d752f979260394632905c15ffe4f65f4ffa376
Reviewed-on: https://go-review.googlesource.com/c/go/+/233939
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-10 19:19:34 +00:00
Filippo Valsorda ea93e68858 crypto/elliptic: make P-521 scalar multiplication constant time
Like for P-224, we do the constant time selects to hide the
point-at-infinity special cases of addition, but not the P = Q case,
which presumably doesn't happen in normal operations.

Runtime increases by about 50%, as expected, since on average we were
able to skip half the additions, and the additions reasonably amounted
to half the runtime. Still, the Fiat code is so much faster than big.Int
that we're still more than three time faster overall than pre-CL 315271.

name                   old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P521-8    4.18ms ± 3%    1.35ms ± 1%  -67.64%  (p=0.000 n=10+10)
ScalarMult/P521-8        4.17ms ± 2%    1.36ms ± 1%  -67.45%  (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P521-8              4.23ms ± 1%    1.44ms ± 1%  -66.02%  (p=0.000 n=9+10)
Verify/P521-8            8.31ms ± 2%    2.73ms ± 2%  -67.08%  (p=0.000 n=9+9)
GenerateKey/P521-8       4.15ms ± 2%    1.35ms ± 2%  -67.41%  (p=0.000 n=10+10)

Updates #40171

Change-Id: I782f2b7f33dd60af9b3b75e46d920d4cb47f719f
Reviewed-on: https://go-review.googlesource.com/c/go/+/315274
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-09 00:08:44 +00:00
Filippo Valsorda 14c3d2aa59 crypto/elliptic: import fiat-crypto P-521 field implementation
Fiat Cryptography (https://github.com/mit-plv/fiat-crypto) is a project
that produces prime order field implementations (the code that does
arithmetic modulo a prime number) based on a formally verified model.

The formal verification covers some of the most subtle and hard to test
parts of an elliptic curve implementation, like carry chains. It would
probably have prevented #20040 and #43786.

This CL imports a 64-bit implementation of the P-521 base field,
replacing the horribly slow and catastrophically variable time big.Int
CurveParams implementation.

The code in p521_fiat64.go is generated reproducibly by fiat-crypto,
building and running the Dockerfile according to the README.

The code in fiat/p521.go is a thin and idiomatic wrapper around the
fiat-crypto code. It includes an Invert method generated with the help
of github.com/mmcloughlin/addchain.

The code in elliptic/p521.go is a line-by-line port of the CurveParams
implementation. Lsh(x, N) was replaced with repeated Add(x, x) calls.
Mul(x, x) was replaced with Square(x). Mod calls were removed, as all
operations are modulo P. Likewise, Add calls to bring values back to
positive were removed. The ScalarMult ladder implementation is now
constant time, copied from p224ScalarMult. Only other notable changes
are adding a p512Point type to keep (x, y, z) together, and making
addJacobian and doubleJacobian methods on that type, with the usual
receiver semantics to save 4 allocations per step.

This amounts to a proof of concept, and is far from a mature elliptic
curve implementation. Here's a non-exhaustive list of things that need
improvement, most of which are pre-existing issues with crypto/elliptic.
Some of these can be fixed without API change, so can't.

- Marshal and Unmarshal still use the slow, variable time big.Int
  arithmetic. The Curve interface does not expose field operations, so
  we'll have to make our own abstraction.

- Point addition uses an incomplete Jacobian formula, which has variable
  time behaviors for points at infinity and equal points. There are
  better, complete formulae these days, but I wanted to keep this CL
  reviewable against the existing code.

- The scalar multiplication ladder is still heavily variable time. This
  is easy to fix and I'll do it in a follow-up CL, but I wanted to keep
  this one easier to review.

- Fundamentally, values have to go in and out of big.Int representation
  when they pass through the Curve interface, which is both slow and
  slightly variable-time.

- There is no scalar field implementation, so crypto/ecdsa ends up using
  big.Int for signing.

- Extending this to P-384 would involve either duplicating all P-521
  code, or coming up with some lower-level interfaces for the base
  field. Even better, generics, which would maybe let us save heap
  allocations due to virtual calls.

- The readability and idiomaticity of the autogenerated code can
  improve, although we have a clear abstraction and well-enforced
  contract, which makes it unlikely we'll have to resort to manually
  modifying the code. See mit-plv/fiat-crypto#949.

- We could also have a 32-bit implementation, since it's almost free to
  have fiat-crypto generate one.

Anyway, it's definitely better than CurveParams, and definitely faster.

name                   old time/op    new time/op    delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P521-8    4.18ms ± 3%    0.86ms ± 2%  -79.50%  (p=0.000 n=10+9)
ScalarMult/P521-8        4.17ms ± 2%    0.85ms ± 6%  -79.68%  (p=0.000 n=10+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P521-8              4.23ms ± 1%    0.94ms ± 0%  -77.70%  (p=0.000 n=9+8)
Verify/P521-8            8.31ms ± 2%    1.75ms ± 4%  -78.99%  (p=0.000 n=9+10)
GenerateKey/P521-8       4.15ms ± 2%    0.85ms ± 2%  -79.49%  (p=0.000 n=10+9)

name                   old alloc/op   new alloc/op   delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P521-8    3.06MB ± 3%    0.00MB ± 0%  -99.97%  (p=0.000 n=10+10)
ScalarMult/P521-8        3.05MB ± 1%    0.00MB ± 0%  -99.97%  (p=0.000 n=9+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P521-8              3.03MB ± 0%    0.01MB ± 0%  -99.74%  (p=0.000 n=10+8)
Verify/P521-8            6.06MB ± 1%    0.00MB ± 0%  -99.93%  (p=0.000 n=9+9)
GenerateKey/P521-8       3.02MB ± 0%    0.00MB ± 0%  -99.96%  (p=0.000 n=9+10)

name                   old allocs/op  new allocs/op  delta
pkg:crypto/elliptic goos:darwin goarch:arm64
ScalarBaseMult/P521-8     19.8k ± 3%      0.0k ± 0%  -99.95%  (p=0.000 n=10+10)
ScalarMult/P521-8         19.7k ± 1%      0.0k ± 0%  -99.95%  (p=0.000 n=9+10)
pkg:crypto/ecdsa goos:darwin goarch:arm64
Sign/P521-8               19.6k ± 0%      0.1k ± 0%  -99.63%  (p=0.000 n=10+10)
Verify/P521-8             39.2k ± 1%      0.1k ± 0%  -99.84%  (p=0.000 n=9+10)
GenerateKey/P521-8        19.5k ± 0%      0.0k ± 0%  -99.91%  (p=0.000 n=9+10)

Updates #40171

Change-Id: Ic898b09a2388382bf51ec007d9a79d72d44efe10
Reviewed-on: https://go-review.googlesource.com/c/go/+/315271
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Trust: Katie Hockman <katie@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
2021-05-09 00:08:05 +00:00
Filippo Valsorda ec4efa4208 crypto/x509: check the private key passed to CreateCertificate
Unfortunately, we can't improve the function signature to refer to
crypto.PrivateKey and crypto.PublicKey, even if they are both
interface{}, because it would break assignments to function types.

Fixes #37845

Change-Id: I627f2ac1e1ba98b128dac5382f9cc2524eaef378
Reviewed-on: https://go-review.googlesource.com/c/go/+/224157
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-09 00:07:34 +00:00
Filippo Valsorda 9d0819b27c crypto/tls: make cipher suite preference ordering automatic
We now have a (well, two, depending on AES hardware support) universal
cipher suite preference order, based on their security and performance.
Peer and application lists are now treated as filters (and AES hardware
support hints) that are applied to this universal order.

This removes a complex and nuanced decision from the application's
responsibilities, one which we are better equipped to make and which
applications usually don't need to have an opinion about. It also lets
us worry less about what suites we support or enable, because we can be
confident that bad ones won't be selected over good ones.

This also moves 3DES suites to InsecureCipherSuites(), even if they are
not disabled by default. Just because we can keep them as a last resort
it doesn't mean they are secure. Thankfully we had not promised that
Insecure means disabled by default.

Notable test changes:

  - TestCipherSuiteCertPreferenceECDSA was testing that we'd pick the
    right certificate regardless of CipherSuite ordering, which is now
    completely ignored, as tested by TestCipherSuitePreference. Removed.

  - The openssl command of TestHandshakeServerExportKeyingMaterial was
    broken for TLS 1.0 in CL 262857, but its golden file was not
    regenerated, so the test kept passing. It now broke because the
    selected suite from the ones in the golden file changed.

  - In TestAESCipherReordering, "server strongly prefers AES-GCM" is
    removed because there is no way for a server to express a strong
    preference anymore; "client prefers AES-GCM and AES-CBC over ChaCha"
    switched to ChaCha20 when the server lacks AES hardware; and finally
    "client supports multiple AES-GCM" changed to always prefer AES-128
    per the universal preference list.

    * this is going back on an explicit decision from CL 262857, and
      while that client order is weird and does suggest a strong dislike
      for ChaCha20, we have a strong dislike for software AES, so it
      didn't feel worth making the logic more complex

  - All Client-* golden files had to be regenerated because the
    ClientHello cipher suites have changed.
    (Even when Config.CipherSuites was limited to one suite, the TLS 1.3
    default order changed.)

Fixes #45430
Fixes #41476 (as 3DES is now always the last resort)

Change-Id: If5f5d356c0f8d1f1c7542fb06644a478d6bad1e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/314609
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
2021-05-08 05:15:48 +00:00
Filippo Valsorda 02ce411821 crypto/x509: remove GODEBUG=x509ignoreCN=0 flag
Common Name and NameConstraintsWithoutSANs are no more.

Fixes #24151 ᕕ(ᐛ)ᕗ

Change-Id: I15058f2a64f981c69e9ee620d3fab00f68967e49
Reviewed-on: https://go-review.googlesource.com/c/go/+/315209
Trust: Filippo Valsorda <filippo@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2021-05-08 05:12:07 +00:00
Roland Shoemaker 90d6bbbe42 crypto/tls: enforce ALPN overlap when negotiated on both sides
During the TLS handshake if the server doesn't support any of the
application protocols requested by the client, send the
no_application_protocol alert and abort the handshake on the server
side. This enforces the requirements of RFC 7301.

Change-Id: Iced2bb5c6efc607497de1c40ee3de9c2b393fa5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/289209
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2021-05-06 18:57:43 +00:00
Roland Shoemaker 51ff3a6965 crypto/x509: rewrite certificate parser
Replaces the encoding/asn1 certificate parser with a
x/crypto/cryptobyte based parser. This provides a significant increase
in performance, mostly due to a reduction of lots of small allocs,
as well as almost entirely removing reflection.

Since this is a rather large rewrite only the certificate parser is
replaced, leaving the parsers for CSRs, CRLs, etc for follow-up work.
Since some of the functions that the other parsers use are replaced
with cryptobyte versions, they still get a not insignificant performance
boost.

name                           old time/op    new time/op    delta
ParseCertificate/ecdsa_leaf-8    44.6µs ± 9%    12.7µs ± 4%  -71.58%  (p=0.000 n=20+18)
ParseCertificate/rsa_leaf-8      46.4µs ± 4%    13.2µs ± 2%  -71.49%  (p=0.000 n=18+19)

name                           old allocs/op  new allocs/op  delta
ParseCertificate/ecdsa_leaf-8       501 ± 0%       164 ± 0%  -67.27%  (p=0.000 n=20+20)
ParseCertificate/rsa_leaf-8         545 ± 0%       182 ± 0%  -66.61%  (p=0.000 n=20+20)

Fixes #21118
Fixes #44237

Change-Id: Id653f6ae5e405c3cbf0c5c48abb30aa831e30107
Reviewed-on: https://go-review.googlesource.com/c/go/+/274234
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
2021-05-06 17:09:23 +00:00