crypto/tls: implement TLS 1.3 middlebox compatibility mode

Looks like the introduction of CCS records in the client second flight
gave time to s_server to send NewSessionTicket messages in between the
client application data and close_notify. There seems to be no way of
turning NewSessionTicket messages off, neither by not sending a
psk_key_exchange_modes extension, nor by command line flag.

Interleaving the client write like that tickled an issue akin to #18701:
on Windows, the client reaches Close() before the last record is drained
from the send buffer, the kernel notices and resets the connection,
cutting short the last flow. There is no good way of synchronizing this,
so we sleep for a RTT before calling close, like in CL 75210. Sigh.

Updates #9671

Change-Id: I44dc1cca17b373695b5a18c2741f218af2990bd1
Reviewed-on: https://go-review.googlesource.com/c/147419
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>

src/crypto/tls/conn.go

@@ -946,7 +946,7 @@ func (c *Conn) writeRecordLocked(typ recordType, data []byte) (int, error) {
 		data = data[m:]
 	}
 
-	if typ == recordTypeChangeCipherSpec {
+	if typ == recordTypeChangeCipherSpec && c.vers != VersionTLS13 {
 		if err := c.out.changeCipherSpec(); err != nil {
 			return n, c.sendAlertLocked(err.(alert))
 		}

src/crypto/tls/handshake_client_test.go

@@ -337,9 +337,14 @@ func (test *clientTest) run(t *testing.T, write bool) {
 
 	doneChan := make(chan bool)
 	go func() {
-		defer func() { doneChan <- true }()
-		defer clientConn.Close()
-		defer client.Close()
+		defer func() {
+			// Give time to the send buffer to drain, to avoid the kernel
+			// sending a RST and cutting off the flow. See Issue 18701.
+			time.Sleep(10 * time.Millisecond)
+			client.Close()
+			clientConn.Close()
+			doneChan <- true
+		}()
 
 		if _, err := client.Write([]byte("hello\n")); err != nil {
 			t.Errorf("Client.Write failed: %s", err)
@@ -482,6 +487,9 @@ func (test *clientTest) run(t *testing.T, write bool) {
 				t.Fatalf("%s #%d: mismatch on read: got:%x want:%x", test.name, i, bb, b)
 			}
 		}
+		// Give time to the send buffer to drain, to avoid the kernel
+		// sending a RST and cutting off the flow. See Issue 18701.
+		time.Sleep(10 * time.Millisecond)
 		serverConn.Close()
 	}
 

src/crypto/tls/handshake_client_tls13.go

@@ -18,6 +18,7 @@ type clientHandshakeStateTLS13 struct {
 	serverHello   *serverHelloMsg
 	hello         *clientHelloMsg
 	certReq       *certificateRequestMsgTLS13
+	sentDummyCCS  bool
 	ecdheParams   ecdheParameters
 	suite         *cipherSuiteTLS13
 	transcript    hash.Hash
@@ -57,6 +58,9 @@ func (hs *clientHandshakeStateTLS13) handshake() error {
 		hs.transcript.Write(chHash)
 		hs.transcript.Write(hs.serverHello.marshal())
 
+		if err := hs.sendDummyChangeCipherSpec(); err != nil {
+			return err
+		}
 		if err := hs.processHelloRetryRequest(); err != nil {
 			return err
 		}
@@ -66,9 +70,13 @@ func (hs *clientHandshakeStateTLS13) handshake() error {
 
 	hs.transcript.Write(hs.serverHello.marshal())
 
+	c.buffering = true
 	if err := hs.processServerHello(); err != nil {
 		return err
 	}
+	if err := hs.sendDummyChangeCipherSpec(); err != nil {
+		return err
+	}
 	if err := hs.establishHandshakeKeys(); err != nil {
 		return err
 	}
@@ -81,8 +89,6 @@ func (hs *clientHandshakeStateTLS13) handshake() error {
 	if err := hs.readServerFinished(); err != nil {
 		return err
 	}
-
-	c.buffering = true
 	if err := hs.sendClientCertificate(); err != nil {
 		return err
 	}
@@ -155,6 +161,18 @@ func (hs *clientHandshakeStateTLS13) checkServerHelloOrHRR() error {
 	return nil
 }
 
+// sendDummyChangeCipherSpec sends a ChangeCipherSpec record for compatibility
+// with middleboxes that didn't implement TLS correctly. See RFC 8446, Appendix D.4.
+func (hs *clientHandshakeStateTLS13) sendDummyChangeCipherSpec() error {
+	if hs.sentDummyCCS {
+		return nil
+	}
+	hs.sentDummyCCS = true
+
+	_, err := hs.c.writeRecord(recordTypeChangeCipherSpec, []byte{1})
+	return err
+}
+
 // processHelloRetryRequest handles the HRR in hs.serverHello, modifies and
 // resends hs.hello, and reads the new ServerHello into hs.serverHello.
 func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {

src/crypto/tls/handshake_server_tls13.go

@@ -20,6 +20,7 @@ type serverHandshakeStateTLS13 struct {
 	c               *Conn
 	clientHello     *clientHelloMsg
 	hello           *serverHelloMsg
+	sentDummyCCS    bool
 	suite           *cipherSuiteTLS13
 	cert            *Certificate
 	sigAlg          SignatureScheme
@@ -205,6 +206,18 @@ GroupSelection:
 	return nil
 }
 
+// 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
+
+	_, err := hs.c.writeRecord(recordTypeChangeCipherSpec, []byte{1})
+	return err
+}
+
 func (hs *serverHandshakeStateTLS13) doHelloRetryRequest(selectedGroup CurveID) error {
 	c := hs.c
 
@@ -231,6 +244,10 @@ func (hs *serverHandshakeStateTLS13) doHelloRetryRequest(selectedGroup CurveID)
 		return err
 	}
 
+	if err := hs.sendDummyChangeCipherSpec(); err != nil {
+		return err
+	}
+
 	msg, err := c.readHandshake()
 	if err != nil {
 		return err
@@ -329,6 +346,10 @@ func (hs *serverHandshakeStateTLS13) sendServerParameters() error {
 		return err
 	}
 
+	if err := hs.sendDummyChangeCipherSpec(); err != nil {
+		return err
+	}
+
 	clientSecret := hs.suite.deriveSecret(hs.handshakeSecret,
 		clientHandshakeTrafficLabel, hs.transcript)
 	c.in.setTrafficSecret(hs.suite, clientSecret)