mirrors/utls
1
0
Fork 0
mirror of https://github.com/refraction-networking/utls.git synced 2025-04-03 20:17:36 +03:00
Code Issues Projects Releases Packages Wiki Activity

crypto/tls: replace all usages of BytesOrPanic

Browse source 
Message marshalling makes use of BytesOrPanic a lot, under the
assumption that it will never panic. This assumption was incorrect, and
specifically crafted handshakes could trigger panics. Rather than just
surgically replacing the usages of BytesOrPanic in paths that could
panic, replace all usages of it with proper error returns in case there
are other ways of triggering panics which we didn't find.

In one specific case, the tree routed by expandLabel, we replace the
usage of BytesOrPanic, but retain a panic. This function already
explicitly panicked elsewhere, and returning an error from it becomes
rather painful because it requires changing a large number of APIs.
The marshalling is unlikely to ever panic, as the inputs are all either
fixed length, or already limited to the sizes required. If it were to
panic, it'd likely only be during development. A close inspection shows
no paths for a user to cause a panic currently.

This patches ends up being rather large, since it requires routing
errors back through functions which previously had no error returns.
Where possible I've tried to use helpers that reduce the verbosity
of frequently repeated stanzas, and to make the diffs as minimal as
possible.

Thanks to Marten Seemann for reporting this issue.

Fixes #58001
Fixes CVE-2022-41724

Change-Id: Ieb55867ef0a3e1e867b33f09421932510cb58851
Reviewed-on: 1679436
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468125
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
This commit is contained in:
Roland Shoemaker 2022-12-14 09:43:16 -08:00 • committed by Gopher Robot
parent 58e7190673
commit ba1a41d66f
13 changed files with 657 additions and 503 deletions
Download patch file Download diff file Expand all files Collapse all files

95
handshake_client.go
View file

@ -162,7 +162,10 @@ func (c *Conn) clientHandshake(ctx context.Context) (err error) {
}
c.serverName = hello.serverName
cacheKey, session, earlySecret, binderKey := c.loadSession(hello)
cacheKey, session, earlySecret, binderKey, err := c.loadSession(hello)
if err != nil {
return err
}
if cacheKey != "" && session != nil {
defer func() {
// If we got a handshake failure when resuming a session, throw away
@ -177,11 +180,12 @@ func (c *Conn) clientHandshake(ctx context.Context) (err error) {
}()
}
if _, err := c.writeRecord(recordTypeHandshake, hello.marshal()); err != nil {
if _, err := c.writeHandshakeRecord(hello, nil); err != nil {
return err
}
msg, err := c.readHandshake()
// serverHelloMsg is not included in the transcript
msg, err := c.readHandshake(nil)
if err != nil {
return err
}
@ -246,9 +250,9 @@ func (c *Conn) clientHandshake(ctx context.Context) (err error) {
}
func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
session *ClientSessionState, earlySecret, binderKey []byte) {
session *ClientSessionState, earlySecret, binderKey []byte, err error) {
if c.config.SessionTicketsDisabled || c.config.ClientSessionCache == nil {
return "", nil, nil, nil
return "", nil, nil, nil, nil
}
hello.ticketSupported = true
@ -263,14 +267,14 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
// renegotiation is primarily used to allow a client to send a client
// certificate, which would be skipped if session resumption occurred.
if c.handshakes != 0 {
return "", nil, nil, nil
return "", nil, nil, nil, nil
}
// Try to resume a previously negotiated TLS session, if available.
cacheKey = clientSessionCacheKey(c.conn.RemoteAddr(), c.config)
session, ok := c.config.ClientSessionCache.Get(cacheKey)
if !ok || session == nil {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
// Check that version used for the previous session is still valid.
@ -282,7 +286,7 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
}
}
if !versOk {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
// Check that the cached server certificate is not expired, and that it's
@ -291,16 +295,16 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
if !c.config.InsecureSkipVerify {
if len(session.verifiedChains) == 0 {
// The original connection had InsecureSkipVerify, while this doesn't.
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
serverCert := session.serverCertificates[0]
if c.config.time().After(serverCert.NotAfter) {
// Expired certificate, delete the entry.
c.config.ClientSessionCache.Put(cacheKey, nil)
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
if err := serverCert.VerifyHostname(c.config.ServerName); err != nil {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
}
@ -308,7 +312,7 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
// In TLS 1.2 the cipher suite must match the resumed session. Ensure we
// are still offering it.
if mutualCipherSuite(hello.cipherSuites, session.cipherSuite) == nil {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
hello.sessionTicket = session.sessionTicket
@ -318,14 +322,14 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
// Check that the session ticket is not expired.
if c.config.time().After(session.useBy) {
c.config.ClientSessionCache.Put(cacheKey, nil)
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
// In TLS 1.3 the KDF hash must match the resumed session. Ensure we
// offer at least one cipher suite with that hash.
cipherSuite := cipherSuiteTLS13ByID(session.cipherSuite)
if cipherSuite == nil {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
cipherSuiteOk := false
for _, offeredID := range hello.cipherSuites {
@ -336,7 +340,7 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
}
}
if !cipherSuiteOk {
return cacheKey, nil, nil, nil
return cacheKey, nil, nil, nil, nil
}
// Set the pre_shared_key extension. See RFC 8446, Section 4.2.11.1.
@ -354,9 +358,15 @@ func (c *Conn) loadSession(hello *clientHelloMsg) (cacheKey string,
earlySecret = cipherSuite.extract(psk, nil)
binderKey = cipherSuite.deriveSecret(earlySecret, resumptionBinderLabel, nil)
transcript := cipherSuite.hash.New()
transcript.Write(hello.marshalWithoutBinders())
helloBytes, err := hello.marshalWithoutBinders()
if err != nil {
return "", nil, nil, nil, err
}
transcript.Write(helloBytes)
pskBinders := [][]byte{cipherSuite.finishedHash(binderKey, transcript)}
hello.updateBinders(pskBinders)
if err := hello.updateBinders(pskBinders); err != nil {
return "", nil, nil, nil, err
}
return
}
@ -401,8 +411,12 @@ func (hs *clientHandshakeState) handshake() error {
hs.finishedHash.discardHandshakeBuffer()
}
hs.finishedHash.Write(hs.hello.marshal())
hs.finishedHash.Write(hs.serverHello.marshal())
if err := transcriptMsg(hs.hello, &hs.finishedHash); err != nil {
return err
}
if err := transcriptMsg(hs.serverHello, &hs.finishedHash); err != nil {
return err
}
c.buffering = true
c.didResume = isResume
@ -473,7 +487,7 @@ func (hs *clientHandshakeState) pickCipherSuite() error {
func (hs *clientHandshakeState) doFullHandshake() error {
c := hs.c
msg, err := c.readHandshake()
msg, err := c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -482,9 +496,8 @@ func (hs *clientHandshakeState) doFullHandshake() error {
c.sendAlert(alertUnexpectedMessage)
return unexpectedMessageError(certMsg, msg)
}
hs.finishedHash.Write(certMsg.marshal())
msg, err = c.readHandshake()
msg, err = c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -502,11 +515,10 @@ func (hs *clientHandshakeState) doFullHandshake() error {
c.sendAlert(alertUnexpectedMessage)
return errors.New("tls: received unexpected CertificateStatus message")
}
hs.finishedHash.Write(cs.marshal())
c.ocspResponse = cs.response
msg, err = c.readHandshake()
msg, err = c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -535,14 +547,13 @@ func (hs *clientHandshakeState) doFullHandshake() error {
skx, ok := msg.(*serverKeyExchangeMsg)
if ok {
hs.finishedHash.Write(skx.marshal())
err = keyAgreement.processServerKeyExchange(c.config, hs.hello, hs.serverHello, c.peerCertificates[0], skx)
if err != nil {
c.sendAlert(alertUnexpectedMessage)
return err
}
msg, err = c.readHandshake()
msg, err = c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -553,7 +564,6 @@ func (hs *clientHandshakeState) doFullHandshake() error {
certReq, ok := msg.(*certificateRequestMsg)
if ok {
certRequested = true
hs.finishedHash.Write(certReq.marshal())
cri := certificateRequestInfoFromMsg(hs.ctx, c.vers, certReq)
if chainToSend, err = c.getClientCertificate(cri); err != nil {
@ -561,7 +571,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
return err
}
msg, err = c.readHandshake()
msg, err = c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -572,7 +582,6 @@ func (hs *clientHandshakeState) doFullHandshake() error {
c.sendAlert(alertUnexpectedMessage)
return unexpectedMessageError(shd, msg)
}
hs.finishedHash.Write(shd.marshal())
// If the server requested a certificate then we have to send a
// Certificate message, even if it's empty because we don't have a
@ -580,8 +589,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
if certRequested {
certMsg = new(certificateMsg)
certMsg.certificates = chainToSend.Certificate
hs.finishedHash.Write(certMsg.marshal())
if _, err := c.writeRecord(recordTypeHandshake, certMsg.marshal()); err != nil {
if _, err := hs.c.writeHandshakeRecord(certMsg, &hs.finishedHash); err != nil {
return err
}
}
@ -592,8 +600,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
return err
}
if ckx != nil {
hs.finishedHash.Write(ckx.marshal())
if _, err := c.writeRecord(recordTypeHandshake, ckx.marshal()); err != nil {
if _, err := hs.c.writeHandshakeRecord(ckx, &hs.finishedHash); err != nil {
return err
}
}
@ -640,8 +647,7 @@ func (hs *clientHandshakeState) doFullHandshake() error {
return err
}
hs.finishedHash.Write(certVerify.marshal())
if _, err := c.writeRecord(recordTypeHandshake, certVerify.marshal()); err != nil {
if _, err := hs.c.writeHandshakeRecord(certVerify, &hs.finishedHash); err != nil {
return err
}
}
@ -776,7 +782,10 @@ func (hs *clientHandshakeState) readFinished(out []byte) error {
return err
}
msg, err := c.readHandshake()
// finishedMsg is included in the transcript, but not until after we
// check the client version, since the state before this message was
// sent is used during verification.
msg, err := c.readHandshake(nil)
if err != nil {
return err
}
@ -792,7 +801,11 @@ func (hs *clientHandshakeState) readFinished(out []byte) error {
c.sendAlert(alertHandshakeFailure)
return errors.New("tls: server's Finished message was incorrect")
}
hs.finishedHash.Write(serverFinished.marshal())
if err := transcriptMsg(serverFinished, &hs.finishedHash); err != nil {
return err
}
copy(out, verify)
return nil
}
@ -803,7 +816,7 @@ func (hs *clientHandshakeState) readSessionTicket() error {
}
c := hs.c
msg, err := c.readHandshake()
msg, err := c.readHandshake(&hs.finishedHash)
if err != nil {
return err
}
@ -812,7 +825,6 @@ func (hs *clientHandshakeState) readSessionTicket() error {
c.sendAlert(alertUnexpectedMessage)
return unexpectedMessageError(sessionTicketMsg, msg)
}
hs.finishedHash.Write(sessionTicketMsg.marshal())
hs.session = &ClientSessionState{
sessionTicket: sessionTicketMsg.ticket,
@ -832,14 +844,13 @@ func (hs *clientHandshakeState) readSessionTicket() error {
func (hs *clientHandshakeState) sendFinished(out []byte) error {
c := hs.c
if _, err := c.writeRecord(recordTypeChangeCipherSpec, []byte{1}); err != nil {
if err := c.writeChangeCipherRecord(); err != nil {
return err
}
finished := new(finishedMsg)
finished.verifyData = hs.finishedHash.clientSum(hs.masterSecret)
hs.finishedHash.Write(finished.marshal())
if _, err := c.writeRecord(recordTypeHandshake, finished.marshal()); err != nil {
if _, err := hs.c.writeHandshakeRecord(finished, &hs.finishedHash); err != nil {
return err
}
copy(out, finished.verifyData)