diff --git a/client.go b/client.go index d13dd814..21d19fed 100644 --- a/client.go +++ b/client.go @@ -286,10 +286,8 @@ func (c *client) handlePacket(remoteAddr net.Addr, packet []byte) { return } - isVersionNegotiationPacket := hdr.VersionFlag /* gQUIC Version Negotiation Packet */ || hdr.Type == protocol.PacketTypeVersionNegotiation /* IETF draft style Version Negotiation Packet */ - // handle Version Negotiation Packets - if isVersionNegotiationPacket { + if hdr.IsVersionNegotiation { // ignore delayed / duplicated version negotiation packets if c.receivedVersionNegotiationPacket || c.versionNegotiated { return diff --git a/internal/protocol/protocol.go b/internal/protocol/protocol.go index 23330707..c098f8c5 100644 --- a/internal/protocol/protocol.go +++ b/internal/protocol/protocol.go @@ -28,8 +28,6 @@ const ( type PacketType uint8 const ( - // PacketTypeVersionNegotiation is the packet type of a Version Negotiation packet - PacketTypeVersionNegotiation PacketType = 1 // PacketTypeInitial is the packet type of a Initial packet PacketTypeInitial PacketType = 2 // PacketTypeRetry is the packet type of a Retry packet @@ -42,8 +40,6 @@ const ( func (t PacketType) String() string { switch t { - case PacketTypeVersionNegotiation: - return "Version Negotiation" case PacketTypeInitial: return "Initial" case PacketTypeRetry: diff --git a/internal/protocol/protocol_test.go b/internal/protocol/protocol_test.go index 27576184..a89f732f 100644 --- a/internal/protocol/protocol_test.go +++ b/internal/protocol/protocol_test.go @@ -8,7 +8,6 @@ import ( var _ = Describe("Protocol", func() { Context("Long Header Packet Types", func() { It("has the correct string representation", func() { - Expect(PacketTypeVersionNegotiation.String()).To(Equal("Version Negotiation")) Expect(PacketTypeInitial.String()).To(Equal("Initial")) Expect(PacketTypeRetry.String()).To(Equal("Retry")) Expect(PacketTypeHandshake.String()).To(Equal("Handshake")) diff --git a/internal/wire/header.go b/internal/wire/header.go index 96066cc9..19c45c3e 100644 --- a/internal/wire/header.go +++ b/internal/wire/header.go @@ -9,13 +9,15 @@ 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 { - Raw []byte - ConnectionID protocol.ConnectionID - OmitConnectionID bool - PacketNumberLen protocol.PacketNumberLen - PacketNumber protocol.PacketNumber - Version protocol.VersionNumber // VersionNumber sent by the client - SupportedVersions []protocol.VersionNumber // Version Number sent in a Version Negotiation Packet by the server + Raw []byte + ConnectionID protocol.ConnectionID + OmitConnectionID bool + PacketNumberLen protocol.PacketNumberLen + PacketNumber protocol.PacketNumber + Version protocol.VersionNumber // VersionNumber sent by the client + + IsVersionNegotiation bool + SupportedVersions []protocol.VersionNumber // Version Number sent in a Version Negotiation Packet by the server // only needed for the gQUIC Public Header VersionFlag bool @@ -40,14 +42,11 @@ func ParseHeaderSentByServer(b *bytes.Reader, version protocol.VersionNumber) (* _ = b.UnreadByte() // unread the type byte var isPublicHeader bool - // As a client, we know the version of the packet that the server sent, except for Version Negotiation Packets. - if typeByte == 0x81 { // IETF draft Version Negotiation Packet + 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 - // IETF QUIC Version Negotiation Packets are sent with the Long Header (indicated by the 0x80 bit) - // gQUIC always has 0x80 unset isPublicHeader = true - } else { // not a Version Negotiation Packet + } else { // the client knows the version that this packet was sent with isPublicHeader = !version.UsesTLS() } diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index d28c8907..8f3b0cd6 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -42,6 +42,7 @@ var _ = Describe("Header", func() { IsLongHeader: true, Type: protocol.PacketType0RTT, PacketNumber: 0x42, + Version: 0x1234, }).writeHeader(buf) Expect(err).ToNot(HaveOccurred()) hdr, err := ParseHeaderSentByClient(bytes.NewReader(buf.Bytes())) @@ -49,6 +50,7 @@ var _ = Describe("Header", func() { Expect(hdr.Type).To(Equal(protocol.PacketType0RTT)) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) Expect(hdr.isPublicHeader).To(BeFalse()) + Expect(hdr.Version).To(Equal(protocol.VersionNumber(0x1234))) }) It("doens't mistake packets with a Short Header for Version Negotiation Packets", func() { @@ -132,15 +134,14 @@ var _ = Describe("Header", func() { It("parses an IETF draft style Version Negotiation Packet", func() { versions := []protocol.VersionNumber{0x13, 0x37} - data := ComposeVersionNegotiation(0x42, 0x77, 0x4321, versions) + data := ComposeVersionNegotiation(0x42, 0x77, 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(Equal(protocol.VersionNumber(0x4321))) Expect(hdr.SupportedVersions).To(Equal(versions)) - Expect(hdr.Type).To(Equal(protocol.PacketTypeVersionNegotiation)) }) }) diff --git a/internal/wire/ietf_header.go b/internal/wire/ietf_header.go index c52128fa..429ada30 100644 --- a/internal/wire/ietf_header.go +++ b/internal/wire/ietf_header.go @@ -21,6 +21,7 @@ func parseHeader(b *bytes.Reader, packetSentBy protocol.Perspective) (*Header, e return parseShortHeader(b, typeByte) } +// parse long header and version negotiation packets func parseLongHeader(b *bytes.Reader, sentBy protocol.Perspective, typeByte byte) (*Header, error) { connID, err := utils.BigEndian.ReadUint64(b) if err != nil { @@ -34,28 +35,17 @@ func parseLongHeader(b *bytes.Reader, sentBy protocol.Perspective, typeByte byte if err != nil { return nil, err } - packetType := protocol.PacketType(typeByte & 0x7f) - if sentBy == protocol.PerspectiveClient && (packetType != protocol.PacketTypeInitial && packetType != protocol.PacketTypeHandshake && packetType != protocol.PacketType0RTT) { - if packetType == protocol.PacketTypeVersionNegotiation { - return nil, qerr.Error(qerr.InvalidVersionNegotiationPacket, "sent by the client") - } - return nil, qerr.Error(qerr.InvalidPacketHeader, fmt.Sprintf("Received packet with invalid packet type: %d", packetType)) - } - if sentBy == protocol.PerspectiveServer && (packetType != protocol.PacketTypeVersionNegotiation && packetType != protocol.PacketTypeRetry && packetType != protocol.PacketTypeHandshake) { - return nil, qerr.Error(qerr.InvalidPacketHeader, fmt.Sprintf("Received packet with invalid packet type: %d", packetType)) - } h := &Header{ - Type: packetType, - IsLongHeader: true, ConnectionID: protocol.ConnectionID(connID), PacketNumber: protocol.PacketNumber(pn), PacketNumberLen: protocol.PacketNumberLen4, Version: protocol.VersionNumber(v), } - if h.Type == protocol.PacketTypeVersionNegotiation { + if v == 0 { // version negotiation packet if b.Len() == 0 { return nil, qerr.Error(qerr.InvalidVersionNegotiationPacket, "empty version list") } + h.IsVersionNegotiation = true h.SupportedVersions = make([]protocol.VersionNumber, b.Len()/4) for i := 0; b.Len() > 0; i++ { v, err := utils.BigEndian.ReadUint32(b) @@ -64,6 +54,15 @@ func parseLongHeader(b *bytes.Reader, sentBy protocol.Perspective, typeByte byte } h.SupportedVersions[i] = protocol.VersionNumber(v) } + return h, nil + } + h.IsLongHeader = true + 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)) + } + if sentBy == protocol.PerspectiveServer && (h.Type != protocol.PacketTypeRetry && h.Type != protocol.PacketTypeHandshake) { + return nil, qerr.Error(qerr.InvalidPacketHeader, fmt.Sprintf("Received packet with invalid packet type: %d", h.Type)) } return h, nil } @@ -102,7 +101,7 @@ func (h *Header) writeHeader(b *bytes.Buffer) error { // TODO: add support for the key phase func (h *Header) writeLongHeader(b *bytes.Buffer) error { - b.WriteByte(byte(0x80 ^ h.Type)) + b.WriteByte(byte(0x80 | h.Type)) utils.BigEndian.WriteUint64(b, uint64(h.ConnectionID)) utils.BigEndian.WriteUint32(b, uint32(h.Version)) utils.BigEndian.WriteUint32(b, uint32(h.PacketNumber)) diff --git a/internal/wire/ietf_header_test.go b/internal/wire/ietf_header_test.go index 8b9b6320..f7531a2f 100644 --- a/internal/wire/ietf_header_test.go +++ b/internal/wire/ietf_header_test.go @@ -17,6 +17,36 @@ import ( var _ = Describe("IETF draft Header", func() { Context("parsing", func() { + Context("Version Negotiation Packets", func() { + It("parses", func() { + versions := []protocol.VersionNumber{0x22334455, 0x33445566} + data := ComposeVersionNegotiation(0x1234567890, 0x1337, 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))) + Expect(h.SupportedVersions).To(Equal(versions)) + }) + + It("errors if it contains versions of the wrong length", func() { + versions := []protocol.VersionNumber{0x22334455, 0x33445566} + data := ComposeVersionNegotiation(0x1234567890, 0x1337, versions) + b := bytes.NewReader(data[:len(data)-2]) + _, err := parseHeader(b, protocol.PerspectiveServer) + Expect(err).To(MatchError(qerr.InvalidVersionNegotiationPacket)) + }) + + It("errors if the version list is emtpy", func() { + versions := []protocol.VersionNumber{0x22334455} + data := ComposeVersionNegotiation(0x1234567890, 0x1337, versions) + _, err := parseHeader(bytes.NewReader(data[:len(data)-4]), protocol.PerspectiveServer) + Expect(err).To(MatchError("InvalidVersionNegotiationPacket: empty version list")) + }) + }) + Context("long headers", func() { generatePacket := func(t protocol.PacketType) []byte { return []byte{ @@ -38,6 +68,7 @@ var _ = Describe("IETF draft Header", func() { Expect(h.PacketNumber).To(Equal(protocol.PacketNumber(0xdecafbad))) Expect(h.PacketNumberLen).To(Equal(protocol.PacketNumberLen4)) Expect(h.Version).To(Equal(protocol.VersionNumber(0x1020304))) + Expect(h.IsVersionNegotiation).To(BeFalse()) Expect(b.Len()).To(BeZero()) }) @@ -66,52 +97,6 @@ var _ = Describe("IETF draft Header", func() { Expect(err).To(Equal(io.EOF)) } }) - - Context("Version Negotiation Packets", func() { - It("parses", func() { - data := append( - generatePacket(protocol.PacketTypeVersionNegotiation), - []byte{ - 0x22, 0x33, 0x44, 0x55, - 0x33, 0x44, 0x55, 0x66, - }..., - ) - b := bytes.NewReader(data) - h, err := parseHeader(b, protocol.PerspectiveServer) - Expect(err).ToNot(HaveOccurred()) - Expect(h.Type).To(Equal(protocol.PacketTypeVersionNegotiation)) - Expect(h.SupportedVersions).To(Equal([]protocol.VersionNumber{ - 0x22334455, - 0x33445566, - })) - }) - - It("errors if it contains versions of the wrong length", func() { - data := append( - generatePacket(protocol.PacketTypeVersionNegotiation), - []byte{0x22, 0x33}..., // too short. Should be 4 bytes. - ) - b := bytes.NewReader(data) - _, err := parseHeader(b, protocol.PerspectiveServer) - Expect(err).To(MatchError(qerr.InvalidVersionNegotiationPacket)) - }) - - It("errors if it was sent by the client", func() { - data := append( - generatePacket(protocol.PacketTypeVersionNegotiation), - []byte{0x22, 0x33, 0x44, 0x55}..., - ) - b := bytes.NewReader(data) - _, err := parseHeader(b, protocol.PerspectiveClient) - Expect(err).To(MatchError("InvalidVersionNegotiationPacket: sent by the client")) - }) - - It("errors if the version list is emtpy", func() { - b := bytes.NewReader(generatePacket(protocol.PacketTypeVersionNegotiation)) - _, err := parseHeader(b, protocol.PerspectiveServer) - Expect(err).To(MatchError("InvalidVersionNegotiationPacket: empty version list")) - }) - }) }) Context("short headers", func() { @@ -129,6 +114,7 @@ var _ = Describe("IETF draft Header", func() { Expect(h.OmitConnectionID).To(BeFalse()) Expect(h.ConnectionID).To(Equal(protocol.ConnectionID(0xdeadbeefcafe1337))) Expect(h.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) + Expect(h.IsVersionNegotiation).To(BeFalse()) Expect(b.Len()).To(BeZero()) }) diff --git a/internal/wire/public_header.go b/internal/wire/public_header.go index ba5c8e69..93ebbcce 100644 --- a/internal/wire/public_header.go +++ b/internal/wire/public_header.go @@ -155,6 +155,7 @@ func parsePublicHeader(b *bytes.Reader, packetSentBy protocol.Perspective) (*Hea if b.Len()%4 != 0 { return nil, qerr.InvalidVersionNegotiationPacket } + header.IsVersionNegotiation = true header.SupportedVersions = make([]protocol.VersionNumber, 0) for { var versionTag uint32 diff --git a/internal/wire/public_header_test.go b/internal/wire/public_header_test.go index 55f50750..75f07ada 100644 --- a/internal/wire/public_header_test.go +++ b/internal/wire/public_header_test.go @@ -22,6 +22,7 @@ var _ = Describe("Public Header", func() { hdr, err := parsePublicHeader(b, protocol.PerspectiveClient) Expect(err).ToNot(HaveOccurred()) Expect(hdr.VersionFlag).To(BeTrue()) + Expect(hdr.IsVersionNegotiation).To(BeFalse()) Expect(hdr.ResetFlag).To(BeFalse()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x4cfa9f9b668619f6))) Expect(hdr.Version).To(Equal(protocol.SupportedVersions[0])) @@ -65,6 +66,7 @@ var _ = Describe("Public Header", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.ResetFlag).To(BeTrue()) Expect(hdr.VersionFlag).To(BeFalse()) + Expect(hdr.IsVersionNegotiation).To(BeFalse()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x0102030405060708))) }) @@ -101,6 +103,7 @@ var _ = Describe("Public Header", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.VersionFlag).To(BeTrue()) Expect(hdr.Version).To(BeZero()) // unitialized + Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.SupportedVersions).To(Equal(protocol.SupportedVersions)) Expect(b.Len()).To(BeZero()) }) @@ -120,6 +123,7 @@ var _ = Describe("Public Header", func() { hdr, err := parsePublicHeader(b, protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) Expect(hdr.VersionFlag).To(BeTrue()) + Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.SupportedVersions).To(Equal([]protocol.VersionNumber{1, protocol.SupportedVersions[0], 99})) Expect(b.Len()).To(BeZero()) }) diff --git a/internal/wire/version_negotiation.go b/internal/wire/version_negotiation.go index 92afb3b4..29ee603a 100644 --- a/internal/wire/version_negotiation.go +++ b/internal/wire/version_negotiation.go @@ -2,6 +2,7 @@ package wire import ( "bytes" + "crypto/rand" "github.com/lucas-clemente/quic-go/internal/protocol" "github.com/lucas-clemente/quic-go/internal/utils" @@ -11,9 +12,10 @@ import ( func ComposeGQUICVersionNegotiation(connID protocol.ConnectionID, versions []protocol.VersionNumber) []byte { fullReply := &bytes.Buffer{} ph := Header{ - ConnectionID: connID, - PacketNumber: 1, - VersionFlag: true, + ConnectionID: connID, + PacketNumber: 1, + VersionFlag: true, + IsVersionNegotiation: true, } if err := ph.writePublicHeader(fullReply, protocol.PerspectiveServer, protocol.VersionWhatever); err != nil { utils.Errorf("error composing version negotiation packet: %s", err.Error()) @@ -29,18 +31,20 @@ func ComposeGQUICVersionNegotiation(connID protocol.ConnectionID, versions []pro func ComposeVersionNegotiation( connID protocol.ConnectionID, pn protocol.PacketNumber, - versionOffered protocol.VersionNumber, versions []protocol.VersionNumber, ) []byte { fullReply := &bytes.Buffer{} - ph := Header{ - IsLongHeader: true, - Type: protocol.PacketTypeVersionNegotiation, - ConnectionID: connID, - PacketNumber: pn, - Version: versionOffered, + 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 := ph.writeHeader(fullReply); err != nil { + if err := h.writeHeader(fullReply); err != nil { utils.Errorf("error composing version negotiation packet: %s", err.Error()) return nil } diff --git a/internal/wire/version_negotiation_test.go b/internal/wire/version_negotiation_test.go index 650c2903..ca69fb8e 100644 --- a/internal/wire/version_negotiation_test.go +++ b/internal/wire/version_negotiation_test.go @@ -19,15 +19,14 @@ var _ = Describe("Version Negotiation Packets", func() { Expect(hdr.SupportedVersions).To(Equal(versions)) }) - It("writes IETF draft style", func() { + It("writes in IETF draft style", func() { versions := []protocol.VersionNumber{1001, 1003} - data := ComposeVersionNegotiation(0x1337, 0x42, 0x1234, versions) + data := ComposeVersionNegotiation(0x1337, 0x42, versions) hdr, err := parseHeader(bytes.NewReader(data), protocol.PerspectiveServer) Expect(err).ToNot(HaveOccurred()) - Expect(hdr.Type).To(Equal(protocol.PacketTypeVersionNegotiation)) + 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(Equal(protocol.VersionNumber(0x1234))) Expect(hdr.SupportedVersions).To(Equal(versions)) }) }) diff --git a/server.go b/server.go index fb73ccb2..10c55805 100644 --- a/server.go +++ b/server.go @@ -280,7 +280,7 @@ func (s *server) handlePacket(pconn net.PacketConn, remoteAddr net.Addr, packet } // send an IETF draft style Version Negotiation Packet, if the client sent an unsupported version with an IETF draft style header if hdr.Type == protocol.PacketTypeInitial && !protocol.IsSupportedVersion(s.config.Versions, hdr.Version) { - _, err := pconn.WriteTo(wire.ComposeVersionNegotiation(hdr.ConnectionID, hdr.PacketNumber, hdr.Version, s.config.Versions), remoteAddr) + _, err := pconn.WriteTo(wire.ComposeVersionNegotiation(hdr.ConnectionID, hdr.PacketNumber, s.config.Versions), remoteAddr) return err } diff --git a/server_test.go b/server_test.go index 64869d6a..252507f5 100644 --- a/server_test.go +++ b/server_test.go @@ -438,6 +438,7 @@ var _ = Describe("Server", func() { IsLongHeader: true, ConnectionID: 0x1337, PacketNumber: 0x55, + Version: 0x1234, } hdr.Write(b, protocol.PerspectiveClient, protocol.VersionTLS) b.Write(bytes.Repeat([]byte{0}, protocol.ClientHelloMinimumSize)) // add a fake CHLO @@ -457,7 +458,7 @@ var _ = Describe("Server", func() { r := bytes.NewReader(conn.dataWritten.Bytes()) packet, err := wire.ParseHeaderSentByServer(r, protocol.VersionUnknown) Expect(err).ToNot(HaveOccurred()) - Expect(packet.Type).To(Equal(protocol.PacketTypeVersionNegotiation)) + 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())