diff --git a/13.go b/13.go index 55f7972..fb920fa 100644 --- a/13.go +++ b/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 } diff --git a/conn.go b/conn.go index 665cbda..d7ae0ab 100644 --- a/conn.go +++ b/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 } diff --git a/handshake_server.go b/handshake_server.go index aa4cc4e..f294a80 100644 --- a/handshake_server.go +++ b/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 {