2018-11-01 01:01:09 -04:00
|
|
|
// Copyright 2018 The Go Authors. All rights reserved.
|
|
|
|
|
// Use of this source code is governed by a BSD-style
|
|
|
|
|
// license that can be found in the LICENSE file.
|
|
|
|
|
|
|
|
|
|
package tls
|
|
|
|
|
|
|
|
|
|
import (
|
|
|
|
|
"bytes"
|
2020-08-01 12:18:31 +01:00
|
|
|
"context"
|
2018-11-01 01:01:09 -04:00
|
|
|
"crypto"
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
"crypto/ecdh"
|
2018-11-01 01:01:09 -04:00
|
|
|
"crypto/hmac"
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
"crypto/rsa"
|
2018-11-01 01:01:09 -04:00
|
|
|
"errors"
|
|
|
|
|
"hash"
|
2018-11-04 18:41:37 -05:00
|
|
|
"time"
|
2018-11-01 01:01:09 -04:00
|
|
|
)
|
|
|
|
|
|
|
|
|
|
type clientHandshakeStateTLS13 struct {
|
2018-11-04 18:41:37 -05:00
|
|
|
c *Conn
|
2020-08-01 12:18:31 +01:00
|
|
|
ctx context.Context
|
2018-11-04 18:41:37 -05:00
|
|
|
serverHello *serverHelloMsg
|
|
|
|
|
hello *clientHelloMsg
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
ecdheKey *ecdh.PrivateKey
|
2018-11-04 18:41:37 -05:00
|
|
|
|
2023-05-21 21:17:56 +02:00
|
|
|
session *SessionState
|
2018-11-04 18:41:37 -05:00
|
|
|
earlySecret []byte
|
|
|
|
|
binderKey []byte
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
certReq *certificateRequestMsgTLS13
|
2018-11-04 18:41:37 -05:00
|
|
|
usingPSK bool
|
2018-11-03 20:04:44 -04:00
|
|
|
sentDummyCCS bool
|
2018-11-01 01:01:09 -04:00
|
|
|
suite *cipherSuiteTLS13
|
|
|
|
|
transcript hash.Hash
|
|
|
|
|
masterSecret []byte
|
|
|
|
|
trafficSecret []byte // client_application_traffic_secret_0
|
|
|
|
|
}
|
|
|
|
|
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
// handshake requires hs.c, hs.hello, hs.serverHello, hs.ecdheKey, and,
|
2018-11-04 18:41:37 -05:00
|
|
|
// optionally, hs.session, hs.earlySecret and hs.binderKey to be set.
|
2018-11-01 01:01:09 -04:00
|
|
|
func (hs *clientHandshakeStateTLS13) handshake() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
2024-01-26 23:22:45 +01:00
|
|
|
if needFIPS() {
|
|
|
|
|
return errors.New("tls: internal error: TLS 1.3 reached in FIPS mode")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
// The server must not select TLS 1.3 in a renegotiation. See RFC 8446,
|
|
|
|
|
// sections 4.1.2 and 4.1.3.
|
|
|
|
|
if c.handshakes > 0 {
|
|
|
|
|
c.sendAlert(alertProtocolVersion)
|
|
|
|
|
return errors.New("tls: server selected TLS 1.3 in a renegotiation")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// Consistency check on the presence of a keyShare and its parameters.
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
if hs.ecdheKey == nil || len(hs.hello.keyShares) != 1 {
|
2018-11-01 01:01:09 -04:00
|
|
|
return c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if err := hs.checkServerHelloOrHRR(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.transcript = hs.suite.hash.New()
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
|
|
|
|
|
if err := transcriptMsg(hs.hello, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
|
|
|
|
|
if bytes.Equal(hs.serverHello.random, helloRetryRequestRandom) {
|
2018-11-03 20:04:44 -04:00
|
|
|
if err := hs.sendDummyChangeCipherSpec(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
if err := hs.processHelloRetryRequest(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if err := transcriptMsg(hs.serverHello, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
|
2018-11-03 20:04:44 -04:00
|
|
|
c.buffering = true
|
2018-11-01 01:01:09 -04:00
|
|
|
if err := hs.processServerHello(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-03 20:04:44 -04:00
|
|
|
if err := hs.sendDummyChangeCipherSpec(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
if err := hs.establishHandshakeKeys(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := hs.readServerParameters(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-04 18:41:37 -05:00
|
|
|
if err := hs.readServerCertificate(); err != nil {
|
2018-11-01 01:01:09 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := hs.readServerFinished(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := hs.sendClientCertificate(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := hs.sendClientFinished(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if _, err := c.flush(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2022-08-09 09:36:17 -07:00
|
|
|
c.isHandshakeComplete.Store(true)
|
2018-11-01 01:01:09 -04:00
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// checkServerHelloOrHRR does validity checks that apply to both ServerHello and
|
|
|
|
|
// HelloRetryRequest messages. It sets hs.suite.
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) checkServerHelloOrHRR() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.supportedVersion == 0 {
|
|
|
|
|
c.sendAlert(alertMissingExtension)
|
|
|
|
|
return errors.New("tls: server selected TLS 1.3 using the legacy version field")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.supportedVersion != VersionTLS13 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected an invalid version after a HelloRetryRequest")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.vers != VersionTLS12 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server sent an incorrect legacy version")
|
|
|
|
|
}
|
|
|
|
|
|
2019-04-29 22:04:09 +00:00
|
|
|
if hs.serverHello.ocspStapling ||
|
2018-11-01 01:01:09 -04:00
|
|
|
hs.serverHello.ticketSupported ||
|
2023-05-24 01:55:45 +02:00
|
|
|
hs.serverHello.extendedMasterSecret ||
|
2018-11-01 01:01:09 -04:00
|
|
|
hs.serverHello.secureRenegotiationSupported ||
|
|
|
|
|
len(hs.serverHello.secureRenegotiation) != 0 ||
|
|
|
|
|
len(hs.serverHello.alpnProtocol) != 0 ||
|
|
|
|
|
len(hs.serverHello.scts) != 0 {
|
|
|
|
|
c.sendAlert(alertUnsupportedExtension)
|
|
|
|
|
return errors.New("tls: server sent a ServerHello extension forbidden in TLS 1.3")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if !bytes.Equal(hs.hello.sessionId, hs.serverHello.sessionId) {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server did not echo the legacy session ID")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.compressionMethod != compressionNone {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected unsupported compression format")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
selectedSuite := mutualCipherSuiteTLS13(hs.hello.cipherSuites, hs.serverHello.cipherSuite)
|
|
|
|
|
if hs.suite != nil && selectedSuite != hs.suite {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server changed cipher suite after a HelloRetryRequest")
|
|
|
|
|
}
|
|
|
|
|
if selectedSuite == nil {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server chose an unconfigured cipher suite")
|
|
|
|
|
}
|
|
|
|
|
hs.suite = selectedSuite
|
|
|
|
|
c.cipherSuite = hs.suite.id
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-03 20:04:44 -04:00
|
|
|
// sendDummyChangeCipherSpec sends a ChangeCipherSpec record for compatibility
|
|
|
|
|
// with middleboxes that didn't implement TLS correctly. See RFC 8446, Appendix D.4.
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) sendDummyChangeCipherSpec() error {
|
2022-10-14 10:48:42 -07:00
|
|
|
if hs.c.quic != nil {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
2018-11-03 20:04:44 -04:00
|
|
|
if hs.sentDummyCCS {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
hs.sentDummyCCS = true
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
return hs.c.writeChangeCipherRecord()
|
2018-11-03 20:04:44 -04:00
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
// processHelloRetryRequest handles the HRR in hs.serverHello, modifies and
|
|
|
|
|
// resends hs.hello, and reads the new ServerHello into hs.serverHello.
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
// The first ClientHello gets double-hashed into the transcript upon a
|
2020-04-29 18:31:50 -04:00
|
|
|
// HelloRetryRequest. (The idea is that the server might offload transcript
|
|
|
|
|
// storage to the client in the cookie.) See RFC 8446, Section 4.4.1.
|
2018-11-04 18:41:37 -05:00
|
|
|
chHash := hs.transcript.Sum(nil)
|
|
|
|
|
hs.transcript.Reset()
|
|
|
|
|
hs.transcript.Write([]byte{typeMessageHash, 0, 0, uint8(len(chHash))})
|
|
|
|
|
hs.transcript.Write(chHash)
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if err := transcriptMsg(hs.serverHello, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-04 18:41:37 -05:00
|
|
|
|
2020-04-29 18:31:50 -04:00
|
|
|
// The only HelloRetryRequest extensions we support are key_share and
|
|
|
|
|
// cookie, and clients must abort the handshake if the HRR would not result
|
|
|
|
|
// in any change in the ClientHello.
|
|
|
|
|
if hs.serverHello.selectedGroup == 0 && hs.serverHello.cookie == nil {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server sent an unnecessary HelloRetryRequest message")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.cookie != nil {
|
|
|
|
|
hs.hello.cookie = hs.serverHello.cookie
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
if hs.serverHello.serverShare.group != 0 {
|
|
|
|
|
c.sendAlert(alertDecodeError)
|
|
|
|
|
return errors.New("tls: received malformed key_share extension")
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-29 18:31:50 -04:00
|
|
|
// If the server sent a key_share extension selecting a group, ensure it's
|
|
|
|
|
// a group we advertised but did not send a key share for, and send a key
|
|
|
|
|
// share for it this time.
|
|
|
|
|
if curveID := hs.serverHello.selectedGroup; curveID != 0 {
|
|
|
|
|
curveOK := false
|
|
|
|
|
for _, id := range hs.hello.supportedCurves {
|
|
|
|
|
if id == curveID {
|
|
|
|
|
curveOK = true
|
|
|
|
|
break
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
2020-04-29 18:31:50 -04:00
|
|
|
if !curveOK {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected unsupported group")
|
|
|
|
|
}
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
if sentID, _ := curveIDForCurve(hs.ecdheKey.Curve()); sentID == curveID {
|
2020-04-29 18:31:50 -04:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server sent an unnecessary HelloRetryRequest key_share")
|
|
|
|
|
}
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
if _, ok := curveForCurveID(curveID); !ok {
|
2020-04-29 18:31:50 -04:00
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return errors.New("tls: CurvePreferences includes unsupported curve")
|
|
|
|
|
}
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
key, err := generateECDHEKey(c.config.rand(), curveID)
|
2020-04-29 18:31:50 -04:00
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
hs.ecdheKey = key
|
|
|
|
|
hs.hello.keyShares = []keyShare{{group: curveID, data: key.PublicKey().Bytes()}}
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.hello.raw = nil
|
2018-11-04 18:41:37 -05:00
|
|
|
if len(hs.hello.pskIdentities) > 0 {
|
|
|
|
|
pskSuite := cipherSuiteTLS13ByID(hs.session.cipherSuite)
|
|
|
|
|
if pskSuite == nil {
|
|
|
|
|
return c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
|
|
|
|
if pskSuite.hash == hs.suite.hash {
|
|
|
|
|
// Update binders and obfuscated_ticket_age.
|
2023-05-21 21:17:56 +02:00
|
|
|
ticketAge := c.config.time().Sub(time.Unix(int64(hs.session.createdAt), 0))
|
|
|
|
|
hs.hello.pskIdentities[0].obfuscatedTicketAge = uint32(ticketAge/time.Millisecond) + hs.session.ageAdd
|
2018-11-04 18:41:37 -05:00
|
|
|
|
|
|
|
|
transcript := hs.suite.hash.New()
|
|
|
|
|
transcript.Write([]byte{typeMessageHash, 0, 0, uint8(len(chHash))})
|
|
|
|
|
transcript.Write(chHash)
|
2023-04-12 10:07:07 +00:00
|
|
|
if err := transcriptMsg(hs.serverHello, transcript); err != nil {
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
helloBytes, err := hs.hello.marshalWithoutBinders()
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
transcript.Write(helloBytes)
|
2018-11-04 18:41:37 -05:00
|
|
|
pskBinders := [][]byte{hs.suite.finishedHash(hs.binderKey, transcript)}
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if err := hs.hello.updateBinders(pskBinders); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-04 18:41:37 -05:00
|
|
|
} else {
|
|
|
|
|
// Server selected a cipher suite incompatible with the PSK.
|
|
|
|
|
hs.hello.pskIdentities = nil
|
|
|
|
|
hs.hello.pskBinders = nil
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2023-05-22 19:23:04 +02:00
|
|
|
if hs.hello.earlyData {
|
|
|
|
|
hs.hello.earlyData = false
|
|
|
|
|
c.quicRejectedEarlyData()
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if _, err := hs.c.writeHandshakeRecord(hs.hello, hs.transcript); err != nil {
|
2018-11-01 01:01:09 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
// serverHelloMsg is not included in the transcript
|
|
|
|
|
msg, err := c.readHandshake(nil)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
serverHello, ok := msg.(*serverHelloMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(serverHello, msg)
|
|
|
|
|
}
|
|
|
|
|
hs.serverHello = serverHello
|
|
|
|
|
|
|
|
|
|
if err := hs.checkServerHelloOrHRR(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) processServerHello() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
if bytes.Equal(hs.serverHello.random, helloRetryRequestRandom) {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return errors.New("tls: server sent two HelloRetryRequest messages")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if len(hs.serverHello.cookie) != 0 {
|
|
|
|
|
c.sendAlert(alertUnsupportedExtension)
|
|
|
|
|
return errors.New("tls: server sent a cookie in a normal ServerHello")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.serverHello.selectedGroup != 0 {
|
|
|
|
|
c.sendAlert(alertDecodeError)
|
|
|
|
|
return errors.New("tls: malformed key_share extension")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
if hs.serverHello.serverShare.group == 0 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server did not send a key share")
|
|
|
|
|
}
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
if sentID, _ := curveIDForCurve(hs.ecdheKey.Curve()); hs.serverHello.serverShare.group != sentID {
|
2018-11-01 01:01:09 -04:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected unsupported group")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
if !hs.serverHello.selectedIdentityPresent {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if int(hs.serverHello.selectedIdentity) >= len(hs.hello.pskIdentities) {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected an invalid PSK")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if len(hs.hello.pskIdentities) != 1 || hs.session == nil {
|
|
|
|
|
return c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
|
|
|
|
pskSuite := cipherSuiteTLS13ByID(hs.session.cipherSuite)
|
|
|
|
|
if pskSuite == nil {
|
|
|
|
|
return c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
|
|
|
|
if pskSuite.hash != hs.suite.hash {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: server selected an invalid PSK and cipher suite pair")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.usingPSK = true
|
|
|
|
|
c.didResume = true
|
2023-05-21 21:17:56 +02:00
|
|
|
c.peerCertificates = hs.session.peerCertificates
|
|
|
|
|
c.activeCertHandles = hs.session.activeCertHandles
|
2018-11-04 18:41:37 -05:00
|
|
|
c.verifiedChains = hs.session.verifiedChains
|
2020-05-15 12:49:04 -07:00
|
|
|
c.ocspResponse = hs.session.ocspResponse
|
|
|
|
|
c.scts = hs.session.scts
|
2018-11-01 01:01:09 -04:00
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) establishHandshakeKeys() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
peerKey, err := hs.ecdheKey.Curve().NewPublicKey(hs.serverHello.serverShare.data)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: invalid server key share")
|
|
|
|
|
}
|
2022-11-14 12:13:46 +01:00
|
|
|
sharedKey, err := hs.ecdheKey.ECDH(peerKey)
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
if err != nil {
|
2018-11-01 01:01:09 -04:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: invalid server key share")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
earlySecret := hs.earlySecret
|
|
|
|
|
if !hs.usingPSK {
|
|
|
|
|
earlySecret = hs.suite.extract(nil, nil)
|
|
|
|
|
}
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
handshakeSecret := hs.suite.extract(sharedKey,
|
|
|
|
|
hs.suite.deriveSecret(earlySecret, "derived", nil))
|
|
|
|
|
|
|
|
|
|
clientSecret := hs.suite.deriveSecret(handshakeSecret,
|
|
|
|
|
clientHandshakeTrafficLabel, hs.transcript)
|
2022-10-14 10:48:42 -07:00
|
|
|
c.out.setTrafficSecret(hs.suite, QUICEncryptionLevelHandshake, clientSecret)
|
2018-11-01 01:01:09 -04:00
|
|
|
serverSecret := hs.suite.deriveSecret(handshakeSecret,
|
|
|
|
|
serverHandshakeTrafficLabel, hs.transcript)
|
2022-10-14 10:48:42 -07:00
|
|
|
c.in.setTrafficSecret(hs.suite, QUICEncryptionLevelHandshake, serverSecret)
|
|
|
|
|
|
|
|
|
|
if c.quic != nil {
|
|
|
|
|
if c.hand.Len() != 0 {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
}
|
|
|
|
|
c.quicSetWriteSecret(QUICEncryptionLevelHandshake, hs.suite.id, clientSecret)
|
|
|
|
|
c.quicSetReadSecret(QUICEncryptionLevelHandshake, hs.suite.id, serverSecret)
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
|
crypto/ecdh: new package
We use crypto/internal/edwards25519/field to implement X25519 directly,
so that golang.org/x/crypto/curve25519 can be dropped from the src
module dependencies, and eventually replaced with a crypto/ecdh wrapper,
removing the need to keep golang.org/x/crypto/curve25519/internal/field
in sync with crypto/internal/edwards25519/field.
In crypto/internal/nistec, we add BytesX to serialize only the x
coordinate, which we'll need for the horrible ECDSA x-coord-to-scalar
operation, too.
In crypto/tls, we replace the ECDHE implementation with crypto/ecdh,
dropping the X25519 special cases and related scaffolding.
Finally, FINALLY, we deprecate the ~white whale~ big.Int-based APIs of
the crypto/elliptic package. •_•) ( •_•)>⌐■-■ (⌐■_■)
Fixes #52182
Fixes #34648
Fixes #52221
Change-Id: Iccdda210319cc892e96bb28a0e7b7123551982c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/398914
Reviewed-by: Fernando Lobato Meeser <felobato@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
2022-04-07 15:15:31 -04:00
|
|
|
err = c.config.writeKeyLog(keyLogLabelClientHandshake, hs.hello.random, clientSecret)
|
2018-11-03 18:13:05 -04:00
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
err = c.config.writeKeyLog(keyLogLabelServerHandshake, hs.hello.random, serverSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
hs.masterSecret = hs.suite.extract(nil,
|
|
|
|
|
hs.suite.deriveSecret(handshakeSecret, "derived", nil))
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) readServerParameters() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
msg, err := c.readHandshake(hs.transcript)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
encryptedExtensions, ok := msg.(*encryptedExtensionsMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(encryptedExtensions, msg)
|
|
|
|
|
}
|
|
|
|
|
|
2022-10-14 10:48:42 -07:00
|
|
|
if err := checkALPN(hs.hello.alpnProtocols, encryptedExtensions.alpnProtocol, c.quic != nil); err != nil {
|
|
|
|
|
// RFC 8446 specifies that no_application_protocol is sent by servers, but
|
|
|
|
|
// does not specify how clients handle the selection of an incompatible protocol.
|
|
|
|
|
// RFC 9001 Section 8.1 specifies that QUIC clients send no_application_protocol
|
|
|
|
|
// in this case. Always sending no_application_protocol seems reasonable.
|
|
|
|
|
c.sendAlert(alertNoApplicationProtocol)
|
2021-06-07 08:24:22 -04:00
|
|
|
return err
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
2021-06-07 08:24:22 -04:00
|
|
|
c.clientProtocol = encryptedExtensions.alpnProtocol
|
2018-11-01 01:01:09 -04:00
|
|
|
|
2022-10-14 10:48:42 -07:00
|
|
|
if c.quic != nil {
|
|
|
|
|
if encryptedExtensions.quicTransportParameters == nil {
|
|
|
|
|
// RFC 9001 Section 8.2.
|
|
|
|
|
c.sendAlert(alertMissingExtension)
|
|
|
|
|
return errors.New("tls: server did not send a quic_transport_parameters extension")
|
|
|
|
|
}
|
|
|
|
|
c.quicSetTransportParameters(encryptedExtensions.quicTransportParameters)
|
|
|
|
|
} else {
|
|
|
|
|
if encryptedExtensions.quicTransportParameters != nil {
|
|
|
|
|
c.sendAlert(alertUnsupportedExtension)
|
|
|
|
|
return errors.New("tls: server sent an unexpected quic_transport_parameters extension")
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2023-05-22 19:23:04 +02:00
|
|
|
if !hs.hello.earlyData && encryptedExtensions.earlyData {
|
|
|
|
|
c.sendAlert(alertUnsupportedExtension)
|
|
|
|
|
return errors.New("tls: server sent an unexpected early_data extension")
|
|
|
|
|
}
|
|
|
|
|
if hs.hello.earlyData && !encryptedExtensions.earlyData {
|
|
|
|
|
c.quicRejectedEarlyData()
|
|
|
|
|
}
|
|
|
|
|
if encryptedExtensions.earlyData {
|
|
|
|
|
if hs.session.cipherSuite != c.cipherSuite {
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
return errors.New("tls: server accepted 0-RTT with the wrong cipher suite")
|
|
|
|
|
}
|
|
|
|
|
if hs.session.alpnProtocol != c.clientProtocol {
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
return errors.New("tls: server accepted 0-RTT with the wrong ALPN")
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
func (hs *clientHandshakeStateTLS13) readServerCertificate() error {
|
2018-11-01 01:01:09 -04:00
|
|
|
c := hs.c
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
// Either a PSK or a certificate is always used, but not both.
|
|
|
|
|
// See RFC 8446, Section 4.1.1.
|
|
|
|
|
if hs.usingPSK {
|
2020-04-20 17:55:37 -04:00
|
|
|
// Make sure the connection is still being verified whether or not this
|
|
|
|
|
// is a resumption. Resumptions currently don't reverify certificates so
|
|
|
|
|
// they don't call verifyServerCertificate. See Issue 31641.
|
|
|
|
|
if c.config.VerifyConnection != nil {
|
|
|
|
|
if err := c.config.VerifyConnection(c.connectionStateLocked()); err != nil {
|
|
|
|
|
c.sendAlert(alertBadCertificate)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
2018-11-04 18:41:37 -05:00
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
msg, err := c.readHandshake(hs.transcript)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certReq, ok := msg.(*certificateRequestMsgTLS13)
|
|
|
|
|
if ok {
|
|
|
|
|
hs.certReq = certReq
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
msg, err = c.readHandshake(hs.transcript)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certMsg, ok := msg.(*certificateMsgTLS13)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(certMsg, msg)
|
|
|
|
|
}
|
|
|
|
|
if len(certMsg.certificate.Certificate) == 0 {
|
|
|
|
|
c.sendAlert(alertDecodeError)
|
|
|
|
|
return errors.New("tls: received empty certificates message")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
c.scts = certMsg.certificate.SignedCertificateTimestamps
|
|
|
|
|
c.ocspResponse = certMsg.certificate.OCSPStaple
|
|
|
|
|
|
|
|
|
|
if err := c.verifyServerCertificate(certMsg.certificate.Certificate); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
// certificateVerifyMsg is included in the transcript, but not until
|
|
|
|
|
// after we verify the handshake signature, since the state before
|
|
|
|
|
// this message was sent is used.
|
|
|
|
|
msg, err = c.readHandshake(nil)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certVerify, ok := msg.(*certificateVerifyMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(certVerify, msg)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// See RFC 8446, Section 4.4.3.
|
2019-02-27 15:39:47 -05:00
|
|
|
if !isSupportedSignatureAlgorithm(certVerify.signatureAlgorithm, supportedSignatureAlgorithms()) {
|
2018-11-01 01:01:09 -04:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: certificate used with invalid signature algorithm")
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
2019-10-29 16:46:26 -04:00
|
|
|
sigType, sigHash, err := typeAndHashFromSignatureScheme(certVerify.signatureAlgorithm)
|
|
|
|
|
if err != nil {
|
|
|
|
|
return c.sendAlert(alertInternalError)
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
if sigType == signaturePKCS1v15 || sigHash == crypto.SHA1 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: certificate used with invalid signature algorithm")
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
2019-05-16 19:13:29 -04:00
|
|
|
signed := signedMessage(sigHash, serverSignatureContext, hs.transcript)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err := verifyHandshakeSignature(sigType, c.peerCertificates[0].PublicKey,
|
2019-05-16 19:13:29 -04:00
|
|
|
sigHash, signed, certVerify.signature); err != nil {
|
2018-11-01 01:01:09 -04:00
|
|
|
c.sendAlert(alertDecryptError)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: invalid signature by the server certificate: " + err.Error())
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if err := transcriptMsg(certVerify, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) readServerFinished() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
// finishedMsg is included in the transcript, but not until after we
|
|
|
|
|
// check the client version, since the state before this message was
|
|
|
|
|
// sent is used during verification.
|
|
|
|
|
msg, err := c.readHandshake(nil)
|
2018-11-01 01:01:09 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
finished, ok := msg.(*finishedMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(finished, msg)
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
expectedMAC := hs.suite.finishedHash(c.in.trafficSecret, hs.transcript)
|
|
|
|
|
if !hmac.Equal(expectedMAC, finished.verifyData) {
|
2018-11-01 01:01:09 -04:00
|
|
|
c.sendAlert(alertDecryptError)
|
2018-11-04 18:41:37 -05:00
|
|
|
return errors.New("tls: invalid server finished hash")
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if err := transcriptMsg(finished, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-01 01:01:09 -04:00
|
|
|
|
|
|
|
|
// Derive secrets that take context through the server Finished.
|
|
|
|
|
|
|
|
|
|
hs.trafficSecret = hs.suite.deriveSecret(hs.masterSecret,
|
|
|
|
|
clientApplicationTrafficLabel, hs.transcript)
|
|
|
|
|
serverSecret := hs.suite.deriveSecret(hs.masterSecret,
|
|
|
|
|
serverApplicationTrafficLabel, hs.transcript)
|
2022-10-14 10:48:42 -07:00
|
|
|
c.in.setTrafficSecret(hs.suite, QUICEncryptionLevelApplication, serverSecret)
|
2018-11-01 01:01:09 -04:00
|
|
|
|
2018-11-03 18:13:05 -04:00
|
|
|
err = c.config.writeKeyLog(keyLogLabelClientTraffic, hs.hello.random, hs.trafficSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
err = c.config.writeKeyLog(keyLogLabelServerTraffic, hs.hello.random, serverSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
c.ekm = hs.suite.exportKeyingMaterial(hs.masterSecret, hs.transcript)
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) sendClientCertificate() error {
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
c := hs.c
|
|
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
if hs.certReq == nil {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
cert, err := c.getClientCertificate(&CertificateRequestInfo{
|
|
|
|
|
AcceptableCAs: hs.certReq.certificateAuthorities,
|
|
|
|
|
SignatureSchemes: hs.certReq.supportedSignatureAlgorithms,
|
2019-11-01 19:40:05 -04:00
|
|
|
Version: c.vers,
|
2020-08-01 12:18:31 +01:00
|
|
|
ctx: hs.ctx,
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
})
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certMsg := new(certificateMsgTLS13)
|
|
|
|
|
|
|
|
|
|
certMsg.certificate = *cert
|
|
|
|
|
certMsg.scts = hs.certReq.scts && len(cert.SignedCertificateTimestamps) > 0
|
|
|
|
|
certMsg.ocspStapling = hs.certReq.ocspStapling && len(cert.OCSPStaple) > 0
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if _, err := hs.c.writeHandshakeRecord(certMsg, hs.transcript); err != nil {
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-29 01:38:07 -05:00
|
|
|
// If we sent an empty certificate message, skip the CertificateVerify.
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
if len(cert.Certificate) == 0 {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certVerifyMsg := new(certificateVerifyMsg)
|
|
|
|
|
certVerifyMsg.hasSignatureAlgorithm = true
|
|
|
|
|
|
crypto/tls: refactor certificate and signature algorithm logic
This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.
In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.
Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.
On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.
On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.
There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.
The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.
Updates #32426
Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
2019-11-01 19:00:33 -04:00
|
|
|
certVerifyMsg.signatureAlgorithm, err = selectSignatureScheme(c.vers, cert, hs.certReq.supportedSignatureAlgorithms)
|
|
|
|
|
if err != nil {
|
2018-11-29 01:38:07 -05:00
|
|
|
// getClientCertificate returned a certificate incompatible with the
|
|
|
|
|
// CertificateRequestInfo supported signature algorithms.
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
crypto/tls: refactor certificate and signature algorithm logic
This refactors a lot of the certificate support logic to make it cleaner
and reusable where possible. These changes will make the following CLs
much simpler.
In particular, the heavily overloaded pickSignatureAlgorithm is gone.
That function used to cover both signing and verifying side, would work
both for pre-signature_algorithms TLS 1.0/1.1 and TLS 1.2, and returned
sigalg, type and hash.
Now, TLS 1.0/1.1 and 1.2 are differentiated at the caller, as they have
effectively completely different logic. TLS 1.0/1.1 simply use
legacyTypeAndHashFromPublicKey as they employ a fixed hash function and
signature algorithm for each public key type. TLS 1.2 is instead routed
through selectSignatureScheme (on the signing side) or
isSupportedSignatureAlgorithm (on the verifying side) and
typeAndHashFromSignatureScheme, like TLS 1.3.
On the signing side, signatureSchemesForCertificate was already version
aware (for PKCS#1 v1.5 vs PSS support), so selectSignatureScheme just
had to learn the Section 7.4.1.4.1 defaults for a missing
signature_algorithms to replace pickSignatureAlgorithm.
On the verifying side, pickSignatureAlgorithm was also checking the
public key type, while isSupportedSignatureAlgorithm +
typeAndHashFromSignatureScheme are not, but that check was redundant
with the one in verifyHandshakeSignature.
There should be no major change in behavior so far. A few minor changes
came from the refactor: we now correctly require signature_algorithms in
TLS 1.3 when using a certificate; we won't use Ed25519 in TLS 1.2 if the
client didn't send signature_algorithms; and we don't send
ec_points_format in the ServerHello (a compatibility measure) if we are
not doing ECDHE anyway because there are no mutually supported curves.
The tests also got simpler because they test simpler functions. The
caller logic switching between TLS 1.0/1.1 and 1.2 is tested by the
transcript tests.
Updates #32426
Change-Id: Ice9dcaea78d204718f661f8d60efdb408ba41577
Reviewed-on: https://go-review.googlesource.com/c/go/+/205061
Reviewed-by: Katie Hockman <katie@golang.org>
2019-11-01 19:00:33 -04:00
|
|
|
return err
|
2018-11-29 01:38:07 -05:00
|
|
|
}
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
|
2019-10-29 16:46:26 -04:00
|
|
|
sigType, sigHash, err := typeAndHashFromSignatureScheme(certVerifyMsg.signatureAlgorithm)
|
|
|
|
|
if err != nil {
|
2018-11-29 01:38:07 -05:00
|
|
|
return c.sendAlert(alertInternalError)
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
}
|
|
|
|
|
|
2019-05-16 19:13:29 -04:00
|
|
|
signed := signedMessage(sigHash, clientSignatureContext, hs.transcript)
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
signOpts := crypto.SignerOpts(sigHash)
|
|
|
|
|
if sigType == signatureRSAPSS {
|
|
|
|
|
signOpts = &rsa.PSSOptions{SaltLength: rsa.PSSSaltLengthEqualsHash, Hash: sigHash}
|
|
|
|
|
}
|
2019-05-16 19:13:29 -04:00
|
|
|
sig, err := cert.PrivateKey.(crypto.Signer).Sign(c.config.rand(), signed, signOpts)
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return errors.New("tls: failed to sign handshake: " + err.Error())
|
|
|
|
|
}
|
|
|
|
|
certVerifyMsg.signature = sig
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if _, err := hs.c.writeHandshakeRecord(certVerifyMsg, hs.transcript); err != nil {
|
crypto/tls: implement TLS 1.3 client authentication
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>
2018-11-05 19:23:25 -05:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return nil
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *clientHandshakeStateTLS13) sendClientFinished() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
finished := &finishedMsg{
|
2018-11-04 18:41:37 -05:00
|
|
|
verifyData: hs.suite.finishedHash(c.out.trafficSecret, hs.transcript),
|
2018-11-01 01:01:09 -04:00
|
|
|
}
|
|
|
|
|
|
crypto/tls: replace all usages of BytesOrPanic
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>
2022-12-14 09:43:16 -08:00
|
|
|
if _, err := hs.c.writeHandshakeRecord(finished, hs.transcript); err != nil {
|
2018-11-01 01:01:09 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2022-10-14 10:48:42 -07:00
|
|
|
c.out.setTrafficSecret(hs.suite, QUICEncryptionLevelApplication, hs.trafficSecret)
|
2018-11-01 01:01:09 -04:00
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
if !c.config.SessionTicketsDisabled && c.config.ClientSessionCache != nil {
|
|
|
|
|
c.resumptionSecret = hs.suite.deriveSecret(hs.masterSecret,
|
|
|
|
|
resumptionLabel, hs.transcript)
|
|
|
|
|
}
|
|
|
|
|
|
2022-10-14 10:48:42 -07:00
|
|
|
if c.quic != nil {
|
|
|
|
|
if c.hand.Len() != 0 {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
}
|
|
|
|
|
c.quicSetWriteSecret(QUICEncryptionLevelApplication, hs.suite.id, hs.trafficSecret)
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (c *Conn) handleNewSessionTicket(msg *newSessionTicketMsgTLS13) error {
|
|
|
|
|
if !c.isClient {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return errors.New("tls: received new session ticket from a client")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if c.config.SessionTicketsDisabled || c.config.ClientSessionCache == nil {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// See RFC 8446, Section 4.6.1.
|
|
|
|
|
if msg.lifetime == 0 {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
lifetime := time.Duration(msg.lifetime) * time.Second
|
2018-11-05 15:59:08 -05:00
|
|
|
if lifetime > maxSessionTicketLifetime {
|
2018-11-04 18:41:37 -05:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: received a session ticket with invalid lifetime")
|
|
|
|
|
}
|
|
|
|
|
|
2023-05-22 19:23:04 +02:00
|
|
|
// RFC 9001, Section 4.6.1
|
|
|
|
|
if c.quic != nil && msg.maxEarlyData != 0 && msg.maxEarlyData != 0xffffffff {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: invalid early data for QUIC connection")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-04 18:41:37 -05:00
|
|
|
cipherSuite := cipherSuiteTLS13ByID(c.cipherSuite)
|
|
|
|
|
if cipherSuite == nil || c.resumptionSecret == nil {
|
|
|
|
|
return c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
|
|
|
|
|
2023-05-21 21:17:56 +02:00
|
|
|
psk := cipherSuite.expandLabel(c.resumptionSecret, "resumption",
|
|
|
|
|
msg.nonce, cipherSuite.hash.Size())
|
|
|
|
|
|
|
|
|
|
session, err := c.sessionState()
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
session.secret = psk
|
|
|
|
|
session.useBy = uint64(c.config.time().Add(lifetime).Unix())
|
|
|
|
|
session.ageAdd = msg.ageAdd
|
2023-05-22 19:23:04 +02:00
|
|
|
session.EarlyData = c.quic != nil && msg.maxEarlyData == 0xffffffff // RFC 9001, Section 4.6.1
|
2023-05-21 21:17:56 +02:00
|
|
|
cs := &ClientSessionState{ticket: msg.label, session: session}
|
|
|
|
|
|
|
|
|
|
if cacheKey := c.clientSessionCacheKey(); cacheKey != "" {
|
|
|
|
|
c.config.ClientSessionCache.Put(cacheKey, cs)
|
2022-10-14 10:48:42 -07:00
|
|
|
}
|
2018-11-04 18:41:37 -05:00
|
|
|
|
2018-11-01 01:01:09 -04:00
|
|
|
return nil
|
|
|
|
|
}
|