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

@@ -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
+}

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())
 

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 {

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{},