From 56a287ab9c9103e8b04594c04fc7969a66cd8cea Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 7 Nov 2016 21:26:25 +0700 Subject: [PATCH] correcty read PublicHeader of PublicReset packets --- benchmark_test.go | 2 +- client.go | 2 +- integrationtests/proxy/udp_proxy.go | 4 +- public_header.go | 60 +++++++++++++-------- public_header_test.go | 84 ++++++++++++++++------------- server.go | 2 +- 6 files changed, 89 insertions(+), 65 deletions(-) diff --git a/benchmark_test.go b/benchmark_test.go index b8fe524d0..1b82588ba 100644 --- a/benchmark_test.go +++ b/benchmark_test.go @@ -40,7 +40,7 @@ func newLinkedConnection(other *Session) *linkedConnection { return } r := bytes.NewReader(packet) - hdr, err := ParsePublicHeader(r) + hdr, err := ParsePublicHeader(r, protocol.PerspectiveClient) if err != nil { Expect(err).NotTo(HaveOccurred()) } diff --git a/client.go b/client.go index a4cffce40..e85e38ff9 100644 --- a/client.go +++ b/client.go @@ -81,7 +81,7 @@ func (c *Client) handlePacket(packet []byte) error { r := bytes.NewReader(packet) - hdr, err := ParsePublicHeader(r) + hdr, err := ParsePublicHeader(r, protocol.PerspectiveServer) if err != nil { return qerr.Error(qerr.InvalidPacketHeader, err.Error()) } diff --git a/integrationtests/proxy/udp_proxy.go b/integrationtests/proxy/udp_proxy.go index 0271d6b60..85c6b9ee6 100644 --- a/integrationtests/proxy/udp_proxy.go +++ b/integrationtests/proxy/udp_proxy.go @@ -126,7 +126,7 @@ func (p *UDPProxy) runProxy() error { raw := buffer[0:n] r := bytes.NewReader(raw) - hdr, err := quic.ParsePublicHeader(r) + hdr, err := quic.ParsePublicHeader(r, protocol.PerspectiveClient) if err != nil { return err } @@ -154,7 +154,7 @@ func (p *UDPProxy) runConnection(conn *connection) error { // TODO: Switch back to using the public header once Chrome properly sets the type byte. // r := bytes.NewReader(raw) - // , err := quic.ParsePublicHeader(r) + // , err := quic.ParsePublicHeader(r, protocol.PerspectiveServer) // if err != nil { // return err // } diff --git a/public_header.go b/public_header.go index 0dc4baefb..292f3c8ce 100644 --- a/public_header.go +++ b/public_header.go @@ -57,7 +57,7 @@ func (h *PublicHeader) Write(b *bytes.Buffer, version protocol.VersionNumber, pe } // only set PacketNumberLen bits if a packet number will be written - if !(h.ResetFlag || (pers == protocol.PerspectiveServer && h.VersionFlag)) { + if h.hasPacketNumber(pers) { switch h.PacketNumberLen { case protocol.PacketNumberLen1: publicFlagByte |= 0x00 @@ -85,7 +85,7 @@ func (h *PublicHeader) Write(b *bytes.Buffer, version protocol.VersionNumber, pe } // if we're a server, and the VersionFlag is set, we must not include anything else in the packet - if h.ResetFlag || (pers == protocol.PerspectiveServer && h.VersionFlag) { + if !h.hasPacketNumber(pers) { return nil } @@ -110,7 +110,8 @@ func (h *PublicHeader) Write(b *bytes.Buffer, version protocol.VersionNumber, pe } // ParsePublicHeader parses a QUIC packet's public header -func ParsePublicHeader(b io.ByteReader) (*PublicHeader, error) { +// the packetSentBy is the perspective of the peer that sent this PublicHeader, i.e. if we're the server, packetSentBy should be PerspectiveClient +func ParsePublicHeader(b io.ByteReader, packetSentBy protocol.Perspective) (*PublicHeader, error) { header := &PublicHeader{} // First byte @@ -131,15 +132,17 @@ func ParsePublicHeader(b io.ByteReader) (*PublicHeader, error) { return nil, errReceivedTruncatedConnectionID } - switch publicFlagByte & 0x30 { - case 0x30: - header.PacketNumberLen = protocol.PacketNumberLen6 - case 0x20: - header.PacketNumberLen = protocol.PacketNumberLen4 - case 0x10: - header.PacketNumberLen = protocol.PacketNumberLen2 - case 0x00: - header.PacketNumberLen = protocol.PacketNumberLen1 + if header.hasPacketNumber(packetSentBy) { + switch publicFlagByte & 0x30 { + case 0x30: + header.PacketNumberLen = protocol.PacketNumberLen6 + case 0x20: + header.PacketNumberLen = protocol.PacketNumberLen4 + case 0x10: + header.PacketNumberLen = protocol.PacketNumberLen2 + case 0x00: + header.PacketNumberLen = protocol.PacketNumberLen1 + } } // Connection ID @@ -147,23 +150,24 @@ func ParsePublicHeader(b io.ByteReader) (*PublicHeader, error) { if err != nil { return nil, err } + header.ConnectionID = protocol.ConnectionID(connID) if header.ConnectionID == 0 { return nil, errInvalidConnectionID } - if !header.ResetFlag { - // Version (optional) - if header.VersionFlag { - var versionTag uint32 - versionTag, err = utils.ReadUint32(b) - if err != nil { - return nil, err - } - header.VersionNumber = protocol.VersionTagToNumber(versionTag) + // Version (optional) + if header.VersionFlag && !header.ResetFlag { + var versionTag uint32 + versionTag, err = utils.ReadUint32(b) + if err != nil { + return nil, err } + header.VersionNumber = protocol.VersionTagToNumber(versionTag) + } - // Packet number + // Packet number + if header.hasPacketNumber(packetSentBy) { packetNumber, err := utils.ReadUintN(b, uint8(header.PacketNumberLen)) if err != nil { return nil, err @@ -192,3 +196,15 @@ func (h *PublicHeader) GetLength() (protocol.ByteCount, error) { length += protocol.ByteCount(h.PacketNumberLen) return length, nil } + +// hasPacketNumber determines if this PublicHeader will contain a packet number +// this depends on the ResetFlag, the VersionFlag and who sent the packet +func (h *PublicHeader) hasPacketNumber(packetSentBy protocol.Perspective) bool { + if h.ResetFlag { + return false + } + if h.VersionFlag && packetSentBy == protocol.PerspectiveServer { + return false + } + return true +} diff --git a/public_header_test.go b/public_header_test.go index 8a49eca70..f292acd62 100644 --- a/public_header_test.go +++ b/public_header_test.go @@ -12,7 +12,7 @@ var _ = Describe("Public Header", func() { Context("when parsing", func() { It("accepts a sample client header", func() { b := bytes.NewReader([]byte{0x09, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0x51, 0x30, 0x33, 0x34, 0x01}) - hdr, err := ParsePublicHeader(b) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveClient) Expect(err).ToNot(HaveOccurred()) Expect(hdr.VersionFlag).To(BeTrue()) Expect(hdr.ResetFlag).To(BeFalse()) @@ -24,51 +24,27 @@ var _ = Describe("Public Header", func() { It("does not accept 0-byte connection ID", func() { b := bytes.NewReader([]byte{0x00, 0x01}) - _, err := ParsePublicHeader(b) + _, err := ParsePublicHeader(b, protocol.PerspectiveClient) Expect(err).To(MatchError(errReceivedTruncatedConnectionID)) }) It("rejects 0 as a connection ID", func() { b := bytes.NewReader([]byte{0x09, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x51, 0x30, 0x33, 0x30, 0x01}) - _, err := ParsePublicHeader(b) + _, err := ParsePublicHeader(b, protocol.PerspectiveClient) Expect(err).To(MatchError(errInvalidConnectionID)) }) - It("accepts 1-byte packet numbers", func() { - b := bytes.NewReader([]byte{0x08, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xde}) - hdr, err := ParsePublicHeader(b) + It("reads a PublicReset packet", func() { + b := bytes.NewReader([]byte{0xa, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8}) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xde))) - Expect(b.Len()).To(BeZero()) - }) - - It("accepts 2-byte packet numbers", func() { - b := bytes.NewReader([]byte{0x18, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xde, 0xca}) - hdr, err := ParsePublicHeader(b) - Expect(err).ToNot(HaveOccurred()) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xcade))) - Expect(b.Len()).To(BeZero()) - }) - - It("accepts 4-byte packet numbers", func() { - b := bytes.NewReader([]byte{0x28, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xad, 0xfb, 0xca, 0xde}) - hdr, err := ParsePublicHeader(b) - Expect(err).ToNot(HaveOccurred()) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xdecafbad))) - Expect(b.Len()).To(BeZero()) - }) - - It("accepts 6-byte packet numbers", func() { - b := bytes.NewReader([]byte{0x38, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0x23, 0x42, 0xad, 0xfb, 0xca, 0xde}) - hdr, err := ParsePublicHeader(b) - Expect(err).ToNot(HaveOccurred()) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xdecafbad4223))) - Expect(b.Len()).To(BeZero()) + Expect(hdr.ResetFlag).To(BeTrue()) + Expect(hdr.ConnectionID).ToNot(BeZero()) }) It("parses a public reset packet", func() { b := bytes.NewReader([]byte{0xa, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}) - hdr, err := ParsePublicHeader(b) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) Expect(hdr.ResetFlag).To(BeTrue()) Expect(hdr.VersionFlag).To(BeFalse()) @@ -81,9 +57,43 @@ var _ = Describe("Public Header", func() { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 0x01, }) - _, err := ParsePublicHeader(b) + _, err := ParsePublicHeader(b, protocol.PerspectiveClient) Expect(err).To(MatchError("diversification nonces should only be sent by servers")) }) + + Context("Packet Number lengths", func() { + It("accepts 1-byte packet numbers", func() { + b := bytes.NewReader([]byte{0x08, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xde}) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveClient) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xde))) + Expect(b.Len()).To(BeZero()) + }) + + It("accepts 2-byte packet numbers", func() { + b := bytes.NewReader([]byte{0x18, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xde, 0xca}) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveClient) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xcade))) + Expect(b.Len()).To(BeZero()) + }) + + It("accepts 4-byte packet numbers", func() { + b := bytes.NewReader([]byte{0x28, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0xad, 0xfb, 0xca, 0xde}) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveClient) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xdecafbad))) + Expect(b.Len()).To(BeZero()) + }) + + It("accepts 6-byte packet numbers", func() { + b := bytes.NewReader([]byte{0x38, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0x23, 0x42, 0xad, 0xfb, 0xca, 0xde}) + hdr, err := ParsePublicHeader(b, protocol.PerspectiveClient) + Expect(err).ToNot(HaveOccurred()) + Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0xdecafbad4223))) + Expect(b.Len()).To(BeZero()) + }) + }) }) Context("when writing", func() { @@ -219,10 +229,8 @@ var _ = Describe("Public Header", func() { It("sets the Reset Flag", func() { b := &bytes.Buffer{} hdr := PublicHeader{ - ResetFlag: true, - ConnectionID: 0x4cfa9f9b668619f6, - PacketNumber: 2, - PacketNumberLen: protocol.PacketNumberLen6, + ResetFlag: true, + ConnectionID: 0x4cfa9f9b668619f6, } err := hdr.Write(b, protocol.VersionWhatever, protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) diff --git a/server.go b/server.go index d2a9aa6c7..0b96a7de0 100644 --- a/server.go +++ b/server.go @@ -138,7 +138,7 @@ func (s *Server) handlePacket(conn *net.UDPConn, remoteAddr *net.UDPAddr, packet r := bytes.NewReader(packet) - hdr, err := ParsePublicHeader(r) + hdr, err := ParsePublicHeader(r, protocol.PerspectiveClient) if err != nil { return qerr.Error(qerr.InvalidPacketHeader, err.Error()) }