only allow a single retry

This commit is contained in:
Marten Seemann
2018-10-24 23:07:03 +07:00
parent cfc8a904d5
commit a7f94d89b6
6 changed files with 33 additions and 37 deletions

View File

@@ -28,7 +28,6 @@ type client struct {
packetHandlers packetHandlerManager packetHandlers packetHandlerManager
token []byte token []byte
numRetries int
versionNegotiated bool // has the server accepted our version versionNegotiated bool // has the server accepted our version
receivedVersionNegotiationPacket bool receivedVersionNegotiationPacket bool
@@ -478,19 +477,18 @@ func (c *client) handleVersionNegotiationPacket(hdr *wire.Header) error {
func (c *client) handleRetryPacket(hdr *wire.Header) { func (c *client) handleRetryPacket(hdr *wire.Header) {
c.logger.Debugf("<- Received Retry") c.logger.Debugf("<- Received Retry")
hdr.Log(c.logger) hdr.Log(c.logger)
// A server that performs multiple retries must use a source connection ID of at least 8 bytes.
// Only a server that won't send additional Retries can use shorter connection IDs.
if hdr.OrigDestConnectionID.Len() < protocol.MinConnectionIDLenInitial {
c.logger.Debugf("Received a Retry with a too short Original Destination Connection ID: %d bytes, must have at least %d bytes.", hdr.OrigDestConnectionID.Len(), protocol.MinConnectionIDLenInitial)
return
}
if !hdr.OrigDestConnectionID.Equal(c.destConnID) { if !hdr.OrigDestConnectionID.Equal(c.destConnID) {
c.logger.Debugf("Received spoofed Retry. Original Destination Connection ID: %s, expected: %s", hdr.OrigDestConnectionID, c.destConnID) c.logger.Debugf("Ignoring spoofed Retry. Original Destination Connection ID: %s, expected: %s", hdr.OrigDestConnectionID, c.destConnID)
return return
} }
c.numRetries++ if hdr.SrcConnectionID.Equal(c.destConnID) {
if c.numRetries > protocol.MaxRetries { c.logger.Debugf("Ignoring Retry, since the server didn't change the Source Connection ID.")
c.session.destroy(qerr.CryptoTooManyRejects) return
}
// If a token is already set, this means that we already received a Retry from the server.
// Ignore this Retry packet.
if len(c.token) > 0 {
c.logger.Debugf("Ignoring Retry, since a Retry was already received.")
return return
} }
c.destConnID = hdr.SrcConnectionID c.destConnID = hdr.SrcConnectionID

View File

@@ -598,7 +598,7 @@ var _ = Describe("Client", func() {
Expect(sessions).To(BeEmpty()) Expect(sessions).To(BeEmpty())
}) })
It("only accepts 3 retries", func() { It("only accepts a single retry", func() {
manager := NewMockPacketHandlerManager(mockCtrl) manager := NewMockPacketHandlerManager(mockCtrl)
manager.EXPECT().Add(gomock.Any(), gomock.Any()).Do(func(id protocol.ConnectionID, handler packetHandler) { manager.EXPECT().Add(gomock.Any(), gomock.Any()).Do(func(id protocol.ConnectionID, handler packetHandler) {
go handler.handlePacket(&receivedPacket{ go handler.handlePacket(&receivedPacket{
@@ -606,7 +606,7 @@ var _ = Describe("Client", func() {
IsLongHeader: true, IsLongHeader: true,
Type: protocol.PacketTypeRetry, Type: protocol.PacketTypeRetry,
Token: []byte("foobar"), Token: []byte("foobar"),
SrcConnectionID: connID, SrcConnectionID: protocol.ConnectionID{1, 2, 3, 4},
DestConnectionID: id, DestConnectionID: id,
OrigDestConnectionID: connID, OrigDestConnectionID: connID,
Version: protocol.VersionTLS, Version: protocol.VersionTLS,
@@ -619,18 +619,23 @@ var _ = Describe("Client", func() {
config := &Config{Versions: []protocol.VersionNumber{protocol.VersionTLS}} config := &Config{Versions: []protocol.VersionNumber{protocol.VersionTLS}}
cl.config = config cl.config = config
sessions := make(chan quicSession, protocol.MaxRetries+1) sessions := make(chan quicSession, 2)
for i := 0; i < protocol.MaxRetries+1; i++ {
run := make(chan error) run := make(chan error)
sess := NewMockQuicSession(mockCtrl) sess := NewMockQuicSession(mockCtrl)
sess.EXPECT().run().DoAndReturn(func() error { sess.EXPECT().run().DoAndReturn(func() error {
return <-run defer GinkgoRecover()
var err error
Eventually(run).Should(Receive(&err))
return err
}) })
sess.EXPECT().destroy(gomock.Any()).Do(func(e error) { sess.EXPECT().destroy(gomock.Any()).Do(func(e error) {
run <- e run <- e
}) })
sessions <- sess sessions <- sess
} doneErr := errors.New("nothing to do")
sess = NewMockQuicSession(mockCtrl)
sess.EXPECT().run().Return(doneErr)
sessions <- sess
newTLSClientSession = func( newTLSClientSession = func(
_ connection, _ connection,
@@ -648,8 +653,7 @@ var _ = Describe("Client", func() {
return <-sessions, nil return <-sessions, nil
} }
_, err := Dial(packetConn, addr, "quic.clemente.io:1337", nil, config) _, err := Dial(packetConn, addr, "quic.clemente.io:1337", nil, config)
Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(doneErr))
Expect(err.(qerr.ErrorCode)).To(Equal(qerr.CryptoTooManyRejects))
Expect(sessions).To(BeEmpty()) Expect(sessions).To(BeEmpty())
}) })
}) })

View File

@@ -200,6 +200,7 @@ var _ = Describe("Handshake RTT tests", func() {
serverConfig.AcceptCookie = func(_ net.Addr, _ *quic.Cookie) bool { serverConfig.AcceptCookie = func(_ net.Addr, _ *quic.Cookie) bool {
return false return false
} }
clientConfig.HandshakeTimeout = 500 * time.Millisecond
runServerAndProxy() runServerAndProxy()
_, err := quic.DialAddr( _, err := quic.DialAddr(
proxy.LocalAddr().String(), proxy.LocalAddr().String(),
@@ -207,7 +208,7 @@ var _ = Describe("Handshake RTT tests", func() {
clientConfig, clientConfig,
) )
Expect(err).To(HaveOccurred()) Expect(err).To(HaveOccurred())
Expect(err.(qerr.ErrorCode)).To(Equal(qerr.CryptoTooManyRejects)) Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.HandshakeTimeout))
}) })
}) })
}) })

View File

@@ -149,6 +149,3 @@ const MinPacingDelay time.Duration = 100 * time.Microsecond
// DefaultConnectionIDLength is the connection ID length that is used for multiplexed connections // DefaultConnectionIDLength is the connection ID length that is used for multiplexed connections
// if no other value is configured. // if no other value is configured.
const DefaultConnectionIDLength = 4 const DefaultConnectionIDLength = 4
// MaxRetries is the maximum number of Retries a client will do before failing the connection.
const MaxRetries = 3

View File

@@ -118,9 +118,6 @@ func (s *serverTLS) handleInitialImpl(p *receivedPacket) (quicSession, protocol.
mconf := s.mintConf.Clone() mconf := s.mintConf.Clone()
mconf.ExtensionHandler = extHandler mconf.ExtensionHandler = extHandler
// A server is allowed to perform multiple Retries.
// It doesn't make much sense, but it's something that our API allows.
// In that case it must use a source connection ID of at least 8 bytes.
connID, err := protocol.GenerateConnectionID(s.config.ConnectionIDLength) connID, err := protocol.GenerateConnectionID(s.config.ConnectionIDLength)
if err != nil { if err != nil {
return nil, nil, err return nil, nil, err
@@ -152,7 +149,7 @@ func (s *serverTLS) sendRetry(remoteAddr net.Addr, hdr *wire.Header) error {
if err != nil { if err != nil {
return err return err
} }
connID, err := protocol.GenerateConnectionIDForInitial() connID, err := protocol.GenerateConnectionID(s.config.ConnectionIDLength)
if err != nil { if err != nil {
return err return err
} }

View File

@@ -79,7 +79,6 @@ var _ = Describe("Stateless TLS handling", func() {
replyHdr := parseHeader(conn.dataWritten.Bytes()) replyHdr := parseHeader(conn.dataWritten.Bytes())
Expect(replyHdr.Type).To(Equal(protocol.PacketTypeRetry)) Expect(replyHdr.Type).To(Equal(protocol.PacketTypeRetry))
Expect(replyHdr.SrcConnectionID).ToNot(Equal(hdr.DestConnectionID)) Expect(replyHdr.SrcConnectionID).ToNot(Equal(hdr.DestConnectionID))
Expect(replyHdr.SrcConnectionID.Len()).To(BeNumerically(">=", protocol.MinConnectionIDLenInitial))
Expect(replyHdr.DestConnectionID).To(Equal(hdr.SrcConnectionID)) Expect(replyHdr.DestConnectionID).To(Equal(hdr.SrcConnectionID))
Expect(replyHdr.OrigDestConnectionID).To(Equal(hdr.DestConnectionID)) Expect(replyHdr.OrigDestConnectionID).To(Equal(hdr.DestConnectionID))
Expect(replyHdr.Token).ToNot(BeEmpty()) Expect(replyHdr.Token).ToNot(BeEmpty())