This change updates the makeClientHello logic to only advertise TLS
1.3 ciphers when tls.Config.MinVersion is set to tls.VersionTLS13
(i.e the client only supports TLS 1.3). Previously, TLS 1.2 ciphers
would be included in the client hello message.
Fixes#57771
Change-Id: Ife4123037b0a4609578ffffb1cdf1e1d4e0a8df6
GitHub-Last-Rev: 45f4275aa9b9550e519e1be5c337b53ab8882007
GitHub-Pull-Request: golang/go#49293
Reviewed-on: https://go-review.googlesource.com/c/go/+/360794
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
There was a bug in TestResumption: the first ExpiredSessionTicket was
inserting a ticket far in the future, so the second ExpiredSessionTicket
wasn't actually supposed to fail. However, there was a bug in
checkForResumption->sendSessionTicket, too: if a session was not resumed
because it was too old, its createdAt was still persisted in the next
ticket. The two bugs used to cancel each other out.
For #60105Fixes#19199
Change-Id: Ic9b2aab943dcbf0de62b8758a6195319dc286e2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/496821
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Another internal change, that allows exposing the new APIs easily in
following CLs.
For #60105
Change-Id: I9c61b9f6e9d29af633f952444f514bcbbe82fe4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/496819
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Ever since session ticket key rotation was introduced in CL 9072, we've
been including a prefix in every ticket to identify what key it's
encrypted with. It's a small privacy gain, but the cost of trial
decryptions is also small, especially since the first key is probably
the most frequently used.
Also reissue tickets on every resumption so that the next connection
can't be linked to all the previous ones. Again the privacy gain is
small but the performance cost is small and it comes with a reduction in
complexity.
For #60105
Change-Id: I852f297162d2b79a3d9bf61f6171e8ce94b2537a
Reviewed-on: https://go-review.googlesource.com/c/go/+/496817
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
When server and client have mismatch in curve preference, the server will
send HelloRetryRequest during TLSv1.3 PSK resumption. There was a bug
introduced by Go1.19.6 or later and Go1.20.1 or later, that makes the client
calculate the PSK binder hash incorrectly. Server will reject the TLS
handshake by sending alert: invalid PSK binder.
Fixes#59424
Change-Id: I2ca8948474275740a36d991c057b62a13392dbb9
GitHub-Last-Rev: 1aad9bcf27f563449c1a7ed6d0dd1d247cc65713
GitHub-Pull-Request: golang/go#59425
Reviewed-on: https://go-review.googlesource.com/c/go/+/481955
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Message marshalling makes use of BytesOrPanic a lot, under the
assumption that it will never panic. This assumption was incorrect, and
specifically crafted handshakes could trigger panics. Rather than just
surgically replacing the usages of BytesOrPanic in paths that could
panic, replace all usages of it with proper error returns in case there
are other ways of triggering panics which we didn't find.
In one specific case, the tree routed by expandLabel, we replace the
usage of BytesOrPanic, but retain a panic. This function already
explicitly panicked elsewhere, and returning an error from it becomes
rather painful because it requires changing a large number of APIs.
The marshalling is unlikely to ever panic, as the inputs are all either
fixed length, or already limited to the sizes required. If it were to
panic, it'd likely only be during development. A close inspection shows
no paths for a user to cause a panic currently.
This patches ends up being rather large, since it requires routing
errors back through functions which previously had no error returns.
Where possible I've tried to use helpers that reduce the verbosity
of frequently repeated stanzas, and to make the diffs as minimal as
possible.
Thanks to Marten Seemann for reporting this issue.
Fixes#58001
Fixes CVE-2022-41724
Change-Id: Ieb55867ef0a3e1e867b33f09421932510cb58851
Reviewed-on: 1679436
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468125
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
It has been agreed that we should prefer the US spelling of words like
"canceling" over "cancelling"; for example, see https://go.dev/cl/14526.
Fix a few occurrences of the "canceling" inconsistency, as well as:
* signaling
* tunneling
* marshaling
Change-Id: I99f3ba0a700a9f0292bc6c1b110af31dd05f1ff0
Reviewed-on: https://go-review.googlesource.com/c/go/+/398734
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
And then revert the bootstrap cmd directories and certain testdata.
And adjust tests as needed.
Not reverting the changes in std that are bootstrapped,
because some of those changes would appear in API docs,
and we want to use any consistently.
Instead, rewrite 'any' to 'interface{}' in cmd/dist for those directories
when preparing the bootstrap copy.
A few files changed as a result of running gofmt -w
not because of interface{} -> any but because they
hadn't been updated for the new //go:build lines.
Fixes#49884.
Change-Id: Ie8045cba995f65bd79c694ec77a1b3d1fe01bb09
Reviewed-on: https://go-review.googlesource.com/c/go/+/368254
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
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>
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>
Simplified both tests significantly by removing logic for writing
the client/server side messages. The flake was likely because of a
race between the closing of the local pipe from inside the test
and closing of the pipe from within the handshakeContext goroutine.
Wait to close the local pipe in the test until after the test
has finished running.
Fixes#45106Fixes#45299
Change-Id: If7ca75aeff7df70cda03c934fa9d8513276d465d
Reviewed-on: https://go-review.googlesource.com/c/go/+/305250
Trust: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This will be reintroduced again once the source of the
flakiness has been determined and fixed.
Fixes#45084
Change-Id: I6677b27fcd71e8c9bb8edbe8e3be70e5a271ebd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/302569
Trust: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Adds the (*tls.Conn).HandshakeContext method. This allows
us to pass the context provided down the call stack to
eventually reach the tls.ClientHelloInfo and
tls.CertificateRequestInfo structs.
These contexts are exposed to the user as read-only via Context()
methods.
This allows users of (*tls.Config).GetCertificate and
(*tls.Config).GetClientCertificate to use the context for
request scoped parameters and cancellation.
Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext
where appropriate, to propagate existing contexts.
Fixes#32406
Change-Id: I259939c744bdc9b805bf51a845a8bc462c042483
Reviewed-on: https://go-review.googlesource.com/c/go/+/295370
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Adds the (*tls.Conn).HandshakeContext method. This allows
us to pass the context provided down the call stack to
eventually reach the tls.ClientHelloInfo and
tls.CertificateRequestInfo structs.
These contexts are exposed to the user as read-only via Context()
methods.
This allows users of (*tls.Config).GetCertificate and
(*tls.Config).GetClientCertificate to use the context for
request scoped parameters and cancellation.
Replace uses of (*tls.Conn).Handshake with (*tls.Conn).HandshakeContext
where appropriate, to propagate existing contexts.
Fixes#32406
Change-Id: I33c228904fe82dcf57683b63627497d3eb841ff2
Reviewed-on: https://go-review.googlesource.com/c/go/+/246338
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Restore previously sent SCTs and stapled OCSP response during session
resumption for both TLS 1.2 and 1.3. This behavior is somewhat
complicated for TLS 1.2 as SCTs are sent during the server hello,
so they override what is saved in ClientSessionState. It is likely
that if the server is sending a different set of SCTs there is probably
a reason for doing so, such as a log being retired, or SCT validation
requirements changing, so it makes sense to defer to the server in
that case.
Fixes#39075
Change-Id: I3c0fa2f69c6bf0247a447c48a1b4c733a882a233
Reviewed-on: https://go-review.googlesource.com/c/go/+/234237
Reviewed-by: Filippo Valsorda <filippo@golang.org>
The ConnectionState's CipherSuite was not set prior
to the VerifyConnection callback in TLS 1.2 servers,
both for full handshakes and resumptions.
Change-Id: Iab91783eff84d1b42ca09c8df08e07861e18da30
Reviewed-on: https://go-review.googlesource.com/c/go/+/236558
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Also add a test that could reproduce this error and
ensure it doesn't occur in other configurations.
Fixes#39012
Change-Id: If792b5131f312c269fd2c5f08c9ed5c00188d1af
Reviewed-on: https://go-review.googlesource.com/c/go/+/233957
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Since the ConnectionState will now be available during
verification, some code was moved around in order to
initialize and make available as much of the fields on
Conn as possible before the ConnectionState is verified.
Fixes#36736
Change-Id: I0e3efa97565ead7de5c48bb8a87e3ea54fbde140
Reviewed-on: https://go-review.googlesource.com/c/go/+/229122
Run-TryBot: Katie Hockman <katie@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Automatically rotate session ticket keys for servers
that don't already have sessionTicketKeys and that
haven't called SetSessionTicketKeys.
Now, session ticket keys will be rotated every 24 hours
with a lifetime of 7 days. This adds a small performance
cost to existing clients that don't provide a session
ticket encrypted with a fresh enough session ticket key,
which would require a full handshake.
Updates #25256
Change-Id: I15b46af7a82aab9a108bceb706bbf66243a1510f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230679
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Also encode the certificates in a way that's more
consistent with TLS 1.3 (with a 24 byte length prefix).
Note that this will have an additional performance cost
requiring clients to do a full handshake every 7 days
where previously they were able to use the same ticket
indefinitely.
Updates #25256
Change-Id: Ic4d1ba0d92773c490b33b5f6c1320d557cc7347d
Reviewed-on: https://go-review.googlesource.com/c/go/+/231317
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This CL changes some unit test functions, making sure that these tests (and goroutines spawned during test) won't block.
Since they are just test functions, I use one CL to fix them all. I hope this won't cause trouble to reviewers and can save time for us.
There are three main categories of incorrect logic fixed by this CL:
1. Use testing.Fatal()/Fatalf() in spawned goroutines, which is forbidden by Go's document.
2. Channels are used in such a way that, when errors or timeout happen, the test will be blocked and never return.
3. Channels are used in such a way that, when errors or timeout happen, the test can return but some spawned goroutines will be leaked, occupying resource until all other tests return and the process is killed.
Change-Id: I3df931ec380794a0cf1404e632c1dd57c65d63e8
Reviewed-on: https://go-review.googlesource.com/c/go/+/219380
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TLS 1.3, which requires RSA-PSS, is now enabled without a GODEBUG
opt-out, and with the introduction of
Certificate.SupportedSignatureAlgorithms (#28660) there is a
programmatic way to avoid RSA-PSS (disable TLS 1.3 with MaxVersion and
use that field to specify only PKCS#1 v1.5 SignatureSchemes).
This effectively reverts 0b3a57b5374bba3fdf88258e2be4c8be65e6a5de,
although following CL 205061 all of the signing-side logic is
conveniently centralized in signatureSchemesForCertificate.
Fixes#32425
Change-Id: I7c9a8893bb5d518d86eae7db82612b9b2cd257d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/205063
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Removed cross-dependencies between handshake_server_test.go and
handshake_client_test.go; moved all initialization to TestMain; replaced
SSLKEYLOGFILE environment variable with -keylog flag.
Change-Id: Ida6712daa44e01a2c00658e8a1896087ee88bcb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/183057
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Signing with RSA-PSS can uncover faulty crypto.Signer implementations,
and it can fail for (broken) small keys. We'll have to take that
breakage eventually, but it would be nice for it to be opt-out at first.
TLS 1.3 requires RSA-PSS and is opt-out in Go 1.13. Instead of making a
TLS 1.3 opt-out influence a TLS 1.2 behavior, let's wait to add RSA-PSS
to TLS 1.2 until TLS 1.3 is on without opt-out.
Note that since the Client Hello is sent before a protocol version is
selected, we have to advertise RSA-PSS there to support TLS 1.3.
That means that we still support RSA-PSS on the client in TLS 1.2 for
verifying server certificates, which is fine, as all issues arise on the
signing side. We have to be careful not to pick (or consider available)
RSA-PSS on the client for client certificates, though.
We'd expect tests to change only in TLS 1.2:
* the server won't pick PSS to sign the key exchange
(Server-TLSv12-* w/ RSA, TestHandshakeServerRSAPSS);
* the server won't advertise PSS in CertificateRequest
(Server-TLSv12-ClientAuthRequested*, TestClientAuth);
* and the client won't pick PSS for its CertificateVerify
(Client-TLSv12-ClientCert-RSA-*, TestHandshakeClientCertRSAPSS,
Client-TLSv12-Renegotiate* because "R" requests a client cert).
Client-TLSv13-ClientCert-RSA-RSAPSS was updated because of a fix in the test.
This effectively reverts 88343530720a52c96b21f2bd5488c8fb607605d7.
Testing was made more complex by the undocumented semantics of OpenSSL's
-[client_]sigalgs (see openssl/openssl#9172).
Updates #32425
Change-Id: Iaddeb2df1f5c75cd090cc8321df2ac8e8e7db349
Reviewed-on: https://go-review.googlesource.com/c/go/+/182339
Reviewed-by: Adam Langley <agl@golang.org>
Shorten some of the longest tests that run during all.bash.
Removes 7r 50u 21s from all.bash.
After this change, all.bash is under 5 minutes again on my laptop.
For #26473.
Change-Id: Ie0460aa935808d65460408feaed210fbaa1d5d79
Reviewed-on: https://go-review.googlesource.com/c/go/+/177559
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Support for Ed25519 certificates was added in CL 175478, this wires them
up into the TLS stack according to RFC 8422 (TLS 1.2) and RFC 8446 (TLS 1.3).
RFC 8422 also specifies support for TLS 1.0 and 1.1, and I initially
implemented that, but even OpenSSL doesn't take the complexity, so I
just dropped it. It would have required keeping a buffer of the
handshake transcript in order to do the direct Ed25519 signatures. We
effectively need to support TLS 1.2 because it shares ClientHello
signature algorithms with TLS 1.3.
While at it, reordered the advertised signature algorithms in the rough
order we would want to use them, also based on what curves have fast
constant-time implementations.
Client and client auth tests changed because of the change in advertised
signature algorithms in ClientHello and CertificateRequest.
Fixes#25355
Change-Id: I9fdd839afde4fd6b13fcbc5cc7017fd8c35085ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/177698
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
In Go 1.13 we will enable RSA-PSS in TLS 1.2 at the same time as we make
TLS 1.3 enabled by default.
This reverts commit 7ccd3583eddcd79679fb29cfc83a6e6fb6973f1e.
Updates #30055
Change-Id: I6f2ddf7652d1172a6b29f4e335ff3a71a89974bc
Reviewed-on: https://go-review.googlesource.com/c/163080
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Most of the issues that led to the decision on #30055 were related to
incompatibility with or faulty support for RSA-PSS (#29831, #29779,
v1.5 signatures). RSA-PSS is required by TLS 1.3, but is also available
to be negotiated in TLS 1.2.
Altering TLS 1.2 behavior based on GODEBUG=tls13=1 feels surprising, so
just disable RSA-PSS entirely in TLS 1.2 until TLS 1.3 is on by default,
so breakage happens all at once.
Updates #30055
Change-Id: Iee90454a20ded8895e5302e8bcbcd32e4e3031c2
Reviewed-on: https://go-review.googlesource.com/c/160998
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
signatureSchemesForCertificate was written to be used with TLS 1.3, but
ended up used for TLS 1.2 client certificates in a refactor. Since it
only supported TLS 1.3 signature algorithms, it would lead to no RSA
client certificates being sent to servers that didn't support RSA-PSS.
TestHandshakeClientCertRSAPKCS1v15 was testing *specifically* for this,
but alas the OpenSSL flag -verify accepts an empty certificates list as
valid, as opposed to -Verify...
Fixes#28925
Change-Id: I61afc02ca501d3d64ab4ad77bbb4cf10931e6f93
Reviewed-on: https://go-review.googlesource.com/c/151660
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Since they are sent after the handshake in TLS 1.3, the client was not
actually consuming them, as it doesn't make any Read calls. They were
then sitting in the kernel receive buffer when the client would call
Close. The kernel would see that and send a RST, which would race the
closeNotify, causing errors.
Also, we get to trim 600 lines of useless test data.
Fixes#28852
Change-Id: I7517feab77dabab7504bfc111098ba09ea07ae5e
Reviewed-on: https://go-review.googlesource.com/c/151659
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TLS_FALLBACK_SCSV is extremely fragile in the presence of sparse
supported_version, but gave it the best try I could.
Set the server random canaries but don't check them yet, waiting for the
browsers to clear the way of misbehaving middleboxes.
Updates #9671
Change-Id: Ie55efdec671d639cf1e716acef0c5f103e91a7ce
Reviewed-on: https://go-review.googlesource.com/c/147617
Reviewed-by: Adam Langley <agl@golang.org>
Note that the SignatureSchemes passed to GetClientCertificate in TLS 1.2
are now filtered by the requested certificate type. This feels like an
improvement anyway, and the full list can be surfaced as well when
support for signature_algorithms_cert is added, which actually matches
the semantics of the CertificateRequest signature_algorithms in TLS 1.2.
Also, note a subtle behavior change in server side resumption: if a
certificate is requested but not required, and the resumed session did
not include one, it used not to invoke VerifyPeerCertificate. However,
if the resumed session did include a certificate, it would. (If a
certificate was required but not in the session, the session is rejected
in checkForResumption.) This inconsistency could be unexpected, even
dangerous, so now VerifyPeerCertificate is always invoked. Still not
consistent with the client behavior, which does not ever invoke
VerifyPeerCertificate on resumption, but it felt too surprising to
entirely change either.
Updates #9671
Change-Id: Ib2b0dbc30e659208dca3ac07d6c687a407d7aaaf
Reviewed-on: https://go-review.googlesource.com/c/147599
Reviewed-by: Adam Langley <agl@golang.org>
Added some assertions to testHandshake, but avoided checking the error
of one of the Close() because the one that would lose the race would
write the closeNotify to a connection closed on the other side which is
broken on js/wasm (#28650). Moved that Close() after the chan sync to
ensure it happens second.
Accepting a ticket with client certificates when NoClientCert is
configured is probably not a problem, and we could hide them to avoid
confusing the application, but the current behavior is to skip the
ticket, and I'd rather keep behavior changes to a minimum.
Updates #9671
Change-Id: I93b56e44ddfe3d48c2bef52c83285ba2f46f297a
Reviewed-on: https://go-review.googlesource.com/c/147445
Reviewed-by: Adam Langley <agl@golang.org>
Also check original certificate validity when resuming TLS 1.0–1.2. Will
refuse to resume a session if the certificate is expired or if the
original connection had InsecureSkipVerify and the resumed one doesn't.
Support only PSK+DHE to protect forward secrecy even with lack of a
strong session ticket rotation story.
Tested with NSS because s_server does not provide any way of getting the
same session ticket key across invocations. Will self-test like TLS
1.0–1.2 once server side is implemented.
Incorporates CL 128477 by @santoshankr.
Fixes#24919
Updates #9671
Change-Id: Id3eaa5b6c77544a1357668bf9ff255f3420ecc34
Reviewed-on: https://go-review.googlesource.com/c/147420
Reviewed-by: Adam Langley <agl@golang.org>
Looks like the introduction of CCS records in the client second flight
gave time to s_server to send NewSessionTicket messages in between the
client application data and close_notify. There seems to be no way of
turning NewSessionTicket messages off, neither by not sending a
psk_key_exchange_modes extension, nor by command line flag.
Interleaving the client write like that tickled an issue akin to #18701:
on Windows, the client reaches Close() before the last record is drained
from the send buffer, the kernel notices and resets the connection,
cutting short the last flow. There is no good way of synchronizing this,
so we sleep for a RTT before calling close, like in CL 75210. Sigh.
Updates #9671
Change-Id: I44dc1cca17b373695b5a18c2741f218af2990bd1
Reviewed-on: https://go-review.googlesource.com/c/147419
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Since TLS 1.3 delivers handshake messages (including KeyUpdate) after
the handshake, the want argument to readRecord had became almost
pointless: it only meant something when set to recordTypeChangeCipherSpec.
Replaced it with a bool to reflect that, and added two shorthands to
avoid anonymous bools in calls.
Took the occasion to simplify and formalize the invariants of readRecord.
The maxConsecutiveEmptyRecords loop became useless when readRecord
started retrying on any non-advancing record in CL 145297.
Replaced panics with errors, because failure is better than undefined
behavior, but contained failure is better than a DoS vulnerability. For
example, I suspect the panic at the top of readRecord was reachable from
handleRenegotiation, which calls readHandshake with handshakeComplete
false. Thankfully it was not a panic in 1.11, and it's allowed now.
Removed Client-TLSv13-RenegotiationRejected because OpenSSL isn't
actually willing to ask for renegotiation over TLS 1.3, the expected
error was due to NewSessionTicket messages, which didn't break the rest
of the tests because they stop too soon.
Updates #9671
Change-Id: I297a81bde5c8020a962a92891b70d6d70b90f5e3
Reviewed-on: https://go-review.googlesource.com/c/147418
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
Also, add support for the SSLKEYLOGFILE environment variable to the
tests, to simplify debugging of unexpected failures.
Updates #9671
Change-Id: I20a34a5824f083da93097b793d51e796d6eb302b
Reviewed-on: https://go-review.googlesource.com/c/147417
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
crypto/x509 already supports PSS signatures (with rsaEncryption OID),
and crypto/tls support was added in CL 79736. Advertise support for the
algorithms and accept them as a peer.
Note that this is about PSS signatures from regular RSA public keys.
RSA-PSS only public keys (with RSASSA-PSS OID) are supported in neither
crypto/tls nor crypto/x509. See RFC 8446, Section 4.2.3.
testdata/Server-TLSv12-ClientAuthRequested* got modified because the
CertificateRequest carries the supported signature algorithms.
The net/smtp tests changed because 512 bits keys are too small for PSS.
Based on Peter Wu's CL 79738, who did all the actual work in CL 79736.
Updates #9671
Change-Id: I4a31e9c6e152ff4c50a5c8a274edd610d5fff231
Reviewed-on: https://go-review.googlesource.com/c/146258
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
RFC 8446 recommends using the supported_versions extension to negotiate
lower versions as well, so begin by implementing it to negotiate the
currently supported versions.
Note that pickTLSVersion was incorrectly negotiating the ServerHello
version down on the client. If the server had illegally sent a version
higher than the ClientHello version, the client would have just
downgraded it, hopefully failing later in the handshake.
In TestGetConfigForClient, we were hitting the record version check
because the server would select TLS 1.1, the handshake would fail on the
client which required TLS 1.2, which would then send a TLS 1.0 record
header on its fatal alert (not having negotiated a version), while the
server would expect a TLS 1.1 header at that point. Now, the client gets
to communicate the minimum version through the extension and the
handshake fails on the server.
Updates #9671
Change-Id: Ie33c7124c0c769f62e10baad51cbed745c424e5b
Reviewed-on: https://go-review.googlesource.com/c/146217
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
The arm5 and mips builders are can't-send-a-packet-to-localhost-in-1s
slow apparently. 1m is less useful, but still better than an obscure
test timeout panic.
Fixes#28405
Change-Id: I2feeae6ea1b095114caccaab4f6709f405faebad
Reviewed-on: https://go-review.googlesource.com/c/145037
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
