tris: prevent sending 0.5-RTT data

Disable 0.5-RTT as it has weaker security properties than 1-RTT. The
same security considerations from TLS False Start (RFC 7918) apply.

Currently the server Handshake method returns as soon as it has sent its
parameters, but it does not wait for the client to authenticate the
handshake via a Finished message. This broke a test that assumed that
the Handshake message performs a full handshake and also
(unintentionally?) enabled the server to send application data before
the handshake is complete ("0.5-RTT data").

Fix this by moving the implicit Finished message check in the handshake
message reader to the server handshake itself (previously readRecord
would process the Finished message as a side-effect of requesting
recordTypeApplicationData). And in the special case where 0-RTT data is
actually desired, process the Finished message in the ConfirmHandshake
and Read functions.

NOTE: 0.5-RTT is not disabled when the server enables 0-RTT. It is the
server responsibility to use ConfirmHandshake before writing anything.

Explicitly panic when ConfirmHandshake is used for client connections,
this is not the intended use of that API.

13.go

@@ -244,12 +244,33 @@ CurvePreferenceLoop:
 	return nil
 }
 
-// readClientFinished13 is called when, on the second flight of the client,
-// a handshake message is received. This might be immediately or after the
-// early data. Once done it sends the session tickets. Under c.in lock.
-func (hs *serverHandshakeState) readClientFinished13() error {
+// readClientFinished13 is called during the server handshake (when no early
+// data it available) or after reading all early data. It discards early data if
+// the server did not accept it and then verifies the Finished message. Once
+// done it sends the session tickets. Under c.in lock.
+func (hs *serverHandshakeState) readClientFinished13(hasConfirmLock bool) error {
 	c := hs.c
 
+	// If the client advertised and sends early data while the server does
+	// not accept it, it must be fully skipped until the Finished message.
+	for c.phase == discardingEarlyData {
+		if err := c.readRecord(recordTypeApplicationData); err != nil {
+			return err
+		}
+		// Assume receipt of Finished message (will be checked below).
+		if c.hand.Len() > 0 {
+			c.phase = waitingClientFinished
+			break
+		}
+	}
+
+	// If the client sends early data followed by a Finished message (but
+	// no end_of_early_data), the server MUST terminate the connection.
+	if c.phase != waitingClientFinished {
+		c.sendAlert(alertUnexpectedMessage)
+		return errors.New("tls: did not expect Client Finished yet")
+	}
+
 	c.phase = readingClientFinished
 	msg, err := c.readHandshake()
 	if err != nil {
@@ -283,7 +304,13 @@ func (hs *serverHandshakeState) readClientFinished13() error {
 	// Any read operation after handshakeRunning and before handshakeConfirmed
 	// will be holding this lock, which we release as soon as the confirmation
 	// happens, even if the Read call might do more work.
-	c.confirmMutex.Unlock()
+	// If a Handshake is pending, c.confirmMutex will never be locked as
+	// ConfirmHandshake will wait for the handshake to complete. If a
+	// handshake was complete, and this was a confirmation, unlock
+	// c.confirmMutex now to allow readers to proceed.
+	if hasConfirmLock {
+		c.confirmMutex.Unlock()
+	}
 
 	return hs.sendSessionTicket13() // TODO: do in a goroutine
 }

conn.go

@@ -844,12 +844,6 @@ Again:
 			return c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage))
 		}
 		c.hand.Write(data)
-		if typ != want && c.phase == waitingClientFinished {
-			if err := c.hs.readClientFinished13(); err != nil {
-				c.in.setErrorLocked(err)
-				break
-			}
-		}
 	}
 
 	if b != nil {
@@ -1315,6 +1309,10 @@ func (c *Conn) handleRenegotiation(*helloRequestMsg) error {
 // client sent valid 0-RTT data. In any other case it's equivalent to
 // calling Handshake.
 func (c *Conn) ConfirmHandshake() error {
+	if c.isClient {
+		panic("ConfirmHandshake should only be called for servers")
+	}
+
 	if err := c.Handshake(); err != nil {
 		return err
 	}
@@ -1341,6 +1339,9 @@ func (c *Conn) ConfirmHandshake() error {
 	defer c.in.Unlock()
 
 	var input *block
+	// Try to read all data (if phase==readingEarlyData) or extract the
+	// remaining data from the previous read that could not fit in the read
+	// buffer (if c.input != nil).
 	if c.phase == readingEarlyData || c.input != nil {
 		buf := &bytes.Buffer{}
 		if _, err := buf.ReadFrom(earlyDataReader{c}); err != nil {
@@ -1350,8 +1351,12 @@ func (c *Conn) ConfirmHandshake() error {
 		input = &block{data: buf.Bytes()}
 	}
 
+	// At this point, earlyDataReader has read all early data and received
+	// the end_of_early_data signal. Expect a Finished message.
+	// Locks held so far: c.confirmMutex, c.in
+	// not confirmed implies c.phase == discardingEarlyData || c.phase == waitingClientFinished
 	for c.phase != handshakeConfirmed {
-		if err := c.readRecord(recordTypeApplicationData); err != nil {
+		if err := c.hs.readClientFinished13(true); err != nil {
 			c.in.setErrorLocked(err)
 			return err
 		}
@@ -1402,6 +1407,7 @@ func (r earlyDataReader) Read(b []byte) (n int, err error) {
 		}
 	}
 
+	// Following early application data, an end_of_early_data is expected.
 	if err == nil && c.phase != readingEarlyData && c.input == nil {
 		err = io.EOF
 	}
@@ -1446,6 +1452,15 @@ func (c *Conn) Read(b []byte) (n int, err error) {
 				return 0, err
 			}
 			if c.hand.Len() > 0 {
+				if c.phase == waitingClientFinished {
+					// Server has received all early data, confirm
+					// by reading the Client Finished message.
+					if err := c.hs.readClientFinished13(true); err != nil {
+						c.in.setErrorLocked(err)
+						return 0, err
+					}
+					continue
+				}
 				if err := c.handlePostHandshake(); err != nil {
 					return 0, err
 				}

handshake_server.go

@@ -77,6 +77,16 @@ func (c *Conn) serverHandshake() error {
 			return err
 		}
 		c.hs = &hs
+		// If the client is sending early data while the server expects
+		// it, delay the Finished check until HandshakeConfirmed() is
+		// called or until all early data is Read(). Otherwise, complete
+		// authenticating the client now (there is no support for
+		// sending 0.5-RTT data to a potential unauthenticated client).
+		if c.phase != readingEarlyData {
+			if err := hs.readClientFinished13(false); err != nil {
+				return err
+			}
+		}
 		c.handshakeComplete = true
 		return nil
 	} else if isResume {