From a4bc7362e0cf867dec7c5b917fa0805e84c0d2d3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 23 Feb 2018 14:19:57 +0800 Subject: [PATCH] fix IETF Version Negotiation Packet, it doesn't have a packet number --- internal/wire/header_test.go | 3 +- internal/wire/ietf_header.go | 16 +++++----- internal/wire/ietf_header_test.go | 7 ++-- internal/wire/version_negotiation.go | 39 ++++++++--------------- internal/wire/version_negotiation_test.go | 4 +-- server_test.go | 1 - server_tls.go | 2 +- 7 files changed, 28 insertions(+), 44 deletions(-) diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index 4d23173f..7f649044 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -137,13 +137,12 @@ var _ = Describe("Header", func() { It("parses an IETF draft style Version Negotiation Packet", func() { versions := []protocol.VersionNumber{0x13, 0x37} - data := ComposeVersionNegotiation(0x42, 0x77, versions) + data := ComposeVersionNegotiation(0x42, versions) hdr, err := ParseHeaderSentByServer(bytes.NewReader(data), protocol.VersionUnknown) Expect(err).ToNot(HaveOccurred()) Expect(hdr.isPublicHeader).To(BeFalse()) Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x42))) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x77))) Expect(hdr.Version).To(BeZero()) // in addition to the versions, the supported versions might contain a reserved version number for _, version := range versions { diff --git a/internal/wire/ietf_header.go b/internal/wire/ietf_header.go index 88bd139f..60e4dd0f 100644 --- a/internal/wire/ietf_header.go +++ b/internal/wire/ietf_header.go @@ -31,15 +31,9 @@ func parseLongHeader(b *bytes.Reader, sentBy protocol.Perspective, typeByte byte if err != nil { return nil, err } - pn, err := utils.BigEndian.ReadUint32(b) - if err != nil { - return nil, err - } h := &Header{ - ConnectionID: protocol.ConnectionID(connID), - PacketNumber: protocol.PacketNumber(pn), - PacketNumberLen: protocol.PacketNumberLen4, - Version: protocol.VersionNumber(v), + ConnectionID: protocol.ConnectionID(connID), + Version: protocol.VersionNumber(v), } if v == 0 { // version negotiation packet if sentBy == protocol.PerspectiveClient { @@ -60,6 +54,12 @@ func parseLongHeader(b *bytes.Reader, sentBy protocol.Perspective, typeByte byte return h, nil } h.IsLongHeader = true + pn, err := utils.BigEndian.ReadUint32(b) + if err != nil { + return nil, err + } + h.PacketNumber = protocol.PacketNumber(pn) + h.PacketNumberLen = protocol.PacketNumberLen4 h.Type = protocol.PacketType(typeByte & 0x7f) if sentBy == protocol.PerspectiveClient && (h.Type != protocol.PacketTypeInitial && h.Type != protocol.PacketTypeHandshake && h.Type != protocol.PacketType0RTT) { return nil, qerr.Error(qerr.InvalidPacketHeader, fmt.Sprintf("Received packet with invalid packet type: %d", h.Type)) diff --git a/internal/wire/ietf_header_test.go b/internal/wire/ietf_header_test.go index 72ff2cba..ec532307 100644 --- a/internal/wire/ietf_header_test.go +++ b/internal/wire/ietf_header_test.go @@ -20,14 +20,13 @@ var _ = Describe("IETF draft Header", func() { Context("Version Negotiation Packets", func() { It("parses", func() { versions := []protocol.VersionNumber{0x22334455, 0x33445566} - data := ComposeVersionNegotiation(0x1234567890, 0x1337, versions) + data := ComposeVersionNegotiation(0x1234567890, versions) b := bytes.NewReader(data) h, err := parseHeader(b, protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) Expect(h.IsVersionNegotiation).To(BeTrue()) Expect(h.Version).To(BeZero()) Expect(h.ConnectionID).To(Equal(protocol.ConnectionID(0x1234567890))) - Expect(h.PacketNumber).To(Equal(protocol.PacketNumber(0x1337))) for _, v := range versions { Expect(h.SupportedVersions).To(ContainElement(v)) } @@ -35,7 +34,7 @@ var _ = Describe("IETF draft Header", func() { It("errors if it contains versions of the wrong length", func() { versions := []protocol.VersionNumber{0x22334455, 0x33445566} - data := ComposeVersionNegotiation(0x1234567890, 0x1337, versions) + data := ComposeVersionNegotiation(0x1234567890, versions) b := bytes.NewReader(data[:len(data)-2]) _, err := parseHeader(b, protocol.PerspectiveServer) Expect(err).To(MatchError(qerr.InvalidVersionNegotiationPacket)) @@ -43,7 +42,7 @@ var _ = Describe("IETF draft Header", func() { It("errors if the version list is emtpy", func() { versions := []protocol.VersionNumber{0x22334455} - data := ComposeVersionNegotiation(0x1234567890, 0x1337, versions) + data := ComposeVersionNegotiation(0x1234567890, versions) // remove 8 bytes (two versions), since ComposeVersionNegotiation also added a reserved version number _, err := parseHeader(bytes.NewReader(data[:len(data)-8]), protocol.PerspectiveServer) Expect(err).To(MatchError("InvalidVersionNegotiationPacket: empty version list")) diff --git a/internal/wire/version_negotiation.go b/internal/wire/version_negotiation.go index b20c43c2..2df5e69d 100644 --- a/internal/wire/version_negotiation.go +++ b/internal/wire/version_negotiation.go @@ -10,50 +10,37 @@ import ( // ComposeGQUICVersionNegotiation composes a Version Negotiation Packet for gQUIC func ComposeGQUICVersionNegotiation(connID protocol.ConnectionID, versions []protocol.VersionNumber) []byte { - fullReply := &bytes.Buffer{} + buf := &bytes.Buffer{} ph := Header{ ConnectionID: connID, PacketNumber: 1, VersionFlag: true, IsVersionNegotiation: true, } - if err := ph.writePublicHeader(fullReply, protocol.PerspectiveServer, protocol.VersionWhatever); err != nil { + if err := ph.writePublicHeader(buf, protocol.PerspectiveServer, protocol.VersionWhatever); err != nil { utils.Errorf("error composing version negotiation packet: %s", err.Error()) return nil } - writeVersions(fullReply, versions) - return fullReply.Bytes() + for _, v := range protocol.GetGreasedVersions(versions) { + utils.BigEndian.WriteUint32(buf, uint32(v)) + } + return buf.Bytes() } // ComposeVersionNegotiation composes a Version Negotiation according to the IETF draft func ComposeVersionNegotiation( connID protocol.ConnectionID, - pn protocol.PacketNumber, versions []protocol.VersionNumber, ) []byte { - fullReply := &bytes.Buffer{} + greasedVersions := protocol.GetGreasedVersions(versions) + buf := bytes.NewBuffer(make([]byte, 0, 1+8+4+len(greasedVersions)*4)) r := make([]byte, 1) _, _ = rand.Read(r) // ignore the error here. It is not critical to have perfect random here. - h := Header{ - IsLongHeader: true, - Type: protocol.PacketType(r[0] | 0x80), - ConnectionID: connID, - PacketNumber: pn, - Version: 0, - IsVersionNegotiation: true, - } - if err := h.writeHeader(fullReply); err != nil { - utils.Errorf("error composing version negotiation packet: %s", err.Error()) - return nil - } - writeVersions(fullReply, versions) - return fullReply.Bytes() -} - -// writeVersions writes the versions for a Version Negotiation Packet. -// It inserts one reserved version number at a random position. -func writeVersions(buf *bytes.Buffer, supported []protocol.VersionNumber) { - for _, v := range protocol.GetGreasedVersions(supported) { + buf.WriteByte(r[0] | 0x80) + utils.BigEndian.WriteUint64(buf, uint64(connID)) + utils.BigEndian.WriteUint32(buf, 0) // version 0 + for _, v := range greasedVersions { utils.BigEndian.WriteUint32(buf, uint32(v)) } + return buf.Bytes() } diff --git a/internal/wire/version_negotiation_test.go b/internal/wire/version_negotiation_test.go index 5bd1a809..75d624aa 100644 --- a/internal/wire/version_negotiation_test.go +++ b/internal/wire/version_negotiation_test.go @@ -25,12 +25,12 @@ var _ = Describe("Version Negotiation Packets", func() { It("writes in IETF draft style", func() { versions := []protocol.VersionNumber{1001, 1003} - data := ComposeVersionNegotiation(0x1337, 0x42, versions) + data := ComposeVersionNegotiation(0x1337, versions) + Expect(data[0] & 0x80).ToNot(BeZero()) hdr, err := parseHeader(bytes.NewReader(data), protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x1337))) - Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) Expect(hdr.Version).To(BeZero()) // the supported versions should include one reserved version number Expect(hdr.SupportedVersions).To(HaveLen(len(versions) + 1)) diff --git a/server_test.go b/server_test.go index 0518f137..f81c3dea 100644 --- a/server_test.go +++ b/server_test.go @@ -529,7 +529,6 @@ var _ = Describe("Server", func() { Expect(err).ToNot(HaveOccurred()) Expect(packet.IsVersionNegotiation).To(BeTrue()) Expect(packet.ConnectionID).To(Equal(protocol.ConnectionID(0x1337))) - Expect(packet.PacketNumber).To(Equal(protocol.PacketNumber(0x55))) Expect(r.Len()).To(BeZero()) Consistently(done).ShouldNot(BeClosed()) }) diff --git a/server_tls.go b/server_tls.go index 5f270e34..a6d974df 100644 --- a/server_tls.go +++ b/server_tls.go @@ -132,7 +132,7 @@ func (s *serverTLS) handleInitialImpl(remoteAddr net.Addr, hdr *wire.Header, dat // check version, if not matching send VNP if !protocol.IsSupportedVersion(s.supportedVersions, hdr.Version) { utils.Debugf("Client offered version %s, sending VersionNegotiationPacket", hdr.Version) - _, err := s.conn.WriteTo(wire.ComposeVersionNegotiation(hdr.ConnectionID, hdr.PacketNumber, s.supportedVersions), remoteAddr) + _, err := s.conn.WriteTo(wire.ComposeVersionNegotiation(hdr.ConnectionID, s.supportedVersions), remoteAddr) return nil, err }