From 6d5c9776e90f377de7d809ddf462c97196c0ff25 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 20 Mar 2017 12:35:34 +0700 Subject: [PATCH] send a Public Reset when receiving a CHLO with the FHL2 tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #411. Chrome sends the FHL2 when it wants to perform a head-of-line blocking experiment, introduced in QUIC version 36 (see https://codereview.chromium.org/2115033002). We don’t support this experiment. By sending a Public Reset when receiving this tag we force Chrome to use the TCP fallback. --- handshake/crypto_setup_server.go | 9 +++++++++ handshake/crypto_setup_server_test.go | 8 ++++++++ handshake/tags.go | 5 +++++ session.go | 2 +- session_test.go | 10 ++++++++++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/handshake/crypto_setup_server.go b/handshake/crypto_setup_server.go index 4d3bf8e05..2a1813d2a 100644 --- a/handshake/crypto_setup_server.go +++ b/handshake/crypto_setup_server.go @@ -47,6 +47,11 @@ type cryptoSetupServer struct { var _ CryptoSetup = &cryptoSetupServer{} +// ErrHOLExperiment is returned when the client sends the FHL2 tag in the CHLO +// this is an expiremnt implemented by Chrome in QUIC 36, which we don't support +// TODO: remove this when dropping support for QUIC 36 +var ErrHOLExperiment = qerr.Error(qerr.InvalidCryptoMessageParameter, "HOL experiment. Unsupported") + // NewCryptoSetup creates a new CryptoSetup instance for a server func NewCryptoSetup( connID protocol.ConnectionID, @@ -95,6 +100,10 @@ func (h *cryptoSetupServer) HandleCryptoStream() error { } func (h *cryptoSetupServer) handleMessage(chloData []byte, cryptoData map[Tag][]byte) (bool, error) { + if _, isHOLExperiment := cryptoData[TagFHL2]; isHOLExperiment { + return false, ErrHOLExperiment + } + sniSlice, ok := cryptoData[TagSNI] if !ok { return false, qerr.Error(qerr.CryptoMessageParameterNotFound, "SNI required") diff --git a/handshake/crypto_setup_server_test.go b/handshake/crypto_setup_server_test.go index a81e29f8d..ba250e1dc 100644 --- a/handshake/crypto_setup_server_test.go +++ b/handshake/crypto_setup_server_test.go @@ -222,6 +222,14 @@ var _ = Describe("Crypto setup", func() { binary.LittleEndian.PutUint64(xlct, crypto.HashCert(cert)) }) + It("doesn't support Chrome's head-of-line blocking experiment", func() { + WriteHandshakeMessage(&stream.dataToRead, TagCHLO, map[Tag][]byte{ + TagFHL2: []byte("foobar"), + }) + err := cs.HandleCryptoStream() + Expect(err).To(MatchError(ErrHOLExperiment)) + }) + It("generates REJ messages", func() { response, err := cs.handleInchoateCHLO("", bytes.Repeat([]byte{'a'}, protocol.ClientHelloMinimumSize), nil) Expect(err).ToNot(HaveOccurred()) diff --git a/handshake/tags.go b/handshake/tags.go index 56a07f6c7..2b3783f64 100644 --- a/handshake/tags.go +++ b/handshake/tags.go @@ -50,6 +50,11 @@ const ( // TagSFCW is the initial stream flow control receive window. TagSFCW Tag = 'S' + 'F'<<8 + 'C'<<16 + 'W'<<24 + // TagFHL2 forces head of line blocking. + // Chrome experiment (see https://codereview.chromium.org/2115033002) + // unsupported by quic-go + TagFHL2 Tag = 'F' + 'H'<<8 + 'L'<<16 + '2'<<24 + // TagSTK is the source-address token TagSTK Tag = 'S' + 'T'<<8 + 'K'<<16 // TagSNO is the server nonce diff --git a/session.go b/session.go index 0e69155c0..de01586e0 100644 --- a/session.go +++ b/session.go @@ -542,7 +542,7 @@ func (s *session) closeImpl(e error, remoteClose bool) error { return nil } - if quicErr.ErrorCode == qerr.DecryptionFailure { + if quicErr.ErrorCode == qerr.DecryptionFailure || quicErr == handshake.ErrHOLExperiment { // If we send a public reset, don't send a CONNECTION_CLOSE s.closeChan <- nil return s.sendPublicReset(s.lastRcvdPacketNumber) diff --git a/session_test.go b/session_test.go index 93d6a7b74..147ab6c30 100644 --- a/session_test.go +++ b/session_test.go @@ -3,6 +3,7 @@ package quic import ( "bytes" "errors" + "fmt" "io" "net" "reflect" @@ -671,6 +672,15 @@ var _ = Describe("Session", func() { Expect(atomic.LoadUint32(&sess.closed) != 0).To(BeTrue()) Expect(mconn.written).To(BeEmpty()) // no CONNECTION_CLOSE or PUBLIC_RESET sent }) + + It("sends a Public Reset if the client is initiating the head-of-line blocking experiment", func() { + sess.Close(handshake.ErrHOLExperiment) + Expect(closeCallbackCalled).To(BeTrue()) + Expect(mconn.written).To(HaveLen(1)) + fmt.Println(string(mconn.written[0])) + Expect(mconn.written[0][0] & 0x02).ToNot(BeZero()) // Public Reset + Expect(sess.runClosed).ToNot(Receive()) // channel should be drained by Close() + }) }) Context("receiving packets", func() {