aboutsummaryrefslogtreecommitdiff
path: root/src/crypto/tls
AgeCommit message (Collapse)Author
2025-07-09crypto/tls: empty server_name conf. ext. from serverDaniel McCarney
When a TLS server uses the information from the server_name extension in a client hello, and the connection isn't resuming, it should return an empty server_name extension in its server hello (or encrypted extensions for TLS 1.3). For TLS <1.3 we we do this in doFullHandshake(), by setting the pre-existing serverHelloMsg.serverNameAck bool. We know that the connection isn't resuming based on the context where this function is called. For TLS 1.3, a new encryptedExtensionsMsg.serverNameAck bool is added, and populated as appropriate in sendServerParameters() based on whether the conn was resumed or not. The encryptedExtensionsMsg marshalling is updated to emit the encrypted extension based on that field. These changes allow enabling the ServerNameExtensionServer-* bogo tests that verify both the presence and absence of the server_name extension based on the relevant specifications. Resolves #74282 Updates #72006 Change-Id: I703bc2ec916b50906bdece7b7483a7faed7aa8e4 Reviewed-on: https://go-review.googlesource.com/c/go/+/684795 TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
2025-07-01crypto/tls: ensure the ECDSA curve matches the signature algorithmFilippo Valsorda
Change-Id: I6a6a4656c1b47ba6bd652d4da18922cb6b80a8ab Reviewed-on: https://go-review.googlesource.com/c/go/+/675836 Reviewed-by: Roland Shoemaker <roland@golang.org> Auto-Submit: Filippo Valsorda <filippo@golang.org> TryBot-Bypass: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
2025-06-30crypto/tls: update bogo versionDaniel McCarney
This commit updates the pinned revision of BoringSSL that's used for the BoGo integration test. Doing this requires a few categories of config changes: * ignoring a few new tests for features crypto/tls doesn't implement * ignoring a few new tests that require further investigation/classification, or that already have an associated tracking issue * updating the error map syntax to accommodate the upstream change that allows a one-to-many mapping One code change is required in the shim test process to adjust how we tear down a connection after an error to account for an upstream change in the test runner. Previously, for error conditions we would immediately close the connection when exiting the shim process. We instead need to do this in a multi-step process: 1. Flush any pending TLS writes to surface any alerts the error condition may have generated. 2. Close the write side of the TCP connection to signal we're not writing anymore. 3. Read and discard any pending data from the peer. 4. Close the read side of the TCP connection to fully close the socket. Without doing this unpredictable timing factors may result in spurious test failures where: 1. The runner sends us data that produces an error. 2. We send an alert, and immediately tear down the connection. 3. The runner tries to perform a write, and hits an error because the pipe is closed. 4. The runner fails the test with the pipe write error, before it reads from the connection to see the expected alert. With the new code we instead swallow the unrelated writes and the runner sees our alert after its ignored write when it tries to read from the conn. The alert is the expected test outcome, and so the test passes. This was previously not an issue because the runner was discarding the write errors. Updates #72006 Change-Id: Ib72a1c5e693aac92144696c8bae888d5f3f6c32f Reviewed-on: https://go-review.googlesource.com/c/go/+/683456 Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-27crypto/tls: enable signature algorithm BoGo tests (and fix two bugs)Filippo Valsorda
The two bugs are very minor: - We were trying to set the ConnectionState CurveID field even if the RSA key exchange was in use - We were sending the wrong alert from TLS 1.2 clients if none of the certificate signature algorithms were supported Change-Id: I6a6a46564f5a9f1a5d44e54fc59a650118ad67d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/675918 Auto-Submit: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Michael Knyszek <mknyszek@google.com>
2025-05-23crypto/tls: signature_algorithms in CertificateRequest can't be emptyFilippo Valsorda
Change-Id: I6a6a4656ab97e1f247df35b2589cd73461b4ac76 Reviewed-on: https://go-review.googlesource.com/c/go/+/675917 Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-05-21crypto/tls: reject duplicate TLS 1.3 EncryptedExtensionsDaniel McCarney
When a TLS 1.3 client processes the server's encryptedExtensionsMsg it should reject instances that contain duplicate extension types. RFC 8446 §4.2 says: There MUST NOT be more than one extension of the same type in a given extension block. This update matches enforcement done in the client hello unmarshalling, but applied to the TLS 1.3 encrypted extensions message unmarshalling. Making this change also allows enabling the DuplicateExtensionClient-TLS-TLS13 BoGo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d2 Reviewed-on: https://go-review.googlesource.com/c/go/+/673757 Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-21crypto/tls: use decode alert for handshake msg unmarshal errDaniel McCarney
Previously if instances of the handshakeMessage interface returned false from unmarshal(), indicating an umarshalling error, the crypto/tls package would emit an unexpected_message alert. This commit changes to use a decode_error alert for this condition instead. The usage-pattern of the handshakeMessage interface is that we switch on the message type, invoke a specific concrete handshakeMessage type's unmarshal function, and then return it to the caller on success. At this point the caller looks at the message type and can determine if the message was unexpected or not. If it was unexpected, the call-sites emit the correct error for that case. Only the caller knows the current protocol state and allowed message types, not the generic handshake decoding logic. With the above in mind, if we find that within the unmarshal logic for a specific message type that the data we have in hand doesn't match the protocol syntax we should emit a decode_error. An unexpected_message error isn't appropriate because we don't yet know if the message is unexpected or not, only that the message can't be decoded based on the spec's syntax for the type the message claimed to be. Notably one unit test, TestQUICPostHandshakeKeyUpdate, had to have its test data adjusted because it was previously not testing the right thing: it was double-encoding the type & length prefix data for a key update message and expecting the QUIC logic to reject it as an inappropriate post-handshake message. In reality it was being rejected sooner as an invalid key update message from the double-encoding and this was masked by the previous alert for this condition matching the expected alert. Finally, changing our alert allows enabling a handful of BoGo tests related to duplicate extensions of the form "DuplicateExtension[Server|Client]-TLS-[TLS1|TLS11|TLS12|TLS13]". One test remains skipped (DuplicateExtensionClient-TLS-TLS13), as it requires additional follow-up. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d1 Reviewed-on: https://go-review.googlesource.com/c/go/+/673738 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Filippo Valsorda <filippo@golang.org>
2025-05-21crypto/tls: disable SHA-1 signature algorithms in TLS 1.2Filippo Valsorda
This implements RFC 9155 by removing support for SHA-1 algorithms: - we don't advertise them in ClientHello and CertificateRequest (where supportedSignatureAlgorithms is used directly) - we don't select them in our ServerKeyExchange and CertificateVerify (where supportedSignatureAlgorithms filters signatureSchemesForCertificate) - we reject them in the peer's ServerKeyExchange and CertificateVerify (where we check against the algorithms we advertised in ClientHello and CertificateRequest) Fixes #72883 Change-Id: I6a6a4656e2aafd2c38cdd32090d3d8a9a8047818 Reviewed-on: https://go-review.googlesource.com/c/go/+/658216 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
2025-05-21crypto/tls: don't advertise TLS 1.2-only sigAlgs in TLS 1.3Filippo Valsorda
If a ClientHello only supports TLS 1.3, or if a CertificateRequest is sent after selecting TLS 1.3, we should not advertise TLS 1.2-only signature_algorithms like PKCS#1 v1.5 or SHA-1. However, since crypto/x509 still supports PKCS#1 v1.5, and a direct CertPool match might not care about the signature in the certificate at all, start sending a separate signature_algorithms_cert extension to indicate support for PKCS#1 v1.5 and SHA-1 in certificates. We were already correctly rejecting these algorithms if the peer selected them in a TLS 1.3 connection. Updates #72883 Change-Id: I6a6a4656ab60e1b7fb20fdedc32604dc156953ae Reviewed-on: https://go-review.googlesource.com/c/go/+/658215 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Filippo Valsorda <filippo@golang.org> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
2025-05-21crypto/tls: update BoGo SessionID test skip reasonsDaniel McCarney
Updates the skip reason for the following BoGo tests: * TLS-ECH-Client-TLS12SessionID * SupportTicketsWithSessionID * ResumeTLS12SessionID-TLS13 The crypto/tls package does not support session ID based resumption at this time, and so any tests that rely on this support need to be skipped. Updates #72006 Updates #25228 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5d0 Reviewed-on: https://go-review.googlesource.com/c/go/+/673737 TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-21crypto/tls: enable BoGo DisabledCurve-HelloRetryRequest-TLS13Daniel McCarney
The crypto/tls package produces the expected error for this test case, and so it can be enabled. Looking at the history of the relevant code it appears the TLS 1.3 implementation has always had the correct behaviour for HRR changing to an unsupported group after the initial hello. I think this test was skipped initially because at the time of initial BoGo config commit we hadn't implemented the -curves argument for the test shim yet, and this test relies on it. We later added support for that flag alongside X25519Kyber768Draft00 KX and I think we missed the chance to enable the test then. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cf Reviewed-on: https://go-review.googlesource.com/c/go/+/673756 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com> TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net>
2025-05-21crypto/tls: match compression method alert across versionsDaniel McCarney
When a pre-TLS 1.3 server processes a client hello message that indicates compression methods that don't include the null compression method, send an illegal parameter alert. Previously we did this for TLS 1.3 server handshakes only, and the legacy TLS versions used alertHandshakeFailure for this circumstance. By switching this to alertIllegalParameter we use a consistent alert across all TLS versions, and can also enable the NoNullCompression-TLS12 BoGo test we were skipping. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5ce Reviewed-on: https://go-review.googlesource.com/c/go/+/673736 TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: David Chase <drchase@google.com>
2025-05-21crypto/tls: delete dead code curveIDForCurveDaniel McCarney
This unexported function has no call-sites. Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cd Reviewed-on: https://go-review.googlesource.com/c/go/+/673755 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Filippo Valsorda <filippo@golang.org> TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net>
2025-05-21crypto/tls: verify server chooses advertised curveDaniel McCarney
When a crypto/tls client using TLS < 1.3 sends supported elliptic_curves in a client hello message the server must limit itself to choosing one of the supported options from our message. If we process a server key exchange message that chooses an unadvertised curve, abort the handshake w/ an error. Previously we would not note that the server chose a curve we didn't include in the client hello message, and would proceed with the handshake as long as the chosen curve was one that we've implemented. However, RFC 8422 5.1 makes it clear this is a server acting out-of-spec, as it says: If a server does not understand the Supported Elliptic Curves Extension, does not understand the Supported Point Formats Extension, or is unable to complete the ECC handshake while restricting itself to the enumerated curves and point formats, it MUST NOT negotiate the use of an ECC cipher suite. Changing our behaviour to enforce this also allows enabling the UnsupportedCurve BoGo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cc Reviewed-on: https://go-review.googlesource.com/c/go/+/673735 TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: Filippo Valsorda <filippo@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-21crypto/tls: have servers prefer TLS 1.3 when supportedDaniel McCarney
Previously the common Config.mutualVersion() code prioritized the selected version based on the provided peerVersions being sent in peer preference order. Instead we would prefer to see TLS 1.3 used whenever it is supported, even if the peer would prefer an older protocol version. This commit updates mutualVersions() to implement this policy change. Our new behaviour matches the behaviour of other TLS stacks, notably BoringSSL, and so also allows enabling the IgnoreClientVersionOrder BoGo test that we otherwise must skip. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5cb Reviewed-on: https://go-review.googlesource.com/c/go/+/673236 Auto-Submit: Daniel McCarney <daniel@binaryparadox.net> TryBot-Bypass: Daniel McCarney <daniel@binaryparadox.net> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-21crypto/tls: add GetEncryptedClientHelloKeysRoland Shoemaker
This allows servers to rotate their ECH keys without needing to restart the server. Fixes #71920 Change-Id: I55591ab3303d5fde639038541c50edcf1fafc9aa Reviewed-on: https://go-review.googlesource.com/c/go/+/670655 TryBot-Bypass: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com> Auto-Submit: Roland Shoemaker <roland@golang.org> Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
2025-05-21crypto/tls: replace custom intern cache with weak cacheRoland Shoemaker
Uses the new weak package to replace the existing custom intern cache with a map of weak.Pointers instead. This simplifies the cache, and means we don't need to store a slice of handles on the Conn anymore. Change-Id: I5c2bf6ef35fac4255e140e184f4e48574b34174c Reviewed-on: https://go-review.googlesource.com/c/go/+/644176 TryBot-Bypass: Roland Shoemaker <roland@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com> Auto-Submit: Roland Shoemaker <roland@golang.org>
2025-05-15crypto/tls: fix bogo IgnoreClientVersionOrder skip reasonDaniel McCarney
The BoGo IgnoreClientVersionOrder test checks that a client that sends a supported_versions extension with the list [TLS 1.2, TLS 1.3] ends up negotiating TLS 1.3. However, the crypto/tls module treats this list as being in client preference order, and so negotiates TLS 1.2, failing the test. Our behaviour appears to be the correct handling based on RFC 8446 §4.2.1 where it says: The extension contains a list of supported versions in preference order, with the most preferred version first. This commit updates the reason we skip this test to cite the RFC instead of saying it's something to be fixed. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5ca Reviewed-on: https://go-review.googlesource.com/c/go/+/671415 Reviewed-by: 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> Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>
2025-05-09crypto/tls: handle client hello version too highDaniel McCarney
If the client hello legacy version is >= TLS 1.3, and no supported_versions extension is sent, negotiate TLS 1.2 or lower when supported. On the topic of supported version negotiation RFC 8446 4.2.1 indicates TLS 1.3 implementations MUST send a supported_versions extension with a list of their supported protocol versions. The crypto/tls package enforces this when the client hello legacy version indicates TLS 1.3 (0x0304), aborting the handshake with an alertMissingExtension alert if no supported_versions were received. However, section 4.2.1 indicates different behaviour should be used when the extension is not present and TLS 1.2 or prior are supported: If this extension is not present, servers which are compliant with this specification and which also support TLS 1.2 MUST negotiate TLS 1.2 or prior as specified in [RFC5246], even if ClientHello.legacy_version is 0x0304 or later. This commit updates the client hello processing logic to allow this behaviour. If no supported_versions extension was received we ignore the legacy version being >= TLS 1.3 and instead negotiate a lower supported version if the server configuration allows. This fix in turn allows enabling the BoGo ClientHelloVersionTooHigh, MinorVersionTolerance, and MajorVersionTolerance tests. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c9 Reviewed-on: https://go-review.googlesource.com/c/go/+/671235 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>
2025-05-09crypto/tls: fix TLS <1.3 client cert required alertDaniel McCarney
Previously for protocol versions older than TLS 1.3 our server handshake implementation sent an alertBadCertificate alert in the case where the server TLS config indicates a client cert is required and none was received. This commit updates the relevant logic to instead send alertHandshakeFailure in these circumstances. For TLS 1.2, RFC 5246 §7.4.6 unambiguously describes this as the correct alert: If the client does not send any certificates, the server MAY at its discretion either continue the handshake without client authentication, or respond with a fatal handshake_failure alert. The TLS 1.1 and 1.0 specs also describe using this alert (RFC 4346 §7.4.6 and RFC 2246 §7.4.6) both say: If client authentication is required by the server for the handshake to continue, it may respond with a fatal handshake failure alert. Making this correction also allows enabling the RequireAnyClientCertificate-TLS1* bogo tests. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c8 Reviewed-on: https://go-review.googlesource.com/c/go/+/671195 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-09crypto/tls: enable more large record bogo testsDaniel McCarney
Previously a handful of large record tests were in the bogo config ignore list. The ignored tests were failing because they used insecure ciphersuites that aren't enabled by default. This commit adds the non-default insecure ciphersuites to the bogo TLS configuration and re-enables the tests. Doing this uncovered a handful of unrelated tests that needed to be fixed, each handled before this commit. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c7 Reviewed-on: https://go-review.googlesource.com/c/go/+/669158 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>
2025-05-09crypto/tls: skip BadRSAClientKeyExchange-[4,5]Daniel McCarney
These two bogo tests mutate the version number used for the premaster secret calculation for a client RSA key exchange, with the expectation the server rejects the handshake. Per the comment in the end of rsaKeyAgreement.processClientKeyExchange we explicitly choose *not* to verify the version number. This commit adds the two version number tests to the ignore list. They coincidentally happen to produced the expected failure because they use a non-default ciphersuite. When we add this ciphersuite to the client config for the bogo test they will start to fail unless ignored. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c6 Reviewed-on: https://go-review.googlesource.com/c/go/+/669175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org>
2025-05-09crypto/tls: err for unsupported point format configsDaniel McCarney
If a client or server explicitly offers point formats, and the point formats don't include the uncompressed format, then error. This matches BoringSSL and Rustls behaviour and allows enabling the PointFormat-Client-MissingUncompressed bogo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c5 Reviewed-on: https://go-review.googlesource.com/c/go/+/669157 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
2025-05-09crypto/tls: update TLS 1.3 client compression validationDaniel McCarney
Unlike in earlier TLS versions, in TLS 1.3 when processing a server hello the legacy_compression_method MUST have the value 0. It is no longer a parameter that offers a choice of compression method. With this in mind, it seems more appropriate to return a decode error when we encounter a non-zero compression method in a server hello message. We haven't found a parameter value we reject, we've found a message that doesn't decode according to its specification. Making this change also aligns with BoringSSL and allows enabling the TLS13-HRR-InvalidCompressionMethod bogo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c4 Reviewed-on: https://go-review.googlesource.com/c/go/+/669156 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>
2025-05-09crypto/tls: use illegal param alert for bad compressionDaniel McCarney
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>
2025-05-08crypto/tls: use runtime.Gosched instead of time.After in TestCertCacheMichael Anthony Knyszek
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>
2025-05-08crypto/tls: add scheduler call to TestCertCache refcount timeout loopMichael Anthony Knyszek
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>
2025-05-07crypto/tls: use runtime.AddCleanup instead of runtime.SetFinalizerCarlos Amedee
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>
2025-04-28crypto/internal/hpke: rename Receipient to RecipientJohn Bampton
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>
2025-04-23crypto/tls: skip part of the test based on GOOS instead of GOARCHNevkontakte
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>
2025-04-16crypto/tls: fix a testing deadlock that occurs on a TLS protocol errorEric Young
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>
2025-04-07crypto/tls: add offered cipher suites to the handshake errorNicola Murino
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>
2025-04-02crypto/tls: use crypto/hkdfqmuntal
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>
2025-03-20crypto/tls: add missing RUnlock in ticketKeysEdoardo Spadolini
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>
2025-03-17crypto/tls: fix ECH compatibility古大羊
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>
2025-03-13crypto/tls/internal/fips140tls: use crypto/fips140qmuntal
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>
2025-03-13crypto/tls: relax native FIPS 140-3 modeFilippo Valsorda
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>
2025-03-13crypto/tls: clean up supported/default/allowed parametersFilippo Valsorda
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>
2025-03-13crypto/tls: add ConnectionState.CurveIDFilippo Valsorda
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>
2025-03-13crypto/tls: allow P-521 in FIPS 140-3 mode and Go+BoringCryptoFilippo Valsorda
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>
2025-03-10crypto/tls: reject TLS 1.3 compat session ID in TLS 1.2Daniel McCarney
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>
2025-03-10crypto/tls: align cert decode alert w/ BSSLDaniel McCarney
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>
2025-03-10crypto/tls: update GREASE-Server-TLS13 BoGo skipDaniel McCarney
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>
2025-03-10crypto/tls: support bogo -wait-for-debuggerDaniel McCarney
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>
2025-03-10crypto/tls: ignore TLS 1.3 user canceled alertsDaniel McCarney
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>
2025-03-10crypto/tls: run SkipNewSessionTicket bogo testDaniel McCarney
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>
2025-03-10crypto/tls: reject empty TLS 1.3 session ticketDaniel McCarney
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>
2025-03-07crypto/tls: small bogo shim test tidyingDaniel McCarney
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>
2025-02-25crypto/tls: require EMS in FIPS 140-3 modeFilippo Valsorda
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>
2025-02-19crypto/tls: improve ech parsing errorsRoland Shoemaker
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>