From 1f5cd31569f96b1520df5a317a9455f4a9c2e71c Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 3 Oct 2017 16:03:31 -0700 Subject: [PATCH 1/4] implement a function to get version slices containing reserved versions --- internal/protocol/version.go | 23 +++++++++++++- internal/protocol/version_test.go | 52 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/internal/protocol/version.go b/internal/protocol/version.go index 5ad04f006..600b508b2 100644 --- a/internal/protocol/version.go +++ b/internal/protocol/version.go @@ -1,11 +1,13 @@ package protocol import ( + "crypto/rand" + "encoding/binary" "fmt" ) // VersionNumber is a version number as int -type VersionNumber int +type VersionNumber int32 // gQUIC version range as defined in the wiki: https://github.com/quicwg/base-drafts/wiki/QUIC-Versions const ( @@ -112,3 +114,22 @@ func ChooseSupportedVersion(ours, theirs []VersionNumber) (VersionNumber, bool) } return 0, false } + +// generateReservedVersion generates a reserved version number (v & 0x0f0f0f0f == 0x0a0a0a0a) +func generateReservedVersion() VersionNumber { + b := make([]byte, 4) + _, _ = rand.Read(b) // ignore the error here. Failure to read random data doesn't break anything + return VersionNumber((binary.BigEndian.Uint32(b) | 0x0a0a0a0a) & 0xfafafafa) +} + +// GetGreasedVersions adds one reserved version number to a slice of version numbers, at a random position +func GetGreasedVersions(supported []VersionNumber) []VersionNumber { + b := make([]byte, 1) + _, _ = rand.Read(b) // ignore the error here. Failure to read random data doesn't break anything + randPos := int(b[0]) % (len(supported) + 1) + greased := make([]VersionNumber, len(supported)+1) + copy(greased, supported[:randPos]) + greased[randPos] = generateReservedVersion() + copy(greased[randPos+1:], supported[randPos:]) + return greased +} diff --git a/internal/protocol/version_test.go b/internal/protocol/version_test.go index c69888dd3..67d600abc 100644 --- a/internal/protocol/version_test.go +++ b/internal/protocol/version_test.go @@ -6,6 +6,10 @@ import ( ) var _ = Describe("Version", func() { + isReservedVersion := func(v VersionNumber) bool { + return v&0x0f0f0f0f == 0x0a0a0a0a + } + // version numbers taken from the wiki: https://github.com/quicwg/base-drafts/wiki/QUIC-Versions It("has the right gQUIC version number", func() { Expect(Version39).To(BeEquivalentTo(0x51303339)) @@ -16,6 +20,11 @@ var _ = Describe("Version", func() { Expect(VersionTLS.UsesTLS()).To(BeTrue()) }) + It("versions don't have reserved version numbers", func() { + Expect(isReservedVersion(Version39)).To(BeFalse()) + Expect(isReservedVersion(VersionTLS)).To(BeFalse()) + }) + It("has the right string representation", func() { Expect(Version39.String()).To(Equal("gQUIC 39")) Expect(VersionTLS.String()).To(ContainSubstring("TLS")) @@ -105,4 +114,47 @@ var _ = Describe("Version", func() { Expect(ok).To(BeFalse()) }) }) + + Context("reserved versions", func() { + It("adds a greased version if passed an empty slice", func() { + greased := GetGreasedVersions([]VersionNumber{}) + Expect(greased).To(HaveLen(1)) + Expect(isReservedVersion(greased[0])).To(BeTrue()) + }) + + It("creates greased lists of version numbers", func() { + supported := []VersionNumber{10, 18, 29} + for _, v := range supported { + Expect(isReservedVersion(v)).To(BeFalse()) + } + var greasedVersionFirst, greasedVersionLast, greasedVersionMiddle int + // check that + // 1. the greased version sometimes appears first + // 2. the greased version sometimes appears in the middle + // 3. the greased version sometimes appears last + // 4. the supported versions are kept in order + for i := 0; i < 100; i++ { + greased := GetGreasedVersions(supported) + Expect(greased).To(HaveLen(4)) + var j int + for i, v := range greased { + if isReservedVersion(v) { + if i == 0 { + greasedVersionFirst++ + } + if i == len(greased)-1 { + greasedVersionLast++ + } + greasedVersionMiddle++ + continue + } + Expect(supported[j]).To(Equal(v)) + j++ + } + } + Expect(greasedVersionFirst).ToNot(BeZero()) + Expect(greasedVersionLast).ToNot(BeZero()) + Expect(greasedVersionMiddle).ToNot(BeZero()) + }) + }) }) From be299636378fd62066542ab1c06711093e50b552 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 30 Nov 2017 11:54:49 +0700 Subject: [PATCH 2/4] send a reserved version number in version negotiation packets --- client_test.go | 2 +- internal/wire/header_test.go | 11 +++++++++-- internal/wire/ietf_header_test.go | 7 +++++-- internal/wire/public_header_test.go | 10 +++++++--- internal/wire/version_negotiation.go | 16 ++++++++++------ internal/wire/version_negotiation_test.go | 13 +++++++++++-- 6 files changed, 43 insertions(+), 16 deletions(-) diff --git a/client_test.go b/client_test.go index 60b2546d1..51518a71a 100644 --- a/client_test.go +++ b/client_test.go @@ -369,7 +369,7 @@ var _ = Describe("Client", func() { Expect(firstSession.closeReason).To(Equal(errCloseSessionForNewVersion)) Consistently(func() bool { return secondSession.closed }).Should(BeFalse()) Expect(cl.connectionID).ToNot(BeEquivalentTo(0x1337)) - Expect(negotiatedVersions).To(Equal([]protocol.VersionNumber{newVersion})) + Expect(negotiatedVersions).To(ContainElement(newVersion)) Expect(initialVersion).To(Equal(actualInitialVersion)) handshakeChan <- handshakeEvent{encLevel: protocol.EncryptionSecure} diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index 8f3b0cd6e..4d23173f9 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -129,7 +129,10 @@ var _ = Describe("Header", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.isPublicHeader).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x42))) - Expect(hdr.SupportedVersions).To(Equal(versions)) + // in addition to the versions, the supported versions might contain a reserved version number + for _, version := range versions { + Expect(hdr.SupportedVersions).To(ContainElement(version)) + } }) It("parses an IETF draft style Version Negotiation Packet", func() { @@ -141,7 +144,11 @@ var _ = Describe("Header", func() { Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x42))) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x77))) - Expect(hdr.SupportedVersions).To(Equal(versions)) + Expect(hdr.Version).To(BeZero()) + // in addition to the versions, the supported versions might contain a reserved version number + for _, version := range versions { + Expect(hdr.SupportedVersions).To(ContainElement(version)) + } }) }) diff --git a/internal/wire/ietf_header_test.go b/internal/wire/ietf_header_test.go index f7531a2f8..c37937fa3 100644 --- a/internal/wire/ietf_header_test.go +++ b/internal/wire/ietf_header_test.go @@ -28,7 +28,9 @@ var _ = Describe("IETF draft Header", func() { 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)) + for _, v := range versions { + Expect(h.SupportedVersions).To(ContainElement(v)) + } }) It("errors if it contains versions of the wrong length", func() { @@ -42,7 +44,8 @@ var _ = Describe("IETF draft Header", func() { 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) + // 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/public_header_test.go b/internal/wire/public_header_test.go index 75f07ada4..7edaceef0 100644 --- a/internal/wire/public_header_test.go +++ b/internal/wire/public_header_test.go @@ -97,14 +97,18 @@ var _ = Describe("Public Header", func() { return data } - It("parses version negotiation packets sent by the server", func() { - b := bytes.NewReader(ComposeGQUICVersionNegotiation(0x1337, protocol.SupportedVersions)) + It("parses", func() { + versions := []protocol.VersionNumber{0x13, 0x37} + b := bytes.NewReader(ComposeGQUICVersionNegotiation(0x1337, versions)) hdr, err := parsePublicHeader(b, protocol.PerspectiveServer) 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)) + // in addition to the versions, the supported versions might contain a reserved version number + for _, version := range versions { + Expect(hdr.SupportedVersions).To(ContainElement(version)) + } Expect(b.Len()).To(BeZero()) }) diff --git a/internal/wire/version_negotiation.go b/internal/wire/version_negotiation.go index 29ee603a2..b20c43c2d 100644 --- a/internal/wire/version_negotiation.go +++ b/internal/wire/version_negotiation.go @@ -21,9 +21,7 @@ func ComposeGQUICVersionNegotiation(connID protocol.ConnectionID, versions []pro utils.Errorf("error composing version negotiation packet: %s", err.Error()) return nil } - for _, v := range versions { - utils.BigEndian.WriteUint32(fullReply, uint32(v)) - } + writeVersions(fullReply, versions) return fullReply.Bytes() } @@ -48,8 +46,14 @@ func ComposeVersionNegotiation( utils.Errorf("error composing version negotiation packet: %s", err.Error()) return nil } - for _, v := range versions { - utils.BigEndian.WriteUint32(fullReply, uint32(v)) - } + 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) { + utils.BigEndian.WriteUint32(buf, uint32(v)) + } +} diff --git a/internal/wire/version_negotiation_test.go b/internal/wire/version_negotiation_test.go index ca69fb8ec..5bd1a8099 100644 --- a/internal/wire/version_negotiation_test.go +++ b/internal/wire/version_negotiation_test.go @@ -16,7 +16,11 @@ var _ = Describe("Version Negotiation Packets", func() { Expect(err).ToNot(HaveOccurred()) Expect(hdr.VersionFlag).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x1337))) - Expect(hdr.SupportedVersions).To(Equal(versions)) + // the supported versions should include one reserved version number + Expect(hdr.SupportedVersions).To(HaveLen(len(versions) + 1)) + for _, version := range versions { + Expect(hdr.SupportedVersions).To(ContainElement(version)) + } }) It("writes in IETF draft style", func() { @@ -27,6 +31,11 @@ var _ = Describe("Version Negotiation Packets", func() { Expect(hdr.IsVersionNegotiation).To(BeTrue()) Expect(hdr.ConnectionID).To(Equal(protocol.ConnectionID(0x1337))) Expect(hdr.PacketNumber).To(Equal(protocol.PacketNumber(0x42))) - Expect(hdr.SupportedVersions).To(Equal(versions)) + Expect(hdr.Version).To(BeZero()) + // the supported versions should include one reserved version number + Expect(hdr.SupportedVersions).To(HaveLen(len(versions) + 1)) + for _, version := range versions { + Expect(hdr.SupportedVersions).To(ContainElement(version)) + } }) }) From 1a3852aec60cc7b3def1f5b95a17b9a3bd8d75ca Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 30 Nov 2017 12:45:55 +0700 Subject: [PATCH 3/4] send a reserved version number in the EncryptedExtensions message --- internal/handshake/tls_extension_handler_server.go | 9 +++++---- internal/handshake/tls_extension_handler_server_test.go | 9 +++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/handshake/tls_extension_handler_server.go b/internal/handshake/tls_extension_handler_server.go index 0324f8526..d8fda2a51 100644 --- a/internal/handshake/tls_extension_handler_server.go +++ b/internal/handshake/tls_extension_handler_server.go @@ -47,13 +47,14 @@ func (h *extensionHandlerServer) Send(hType mint.HandshakeType, el *mint.Extensi // TODO(#855): generate a real token transportParameter{statelessResetTokenParameterID, bytes.Repeat([]byte{42}, 16)}, ) - supportedVersions := make([]uint32, len(h.supportedVersions)) - for i, v := range h.supportedVersions { - supportedVersions[i] = uint32(v) + supportedVersions := protocol.GetGreasedVersions(h.supportedVersions) + versions := make([]uint32, len(supportedVersions)) + for i, v := range supportedVersions { + versions[i] = uint32(v) } data, err := syntax.Marshal(encryptedExtensionsTransportParameters{ NegotiatedVersion: uint32(h.version), - SupportedVersions: supportedVersions, + SupportedVersions: versions, Parameters: transportParams, }) if err != nil { diff --git a/internal/handshake/tls_extension_handler_server_test.go b/internal/handshake/tls_extension_handler_server_test.go index fa52ebad2..41a651618 100644 --- a/internal/handshake/tls_extension_handler_server_test.go +++ b/internal/handshake/tls_extension_handler_server_test.go @@ -45,7 +45,8 @@ var _ = Describe("TLS Extension Handler, for the server", func() { It("adds TransportParameters to the EncryptedExtensions message", func() { handler.version = 666 - handler.supportedVersions = []protocol.VersionNumber{13, 37, 42} + versions := []protocol.VersionNumber{13, 37, 42} + handler.supportedVersions = versions err := handler.Send(mint.HandshakeTypeEncryptedExtensions, &el) Expect(err).ToNot(HaveOccurred()) Expect(el).To(HaveLen(1)) @@ -55,8 +56,12 @@ var _ = Describe("TLS Extension Handler, for the server", func() { eetp := &encryptedExtensionsTransportParameters{} _, err = syntax.Unmarshal(ext.data, eetp) Expect(err).ToNot(HaveOccurred()) - Expect(eetp.SupportedVersions).To(Equal([]uint32{13, 37, 42})) Expect(eetp.NegotiatedVersion).To(BeEquivalentTo(666)) + // the SupportedVersions will contain one reserved version number + Expect(eetp.SupportedVersions).To(HaveLen(len(versions) + 1)) + for _, version := range versions { + Expect(eetp.SupportedVersions).To(ContainElement(uint32(version))) + } }) }) From 8c2404edf53b7967b93ce81fe28c4d401a6734b8 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 30 Nov 2017 12:59:53 +0700 Subject: [PATCH 4/4] send a reserved version number in the SHLO --- internal/handshake/crypto_setup_server.go | 2 +- internal/handshake/crypto_setup_server_test.go | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/handshake/crypto_setup_server.go b/internal/handshake/crypto_setup_server.go index 50e261831..ddaf75e93 100644 --- a/internal/handshake/crypto_setup_server.go +++ b/internal/handshake/crypto_setup_server.go @@ -431,7 +431,7 @@ func (h *cryptoSetupServer) handleCHLO(sni string, data []byte, cryptoData map[T replyMap := h.params.getHelloMap() // add crypto parameters verTag := &bytes.Buffer{} - for _, v := range h.supportedVersions { + for _, v := range protocol.GetGreasedVersions(h.supportedVersions) { utils.BigEndian.WriteUint32(verTag, uint32(v)) } replyMap[TagPUBS] = ephermalKex.PublicKey() diff --git a/internal/handshake/crypto_setup_server_test.go b/internal/handshake/crypto_setup_server_test.go index df57b8711..77ec1cef3 100644 --- a/internal/handshake/crypto_setup_server_test.go +++ b/internal/handshake/crypto_setup_server_test.go @@ -313,12 +313,17 @@ var _ = Describe("Server Crypto Setup", func() { }) Expect(err).ToNot(HaveOccurred()) Expect(response).To(HavePrefix("SHLO")) - Expect(response).To(ContainSubstring("ephermal pub")) - Expect(response).To(ContainSubstring("SNO\x00")) + message, err := ParseHandshakeMessage(bytes.NewReader(response)) + Expect(err).ToNot(HaveOccurred()) + Expect(message.Data).To(HaveKeyWithValue(TagPUBS, []byte("ephermal pub"))) + Expect(message.Data).To(HaveKey(TagSNO)) + Expect(message.Data).To(HaveKey(TagVER)) + // the supported versions should include one reserved version number + Expect(message.Data[TagVER]).To(HaveLen(4*len(supportedVersions) + 4)) for _, v := range supportedVersions { b := &bytes.Buffer{} utils.BigEndian.WriteUint32(b, uint32(v)) - Expect(response).To(ContainSubstring(string(b.Bytes()))) + Expect(message.Data[TagVER]).To(ContainSubstring(string(b.Bytes()))) } Expect(checkedSecure).To(BeTrue()) Expect(checkedForwardSecure).To(BeTrue())