From acb45c0ef1c76a75cc36fd95691d9f236b332cb6 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 15 Aug 2018 15:23:28 +0700 Subject: [PATCH] always allow sending of IETF QUIC Version Negotiation Packets When receiving a packet with an IETF QUIC Header using an unsupported version, we should send a IETF QUIC Version Negotiation Packet, even if none of the supported versions is IETF QUIC. --- server.go | 42 +++++++++++++++++++++++++++--------------- server_test.go | 2 +- server_tls.go | 40 ++++++++++++++-------------------------- server_tls_test.go | 16 ---------------- 4 files changed, 42 insertions(+), 58 deletions(-) diff --git a/server.go b/server.go index 88c15cfb..5084d81f 100644 --- a/server.go +++ b/server.go @@ -325,13 +325,19 @@ func (s *server) handlePacket(p *receivedPacket) { func (s *server) handlePacketImpl(p *receivedPacket) error { hdr := p.header - version := hdr.Version + if hdr.VersionFlag || hdr.IsLongHeader { + // send a Version Negotiation Packet if the client is speaking a different protocol version + if !protocol.IsSupportedVersion(s.config.Versions, hdr.Version) { + return s.sendVersionNegotiationPacket(p) + } + } if hdr.Type == protocol.PacketTypeInitial { go s.serverTLS.HandleInitial(p) return nil } + // TODO(#943): send Stateless Reset, if this an IETF QUIC packet if !hdr.VersionFlag { _, err := s.conn.WriteTo(wire.WritePublicReset(hdr.DestConnectionID, 0, 0), p.remoteAddr) return err @@ -343,23 +349,11 @@ func (s *server) handlePacketImpl(p *receivedPacket) error { return errors.New("dropping small packet for unknown connection") } - // send a Version Negotiation Packet if the client is speaking a different protocol version - // since the client send a Public Header (only gQUIC has a Version Flag), we need to send a gQUIC Version Negotiation Packet - if hdr.VersionFlag && !protocol.IsSupportedVersion(s.config.Versions, version) { - s.logger.Infof("Client offered version %s, sending Version Negotiation Packet", version) - _, err := s.conn.WriteTo(wire.ComposeGQUICVersionNegotiation(hdr.DestConnectionID, s.config.Versions), p.remoteAddr) - return err - } - - if !protocol.IsSupportedVersion(s.config.Versions, version) { - return errors.New("Server BUG: negotiated version not supported") - } - - s.logger.Infof("Serving new connection: %s, version %s from %v", hdr.DestConnectionID, version, p.remoteAddr) + s.logger.Infof("Serving new connection: %s, version %s from %v", hdr.DestConnectionID, hdr.Version, p.remoteAddr) sess, err := s.newSession( &conn{pconn: s.conn, currentAddr: p.remoteAddr}, s.sessionRunner, - version, + hdr.Version, hdr.DestConnectionID, s.scfg, s.tlsConf, @@ -374,3 +368,21 @@ func (s *server) handlePacketImpl(p *receivedPacket) error { sess.handlePacket(p) return nil } + +func (s *server) sendVersionNegotiationPacket(p *receivedPacket) error { + hdr := p.header + s.logger.Debugf("Client offered version %s, sending VersionNegotiationPacket", hdr.Version) + + var data []byte + if hdr.Version.UsesIETFFrameFormat() { + var err error + data, err = wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.config.Versions) + if err != nil { + return err + } + } else { + data = wire.ComposeGQUICVersionNegotiation(hdr.DestConnectionID, s.config.Versions) + } + _, err := s.conn.WriteTo(data, p.remoteAddr) + return err +} diff --git a/server_test.go b/server_test.go index 10f58ced..39e90b9e 100644 --- a/server_test.go +++ b/server_test.go @@ -379,6 +379,7 @@ var _ = Describe("Server", func() { DestConnectionID: connID, PacketNumber: 1, PacketNumberLen: protocol.PacketNumberLen2, + Version: protocol.Version39 - 1, } Expect(hdr.Write(b, protocol.PerspectiveClient, 13 /* not a valid QUIC version */)).To(Succeed()) b.Write(bytes.Repeat([]byte{0}, protocol.MinClientHelloSize)) // add a fake CHLO @@ -411,7 +412,6 @@ var _ = Describe("Server", func() { }) It("sends an IETF draft style Version Negotaion Packet, if the client sent a IETF draft style header", func() { - config.Versions = append(config.Versions, protocol.VersionTLS) ln, err := Listen(conn, testdata.GetTLSConfig(), config) Expect(err).ToNot(HaveOccurred()) diff --git a/server_tls.go b/server_tls.go index b894e48b..b1e9b4f8 100644 --- a/server_tls.go +++ b/server_tls.go @@ -19,12 +19,11 @@ type tlsSession struct { } type serverTLS struct { - conn net.PacketConn - config *Config - supportedVersions []protocol.VersionNumber - mintConf *mint.Config - params *handshake.TransportParameters - cookieGenerator *handshake.CookieGenerator + conn net.PacketConn + config *Config + mintConf *mint.Config + params *handshake.TransportParameters + cookieGenerator *handshake.CookieGenerator newSession func(connection, sessionRunner, protocol.ConnectionID, protocol.ConnectionID, protocol.ConnectionID, protocol.PacketNumber, *Config, *mint.Config, *handshake.TransportParameters, utils.Logger, protocol.VersionNumber) (quicSession, error) @@ -60,16 +59,15 @@ func newServerTLS( sessionChan := make(chan tlsSession) s := &serverTLS{ - conn: conn, - config: config, - supportedVersions: config.Versions, - mintConf: mconf, - sessionRunner: runner, - sessionChan: sessionChan, - cookieGenerator: cookieGenerator, - params: params, - newSession: newTLSServerSession, - logger: logger, + conn: conn, + config: config, + mintConf: mconf, + sessionRunner: runner, + sessionChan: sessionChan, + cookieGenerator: cookieGenerator, + params: params, + newSession: newTLSServerSession, + logger: logger, } return s, sessionChan, nil } @@ -99,16 +97,6 @@ func (s *serverTLS) handleInitialImpl(p *receivedPacket) (quicSession, protocol. if len(hdr.Raw)+len(p.data) < protocol.MinInitialPacketSize { return nil, nil, errors.New("dropping too small Initial packet") } - // check version, if not matching send VNP - if !protocol.IsSupportedVersion(s.supportedVersions, hdr.Version) { - s.logger.Debugf("Client offered version %s, sending VersionNegotiationPacket", hdr.Version) - vnp, err := wire.ComposeVersionNegotiation(hdr.SrcConnectionID, hdr.DestConnectionID, s.supportedVersions) - if err != nil { - return nil, nil, err - } - _, err = s.conn.WriteTo(vnp, p.remoteAddr) - return nil, nil, err - } var cookie *handshake.Cookie if len(hdr.Token) > 0 { diff --git a/server_tls_test.go b/server_tls_test.go index e9ef4665..4a2024cf 100644 --- a/server_tls_test.go +++ b/server_tls_test.go @@ -41,22 +41,6 @@ var _ = Describe("Stateless TLS handling", func() { return hdr } - It("sends a Version Negotiation Packet if it doesn't support the version", func() { - server.HandleInitial(&receivedPacket{ - header: &wire.Header{ - DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - SrcConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - PacketNumberLen: protocol.PacketNumberLen1, - Version: 0x1337, - }, - data: bytes.Repeat([]byte{0}, protocol.MinInitialPacketSize), - }) - Expect(conn.dataWritten.Len()).ToNot(BeZero()) - hdr := parseHeader(conn.dataWritten.Bytes()) - Expect(hdr.IsVersionNegotiation).To(BeTrue()) - Expect(sessionChan).ToNot(Receive()) - }) - It("drops too small packets", func() { server.HandleInitial(&receivedPacket{ header: &wire.Header{},