From 372463db49758db30ace123b82f276a4ec562f39 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 21 May 2018 11:27:15 +0800 Subject: [PATCH] don't pass the client's version to the header parser --- client.go | 2 +- internal/wire/header.go | 9 +++------ internal/wire/header_test.go | 14 +++++++------- packet_packer_test.go | 4 ++-- server_test.go | 4 ++-- server_tls_test.go | 6 +++--- 6 files changed, 18 insertions(+), 21 deletions(-) diff --git a/client.go b/client.go index 692ca3c1..98c5590c 100644 --- a/client.go +++ b/client.go @@ -304,7 +304,7 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { rcvTime := time.Now() r := bytes.NewReader(packet) - hdr, err := wire.ParseHeaderSentByServer(r, c.version) + hdr, err := wire.ParseHeaderSentByServer(r) // drop the packet if we can't parse the header if err != nil { return fmt.Errorf("error parsing packet from %s: %s", remoteAddr.String(), err.Error()) diff --git a/internal/wire/header.go b/internal/wire/header.go index 41261333..076e4d43 100644 --- a/internal/wire/header.go +++ b/internal/wire/header.go @@ -39,7 +39,7 @@ type Header struct { } // ParseHeaderSentByServer parses the header for a packet that was sent by the server. -func ParseHeaderSentByServer(b *bytes.Reader, version protocol.VersionNumber) (*Header, error) { +func ParseHeaderSentByServer(b *bytes.Reader) (*Header, error) { typeByte, err := b.ReadByte() if err != nil { return nil, err @@ -49,13 +49,10 @@ func ParseHeaderSentByServer(b *bytes.Reader, version protocol.VersionNumber) (* var isPublicHeader bool if typeByte&0x80 > 0 { // gQUIC always has 0x80 unset. IETF Long Header or Version Negotiation isPublicHeader = false - } else if typeByte&0xcf == 0x9 { // gQUIC Version Negotiation Packet - isPublicHeader = true } else { - // the client knows the version that this packet was sent with - isPublicHeader = !version.UsesTLS() + // gQUIC never uses 6 byte packet numbers, so the third and fourth bit will never be 11 + isPublicHeader = typeByte&0x30 != 0x30 } - return parsePacketHeader(b, protocol.PerspectiveServer, isPublicHeader) } diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index b897266b..9a000743 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -68,7 +68,7 @@ var _ = Describe("Header", func() { PacketNumber: 0x42, }).writeHeader(buf) Expect(err).ToNot(HaveOccurred()) - hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes()), versionIETFHeader) + hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes())) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsPublicHeader).To(BeFalse()) }) @@ -94,7 +94,7 @@ var _ = Describe("Header", func() { Expect(hdr.IsPublicHeader).To(BeTrue()) }) - It("parses a gQUIC Public Header, when the version is known", func() { + It("parses a gQUIC Public Header", func() { connID := protocol.ConnectionID{8, 7, 6, 5, 4, 3, 2, 1} buf := &bytes.Buffer{} err := (&Header{ @@ -105,7 +105,7 @@ var _ = Describe("Header", func() { DiversificationNonce: bytes.Repeat([]byte{'f'}, 32), }).writePublicHeader(buf, protocol.PerspectiveServer, versionPublicHeader) Expect(err).ToNot(HaveOccurred()) - hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes()), versionPublicHeader) + hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes())) Expect(err).ToNot(HaveOccurred()) Expect(hdr.DestConnectionID).To(Equal(connID)) Expect(hdr.SrcConnectionID).To(Equal(connID)) @@ -130,7 +130,7 @@ var _ = Describe("Header", func() { }) It("errors when given no data", func() { - _, err := ParseHeaderSentByServer(bytes.NewReader([]byte{}), protocol.VersionUnknown) + _, err := ParseHeaderSentByServer(bytes.NewReader([]byte{})) Expect(err).To(MatchError(io.EOF)) _, err = ParseHeaderSentByClient(bytes.NewReader([]byte{})) Expect(err).To(MatchError(io.EOF)) @@ -140,7 +140,7 @@ var _ = Describe("Header", func() { connID := protocol.ConnectionID{0xde, 0xca, 0xfb, 0xad, 0xde, 0xca, 0xfb, 0xad} versions := []protocol.VersionNumber{0x13, 0x37} data := ComposeGQUICVersionNegotiation(connID, versions) - hdr, err := ParseHeaderSentByServer(bytes.NewReader(data), protocol.VersionUnknown) + hdr, err := ParseHeaderSentByServer(bytes.NewReader(data)) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsPublicHeader).To(BeTrue()) Expect(hdr.DestConnectionID).To(Equal(connID)) @@ -157,7 +157,7 @@ var _ = Describe("Header", func() { versions := []protocol.VersionNumber{0x13, 0x37} data, err := ComposeVersionNegotiation(destConnID, srcConnID, versions) Expect(err).ToNot(HaveOccurred()) - hdr, err := ParseHeaderSentByServer(bytes.NewReader(data), protocol.VersionUnknown) + hdr, err := ParseHeaderSentByServer(bytes.NewReader(data)) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsPublicHeader).To(BeFalse()) Expect(hdr.IsVersionNegotiation).To(BeTrue()) @@ -199,7 +199,7 @@ var _ = Describe("Header", func() { } err := hdr.Write(buf, protocol.PerspectiveServer, versionIETFHeader) Expect(err).ToNot(HaveOccurred()) - _, err = ParseHeaderSentByServer(bytes.NewReader(buf.Bytes()), versionIETFFrames) + _, err = ParseHeaderSentByServer(bytes.NewReader(buf.Bytes())) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsPublicHeader).To(BeFalse()) }) diff --git a/packet_packer_test.go b/packet_packer_test.go index 6e69990c..d682cc95 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -265,7 +265,7 @@ var _ = Describe("Packet packer", func() { Expect(err).ToNot(HaveOccurred()) // parse the packet r := bytes.NewReader(p.raw) - hdr, err := wire.ParseHeaderSentByServer(r, packer.version) + hdr, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) Expect(hdr.PayloadLen).To(BeEquivalentTo(r.Len())) }) @@ -614,7 +614,7 @@ var _ = Describe("Packet packer", func() { Expect(p.header.IsLongHeader).To(BeTrue()) // parse the packet r := bytes.NewReader(p.raw) - hdr, err := wire.ParseHeaderSentByServer(r, packer.version) + hdr, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) Expect(hdr.PayloadLen).To(BeEquivalentTo(r.Len())) }) diff --git a/server_test.go b/server_test.go index 9461a718..0642bd72 100644 --- a/server_test.go +++ b/server_test.go @@ -571,7 +571,7 @@ var _ = Describe("Server", func() { Eventually(func() int { return conn.dataWritten.Len() }).ShouldNot(BeZero()) Expect(conn.dataWrittenTo).To(Equal(udpAddr)) r := bytes.NewReader(conn.dataWritten.Bytes()) - packet, err := wire.ParseHeaderSentByServer(r, protocol.VersionUnknown) + packet, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) Expect(packet.VersionFlag).To(BeTrue()) Expect(packet.DestConnectionID).To(Equal(connID)) @@ -614,7 +614,7 @@ var _ = Describe("Server", func() { Eventually(func() int { return conn.dataWritten.Len() }).ShouldNot(BeZero()) Expect(conn.dataWrittenTo).To(Equal(udpAddr)) r := bytes.NewReader(conn.dataWritten.Bytes()) - packet, err := wire.ParseHeaderSentByServer(r, protocol.VersionUnknown) + packet, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) Expect(packet.IsVersionNegotiation).To(BeTrue()) Expect(packet.DestConnectionID).To(Equal(connID)) diff --git a/server_tls_test.go b/server_tls_test.go index b1246ab6..a03954b8 100644 --- a/server_tls_test.go +++ b/server_tls_test.go @@ -71,7 +71,7 @@ var _ = Describe("Stateless TLS handling", func() { unpackPacket := func(data []byte) (*wire.Header, []byte) { r := bytes.NewReader(conn.dataWritten.Bytes()) - hdr, err := wire.ParseHeaderSentByServer(r, protocol.VersionTLS) + hdr, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) hdr.Raw = data[:len(data)-r.Len()] aead, err := crypto.NewNullAEAD(protocol.PerspectiveClient, hdr.SrcConnectionID, protocol.VersionTLS) @@ -89,7 +89,7 @@ var _ = Describe("Stateless TLS handling", func() { } server.HandleInitial(nil, hdr, bytes.Repeat([]byte{0}, protocol.MinInitialPacketSize)) Expect(conn.dataWritten.Len()).ToNot(BeZero()) - hdr, err := wire.ParseHeaderSentByServer(bytes.NewReader(conn.dataWritten.Bytes()), protocol.VersionUnknown) + hdr, err := wire.ParseHeaderSentByServer(bytes.NewReader(conn.dataWritten.Bytes())) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(sessionChan).ToNot(Receive()) @@ -118,7 +118,7 @@ var _ = Describe("Stateless TLS handling", func() { server.HandleInitial(nil, hdr, data) Expect(conn.dataWritten.Len()).ToNot(BeZero()) r := bytes.NewReader(conn.dataWritten.Bytes()) - replyHdr, err := wire.ParseHeaderSentByServer(r, protocol.VersionTLS) + replyHdr, err := wire.ParseHeaderSentByServer(r) Expect(err).ToNot(HaveOccurred()) Expect(replyHdr.Type).To(Equal(protocol.PacketTypeRetry)) Expect(replyHdr.SrcConnectionID).To(Equal(hdr.DestConnectionID))