From 7c3d6abb4b34b70f4983b383c11f2c3c83d21c29 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Sun, 12 Nov 2017 11:52:53 +0800
Subject: [PATCH] fix parsing of the Header type byte

In order to determine if a packet is a Version Negotiation Packet, it is
not sufficient to just look at bit 0x1. Other packet types also have
that bit set, e.g. the Retry packet (packet type 0x3). Instead, we have
to look at the last 3 bits. This fix will work as long as IETF QUIC
doesn't define more than 8 long header types.
---
 internal/wire/header.go      | 11 ++++++-----
 internal/wire/header_test.go | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/internal/wire/header.go b/internal/wire/header.go
index 5bcabb36..96066cc9 100644
--- a/internal/wire/header.go
+++ b/internal/wire/header.go
@@ -41,13 +41,14 @@ func ParseHeaderSentByServer(b *bytes.Reader, version protocol.VersionNumber) (*
 
 	var isPublicHeader bool
 	// As a client, we know the version of the packet that the server sent, except for Version Negotiation Packets.
-	// Both gQUIC and IETF QUIC Version Negotiation Packets have 0x1 set.
-	if typeByte&0x1 > 0 {
+	if typeByte == 0x81 { // IETF draft Version Negotiation Packet
+		isPublicHeader = false
+	} else if typeByte&0xcf == 0x9 { // gQUIC Version Negotiation Packet
 		// IETF QUIC Version Negotiation Packets are sent with the Long Header (indicated by the 0x80 bit)
 		// gQUIC always has 0x80 unset
-		isPublicHeader = typeByte&0x80 == 0
-	} else {
-		// For all packets that are not Version Negotiation Packets, the client knows the version that this packet was sent with
+		isPublicHeader = true
+	} else { // not a Version Negotiation Packet
+		// the client knows the version that this packet was sent with
 		isPublicHeader = !version.UsesTLS()
 	}
 	return parsePacketHeader(b, protocol.PerspectiveServer, isPublicHeader)
diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go
index 65871eb8..d28c8907 100644
--- a/internal/wire/header_test.go
+++ b/internal/wire/header_test.go
@@ -19,7 +19,7 @@ var _ = Describe("Header", func() {
 	)
 
 	Context("parsing", func() {
-		It("parses an IETF draft header, when the QUIC version supports TLS", func() {
+		It("parses an IETF draft Short Header, when the QUIC version supports TLS", func() {
 			buf := &bytes.Buffer{}
 			// use a short header, which isn't distinguishable from the gQUIC Public Header when looking at the type byte
 			err := (&Header{
@@ -51,6 +51,21 @@ var _ = Describe("Header", func() {
 			Expect(hdr.isPublicHeader).To(BeFalse())
 		})
 
+		It("doens't mistake packets with a Short Header for Version Negotiation Packets", func() {
+			// make sure this packet could be mistaken for a Version Negotiation Packet, if we only look at the 0x1 bit
+			buf := &bytes.Buffer{}
+			err := (&Header{
+				IsLongHeader:    false,
+				PacketNumberLen: protocol.PacketNumberLen1,
+				PacketNumber:    0x42,
+			}).writeHeader(buf)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(buf.Bytes()[0] & 0x1).To(BeEquivalentTo(0x1))
+			hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes()), versionIETFHeader)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(hdr.isPublicHeader).To(BeFalse())
+		})
+
 		It("parses a gQUIC Public Header, when the version is not known", func() {
 			buf := &bytes.Buffer{}
 			err := (&Header{