A reference to a function in a "var _ = ..." init-time
initialization keeps the symbol live. Move references to
Config.EncryptTicket and Config.DecryptTicket into tests.
These references increase the size of an unused import of
crypto/tls by about 1MiB.
Change-Id: I6d62a6dcbd73e22972a217afcda7395e909b52cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/498595
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We used to inconsistently run certificate verification on the server on
resumption, but not on the client. This made TLS 1.3 resumption pretty
much useless, as it didn't save bytes, CPU, or round-trips.
This requires serializing the verified chains into the session ticket,
so it's a tradeoff making the ticket bigger to save computation (and for
consistency).
The previous behavior also had a "stickyness" issue: if a ticket
contained invalid certificates, they would be used even if the client
had in the meantime configured valid certificates for a full handshake.
We also didn't check expiration on the client side on resumption if
InsecureSkipVerify was set. Again for consistency, we do that now.
Also, we used to run VerifyPeerCertificates on resumption even if
NoClientCerts was set.
Fixes#31641
Change-Id: Icc88269ea4adb544fa81158114aae76f3c91a15f
Reviewed-on: https://go-review.googlesource.com/c/go/+/497895
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
All OpenSSL tests now test operation with EMS. To test a handshake
*without* EMS we need to pass -Options=-ExtendedMasterSecret which is
only available in OpenSSL 3.1, which breaks a number of other tests.
Updates #43922
Change-Id: Ib9ac79a1d03fab6bfba5fe9cd66689cff661cda7
Reviewed-on: https://go-review.googlesource.com/c/go/+/497376
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
This can be used by applications to store additional data in a session.
Fixes#57753
For #60105
Change-Id: Ib42387ad64750fa8dbbdf51de5e9c86378bef0ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/496822
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
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>
This change by itself is useless, because the application has no way to
access or provide SessionStates to crypto/tls, but they will be provided
in following CLs.
For #60105
Change-Id: I8d5de79b1eda0a778420134cf6f346246a1bb296
Reviewed-on: https://go-review.googlesource.com/c/go/+/496818
Reviewed-by: Marten Seemann <martenseemann@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@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>
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: https://team-review.git.corp.google.com/c/golang/go-private/+/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>
