Conversation
We don't support SSLv2, at all. Change-Id: Icd0579b81393fbd82bf5b4d961470928faa7d09d Reviewed-on: https://go-review.googlesource.com/c/go/+/644017 Reviewed-by: Neal Patel <nealpatel@google.com> Auto-Submit: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I9ef075bbb0e3c65f3c2a9d49e599ef50b18aa9be Reviewed-on: https://go-review.googlesource.com/c/go/+/639535 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I6a6a465612cf76d148b9758ee3fcdc8606497830 Reviewed-on: https://go-review.googlesource.com/c/go/+/648835 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Updates #71706 Change-Id: Id689ec476eb3f76500dbf59d716a4376749de700 Reviewed-on: https://go-review.googlesource.com/c/go/+/650719 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Make the errors we return when parsing an ECHConfig slightly more verbose. Fixes #71706 Change-Id: Id138fd9defec71ce492a490a71af4981cb9ede51 Reviewed-on: https://go-review.googlesource.com/c/go/+/650720 Auto-Submit: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Michael Knyszek <mknyszek@google.com>
See Implementation Guidance D.Q. Change-Id: I6a6a465607da94f2bb249934f0561ae04a55e7b7 Reviewed-on: https://go-review.googlesource.com/c/go/+/650575 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
1. onResumeShimWritesFirst is unused, replace the binding with an underscore. 2. in the bogoShim() function when looping through resumeCount+1 the tlsConn read for loop only breaks for non-nil err, so there's no need to check that again after the loop body. Updates #72006 Change-Id: Ieff45d26df33d71003a2509ea5b2b06c5fa0e1d6 Reviewed-on: https://go-review.googlesource.com/c/go/+/650715 Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
While not clearly motivated by normative language in RFC 8446 it seems clear that an empty opaque ticket value is non-operable, and so we should reject it with an appropriate alert/error. This allows removing the SendEmptySessionTicket-TLS13 BoGo test from the bogo excluded tests configuration. Fixes #70513 Updates #72006 Change-Id: I589b34e86fb1eb27a349a230e920c22284597cde Reviewed-on: https://go-review.googlesource.com/c/go/+/650735 Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
This commit removes SkipNewSessionTicket from the bogo_config.json excluded tests list. Previously this test was being skipped with a TODO that there might be a bug here. In practice it seems like there's no bug and the test is handled correctly by crypto/tls. When activated, a TLS 1.2 client connecting to the bogo dispatcher goes through the normal handshake process with the exception that the server skips sending the NewSessionTicket msg expected by the client in response to the client's final flight of handshake msgs. The crypto/tls TLS 1.2 client_handshake.go logic correctly rejects the unexpected message that follows (ChangeCipherSpec) when trying to read the bytes necessary to unmarshal the expected NewSessionTicket message that was omitted. Updates #72006 Change-Id: I9faea4d18589d10b163211aa17b2d0da8af1187e Reviewed-on: https://go-review.googlesource.com/c/go/+/650736 Reviewed-by: Junyang Shao <shaojunyang@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
When encountering alertUserCanceled in a TLS 1.3 handshake, ignore the alert and retry reading a record. This matches existing logic for how TLS 1.2 alertLevelWarning alerts are handled. For broader context, TLS 1.3 removed warning-level alerts except for alertUserCanceled (RFC 8446, § 6.1). Since at least one major implementation (https://bugs.openjdk.org/browse/JDK-8323517) misuses this alert, many TLS stacks now ignore it outright when seen in a TLS 1.3 handshake (e.g. BoringSSL, NSS, Rustls). With the crypto/tls behaviour changed to match peer implementations we can now enable the "SendUserCanceledAlerts-TLS13" BoGo test. "SendUserCanceledAlerts-TooMany-TLS13" remains ignored, because like "SendWarningAlerts*" fixing the test requires some general spam protocol message enhancements be done first. Updates #72006 Change-Id: I570c1fa674b5a4760836c514d35ee17f746fe28d Reviewed-on: https://go-review.googlesource.com/c/go/+/650716 Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
When this command line flag is provided to the BoGo runner it will: * Disable some timeouts * Limit concurrency to 1 worker at a time * Pass the -wait-for-debugger flag to the shim process * Print the PID of the shim process to status output On the shim-side, we need to react to -wait-for-debugger by sending ourselves a SIGSTOP signal. When a debugger attaches to the shim the process will be resumed. This makes it possible to debug both the runner side and the shim side of a BoGo interaction without resorting to print style debugging. Since SIGSTOP is not a signal we can use on Windows this functionality is limited to unix builds. Updates #72006 Change-Id: Iafa08cf71830cdfde3e6ee4826914236e3cd7e57 Reviewed-on: https://go-review.googlesource.com/c/go/+/650737 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously this test was skipped without a comment clarifying why. In practice it's because crypto/tls doesn't generate GREASE extensions at this time, and the test expects to find one in the NewSessionTicket message extensions produced by a server. We're already skipping some other GREASE related test as not-yet-implemented without explicit bogo_config.json exclusion by way of the -enable-grease flag not being implemented, however for TLS 1.3 servers the BoGo expectation is that they _always_ send GREASE, and so the -enable-grease flag isn't provided and an explicit skip must be used. We should revisit this alongside implementing GREASE ext production in general for both clients and servers. Updates #72006 Change-Id: I8af4b555ac8c32cad42215fbf26aa0feae90fa21 Reviewed-on: https://go-review.googlesource.com/c/go/+/650717 Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Junyang Shao <shaojunyang@google.com>
For malformed client/server certificates in a TLS handshake send a decode_error alert, matching BoringSSL behaviour. Previously crypto/tls used a bad_certificate alert for this purpose. The TLS specification is imprecise enough to allow this to be considered a spec. justified choice, but since all other places in the protocol encourage using decode_error for structurally malformed messages we may as well do the same here and get some extra cross-impl consistency for free. This also allows un-ignoring the BoGo GarbageCertificate-[Client|Server]-[TLS12|TLS13] tests. Updates #72006 Change-Id: Ide45ba1602816e71c3289a60e77587266c3b9036 Reviewed-on: https://go-review.googlesource.com/c/go/+/652995 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
If we weren't resuming an existing session, and we constructed a TLS 1.3 compatible client hello, ensure the server doesn't echo back the made up compatibility session ID if we end up handshaking for TLS 1.2. As part of an effort to make the initial stages of a TLS 1.3 handshake compatible with TLS 1.2 middleboxes, TLS 1.3 requires that the client hello contain a non-empty legacy_session_id value. For anti-ossification purposes it's recommended this ID be randomly generated. This is the strategy the crypto/tls package takes. When we follow this approach, but then end up negotiating TLS 1.2, the server should not have echoed back that random ID to us. It's impossible for the server to have had a session with a matching ID and so it is misbehaving and it's prudent for our side to abort the handshake. See RFC 8446 Section 4.1.2 for more detail: https://www.rfc-editor.org/rfc/rfc8446#section-4.1.2 Adopting this behaviour allows un-ignoring the BoGo EchoTLS13CompatibilitySessionID testcase. Updates #72006 Change-Id: I1e52075177a13a7aa103b45498eae38d8a4c34b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/652997 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
Partially reverts CL 587296, restoring the Go+BoringCrypto 1.23 behavior in terms of supported curves. Updates #71757 Change-Id: I6a6a465651a8407056fd0fae091d10a945b37997 Reviewed-on: https://go-review.googlesource.com/c/go/+/657095 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org>
This required adding a new field to SessionState for TLS 1.0–1.2, since the key exchange is not repeated on resumption. The additional field is unfortunately not backwards compatible because current Go versions check that the encoding has no extra data at the end, but will cause cross-version tickets to be ignored. Relaxed that so we can add fields in a backwards compatible way the next time. For the cipher suite, we check that the session's is still acceptable per the Config. That would arguably make sense here, too: if a Config for example requires PQ, we should reject resumptions of connections that didn't use PQ. However, that only applies to pre-TLS 1.3 connections, since in TLS 1.3 we always do a fresh key exchange on resumption. Since PQ is the only main differentiator between key exchanges (aside from off-by-default non-PFS RSA, which are controlled by the cipher suite in TLS 1.0–1.2) and it's PQ-only, we can skip that check. Fixes #67516 Change-Id: I6a6a465681a6292edf66c7b8df8f4aba4171a76b Reviewed-on: https://go-review.googlesource.com/c/go/+/653315 Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Cleaned up a lot of the plumbing to make it consistently follow this logic: clone the preference order; filter by user preference; filter by FIPS policy. There should be no behavior changes. Updates #71757 Change-Id: I6a6a4656eb02e56d079f0a22f98212275a400000 Reviewed-on: https://go-review.googlesource.com/c/go/+/657096 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com>
We are going to stick to BoringSSL's policy for Go+BoringCrypto, but when using the native FIPS 140-3 module we can allow Ed25519, ML-KEM, and P-521. NIST SP 800-52r2 is stricter, but it only applies to some entities, so they can restrict the profile with Config. Fixes #71757 Change-Id: I6a6a4656eb02e56d079f0a22f98212275a40a679 Reviewed-on: https://go-review.googlesource.com/c/go/+/650576 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com>
There is no need for fips140tls to depend on an internal package, it can use crypto/fips140 directly. Both approaches are equivalent, but using crypto/fips140 makes us exercise a public API and sets precedence. Change-Id: I668e80ee62b711bc60821cee3a54232a33295ee1 Reviewed-on: https://go-review.googlesource.com/c/go/+/642035 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Junyang Shao <shaojunyang@google.com>
Previously, the code only checked supportedVersions[0] for TLS 1.3
However, Chromium-based
browsers may list TLS 1.3 at different positions, causing ECH failures.
This fix:
Iterates through supportedVersions to accept connections as long as TLS 1.3 is present.
Improves ECH compatibility, ensuring Chrome, Edge, and other browsers work properly.
Fixes #71642
Change-Id: I32f4219fb6654d5cc22c7f33497c6142c0acb4f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/648015
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
If GetConfigForClient returns a tls.Config that has SessionTicketsDisabled set, the TLS server handshake currently leaves the Config's internal RWMutex read locked after calculating the ticketKeys to use for the handshake. Change-Id: I07e450a0d2edda9e80f51fc2c20af633aa271684 GitHub-Last-Rev: 693d7acf952e9478708fe4cd69788f3115e6fe23 GitHub-Pull-Request: golang/go#68607 Reviewed-on: https://go-review.googlesource.com/c/go/+/601335 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Sean Liao <sean@liao.dev> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Roland Shoemaker <roland@golang.org>
For consistency, prefer crypto/hkdf over crypto/internal/fips140/hkdf. Both should have the same behavior given the constrained use of HKDF in TLS. Change-Id: Ia982b9f7a6ea66537d748eb5ecae1ac1eade68a5 Reviewed-on: https://go-review.googlesource.com/c/go/+/658217 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
This change makes debugging easier if the server handshake fails because the client only offers unsupported algorithms. Change-Id: I7daac173a16af2e073aec3d9b59709560f540c6f Reviewed-on: https://go-review.googlesource.com/c/go/+/631555 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
A Go routine was, on an error, returning without sending a message on its signaling channel, so the main program was blocking forever waiting for a message that was never sent. Found while breaking crypto/tls. Change-Id: Id0b3c070a27cabd852f74e86bb9eff5c66b86d28 GitHub-Last-Rev: 4d84fb8b556589ec98eba6142a553fbd45683b96 GitHub-Pull-Request: golang/go#53216 Reviewed-on: https://go-review.googlesource.com/c/go/+/410274 Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Sean Liao <sean@liao.dev> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This allows to skip the last part of the test under GopherJS as well as WebAssembly, since GopherJS shares GOOS=js with wasm. Change-Id: I41adad788043c1863b23eb2a6da9bc9aa2833092 GitHub-Last-Rev: d8d42a3b7ccb2bee6479306b6ac1a319443702ec GitHub-Pull-Request: golang/go#51827 Reviewed-on: https://go-review.googlesource.com/c/go/+/394114 Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Sean Liao <sean@liao.dev> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Sean Liao <sean@liao.dev> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
receipient -> recipient Change-Id: I9ed5937acf0f3808283e35221f8b4f41408eee7c GitHub-Last-Rev: 0ed5ff7a46808d5311af3620b6577734a1e557f4 GitHub-Pull-Request: golang/go#73131 Reviewed-on: https://go-review.googlesource.com/c/go/+/662175 Reviewed-by: Carlos Amedee <carlos@golang.org> Auto-Submit: Jorropo <jorropo.pgm@gmail.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Jorropo <jorropo.pgm@gmail.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Replace the usage of runtime.SetFinalizer with runtime.AddCleanup in the certificate cache. Updates #70907 Change-Id: Ieab6ff88dbc4083f11c1b475f11bd61521dbc638 Reviewed-on: https://go-review.googlesource.com/c/go/+/664275 Auto-Submit: Carlos Amedee <carlos@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently TestCertCache will busy loop waiting for a cleanup (in the runtime.AddCleanup sense) to execute. If we ever get into this busy loop, then on single-threaded platforms like js/wasm, we'll end up _always_ timing out. This doesn't happen right now because we're getting lucky. The finalizer goroutine is scheduled into the runnext slot with 'ready' and is thus scheduled immediately after the GC call. In a follow-up CL, scheduling cleanup goroutines becomes less aggressive, and thus this test fails. Although perhaps that CL should schedule cleanup goroutines more aggressively, the test is still technically buggy, because it expects busy loops like this to call into the scheduler, but that won't happen on certain platforms. Change-Id: I8efe5975be97f4314aec1c8c6e9e22f396be9c94 Reviewed-on: https://go-review.googlesource.com/c/go/+/670755 Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
I noticed a failure of this test on a linux/amd64 builder and reproduced it locally. I can only really reproduce it in a stress test when I overload my system (`stress2 ./tls.test -test.run=TestCertCache`) but this points to the root of the problem: it's possible for a timer to get delayed and the timeout fires before we ever get the chance to check. After copious debugging printlns, this is essentially what I'd observed. There would only be one failed check of the reference count from before it was updated. Change the test to be a busy-loop again, but call runtime.Gosched. This is also what we do for the os.Root tests, and in hindsight should've been my go-to. This has a much higher likelihood of executing promptly. We may want to go back and understand why the 1 ms timer would fire so hilariously late the second time. This might be a real bug. For now, this change makes the test more stable. It no longer fails when it's hammered under `stress2`. Fixes #73637. Change-Id: I316bd9e30946f4c055e61d179c4efc5fe029c608 Reviewed-on: https://go-review.googlesource.com/c/go/+/671175 Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Previously if the clientHandshakeState for the TLS 1.2 client code encountered a server helo message that contained a compression method other than compressionNone, we would emit an unexpected message alert. Instead, it seems more appropriate to return an illegal parameter alert. The server hello message _was_ expected, it just contained a bad parameter option. Making this change also allows enabling the InvalidCompressionMethod bogo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c3 Reviewed-on: https://go-review.googlesource.com/c/go/+/669155 Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit adds test coverage of path building and name constraint verification using the suite of test data provided by Netflix's BetterTLS project. Since the uncompressed raw JSON test data exported by BetterTLS for external test integrations is ~31MB we use a similar approach to the BoGo and ACVP test integrations and fetch the BetterTLS Go module, and run its export tool on-the-fly to generate the test data in a tempdir. As expected, all tests pass currently and this coverage is mainly helpful in catching regressions, especially with tricky/cursed name constraints. Change-Id: I23d7c24232e314aece86bcbfd133b7f02c9e71b5 Reviewed-on: https://go-review.googlesource.com/c/go/+/717420 TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Michael Pratt <mpratt@google.com>
This change mechanically replaces all occurrences of interface{}
by 'any' (where deemed safe by the 'any' modernizer) throughout
std and cmd, minus their vendor trees.
Since this fix is relatively numerous, it gets its own CL.
Also, 'go generate go/types'.
Change-Id: I14a6b52856c3291c1d27935409bca8d5fd4242a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/719702
Commit-Queue: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
Move the BetterTLS test suite from crypto/tls to crypto/x509. Despite the name, the test suites we care about are actually related to X.509 path building and name constraint checking. As such it makes more sense to include these in the crypto/x509 package, so we are more likely to catch breaking behaviors during local testing. Change-Id: I5237903dcc9d9f60d6c7070db3c996ceb643b04c Reviewed-on: https://go-review.googlesource.com/c/go/+/719120 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Junyang Shao <shaojunyang@google.com> Auto-Submit: Roland Shoemaker <roland@golang.org>
For #72850 Change-Id: I07e64f05c82a34b1dadb9a72e16f5045e68cbd24 Reviewed-on: https://go-review.googlesource.com/c/go/+/720642 Auto-Submit: Alan Donovan <adonovan@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Add a new QUICEvent type for reporting errors. This provides a way to report errors that don't occur as a result of QUICConn.Start, QUICConn.HandleData, or QUICConn.SendSessionTicket. Fixes #75108 Change-Id: I941371a21f26b940e75287a66d7e0211fc0baab1 Reviewed-on: https://go-review.googlesource.com/c/go/+/719040 Auto-Submit: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
Updates #75300 Change-Id: I6a6a6964de449b36bc6f5594e08c3c47a0a2f17f Reviewed-on: https://go-review.googlesource.com/c/go/+/701435 Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Mark Freeman <markfreeman@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com>
Updates #75300 Change-Id: I87ed26e8f57180d741408bdbda1696d46a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/719560 Reviewed-by: Mark Freeman <markfreeman@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Filippo Valsorda <filippo@golang.org>
I don't think we have good QUIC ECH tests. BoGo has some for this, but I'm not sure how easy it would be to enable those for QUIC. Fixes #76283 Change-Id: I0ffa535fd89a624b7f9bfd73441ce2a1683e0549 Reviewed-on: https://go-review.googlesource.com/c/go/+/720920 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Damien Neil <dneil@google.com>
This commit adds fields to the ClientHelloInfo and ConnectionState structures to represent hello retry request state information. ClientHelloInfo gains a new HelloRetryRequest bool field that indicates if the client hello was sent in response to a TLS 1.3 hello retry request message previously emitted by the server. ConnectionState gains a new HelloRetryRequest bool field that indicates (depending on the connection role) whether the client received a TLS 1.3 hello retry request message from the server, or whether the server sent such a message to a client. Fixes #74425 Change-Id: Ic1a5290b8a4ba1568da1d2c2cf9f148150955fa5 Reviewed-on: https://go-review.googlesource.com/c/go/+/717440 Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
Check for invalid encodings and keys more systematically in ParseRawPrivateKey/PrivateKey.Bytes, ParseUncompressedPublicKey/PublicKey.Bytes, and fips140/ecdsa.NewPrivateKey/NewPublicKey. Also, use these functions throughout the codebase. This should not change any observable behavior, because there were multiple layers of checks and every path would hit at least one. Change-Id: I6a6a46566c95de871a5a37996835a0e51495f1d8 Reviewed-on: https://go-review.googlesource.com/c/go/+/724000 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Fixes #75300 Change-Id: I6a83e0d040dba3366819d2afff704f886a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/723560 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Filippo Valsorda <filippo@golang.org> TryBot-Bypass: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
…exchanges Fixes #71206 Change-Id: If3cf75261c56828b87ae6805bd2913f56a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/722140 Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes #75656 Change-Id: I6bc71c80973765ef995d17b1450ea2026a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/724820 Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
…balRandom First, we centralize all random bytes generation through drbg.Read. The rest of the FIPS 140-3 module can't use external functions anyway, so drbg.Read needs to have all the logic. Then, make sure that the crypto/... tree uses drbg.Read (or the new crypto/internal/rand.Reader wrapper) instead of crypto/rand, so it is unaffected by applications setting crypto/rand.Reader. Next, pass all unspecified random io.Reader parameters through the new crypto/internal/rand.CustomReader, which just redirects to drbg.Read unless GODEBUG=cryptocustomrand=1 is set. Move all the calls to MaybeReadByte there, since it's only needed for these custom Readers. Finally, add testing/cryptotest.SetGlobalRandom which sets crypto/rand.Reader to a locked deterministic source and overrides drbg.Read. This way SetGlobalRandom should affect all cryptographic randomness in the standard library. Fixes #70942 Co-authored-by: qiulaidongfeng <2645477756@qq.com> Change-Id: I6a6a69641311d9fac318abcc6d79677f0e406100 Reviewed-on: https://go-review.googlesource.com/c/go/+/724480 Reviewed-by: Nicholas Husin <nsh@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I5dec35b1432705b3a52859c38e758220282226af Reviewed-on: https://go-review.googlesource.com/c/go/+/726700 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Sean Liao <sean@liao.dev> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Consistently use testenv.Command and testenv.Executable, avoid redundant testenv.Must, use testenv.CleanCmdEnv where the output is parsed, always log the output with a preceding newline, invoke tests with -v, and always use cmd.Environ() to preserve existing env. Change-Id: I647ff1a8b7d162e5e8df9424030fac446a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/728641 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
For TLS 1.3, after procesesing the server/client hello, if there isn't a CCS message, reject the trailing messages which were appended to the hello messages. This prevents an on-path attacker from injecting plaintext messages into the handshake. Additionally, check that we don't have any buffered messages before we switch the read traffic secret regardless, since any buffered messages would have been under an old key which is no longer appropriate. We also invert the ordering of setting the read/write secrets so that if we fail when changing the read secret we send the alert using the correct write secret. Fixes #76443 Fixes CVE-2025-61730 Change-Id: If6ba8ad16f48d5cd5db5574824062ad4244a5b52 Reviewed-on: https://go-review.googlesource.com/c/go/+/724120 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Coia Prant <coiaprant@gmail.com>
Change-Id: Ib0ec1f05e51a4295a9369d6e8c6b61976a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/735260 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
…icket keys in Config.Clone Once a tls.Config is used, it is not safe to mutate. We provide the Clone method in order to allow users to copy and modify a Config that is in use. If Config.SessionTicketKey is not populated, and if Config.SetSessionTicketKeys has not been called, we automatically populate and rotate session ticket keys. Clone was previously copying these keys into the new Config, meaning that two Configs could share the same auto-rotated session ticket keys. This could allow sessions to be resumed across different Configs, which may have completely different configurations. This change updates Clone to not copy the auto-rotated session ticket keys. Additionally, when resuming a session, check that not just that the leaf certificate is unexpired, but that the entire certificate chain is still unexpired. Fixes #77113 Fixes CVE-2025-68121 Change-Id: I011df7329de83068d11b3f0c793763692d018a98 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3300 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3340 Reviewed-by: Roland Shoemaker <bracewell@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/736704 TryBot-Bypass: Michael Pratt <mpratt@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
Once a tls.Config is used, it is not safe to mutate. We provide the Clone method in order to allow users to copy and modify a Config that is in use. If Config.SessionTicketKey is not populated, and if Config.SetSessionTicketKeys has not been called, we automatically populate and rotate session ticket keys. Clone was previously copying these keys into the new Config, meaning that two Configs could share the same auto-rotated session ticket keys. This could allow sessions to be resumed across different Configs, which may have completely different configurations. This change updates Clone to not copy the auto-rotated session ticket keys. Additionally, when resuming a session, check that not just that the leaf certificate is unexpired, but that the entire certificate chain is still unexpired. Fixes #77113 Fixes CVE-2025-68121 Change-Id: I011df7329de83068d11b3f0c793763692d018a98 Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/3300 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/736709 Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Junyang Shao <shaojunyang@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
…ch.go1.26 For #76474. Change-Id: Ic50e288f99b731b62af9aa73ce6721f0f2f3596d
…ession ticket keys in Config.Clone" This reverts CL 736709 (commit bba24719a4cad5cc8d771fc9cfff5a38019d554a). Updates #77113 Updates #77357 Updates CVE-2025-68121 Change-Id: I0261cb75e9adf9d0ac9890dc91ae8476b8988ba0 Reviewed-on: https://go-review.googlesource.com/c/go/+/739320 Reviewed-by: Coia Prant <coiaprant@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/740002 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
…cking during resumption When resuming a session, check that the verifiedChains contain at least one chain that is still valid at the time of resumption. If not, trigger a new handshake. Updates #77113 Updates #77357 Updates CVE-2025-68121 Change-Id: I14f585c43da17802513cbdd5b10c552d7a38b34e Reviewed-on: https://go-review.googlesource.com/c/go/+/739321 Reviewed-by: Coia Prant <coiaprant@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Auto-Submit: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/740003 Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
…esuming sessions When resuming TLS sessions, on the server and client verify that the chains stored in the session state (verifiedChains) are still acceptable with regards to the Config by checking for the inclusion of the root in either ClientCAs (server) or RootCAs (client). This prevents resuming a session with a certificate chain that would be rejected during a full handshake due to an untrusted root. Updates #77113 Updates #77357 Updates CVE-2025-68121 Change-Id: I11fe00909ef1961c24ecf80bf5b97f7b1121d359 Reviewed-on: https://go-review.googlesource.com/c/go/+/737700 Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Coia Prant <coiaprant@gmail.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-on: https://go-review.googlesource.com/c/go/+/740004 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Nicholas Husin <husin@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Nicholas Husin <nsh@golang.org>
…ion on Windows and macOS TestHandshakeChangeRootCAsResumption and TestHandshakeGetConfigForClientDifferentClientCAs changed because previously rootA and rootB shared Subject and SPKI, which made the new full-chain revalidation check succeed, as the same leaf would verify against both roots. Updates #77376 Fixes #77426 Cq-Include-Trybots: luci.golang.try:go1.26-darwin-arm64-longtest Change-Id: I60bed694bdc621c9e83f1bd8a8224c016a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/741361 Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Roland Shoemaker <roland@golang.org> (cherry picked from commit b691a2edc7f5863f61a07c4a4f087eef1a15a704) Reviewed-on: https://go-review.googlesource.com/c/go/+/741680 Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Michael Pratt <mpratt@google.com>
…ss Configs Updates #77113 Updates #77217 Updates CVE-2025-68121 Change-Id: Ia47904a9ed001275aad0243a6a0ce57e6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/740240 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> (cherry picked from commit 1c9abbdc8e9032cd613bd147c78b166ebacc8a2e) Reviewed-on: https://go-review.googlesource.com/c/go/+/741181 Auto-Submit: Michael Pratt <mpratt@google.com>
…QUICConn's Context Methods on QUICConn are synchronous: The connection state is expected to change only in reaction to a user calling a QUICConn method, and the state change should finish completely before the method returns. The connection context provided to QUICConn.Start violates this model, because canceling the context causes an asynchronous state change. Prior to CL 719040, this caused no problems because canceling the context did not cause any user-visible state changes. In particular, canceling the context did not cause any new events to be immediately returned by QUICConn.NextEvent. CL 719040 introduced a new error event. Now, canceling a QUICConn's context causes a new connection event to be generated. Receiving this event causes a data race visible to the race detector, but the core problem is not the data race itself: It's that an asynchronous event (canceling the connection context) causes an change to the connection events. Fix this race by reworking the handling of QUICConn context cancellation a bit. We no longer react to cancellation while control of the connection lies with the user. We only process cancellation as part of a user call, such as QUICConn.Close or QUICConn.HandleData. Fixes #77274 Change-Id: If2e0f73618c4852114e0931b6bd0cb0b6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/go/+/742561 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit d4febb45179fa99ee1d5783bcb693ed7ba14115c) Reviewed-on: https://go-review.googlesource.com/c/go/+/742761 TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org> Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.