2018-11-02 00:57:30 -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-02 00:57:30 -04:00
|
|
|
"crypto"
|
|
|
|
|
"crypto/hmac"
|
|
|
|
|
"crypto/rsa"
|
2022-05-12 14:58:29 -04:00
|
|
|
"encoding/binary"
|
2018-11-02 00:57:30 -04:00
|
|
|
"errors"
|
|
|
|
|
"hash"
|
|
|
|
|
"io"
|
2018-11-05 15:59:08 -05:00
|
|
|
"time"
|
2018-11-02 00:57:30 -04:00
|
|
|
)
|
|
|
|
|
|
2018-11-05 15:59:08 -05:00
|
|
|
// maxClientPSKIdentities is the number of client PSK identities the server will
|
|
|
|
|
// attempt to validate. It will ignore the rest not to let cheap ClientHello
|
|
|
|
|
// messages cause too much work in session ticket decryption attempts.
|
|
|
|
|
const maxClientPSKIdentities = 5
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
type serverHandshakeStateTLS13 struct {
|
|
|
|
|
c *Conn
|
2020-08-01 12:18:31 +01:00
|
|
|
ctx context.Context
|
2018-11-02 00:57:30 -04:00
|
|
|
clientHello *clientHelloMsg
|
|
|
|
|
hello *serverHelloMsg
|
2018-11-03 20:04:44 -04:00
|
|
|
sentDummyCCS bool
|
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
|
|
|
usingPSK bool
|
2018-11-02 00:57:30 -04:00
|
|
|
suite *cipherSuiteTLS13
|
|
|
|
|
cert *Certificate
|
|
|
|
|
sigAlg SignatureScheme
|
2018-11-05 15:59:08 -05:00
|
|
|
earlySecret []byte
|
|
|
|
|
sharedKey []byte
|
2018-11-02 00:57:30 -04:00
|
|
|
handshakeSecret []byte
|
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
|
|
|
masterSecret []byte
|
2018-11-02 00:57:30 -04:00
|
|
|
trafficSecret []byte // client_application_traffic_secret_0
|
|
|
|
|
transcript hash.Hash
|
2018-11-05 15:59:08 -05:00
|
|
|
clientFinished []byte
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) handshake() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
2018-11-14 12:34:38 -05:00
|
|
|
if needFIPS() {
|
|
|
|
|
return errors.New("tls: internal error: TLS 1.3 reached in FIPS mode")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
// For an overview of the TLS 1.3 handshake, see RFC 8446, Section 2.
|
|
|
|
|
if err := hs.processClientHello(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
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 := hs.checkForResumption(); err != nil {
|
2018-11-05 15:59:08 -05:00
|
|
|
return err
|
|
|
|
|
}
|
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 := hs.pickCertificate(); err != nil {
|
|
|
|
|
return err
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
2018-11-02 00:57:30 -04:00
|
|
|
c.buffering = true
|
|
|
|
|
if err := hs.sendServerParameters(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
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 := hs.sendServerCertificate(); err != nil {
|
|
|
|
|
return err
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
if err := hs.sendServerFinished(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-05 15:59:08 -05:00
|
|
|
// Note that at this point we could start sending application data without
|
|
|
|
|
// waiting for the client's second flight, but the application might not
|
|
|
|
|
// expect the lack of replay protection of the ClientHello parameters.
|
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 := c.flush(); err != nil {
|
2018-11-05 15:59:08 -05:00
|
|
|
return err
|
|
|
|
|
}
|
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 := hs.readClientCertificate(); err != nil {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if err := hs.readClientFinished(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2022-08-09 09:36:17 -07:00
|
|
|
c.isHandshakeComplete.Store(true)
|
2018-11-02 00:57:30 -04:00
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) processClientHello() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
hs.hello = new(serverHelloMsg)
|
|
|
|
|
|
|
|
|
|
// TLS 1.3 froze the ServerHello.legacy_version field, and uses
|
|
|
|
|
// supported_versions instead. See RFC 8446, sections 4.1.3 and 4.2.1.
|
|
|
|
|
hs.hello.vers = VersionTLS12
|
|
|
|
|
hs.hello.supportedVersion = c.vers
|
|
|
|
|
|
|
|
|
|
if len(hs.clientHello.supportedVersions) == 0 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: client used the legacy version field to negotiate TLS 1.3")
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-05 20:39:45 -05:00
|
|
|
// Abort if the client is doing a fallback and landing lower than what we
|
|
|
|
|
// support. See RFC 7507, which however does not specify the interaction
|
|
|
|
|
// with supported_versions. The only difference is that with
|
|
|
|
|
// supported_versions a client has a chance to attempt a [TLS 1.2, TLS 1.4]
|
|
|
|
|
// handshake in case TLS 1.3 is broken but 1.2 is not. Alas, in that case,
|
|
|
|
|
// it will have to drop the TLS_FALLBACK_SCSV protection if it falls back to
|
|
|
|
|
// TLS 1.2, because a TLS 1.3 server would abort here. The situation before
|
|
|
|
|
// supported_versions was not better because there was just no way to do a
|
|
|
|
|
// TLS 1.4 handshake without risking the server selecting TLS 1.3.
|
|
|
|
|
for _, id := range hs.clientHello.cipherSuites {
|
|
|
|
|
if id == TLS_FALLBACK_SCSV {
|
|
|
|
|
// Use c.vers instead of max(supported_versions) because an attacker
|
|
|
|
|
// could defeat this by adding an arbitrary high version otherwise.
|
2021-10-31 23:13:18 -04:00
|
|
|
if c.vers < c.config.maxSupportedVersion(roleServer) {
|
2018-11-05 20:39:45 -05:00
|
|
|
c.sendAlert(alertInappropriateFallback)
|
|
|
|
|
return errors.New("tls: client using inappropriate protocol fallback")
|
|
|
|
|
}
|
|
|
|
|
break
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
if len(hs.clientHello.compressionMethods) != 1 ||
|
|
|
|
|
hs.clientHello.compressionMethods[0] != compressionNone {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: TLS 1.3 client supports illegal compression methods")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.hello.random = make([]byte, 32)
|
|
|
|
|
if _, err := io.ReadFull(c.config.rand(), hs.hello.random); err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if len(hs.clientHello.secureRenegotiation) != 0 {
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
return errors.New("tls: initial handshake had non-empty renegotiation extension")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.clientHello.earlyData {
|
crypto/tls: enable TLS 1.3 and update tests
To disable TLS 1.3, simply remove VersionTLS13 from supportedVersions,
as tested by TestEscapeRoute, and amend documentation. To make it
opt-in, revert the change to (*Config).supportedVersions from this CL.
I did not have the heart to implement the early data skipping feature
when I realized that it did not offer a choice between two
abstraction-breaking options, but demanded them both (look for handshake
type in case of HelloRetryRequest, trial decryption otherwise). It's a
lot of complexity for an apparently small gain, but if anyone has strong
opinions about it let me know.
Note that in TLS 1.3 alerts are encrypted, so the close_notify peeking
to return (n > 0, io.EOF) from Read doesn't work. If we are lucky, those
servers that unexpectedly close connections after serving a single
request will have stopped (maybe thanks to H/2) before they got updated
to TLS 1.3.
Relatedly, session tickets are now provisioned on the client first Read
instead of at Handshake time, because they are, well, post-handshake
messages. If this proves to be a problem we might try to peek at them.
Doubled the tests that cover logic that's different in TLS 1.3.
The benchmarks for TLS 1.2 compared to be0f3c286b5 (before TLS 1.3 and
its refactors, after CL 142817 changed them to use real connections)
show little movement.
name old time/op new time/op delta
HandshakeServer/RSA-8 795µs ± 1% 798µs ± 1% ~ (p=0.057 n=10+18)
HandshakeServer/ECDHE-P256-RSA-8 903µs ± 0% 909µs ± 1% +0.68% (p=0.000 n=8+17)
HandshakeServer/ECDHE-P256-ECDSA-P256-8 198µs ± 0% 204µs ± 1% +3.24% (p=0.000 n=9+18)
HandshakeServer/ECDHE-X25519-ECDSA-P256-8 202µs ± 3% 208µs ± 1% +2.98% (p=0.000 n=9+20)
HandshakeServer/ECDHE-P521-ECDSA-P521-8 15.5ms ± 1% 15.9ms ± 2% +2.49% (p=0.000 n=10+20)
Throughput/MaxPacket/1MB-8 5.81ms ±23% 6.14ms ±44% ~ (p=0.605 n=8+18)
Throughput/MaxPacket/2MB-8 8.91ms ±22% 8.74ms ±33% ~ (p=0.498 n=9+19)
Throughput/MaxPacket/4MB-8 12.8ms ± 3% 14.0ms ±10% +9.74% (p=0.000 n=10+17)
Throughput/MaxPacket/8MB-8 25.1ms ± 7% 24.6ms ±16% ~ (p=0.129 n=9+19)
Throughput/MaxPacket/16MB-8 46.3ms ± 4% 45.9ms ±12% ~ (p=0.340 n=9+20)
Throughput/MaxPacket/32MB-8 88.5ms ± 4% 86.0ms ± 4% -2.82% (p=0.004 n=10+20)
Throughput/MaxPacket/64MB-8 173ms ± 2% 167ms ± 7% -3.42% (p=0.001 n=10+19)
Throughput/DynamicPacket/1MB-8 5.88ms ± 4% 6.59ms ±64% ~ (p=0.232 n=9+18)
Throughput/DynamicPacket/2MB-8 9.08ms ±12% 8.73ms ±21% ~ (p=0.408 n=10+18)
Throughput/DynamicPacket/4MB-8 14.2ms ± 5% 14.0ms ±11% ~ (p=0.188 n=9+19)
Throughput/DynamicPacket/8MB-8 25.1ms ± 6% 24.0ms ± 7% -4.39% (p=0.000 n=10+18)
Throughput/DynamicPacket/16MB-8 45.6ms ± 3% 43.3ms ± 1% -5.22% (p=0.000 n=10+8)
Throughput/DynamicPacket/32MB-8 88.4ms ± 3% 84.8ms ± 2% -4.06% (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8 175ms ± 3% 167ms ± 2% -4.63% (p=0.000 n=10+10)
Latency/MaxPacket/200kbps-8 694ms ± 0% 694ms ± 0% -0.02% (p=0.000 n=9+9)
Latency/MaxPacket/500kbps-8 279ms ± 0% 279ms ± 0% -0.09% (p=0.000 n=10+10)
Latency/MaxPacket/1000kbps-8 140ms ± 0% 140ms ± 0% -0.15% (p=0.000 n=10+9)
Latency/MaxPacket/2000kbps-8 71.1ms ± 0% 71.0ms ± 0% -0.09% (p=0.001 n=8+9)
Latency/MaxPacket/5000kbps-8 30.5ms ± 6% 30.1ms ± 6% ~ (p=0.905 n=10+9)
Latency/DynamicPacket/200kbps-8 134ms ± 0% 134ms ± 0% ~ (p=0.796 n=9+9)
Latency/DynamicPacket/500kbps-8 54.8ms ± 0% 54.7ms ± 0% -0.18% (p=0.000 n=8+10)
Latency/DynamicPacket/1000kbps-8 28.5ms ± 0% 29.1ms ± 8% ~ (p=0.173 n=8+10)
Latency/DynamicPacket/2000kbps-8 15.3ms ± 6% 15.9ms ±10% ~ (p=0.905 n=9+10)
Latency/DynamicPacket/5000kbps-8 9.14ms ±21% 9.65ms ±82% ~ (p=0.529 n=10+10)
name old speed new speed delta
Throughput/MaxPacket/1MB-8 175MB/s ±13% 167MB/s ±64% ~ (p=0.646 n=7+20)
Throughput/MaxPacket/2MB-8 241MB/s ±25% 241MB/s ±40% ~ (p=0.660 n=9+20)
Throughput/MaxPacket/4MB-8 328MB/s ± 3% 300MB/s ± 9% -8.70% (p=0.000 n=10+17)
Throughput/MaxPacket/8MB-8 335MB/s ± 7% 340MB/s ±17% ~ (p=0.212 n=9+20)
Throughput/MaxPacket/16MB-8 363MB/s ± 4% 367MB/s ±11% ~ (p=0.340 n=9+20)
Throughput/MaxPacket/32MB-8 379MB/s ± 4% 390MB/s ± 4% +2.93% (p=0.004 n=10+20)
Throughput/MaxPacket/64MB-8 388MB/s ± 2% 401MB/s ± 7% +3.25% (p=0.004 n=10+20)
Throughput/DynamicPacket/1MB-8 178MB/s ± 4% 157MB/s ±73% ~ (p=0.127 n=9+20)
Throughput/DynamicPacket/2MB-8 232MB/s ±11% 243MB/s ±18% ~ (p=0.415 n=10+18)
Throughput/DynamicPacket/4MB-8 296MB/s ± 5% 299MB/s ±15% ~ (p=0.295 n=9+20)
Throughput/DynamicPacket/8MB-8 334MB/s ± 6% 350MB/s ± 7% +4.58% (p=0.000 n=10+18)
Throughput/DynamicPacket/16MB-8 368MB/s ± 3% 388MB/s ± 1% +5.48% (p=0.000 n=10+8)
Throughput/DynamicPacket/32MB-8 380MB/s ± 3% 396MB/s ± 2% +4.20% (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8 384MB/s ± 3% 403MB/s ± 2% +4.83% (p=0.000 n=10+10)
Comparing TLS 1.2 and TLS 1.3 at tip shows a slight (~5-10%) slowdown of
handshakes, which might be worth looking at next cycle, but the latency
improvements are expected to overshadow that.
name old time/op new time/op delta
HandshakeServer/ECDHE-P256-RSA-8 909µs ± 1% 963µs ± 0% +5.87% (p=0.000 n=17+18)
HandshakeServer/ECDHE-P256-ECDSA-P256-8 204µs ± 1% 225µs ± 2% +10.20% (p=0.000 n=18+20)
HandshakeServer/ECDHE-X25519-ECDSA-P256-8 208µs ± 1% 230µs ± 2% +10.35% (p=0.000 n=20+18)
HandshakeServer/ECDHE-P521-ECDSA-P521-8 15.9ms ± 2% 15.9ms ± 1% ~ (p=0.444 n=20+19)
Throughput/MaxPacket/1MB-8 6.14ms ±44% 7.07ms ±46% ~ (p=0.057 n=18+19)
Throughput/MaxPacket/2MB-8 8.74ms ±33% 8.61ms ± 9% ~ (p=0.552 n=19+17)
Throughput/MaxPacket/4MB-8 14.0ms ±10% 14.1ms ±12% ~ (p=0.707 n=17+20)
Throughput/MaxPacket/8MB-8 24.6ms ±16% 25.6ms ±14% ~ (p=0.107 n=19+20)
Throughput/MaxPacket/16MB-8 45.9ms ±12% 44.7ms ± 6% ~ (p=0.607 n=20+19)
Throughput/MaxPacket/32MB-8 86.0ms ± 4% 87.9ms ± 8% ~ (p=0.113 n=20+19)
Throughput/MaxPacket/64MB-8 167ms ± 7% 169ms ± 2% +1.26% (p=0.011 n=19+19)
Throughput/DynamicPacket/1MB-8 6.59ms ±64% 6.79ms ±43% ~ (p=0.480 n=18+19)
Throughput/DynamicPacket/2MB-8 8.73ms ±21% 9.58ms ±13% +9.71% (p=0.006 n=18+20)
Throughput/DynamicPacket/4MB-8 14.0ms ±11% 13.9ms ±10% ~ (p=0.687 n=19+20)
Throughput/DynamicPacket/8MB-8 24.0ms ± 7% 24.6ms ± 8% +2.36% (p=0.045 n=18+17)
Throughput/DynamicPacket/16MB-8 43.3ms ± 1% 44.3ms ± 2% +2.48% (p=0.001 n=8+9)
Throughput/DynamicPacket/32MB-8 84.8ms ± 2% 86.7ms ± 2% +2.27% (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8 167ms ± 2% 170ms ± 3% +1.89% (p=0.005 n=10+10)
Latency/MaxPacket/200kbps-8 694ms ± 0% 699ms ± 0% +0.65% (p=0.000 n=9+10)
Latency/MaxPacket/500kbps-8 279ms ± 0% 280ms ± 0% +0.68% (p=0.000 n=10+10)
Latency/MaxPacket/1000kbps-8 140ms ± 0% 141ms ± 0% +0.59% (p=0.000 n=9+9)
Latency/MaxPacket/2000kbps-8 71.0ms ± 0% 71.3ms ± 0% +0.42% (p=0.000 n=9+9)
Latency/MaxPacket/5000kbps-8 30.1ms ± 6% 30.7ms ±10% +1.93% (p=0.019 n=9+9)
Latency/DynamicPacket/200kbps-8 134ms ± 0% 138ms ± 0% +3.22% (p=0.000 n=9+10)
Latency/DynamicPacket/500kbps-8 54.7ms ± 0% 56.3ms ± 0% +3.03% (p=0.000 n=10+8)
Latency/DynamicPacket/1000kbps-8 29.1ms ± 8% 29.1ms ± 0% ~ (p=0.173 n=10+8)
Latency/DynamicPacket/2000kbps-8 15.9ms ±10% 16.4ms ±36% ~ (p=0.633 n=10+8)
Latency/DynamicPacket/5000kbps-8 9.65ms ±82% 8.32ms ± 8% ~ (p=0.573 n=10+8)
name old speed new speed delta
Throughput/MaxPacket/1MB-8 167MB/s ±64% 155MB/s ±55% ~ (p=0.224 n=20+19)
Throughput/MaxPacket/2MB-8 241MB/s ±40% 244MB/s ± 9% ~ (p=0.407 n=20+17)
Throughput/MaxPacket/4MB-8 300MB/s ± 9% 298MB/s ±11% ~ (p=0.707 n=17+20)
Throughput/MaxPacket/8MB-8 340MB/s ±17% 330MB/s ±13% ~ (p=0.201 n=20+20)
Throughput/MaxPacket/16MB-8 367MB/s ±11% 375MB/s ± 5% ~ (p=0.607 n=20+19)
Throughput/MaxPacket/32MB-8 390MB/s ± 4% 382MB/s ± 8% ~ (p=0.113 n=20+19)
Throughput/MaxPacket/64MB-8 401MB/s ± 7% 397MB/s ± 2% -0.96% (p=0.030 n=20+19)
Throughput/DynamicPacket/1MB-8 157MB/s ±73% 156MB/s ±39% ~ (p=0.738 n=20+20)
Throughput/DynamicPacket/2MB-8 243MB/s ±18% 220MB/s ±14% -9.65% (p=0.006 n=18+20)
Throughput/DynamicPacket/4MB-8 299MB/s ±15% 303MB/s ± 9% ~ (p=0.512 n=20+20)
Throughput/DynamicPacket/8MB-8 350MB/s ± 7% 342MB/s ± 8% -2.27% (p=0.045 n=18+17)
Throughput/DynamicPacket/16MB-8 388MB/s ± 1% 378MB/s ± 2% -2.41% (p=0.001 n=8+9)
Throughput/DynamicPacket/32MB-8 396MB/s ± 2% 387MB/s ± 2% -2.21% (p=0.000 n=10+10)
Throughput/DynamicPacket/64MB-8 403MB/s ± 2% 396MB/s ± 3% -1.84% (p=0.005 n=10+10)
Fixes #9671
Change-Id: Ieb57c5140eb2c083b8be0d42b240cd2eeec0dcf6
Reviewed-on: https://go-review.googlesource.com/c/147638
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
2018-11-05 22:52:51 -05:00
|
|
|
// See RFC 8446, Section 4.2.10 for the complicated behavior required
|
|
|
|
|
// here. The scenario is that a different server at our address offered
|
|
|
|
|
// to accept early data in the past, which we can't handle. For now, all
|
|
|
|
|
// 0-RTT enabled session tickets need to expire before a Go server can
|
|
|
|
|
// replace a server or join a pool. That's the same requirement that
|
|
|
|
|
// applies to mixing or replacing with any TLS 1.2 server.
|
|
|
|
|
c.sendAlert(alertUnsupportedExtension)
|
|
|
|
|
return errors.New("tls: client sent unexpected early data")
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.hello.sessionId = hs.clientHello.sessionId
|
|
|
|
|
hs.hello.compressionMethod = compressionNone
|
|
|
|
|
|
crypto/tls: make cipher suite preference ordering automatic
We now have a (well, two, depending on AES hardware support) universal
cipher suite preference order, based on their security and performance.
Peer and application lists are now treated as filters (and AES hardware
support hints) that are applied to this universal order.
This removes a complex and nuanced decision from the application's
responsibilities, one which we are better equipped to make and which
applications usually don't need to have an opinion about. It also lets
us worry less about what suites we support or enable, because we can be
confident that bad ones won't be selected over good ones.
This also moves 3DES suites to InsecureCipherSuites(), even if they are
not disabled by default. Just because we can keep them as a last resort
it doesn't mean they are secure. Thankfully we had not promised that
Insecure means disabled by default.
Notable test changes:
- TestCipherSuiteCertPreferenceECDSA was testing that we'd pick the
right certificate regardless of CipherSuite ordering, which is now
completely ignored, as tested by TestCipherSuitePreference. Removed.
- The openssl command of TestHandshakeServerExportKeyingMaterial was
broken for TLS 1.0 in CL 262857, but its golden file was not
regenerated, so the test kept passing. It now broke because the
selected suite from the ones in the golden file changed.
- In TestAESCipherReordering, "server strongly prefers AES-GCM" is
removed because there is no way for a server to express a strong
preference anymore; "client prefers AES-GCM and AES-CBC over ChaCha"
switched to ChaCha20 when the server lacks AES hardware; and finally
"client supports multiple AES-GCM" changed to always prefer AES-128
per the universal preference list.
* this is going back on an explicit decision from CL 262857, and
while that client order is weird and does suggest a strong dislike
for ChaCha20, we have a strong dislike for software AES, so it
didn't feel worth making the logic more complex
- All Client-* golden files had to be regenerated because the
ClientHello cipher suites have changed.
(Even when Config.CipherSuites was limited to one suite, the TLS 1.3
default order changed.)
Fixes #45430
Fixes #41476 (as 3DES is now always the last resort)
Change-Id: If5f5d356c0f8d1f1c7542fb06644a478d6bad1e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/314609
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
2021-04-28 01:37:09 -04:00
|
|
|
preferenceList := defaultCipherSuitesTLS13
|
|
|
|
|
if !hasAESGCMHardwareSupport || !aesgcmPreferred(hs.clientHello.cipherSuites) {
|
|
|
|
|
preferenceList = defaultCipherSuitesTLS13NoAES
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
for _, suiteID := range preferenceList {
|
crypto/tls: make cipher suite preference ordering automatic
We now have a (well, two, depending on AES hardware support) universal
cipher suite preference order, based on their security and performance.
Peer and application lists are now treated as filters (and AES hardware
support hints) that are applied to this universal order.
This removes a complex and nuanced decision from the application's
responsibilities, one which we are better equipped to make and which
applications usually don't need to have an opinion about. It also lets
us worry less about what suites we support or enable, because we can be
confident that bad ones won't be selected over good ones.
This also moves 3DES suites to InsecureCipherSuites(), even if they are
not disabled by default. Just because we can keep them as a last resort
it doesn't mean they are secure. Thankfully we had not promised that
Insecure means disabled by default.
Notable test changes:
- TestCipherSuiteCertPreferenceECDSA was testing that we'd pick the
right certificate regardless of CipherSuite ordering, which is now
completely ignored, as tested by TestCipherSuitePreference. Removed.
- The openssl command of TestHandshakeServerExportKeyingMaterial was
broken for TLS 1.0 in CL 262857, but its golden file was not
regenerated, so the test kept passing. It now broke because the
selected suite from the ones in the golden file changed.
- In TestAESCipherReordering, "server strongly prefers AES-GCM" is
removed because there is no way for a server to express a strong
preference anymore; "client prefers AES-GCM and AES-CBC over ChaCha"
switched to ChaCha20 when the server lacks AES hardware; and finally
"client supports multiple AES-GCM" changed to always prefer AES-128
per the universal preference list.
* this is going back on an explicit decision from CL 262857, and
while that client order is weird and does suggest a strong dislike
for ChaCha20, we have a strong dislike for software AES, so it
didn't feel worth making the logic more complex
- All Client-* golden files had to be regenerated because the
ClientHello cipher suites have changed.
(Even when Config.CipherSuites was limited to one suite, the TLS 1.3
default order changed.)
Fixes #45430
Fixes #41476 (as 3DES is now always the last resort)
Change-Id: If5f5d356c0f8d1f1c7542fb06644a478d6bad1e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/314609
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Trust: Filippo Valsorda <filippo@golang.org>
2021-04-28 01:37:09 -04:00
|
|
|
hs.suite = mutualCipherSuiteTLS13(hs.clientHello.cipherSuites, suiteID)
|
2018-11-02 00:57:30 -04:00
|
|
|
if hs.suite != nil {
|
|
|
|
|
break
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if hs.suite == nil {
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
return errors.New("tls: no cipher suite supported by both client and server")
|
|
|
|
|
}
|
|
|
|
|
c.cipherSuite = hs.suite.id
|
|
|
|
|
hs.hello.cipherSuite = hs.suite.id
|
|
|
|
|
hs.transcript = hs.suite.hash.New()
|
|
|
|
|
|
|
|
|
|
// Pick the ECDHE group in server preference order, but give priority to
|
|
|
|
|
// groups with a key share, to avoid a HelloRetryRequest round-trip.
|
|
|
|
|
var selectedGroup CurveID
|
|
|
|
|
var clientKeyShare *keyShare
|
|
|
|
|
GroupSelection:
|
|
|
|
|
for _, preferredGroup := range c.config.curvePreferences() {
|
|
|
|
|
for _, ks := range hs.clientHello.keyShares {
|
|
|
|
|
if ks.group == preferredGroup {
|
|
|
|
|
selectedGroup = ks.group
|
|
|
|
|
clientKeyShare = &ks
|
|
|
|
|
break GroupSelection
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if selectedGroup != 0 {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
for _, group := range hs.clientHello.supportedCurves {
|
|
|
|
|
if group == preferredGroup {
|
|
|
|
|
selectedGroup = group
|
|
|
|
|
break
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if selectedGroup == 0 {
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
return errors.New("tls: no ECDHE curve supported by both client and server")
|
|
|
|
|
}
|
|
|
|
|
if clientKeyShare == nil {
|
|
|
|
|
if err := hs.doHelloRetryRequest(selectedGroup); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
clientKeyShare = &hs.clientHello.keyShares[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
|
|
|
if _, ok := curveForCurveID(selectedGroup); !ok {
|
2018-11-02 00:57:30 -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(), selectedGroup)
|
2018-11-02 00:57:30 -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.hello.serverShare = keyShare{group: selectedGroup, data: key.PublicKey().Bytes()}
|
|
|
|
|
peerKey, err := key.Curve().NewPublicKey(clientKeyShare.data)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: invalid client key share")
|
|
|
|
|
}
|
2022-11-14 12:13:46 +01:00
|
|
|
hs.sharedKey, err = key.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-02 00:57:30 -04:00
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: invalid client key share")
|
|
|
|
|
}
|
2018-11-05 15:59:08 -05:00
|
|
|
|
2018-11-09 22:04:58 -05:00
|
|
|
c.serverName = hs.clientHello.serverName
|
2018-11-05 15:59:08 -05:00
|
|
|
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
|
|
|
func (hs *serverHandshakeStateTLS13) checkForResumption() error {
|
2018-11-05 15:59:08 -05:00
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
if c.config.SessionTicketsDisabled {
|
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 nil
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
modeOK := false
|
|
|
|
|
for _, mode := range hs.clientHello.pskModes {
|
|
|
|
|
if mode == pskModeDHE {
|
|
|
|
|
modeOK = true
|
|
|
|
|
break
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
if !modeOK {
|
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 nil
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if len(hs.clientHello.pskIdentities) != len(hs.clientHello.pskBinders) {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
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 errors.New("tls: invalid or missing PSK binders")
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
if len(hs.clientHello.pskIdentities) == 0 {
|
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 nil
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
for i, identity := range hs.clientHello.pskIdentities {
|
|
|
|
|
if i >= maxClientPSKIdentities {
|
|
|
|
|
break
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
plaintext, _ := c.decryptTicket(identity.label)
|
|
|
|
|
if plaintext == nil {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
sessionState := new(sessionStateTLS13)
|
|
|
|
|
if ok := sessionState.unmarshal(plaintext); !ok {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
createdAt := time.Unix(int64(sessionState.createdAt), 0)
|
|
|
|
|
if c.config.time().Sub(createdAt) > maxSessionTicketLifetime {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// We don't check the obfuscated ticket age because it's affected by
|
|
|
|
|
// clock skew and it's only a freshness signal useful for shrinking the
|
|
|
|
|
// window for replay attacks, which don't affect us as we don't do 0-RTT.
|
|
|
|
|
|
|
|
|
|
pskSuite := cipherSuiteTLS13ByID(sessionState.cipherSuite)
|
|
|
|
|
if pskSuite == nil || pskSuite.hash != hs.suite.hash {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// PSK connections don't re-establish client certificates, but carry
|
|
|
|
|
// them over in the session ticket. Ensure the presence of client certs
|
|
|
|
|
// in the ticket is consistent with the configured requirements.
|
|
|
|
|
sessionHasClientCerts := len(sessionState.certificate.Certificate) != 0
|
|
|
|
|
needClientCerts := requiresClientCert(c.config.ClientAuth)
|
|
|
|
|
if needClientCerts && !sessionHasClientCerts {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
if sessionHasClientCerts && c.config.ClientAuth == NoClientCert {
|
|
|
|
|
continue
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
psk := hs.suite.expandLabel(sessionState.resumptionSecret, "resumption",
|
|
|
|
|
nil, hs.suite.hash.Size())
|
|
|
|
|
hs.earlySecret = hs.suite.extract(psk, nil)
|
|
|
|
|
binderKey := hs.suite.deriveSecret(hs.earlySecret, resumptionBinderLabel, nil)
|
|
|
|
|
// Clone the transcript in case a HelloRetryRequest was recorded.
|
|
|
|
|
transcript := cloneHash(hs.transcript, hs.suite.hash)
|
|
|
|
|
if transcript == nil {
|
|
|
|
|
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
|
|
|
return errors.New("tls: internal error: failed to clone hash")
|
2018-11-05 15:59:08 -05: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
|
|
|
clientHelloBytes, err := hs.clientHello.marshalWithoutBinders()
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
transcript.Write(clientHelloBytes)
|
2018-11-05 15:59:08 -05:00
|
|
|
pskBinder := hs.suite.finishedHash(binderKey, transcript)
|
|
|
|
|
if !hmac.Equal(hs.clientHello.pskBinders[i], pskBinder) {
|
|
|
|
|
c.sendAlert(alertDecryptError)
|
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 errors.New("tls: invalid PSK binder")
|
|
|
|
|
}
|
|
|
|
|
|
2020-04-20 17:55:37 -04:00
|
|
|
c.didResume = true
|
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 := c.processCertsFromClient(sessionState.certificate); err != nil {
|
|
|
|
|
return err
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.hello.selectedIdentityPresent = true
|
|
|
|
|
hs.hello.selectedIdentity = uint16(i)
|
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
|
|
|
hs.usingPSK = true
|
|
|
|
|
return nil
|
2018-11-05 15:59:08 -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
|
|
|
return nil
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// cloneHash uses the encoding.BinaryMarshaler and encoding.BinaryUnmarshaler
|
|
|
|
|
// interfaces implemented by standard library hashes to clone the state of in
|
|
|
|
|
// to a new instance of h. It returns nil if the operation fails.
|
|
|
|
|
func cloneHash(in hash.Hash, h crypto.Hash) hash.Hash {
|
|
|
|
|
// Recreate the interface to avoid importing encoding.
|
|
|
|
|
type binaryMarshaler interface {
|
|
|
|
|
MarshalBinary() (data []byte, err error)
|
|
|
|
|
UnmarshalBinary(data []byte) error
|
|
|
|
|
}
|
|
|
|
|
marshaler, ok := in.(binaryMarshaler)
|
|
|
|
|
if !ok {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
state, err := marshaler.MarshalBinary()
|
|
|
|
|
if err != nil {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
out := h.New()
|
|
|
|
|
unmarshaler, ok := out.(binaryMarshaler)
|
|
|
|
|
if !ok {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
if err := unmarshaler.UnmarshalBinary(state); err != nil {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
return out
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) pickCertificate() error {
|
|
|
|
|
c := hs.c
|
2018-11-02 00:57:30 -04: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
|
|
|
// Only one of PSK and certificates are used at a time.
|
|
|
|
|
if hs.usingPSK {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
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
|
|
|
// signature_algorithms is required in TLS 1.3. See RFC 8446, Section 4.2.3.
|
|
|
|
|
if len(hs.clientHello.supportedSignatureAlgorithms) == 0 {
|
|
|
|
|
return c.sendAlert(alertMissingExtension)
|
|
|
|
|
}
|
|
|
|
|
|
2020-08-01 12:18:31 +01:00
|
|
|
certificate, err := c.config.getCertificate(clientHelloInfo(hs.ctx, c, hs.clientHello))
|
2018-11-02 00:57:30 -04:00
|
|
|
if err != nil {
|
crypto/tls: select only compatible chains from Certificates
Now that we have a full implementation of the logic to check certificate
compatibility, we can let applications just list multiple chains in
Certificates (for example, an RSA and an ECDSA one) and choose the most
appropriate automatically.
NameToCertificate only maps each name to one chain, so simply deprecate
it, and while at it simplify its implementation by not stripping
trailing dots from the SNI (which is specified not to have any, see RFC
6066, Section 3) and by not supporting multi-level wildcards, which are
not a thing in the WebPKI (and in crypto/x509).
The performance of SupportsCertificate without Leaf is poor, but doesn't
affect current users. For now document that, and address it properly in
the next cycle. See #35504.
While cleaning up the Certificates/GetCertificate/GetConfigForClient
behavior, also support leaving Certificates/GetCertificate nil if
GetConfigForClient is set, and send unrecognized_name when there are no
available certificates.
Fixes #29139
Fixes #18377
Change-Id: I26604db48806fe4d608388e55da52f34b7ca4566
Reviewed-on: https://go-review.googlesource.com/c/go/+/205059
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
2019-11-02 14:43:34 -04:00
|
|
|
if err == errNoCertificates {
|
|
|
|
|
c.sendAlert(alertUnrecognizedName)
|
|
|
|
|
} else {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
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
|
|
|
hs.sigAlg, err = selectSignatureScheme(c.vers, certificate, hs.clientHello.supportedSignatureAlgorithms)
|
|
|
|
|
if err != nil {
|
|
|
|
|
// getCertificate returned a certificate that is unsupported or
|
|
|
|
|
// incompatible with the client's signature algorithms.
|
2018-11-02 00:57:30 -04:00
|
|
|
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-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
hs.cert = certificate
|
|
|
|
|
|
|
|
|
|
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 *serverHandshakeStateTLS13) sendDummyChangeCipherSpec() error {
|
|
|
|
|
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-02 00:57:30 -04:00
|
|
|
func (hs *serverHandshakeStateTLS13) doHelloRetryRequest(selectedGroup CurveID) error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
// The first ClientHello gets double-hashed into the transcript upon a
|
|
|
|
|
// HelloRetryRequest. See RFC 8446, Section 4.4.1.
|
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.clientHello, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-02 00:57:30 -04:00
|
|
|
chHash := hs.transcript.Sum(nil)
|
|
|
|
|
hs.transcript.Reset()
|
|
|
|
|
hs.transcript.Write([]byte{typeMessageHash, 0, 0, uint8(len(chHash))})
|
|
|
|
|
hs.transcript.Write(chHash)
|
|
|
|
|
|
|
|
|
|
helloRetryRequest := &serverHelloMsg{
|
|
|
|
|
vers: hs.hello.vers,
|
|
|
|
|
random: helloRetryRequestRandom,
|
|
|
|
|
sessionId: hs.hello.sessionId,
|
|
|
|
|
cipherSuite: hs.hello.cipherSuite,
|
|
|
|
|
compressionMethod: hs.hello.compressionMethod,
|
|
|
|
|
supportedVersion: hs.hello.supportedVersion,
|
|
|
|
|
selectedGroup: selectedGroup,
|
|
|
|
|
}
|
|
|
|
|
|
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(helloRetryRequest, hs.transcript); err != nil {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-03 20:04:44 -04:00
|
|
|
if err := hs.sendDummyChangeCipherSpec(); 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
|
|
|
// clientHelloMsg is not included in the transcript.
|
|
|
|
|
msg, err := c.readHandshake(nil)
|
2018-11-02 00:57:30 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
clientHello, ok := msg.(*clientHelloMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(clientHello, msg)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if len(clientHello.keyShares) != 1 || clientHello.keyShares[0].group != selectedGroup {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: client sent invalid key share in second ClientHello")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if clientHello.earlyData {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: client indicated early data in second ClientHello")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if illegalClientHelloChange(clientHello, hs.clientHello) {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
|
|
|
|
return errors.New("tls: client illegally modified second ClientHello")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
hs.clientHello = clientHello
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-22 11:46:44 +01:00
|
|
|
// illegalClientHelloChange reports whether the two ClientHello messages are
|
2018-11-02 00:57:30 -04:00
|
|
|
// different, with the exception of the changes allowed before and after a
|
|
|
|
|
// HelloRetryRequest. See RFC 8446, Section 4.1.2.
|
|
|
|
|
func illegalClientHelloChange(ch, ch1 *clientHelloMsg) bool {
|
|
|
|
|
if len(ch.supportedVersions) != len(ch1.supportedVersions) ||
|
|
|
|
|
len(ch.cipherSuites) != len(ch1.cipherSuites) ||
|
|
|
|
|
len(ch.supportedCurves) != len(ch1.supportedCurves) ||
|
|
|
|
|
len(ch.supportedSignatureAlgorithms) != len(ch1.supportedSignatureAlgorithms) ||
|
|
|
|
|
len(ch.supportedSignatureAlgorithmsCert) != len(ch1.supportedSignatureAlgorithmsCert) ||
|
|
|
|
|
len(ch.alpnProtocols) != len(ch1.alpnProtocols) {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.supportedVersions {
|
|
|
|
|
if ch.supportedVersions[i] != ch1.supportedVersions[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.cipherSuites {
|
|
|
|
|
if ch.cipherSuites[i] != ch1.cipherSuites[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.supportedCurves {
|
|
|
|
|
if ch.supportedCurves[i] != ch1.supportedCurves[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.supportedSignatureAlgorithms {
|
|
|
|
|
if ch.supportedSignatureAlgorithms[i] != ch1.supportedSignatureAlgorithms[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.supportedSignatureAlgorithmsCert {
|
|
|
|
|
if ch.supportedSignatureAlgorithmsCert[i] != ch1.supportedSignatureAlgorithmsCert[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
for i := range ch.alpnProtocols {
|
|
|
|
|
if ch.alpnProtocols[i] != ch1.alpnProtocols[i] {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
return ch.vers != ch1.vers ||
|
|
|
|
|
!bytes.Equal(ch.random, ch1.random) ||
|
|
|
|
|
!bytes.Equal(ch.sessionId, ch1.sessionId) ||
|
|
|
|
|
!bytes.Equal(ch.compressionMethods, ch1.compressionMethods) ||
|
|
|
|
|
ch.serverName != ch1.serverName ||
|
|
|
|
|
ch.ocspStapling != ch1.ocspStapling ||
|
|
|
|
|
!bytes.Equal(ch.supportedPoints, ch1.supportedPoints) ||
|
|
|
|
|
ch.ticketSupported != ch1.ticketSupported ||
|
|
|
|
|
!bytes.Equal(ch.sessionTicket, ch1.sessionTicket) ||
|
|
|
|
|
ch.secureRenegotiationSupported != ch1.secureRenegotiationSupported ||
|
|
|
|
|
!bytes.Equal(ch.secureRenegotiation, ch1.secureRenegotiation) ||
|
|
|
|
|
ch.scts != ch1.scts ||
|
|
|
|
|
!bytes.Equal(ch.cookie, ch1.cookie) ||
|
|
|
|
|
!bytes.Equal(ch.pskModes, ch1.pskModes)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) sendServerParameters() 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
|
|
|
if err := transcriptMsg(hs.clientHello, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
if _, err := hs.c.writeHandshakeRecord(hs.hello, hs.transcript); err != nil {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-03 20:04:44 -04:00
|
|
|
if err := hs.sendDummyChangeCipherSpec(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-05 15:59:08 -05:00
|
|
|
earlySecret := hs.earlySecret
|
|
|
|
|
if earlySecret == nil {
|
|
|
|
|
earlySecret = hs.suite.extract(nil, nil)
|
|
|
|
|
}
|
|
|
|
|
hs.handshakeSecret = hs.suite.extract(hs.sharedKey,
|
|
|
|
|
hs.suite.deriveSecret(earlySecret, "derived", nil))
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
clientSecret := hs.suite.deriveSecret(hs.handshakeSecret,
|
|
|
|
|
clientHandshakeTrafficLabel, hs.transcript)
|
|
|
|
|
c.in.setTrafficSecret(hs.suite, clientSecret)
|
|
|
|
|
serverSecret := hs.suite.deriveSecret(hs.handshakeSecret,
|
|
|
|
|
serverHandshakeTrafficLabel, hs.transcript)
|
|
|
|
|
c.out.setTrafficSecret(hs.suite, serverSecret)
|
|
|
|
|
|
2018-11-03 18:13:05 -04:00
|
|
|
err := c.config.writeKeyLog(keyLogLabelClientHandshake, hs.clientHello.random, clientSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
err = c.config.writeKeyLog(keyLogLabelServerHandshake, hs.clientHello.random, serverSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
encryptedExtensions := new(encryptedExtensionsMsg)
|
|
|
|
|
|
2021-06-07 08:24:22 -04:00
|
|
|
selectedProto, err := negotiateALPN(c.config.NextProtos, hs.clientHello.alpnProtocols)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertNoApplicationProtocol)
|
|
|
|
|
return err
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
2021-06-07 08:24:22 -04:00
|
|
|
encryptedExtensions.alpnProtocol = selectedProto
|
|
|
|
|
c.clientProtocol = selectedProto
|
2018-11-02 00:57:30 -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(encryptedExtensions, hs.transcript); err != nil {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
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
|
|
|
func (hs *serverHandshakeStateTLS13) requestClientCert() bool {
|
|
|
|
|
return hs.c.config.ClientAuth >= RequestClientCert && !hs.usingPSK
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
func (hs *serverHandshakeStateTLS13) sendServerCertificate() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
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
|
|
|
// Only one of PSK and certificates are used at a time.
|
|
|
|
|
if hs.usingPSK {
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if hs.requestClientCert() {
|
|
|
|
|
// Request a client certificate
|
|
|
|
|
certReq := new(certificateRequestMsgTLS13)
|
|
|
|
|
certReq.ocspStapling = true
|
|
|
|
|
certReq.scts = true
|
2019-02-27 15:39:47 -05:00
|
|
|
certReq.supportedSignatureAlgorithms = supportedSignatureAlgorithms()
|
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 c.config.ClientCAs != nil {
|
|
|
|
|
certReq.certificateAuthorities = c.config.ClientCAs.Subjects()
|
|
|
|
|
}
|
|
|
|
|
|
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(certReq, 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-02 00:57:30 -04:00
|
|
|
certMsg := new(certificateMsgTLS13)
|
|
|
|
|
|
|
|
|
|
certMsg.certificate = *hs.cert
|
|
|
|
|
certMsg.scts = hs.clientHello.scts && len(hs.cert.SignedCertificateTimestamps) > 0
|
|
|
|
|
certMsg.ocspStapling = hs.clientHello.ocspStapling && len(hs.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 {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
certVerifyMsg := new(certificateVerifyMsg)
|
|
|
|
|
certVerifyMsg.hasSignatureAlgorithm = true
|
|
|
|
|
certVerifyMsg.signatureAlgorithm = hs.sigAlg
|
|
|
|
|
|
2019-10-29 16:46:26 -04:00
|
|
|
sigType, sigHash, err := typeAndHashFromSignatureScheme(hs.sigAlg)
|
|
|
|
|
if err != nil {
|
2018-11-29 01:38:07 -05:00
|
|
|
return c.sendAlert(alertInternalError)
|
2018-11-02 00:57:30 -04:00
|
|
|
}
|
|
|
|
|
|
2019-05-16 19:13:29 -04:00
|
|
|
signed := signedMessage(sigHash, serverSignatureContext, hs.transcript)
|
2018-11-02 00:57:30 -04: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 := hs.cert.PrivateKey.(crypto.Signer).Sign(c.config.rand(), signed, signOpts)
|
2018-11-02 00:57:30 -04:00
|
|
|
if err != nil {
|
2019-01-18 17:33:49 -05:00
|
|
|
public := hs.cert.PrivateKey.(crypto.Signer).Public()
|
|
|
|
|
if rsaKey, ok := public.(*rsa.PublicKey); ok && sigType == signatureRSAPSS &&
|
|
|
|
|
rsaKey.N.BitLen()/8 < sigHash.Size()*2+2 { // key too small for RSA-PSS
|
|
|
|
|
c.sendAlert(alertHandshakeFailure)
|
|
|
|
|
} else {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
}
|
2018-11-02 00:57:30 -04:00
|
|
|
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 {
|
2018-11-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) sendServerFinished() 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-02 00:57:30 -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-02 00:57:30 -04:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// Derive secrets that take context through the server Finished.
|
|
|
|
|
|
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
|
|
|
hs.masterSecret = hs.suite.extract(nil,
|
2018-11-02 00:57:30 -04:00
|
|
|
hs.suite.deriveSecret(hs.handshakeSecret, "derived", 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
|
|
|
hs.trafficSecret = hs.suite.deriveSecret(hs.masterSecret,
|
2018-11-02 00:57:30 -04:00
|
|
|
clientApplicationTrafficLabel, 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
|
|
|
serverSecret := hs.suite.deriveSecret(hs.masterSecret,
|
2018-11-02 00:57:30 -04:00
|
|
|
serverApplicationTrafficLabel, hs.transcript)
|
|
|
|
|
c.out.setTrafficSecret(hs.suite, serverSecret)
|
|
|
|
|
|
2018-11-03 18:13:05 -04:00
|
|
|
err := c.config.writeKeyLog(keyLogLabelClientTraffic, hs.clientHello.random, hs.trafficSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
err = c.config.writeKeyLog(keyLogLabelServerTraffic, hs.clientHello.random, serverSecret)
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
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.ekm = hs.suite.exportKeyingMaterial(hs.masterSecret, hs.transcript)
|
2018-11-02 00:57:30 -04: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
|
|
|
// If we did not request client certificates, at this point we can
|
|
|
|
|
// precompute the client finished and roll the transcript forward to send
|
|
|
|
|
// session tickets in our first flight.
|
|
|
|
|
if !hs.requestClientCert() {
|
|
|
|
|
if err := hs.sendSessionTickets(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
2018-11-05 15:59:08 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) shouldSendSessionTickets() bool {
|
|
|
|
|
if hs.c.config.SessionTicketsDisabled {
|
|
|
|
|
return false
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// Don't send tickets the client wouldn't use. See RFC 8446, Section 4.2.9.
|
|
|
|
|
for _, pskMode := range hs.clientHello.pskModes {
|
|
|
|
|
if pskMode == pskModeDHE {
|
|
|
|
|
return true
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
return false
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
func (hs *serverHandshakeStateTLS13) sendSessionTickets() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
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
|
|
|
hs.clientFinished = hs.suite.finishedHash(c.in.trafficSecret, hs.transcript)
|
|
|
|
|
finishedMsg := &finishedMsg{
|
|
|
|
|
verifyData: hs.clientFinished,
|
|
|
|
|
}
|
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(finishedMsg, hs.transcript); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
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
|
|
|
|
2018-11-05 15:59:08 -05:00
|
|
|
if !hs.shouldSendSessionTickets() {
|
|
|
|
|
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
|
|
|
resumptionSecret := hs.suite.deriveSecret(hs.masterSecret,
|
|
|
|
|
resumptionLabel, hs.transcript)
|
|
|
|
|
|
2018-11-05 15:59:08 -05:00
|
|
|
m := new(newSessionTicketMsgTLS13)
|
|
|
|
|
|
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
|
|
|
var certsFromClient [][]byte
|
|
|
|
|
for _, cert := range c.peerCertificates {
|
|
|
|
|
certsFromClient = append(certsFromClient, cert.Raw)
|
|
|
|
|
}
|
2018-11-05 15:59:08 -05:00
|
|
|
state := sessionStateTLS13{
|
|
|
|
|
cipherSuite: hs.suite.id,
|
|
|
|
|
createdAt: uint64(c.config.time().Unix()),
|
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
|
|
|
resumptionSecret: resumptionSecret,
|
|
|
|
|
certificate: Certificate{
|
|
|
|
|
Certificate: certsFromClient,
|
|
|
|
|
OCSPStaple: c.ocspResponse,
|
|
|
|
|
SignedCertificateTimestamps: c.scts,
|
|
|
|
|
},
|
2018-11-05 15:59:08 -05: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
|
|
|
stateBytes, err := state.marshal()
|
|
|
|
|
if err != nil {
|
|
|
|
|
c.sendAlert(alertInternalError)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
m.label, err = c.encryptTicket(stateBytes)
|
2018-11-05 15:59:08 -05:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
m.lifetime = uint32(maxSessionTicketLifetime / time.Second)
|
|
|
|
|
|
2022-05-12 14:58:29 -04:00
|
|
|
// ticket_age_add is a random 32-bit value. See RFC 8446, section 4.6.1
|
|
|
|
|
// The value is not stored anywhere; we never need to check the ticket age
|
|
|
|
|
// because 0-RTT is not supported.
|
|
|
|
|
ageAdd := make([]byte, 4)
|
|
|
|
|
_, err = hs.c.config.rand().Read(ageAdd)
|
|
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
m.ageAdd = binary.LittleEndian.Uint32(ageAdd)
|
|
|
|
|
|
|
|
|
|
// ticket_nonce, which must be unique per connection, is always left at
|
|
|
|
|
// zero because we only ever send one ticket per connection.
|
|
|
|
|
|
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 := c.writeHandshakeRecord(m, nil); err != nil {
|
2018-11-05 15:59:08 -05:00
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
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
|
|
|
func (hs *serverHandshakeStateTLS13) readClientCertificate() error {
|
|
|
|
|
c := hs.c
|
|
|
|
|
|
|
|
|
|
if !hs.requestClientCert() {
|
2020-04-20 17:55:37 -04:00
|
|
|
// Make sure the connection is still being verified whether or not
|
|
|
|
|
// the server requested a client certificate.
|
|
|
|
|
if c.config.VerifyConnection != nil {
|
|
|
|
|
if err := c.config.VerifyConnection(c.connectionStateLocked()); err != nil {
|
|
|
|
|
c.sendAlert(alertBadCertificate)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
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 nil
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
// If we requested a client certificate, then the client must send a
|
|
|
|
|
// certificate message. If it's empty, no CertificateVerify is sent.
|
|
|
|
|
|
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)
|
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, ok := msg.(*certificateMsgTLS13)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(certMsg, msg)
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if err := c.processCertsFromClient(certMsg.certificate); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
2020-05-13 17:44:20 -04:00
|
|
|
if c.config.VerifyConnection != nil {
|
|
|
|
|
if err := c.config.VerifyConnection(c.connectionStateLocked()); err != nil {
|
|
|
|
|
c.sendAlert(alertBadCertificate)
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
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(certMsg.certificate.Certificate) != 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
|
|
|
// 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)
|
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
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
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()) {
|
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.sendAlert(alertIllegalParameter)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: client certificate used with invalid signature algorithm")
|
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(certVerify.signatureAlgorithm)
|
|
|
|
|
if err != nil {
|
|
|
|
|
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
|
|
|
}
|
|
|
|
|
if sigType == signaturePKCS1v15 || sigHash == crypto.SHA1 {
|
|
|
|
|
c.sendAlert(alertIllegalParameter)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: client certificate used with invalid signature algorithm")
|
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
|
|
|
if err := verifyHandshakeSignature(sigType, c.peerCertificates[0].PublicKey,
|
2019-05-16 19:13:29 -04:00
|
|
|
sigHash, signed, certVerify.signature); 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
|
|
|
c.sendAlert(alertDecryptError)
|
2019-10-29 16:46:26 -04:00
|
|
|
return errors.New("tls: invalid signature by the client certificate: " + err.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
|
|
|
}
|
|
|
|
|
|
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
|
|
|
|
|
}
|
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 we waited until the client certificates to send session tickets, we
|
|
|
|
|
// are ready to do it now.
|
|
|
|
|
if err := hs.sendSessionTickets(); err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-02 00:57:30 -04:00
|
|
|
func (hs *serverHandshakeStateTLS13) readClientFinished() 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 not included in the transcript.
|
|
|
|
|
msg, err := c.readHandshake(nil)
|
2018-11-02 00:57:30 -04:00
|
|
|
if err != nil {
|
|
|
|
|
return err
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
finished, ok := msg.(*finishedMsg)
|
|
|
|
|
if !ok {
|
|
|
|
|
c.sendAlert(alertUnexpectedMessage)
|
|
|
|
|
return unexpectedMessageError(finished, msg)
|
|
|
|
|
}
|
|
|
|
|
|
2018-11-05 15:59:08 -05:00
|
|
|
if !hmac.Equal(hs.clientFinished, finished.verifyData) {
|
2018-11-02 00:57:30 -04:00
|
|
|
c.sendAlert(alertDecryptError)
|
|
|
|
|
return errors.New("tls: invalid client finished hash")
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
c.in.setTrafficSecret(hs.suite, hs.trafficSecret)
|
|
|
|
|
|
|
|
|
|
return nil
|
|
|
|
|
}
|