From 683b5823e4e38de37d82b7d51997b193decd6ddc Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 7 May 2018 13:24:43 +0900 Subject: [PATCH] handle gQUIC and IETF QUIC packets separately in the client --- client.go | 64 +++++++++++++++++++++++++----------- client_test.go | 1 + internal/wire/header.go | 11 +++---- internal/wire/header_test.go | 20 +++++------ 4 files changed, 61 insertions(+), 35 deletions(-) diff --git a/client.go b/client.go index e03f822b..7ffabd69 100644 --- a/client.go +++ b/client.go @@ -316,6 +316,35 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { hdr.Raw = packet[:len(packet)-r.Len()] packetData := packet[len(packet)-r.Len():] + c.mutex.Lock() + defer c.mutex.Unlock() + + // handle Version Negotiation Packets + if hdr.IsVersionNegotiation { + // ignore delayed / duplicated version negotiation packets + if c.receivedVersionNegotiationPacket || c.versionNegotiated { + return errors.New("received a delayed Version Negotiation Packet") + } + + // version negotiation packets have no payload + if err := c.handleVersionNegotiationPacket(hdr); err != nil { + c.session.Close(err) + } + return nil + } + + if hdr.IsPublicHeader { + return c.handleGQUICPacket(hdr, r, packetData, remoteAddr, rcvTime) + } + return c.handleIETFQUICPacket(hdr, packetData, remoteAddr, rcvTime) +} + +func (c *client) handleIETFQUICPacket(hdr *wire.Header, packetData []byte, remoteAddr net.Addr, rcvTime time.Time) error { + // TODO(#1003): add support for server-chosen connection IDs + // reject packets with the wrong connection ID + if !hdr.DestConnectionID.Equal(c.srcConnID) { + return fmt.Errorf("received a packet with an unexpected connection ID (%s, expected %s)", hdr.DestConnectionID, c.srcConnID) + } if hdr.IsLongHeader { c.logger.Debugf("len(packet data): %d, payloadLen: %d", len(packetData), hdr.PayloadLen) if protocol.ByteCount(len(packetData)) < hdr.PayloadLen { @@ -325,11 +354,24 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { // TODO(#1312): implement parsing of compound packets } - c.mutex.Lock() - defer c.mutex.Unlock() + // this is the first packet we are receiving + // since it is not a Version Negotiation Packet, this means the server supports the suggested version + if !c.versionNegotiated { + c.versionNegotiated = true + close(c.versionNegotiationChan) + } + c.session.handlePacket(&receivedPacket{ + remoteAddr: remoteAddr, + header: hdr, + data: packetData, + rcvTime: rcvTime, + }) + return nil +} + +func (c *client) handleGQUICPacket(hdr *wire.Header, r *bytes.Reader, packetData []byte, remoteAddr net.Addr, rcvTime time.Time) error { // reject packets with the wrong connection ID - // TODO(#1003): add support for server-chosen connection IDs if !hdr.OmitConnectionID && !hdr.DestConnectionID.Equal(c.srcConnID) { return fmt.Errorf("received a packet with an unexpected connection ID (%s, expected %s)", hdr.DestConnectionID, c.srcConnID) } @@ -350,20 +392,6 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { return nil } - // handle Version Negotiation Packets - if hdr.IsVersionNegotiation { - // ignore delayed / duplicated version negotiation packets - if c.receivedVersionNegotiationPacket || c.versionNegotiated { - return errors.New("received a delayed Version Negotiation Packet") - } - - // version negotiation packets have no payload - if err := c.handleVersionNegotiationPacket(hdr); err != nil { - c.session.Close(err) - } - return nil - } - // this is the first packet we are receiving // since it is not a Version Negotiation Packet, this means the server supports the suggested version if !c.versionNegotiated { @@ -371,8 +399,6 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) error { close(c.versionNegotiationChan) } - // TODO: validate packet number and connection ID on Retry packets (for IETF QUIC) - c.session.handlePacket(&receivedPacket{ remoteAddr: remoteAddr, header: hdr, diff --git a/client_test.go b/client_test.go index 6b5ad873..c22e3b4a 100644 --- a/client_test.go +++ b/client_test.go @@ -580,6 +580,7 @@ var _ = Describe("Client", func() { SrcConnectionID: connID, PacketNumber: 1, PacketNumberLen: 1, + Version: versionIETFFrames, }).Write(buf, protocol.PerspectiveServer, versionIETFFrames) Expect(err).ToNot(HaveOccurred()) err = cl.handlePacket(addr, buf.Bytes()) diff --git a/internal/wire/header.go b/internal/wire/header.go index 1bc1897e..ec0ae24d 100644 --- a/internal/wire/header.go +++ b/internal/wire/header.go @@ -10,6 +10,8 @@ import ( // Header is the header of a QUIC packet. // It contains fields that are only needed for the gQUIC Public Header and the IETF draft Header. type Header struct { + IsPublicHeader bool + Raw []byte Version protocol.VersionNumber @@ -34,9 +36,6 @@ type Header struct { IsLongHeader bool KeyPhase int PayloadLen protocol.ByteCount - - // only needed for logging - isPublicHeader bool } // ParseHeaderSentByServer parses the header for a packet that was sent by the server. @@ -85,7 +84,7 @@ func parsePacketHeader(b *bytes.Reader, sentBy protocol.Perspective, isPublicHea if err != nil { return nil, err } - hdr.isPublicHeader = true // save that this is a Public Header, so we can log it correctly later + hdr.IsPublicHeader = true // save that this is a Public Header, so we can log it correctly later return hdr, nil } return parseHeader(b, sentBy) @@ -94,7 +93,7 @@ func parsePacketHeader(b *bytes.Reader, sentBy protocol.Perspective, isPublicHea // Write writes the Header. func (h *Header) Write(b *bytes.Buffer, pers protocol.Perspective, version protocol.VersionNumber) error { if !version.UsesTLS() { - h.isPublicHeader = true // save that this is a Public Header, so we can log it correctly later + h.IsPublicHeader = true // save that this is a Public Header, so we can log it correctly later return h.writePublicHeader(b, pers, version) } return h.writeHeader(b) @@ -110,7 +109,7 @@ func (h *Header) GetLength(pers protocol.Perspective, version protocol.VersionNu // Log logs the Header func (h *Header) Log(logger utils.Logger) { - if h.isPublicHeader { + if h.IsPublicHeader { h.logPublicHeader(logger) } else { h.logHeader(logger) diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index 4a30de32..24ebb92a 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -35,7 +35,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.KeyPhase).To(BeEquivalentTo(1)) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) - Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.IsPublicHeader).To(BeFalse()) }) It("parses an IETF draft header, when the version is not known, but it has Long Header format", func() { @@ -53,7 +53,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.Type).To(Equal(protocol.PacketType0RTT)) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) - Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.IsPublicHeader).To(BeFalse()) Expect(hdr.Version).To(Equal(protocol.VersionNumber(0x1234))) }) @@ -70,7 +70,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) hdr, err := ParseHeaderSentByServer(bytes.NewReader(buf.Bytes()), versionIETFHeader) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.IsPublicHeader).To(BeFalse()) }) It("parses a gQUIC Public Header, when the version is not known", func() { @@ -91,7 +91,7 @@ var _ = Describe("Header", func() { Expect(hdr.SrcConnectionID).To(Equal(connID)) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x1337))) Expect(hdr.Version).To(Equal(versionPublicHeader)) - Expect(hdr.isPublicHeader).To(BeTrue()) + Expect(hdr.IsPublicHeader).To(BeTrue()) }) It("parses a gQUIC Public Header, when the version is known", func() { @@ -111,7 +111,7 @@ var _ = Describe("Header", func() { Expect(hdr.SrcConnectionID).To(Equal(connID)) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x1337))) Expect(hdr.DiversificationNonce).To(HaveLen(32)) - Expect(hdr.isPublicHeader).To(BeTrue()) + Expect(hdr.IsPublicHeader).To(BeTrue()) }) It("errors when parsing the gQUIC header fails", func() { @@ -142,7 +142,7 @@ var _ = Describe("Header", func() { data := ComposeGQUICVersionNegotiation(connID, versions) hdr, err := ParseHeaderSentByServer(bytes.NewReader(data), protocol.VersionUnknown) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.isPublicHeader).To(BeTrue()) + Expect(hdr.IsPublicHeader).To(BeTrue()) Expect(hdr.DestConnectionID).To(Equal(connID)) Expect(hdr.SrcConnectionID).To(Equal(connID)) // in addition to the versions, the supported versions might contain a reserved version number @@ -159,7 +159,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) hdr, err := ParseHeaderSentByServer(bytes.NewReader(data), protocol.VersionUnknown) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.IsPublicHeader).To(BeFalse()) Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.DestConnectionID).To(Equal(destConnID)) Expect(hdr.SrcConnectionID).To(Equal(srcConnID)) @@ -184,7 +184,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) _, err = parsePublicHeader(bytes.NewReader(buf.Bytes()), protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.isPublicHeader).To(BeTrue()) + Expect(hdr.IsPublicHeader).To(BeTrue()) }) It("writes a IETF draft header", func() { @@ -200,7 +200,7 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) _, err = parseHeader(bytes.NewReader(buf.Bytes()), protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.IsPublicHeader).To(BeFalse()) }) }) @@ -278,7 +278,7 @@ var _ = Describe("Header", func() { It("logs a Public Header", func() { (&Header{ - isPublicHeader: true, + IsPublicHeader: true, DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, SrcConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, }).Log(logger)