diff --git a/fuzzing/header/main.go b/fuzzing/header/main.go index 41af5a00..13dff303 100644 --- a/fuzzing/header/main.go +++ b/fuzzing/header/main.go @@ -83,15 +83,6 @@ func main() { Length: protocol.ByteCount(rand.Intn(1000)), Version: version, }, - wire.Header{ // Retry Packet - IsLongHeader: true, - SrcConnectionID: protocol.ConnectionID(getRandomData(8)), - DestConnectionID: protocol.ConnectionID(getRandomData(9)), - OrigDestConnectionID: protocol.ConnectionID(getRandomData(10)), - Type: protocol.PacketTypeRetry, - Token: getRandomData(10), - Version: version, - }, wire.Header{ // Retry Packet, with empty orig dest conn id IsLongHeader: true, SrcConnectionID: protocol.ConnectionID(getRandomData(8)), @@ -100,14 +91,6 @@ func main() { Token: getRandomData(1000), Version: version, }, - wire.Header{ // Retry Packet, with zero-length dest conn id - IsLongHeader: true, - SrcConnectionID: protocol.ConnectionID(getRandomData(8)), - OrigDestConnectionID: protocol.ConnectionID(getRandomData(10)), - Type: protocol.PacketTypeRetry, - Token: getRandomData(1000), - Version: version, - }, wire.Header{ // Short-Header DestConnectionID: protocol.ConnectionID(getRandomData(8)), }, @@ -123,6 +106,9 @@ func main() { if err := extHdr.Write(b, version); err != nil { panic(err) } + if h.Type == protocol.PacketTypeRetry { + b.Write([]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}) + } if h.Length > 0 { b.Write(make([]byte, h.Length)) } diff --git a/internal/testutils/testutils.go b/internal/testutils/testutils.go index a0227c65..743d062a 100644 --- a/internal/testutils/testutils.go +++ b/internal/testutils/testutils.go @@ -110,17 +110,23 @@ func ComposeInitialPacket(srcConnID protocol.ConnectionID, destConnID protocol.C } // ComposeRetryPacket returns a new raw Retry Packet -func ComposeRetryPacket(srcConnID protocol.ConnectionID, destConnID protocol.ConnectionID, origDestConnID protocol.ConnectionID, token []byte, version protocol.VersionNumber) []byte { +func ComposeRetryPacket( + srcConnID protocol.ConnectionID, + destConnID protocol.ConnectionID, + origDestConnID protocol.ConnectionID, + token []byte, + version protocol.VersionNumber, +) []byte { hdr := &wire.ExtendedHeader{ Header: wire.Header{ - IsLongHeader: true, - Type: protocol.PacketTypeRetry, - SrcConnectionID: srcConnID, - DestConnectionID: destConnID, - OrigDestConnectionID: origDestConnID, - Token: token, - Version: version, + IsLongHeader: true, + Type: protocol.PacketTypeRetry, + SrcConnectionID: srcConnID, + DestConnectionID: destConnID, + Token: token, + Version: version, }, } - return writePacket(hdr, nil) + data := writePacket(hdr, nil) + return append(data, handshake.GetRetryIntegrityTag(data, origDestConnID)[:]...) } diff --git a/internal/wire/extended_header.go b/internal/wire/extended_header.go index 472be992..26aa453a 100644 --- a/internal/wire/extended_header.go +++ b/internal/wire/extended_header.go @@ -120,9 +120,6 @@ func (h *ExtendedHeader) Write(b *bytes.Buffer, ver protocol.VersionNumber) erro if h.SrcConnectionID.Len() > protocol.MaxConnIDLen { return fmt.Errorf("invalid connection ID length: %d bytes", h.SrcConnectionID.Len()) } - if h.OrigDestConnectionID.Len() > protocol.MaxConnIDLen { - return fmt.Errorf("invalid connection ID length: %d bytes", h.OrigDestConnectionID.Len()) - } if h.IsLongHeader { return h.writeLongHeader(b, ver) } @@ -156,8 +153,6 @@ func (h *ExtendedHeader) writeLongHeader(b *bytes.Buffer, _ protocol.VersionNumb switch h.Type { case protocol.PacketTypeRetry: - b.WriteByte(uint8(h.OrigDestConnectionID.Len())) - b.Write(h.OrigDestConnectionID.Bytes()) b.Write(h.Token) return nil case protocol.PacketTypeInitial: @@ -227,7 +222,7 @@ func (h *ExtendedHeader) Log(logger utils.Logger) { token = fmt.Sprintf("Token: %#x, ", h.Token) } if h.Type == protocol.PacketTypeRetry { - logger.Debugf("\tLong Header{Type: %s, DestConnectionID: %s, SrcConnectionID: %s, %sOrigDestConnectionID: %s, Version: %s}", h.Type, h.DestConnectionID, h.SrcConnectionID, token, h.OrigDestConnectionID, h.Version) + logger.Debugf("\tLong Header{Type: %s, DestConnectionID: %s, SrcConnectionID: %s, %sVersion: %s}", h.Type, h.DestConnectionID, h.SrcConnectionID, token, h.Version) return } } diff --git a/internal/wire/extended_header_test.go b/internal/wire/extended_header_test.go index e4a0b201..442452e9 100644 --- a/internal/wire/extended_header_test.go +++ b/internal/wire/extended_header_test.go @@ -100,34 +100,20 @@ var _ = Describe("Header", func() { It("writes a Retry packet", func() { token := []byte("Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.") Expect((&ExtendedHeader{Header: Header{ - IsLongHeader: true, - Version: 0x1020304, - Type: protocol.PacketTypeRetry, - Token: token, - OrigDestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8, 9}, + IsLongHeader: true, + Version: 0x1020304, + Type: protocol.PacketTypeRetry, + Token: token, }}).Write(buf, versionIETFHeader)).To(Succeed()) expected := []byte{ 0xc0 | 0x3<<4, 0x1, 0x2, 0x3, 0x4, // version number - 0x0, // dest connection ID length - 0x0, // src connection ID length - 0x9, // orig dest connection ID length - 1, 2, 3, 4, 5, 6, 7, 8, 9, // Orig Dest Connection ID + 0x0, // dest connection ID length + 0x0, // src connection ID length } expected = append(expected, token...) Expect(buf.Bytes()).To(Equal(expected)) }) - - It("refuses to write a Retry packet with an invalid Orig Destination Connection ID length", func() { - err := (&ExtendedHeader{Header: Header{ - IsLongHeader: true, - Version: 0x1020304, - Type: protocol.PacketTypeRetry, - Token: []byte("foobar"), - OrigDestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21}, // connection IDs must be at most 20 bytes long - }}).Write(buf, versionIETFHeader) - Expect(err).To(MatchError("invalid connection ID length: 21 bytes")) - }) }) Context("short header", func() { @@ -359,7 +345,7 @@ var _ = Describe("Header", func() { Expect(buf.String()).To(ContainSubstring("Long Header{Type: Initial, DestConnectionID: 0xcafe1337, SrcConnectionID: 0xdecafbad, Token: 0xdeadbeef, PacketNumber: 0x42, PacketNumberLen: 2, Length: 100, Version: 0xfeed}")) }) - It("logs Initial Packets without a Token", func() { + It("logs Initial packets without a Token", func() { (&ExtendedHeader{ Header: Header{ IsLongHeader: true, @@ -375,19 +361,18 @@ var _ = Describe("Header", func() { Expect(buf.String()).To(ContainSubstring("Long Header{Type: Initial, DestConnectionID: 0xcafe1337, SrcConnectionID: 0xdecafbad, Token: (empty), PacketNumber: 0x42, PacketNumberLen: 2, Length: 100, Version: 0xfeed}")) }) - It("logs Initial Packets with a Token", func() { + It("logs Retry packets with a Token", func() { (&ExtendedHeader{ Header: Header{ - IsLongHeader: true, - DestConnectionID: protocol.ConnectionID{0xca, 0xfe, 0x13, 0x37}, - SrcConnectionID: protocol.ConnectionID{0xde, 0xca, 0xfb, 0xad}, - Type: protocol.PacketTypeRetry, - OrigDestConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, - Token: []byte{0x12, 0x34, 0x56}, - Version: 0xfeed, + IsLongHeader: true, + DestConnectionID: protocol.ConnectionID{0xca, 0xfe, 0x13, 0x37}, + SrcConnectionID: protocol.ConnectionID{0xde, 0xca, 0xfb, 0xad}, + Type: protocol.PacketTypeRetry, + Token: []byte{0x12, 0x34, 0x56}, + Version: 0xfeed, }, }).Log(logger) - Expect(buf.String()).To(ContainSubstring("Long Header{Type: Retry, DestConnectionID: 0xcafe1337, SrcConnectionID: 0xdecafbad, Token: 0x123456, OrigDestConnectionID: 0xdeadbeef, Version: 0xfeed}")) + Expect(buf.String()).To(ContainSubstring("Long Header{Type: Retry, DestConnectionID: 0xcafe1337, SrcConnectionID: 0xdecafbad, Token: 0x123456, Version: 0xfeed}")) }) It("logs Short Headers containing a connection ID", func() { diff --git a/internal/wire/header.go b/internal/wire/header.go index 6d5e8a23..8c68b891 100644 --- a/internal/wire/header.go +++ b/internal/wire/header.go @@ -56,9 +56,8 @@ type Header struct { Length protocol.ByteCount - Token []byte - SupportedVersions []protocol.VersionNumber // sent in a Version Negotiation Packet - OrigDestConnectionID protocol.ConnectionID // sent in the Retry packet + Token []byte + SupportedVersions []protocol.VersionNumber // sent in a Version Negotiation Packet parsedLen protocol.ByteCount // how many bytes were read while parsing this header } @@ -176,19 +175,16 @@ func (h *Header) parseLongHeader(b *bytes.Reader) error { } if h.Type == protocol.PacketTypeRetry { - origDestConnIDLen, err := b.ReadByte() - if err != nil { - return err + tokenLen := b.Len() - 16 + if tokenLen <= 0 { + return io.EOF } - h.OrigDestConnectionID, err = protocol.ReadConnectionID(b, int(origDestConnIDLen)) - if err != nil { - return err - } - h.Token = make([]byte, b.Len()) + h.Token = make([]byte, tokenLen) if _, err := io.ReadFull(b, h.Token); err != nil { return err } - return nil + _, err := b.Seek(16, io.SeekCurrent) + return err } if h.Type == protocol.PacketTypeInitial { diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index 36bb6104..48c0ea7a 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -271,19 +271,33 @@ var _ = Describe("Header Parsing", func() { It("parses a Retry packet", func() { data := []byte{0xc0 | 0x3<<4 | (10 - 3) /* connection ID length */} data = appendVersion(data, versionIETFFrames) - data = append(data, []byte{0x0, 0x0}...) // dest and src conn ID lengths - data = append(data, 0xa) // orig dest conn ID len + data = append(data, []byte{6}...) // dest conn ID len + data = append(data, []byte{6, 5, 4, 3, 2, 1}...) // dest conn ID + data = append(data, []byte{10}...) // src conn ID len data = append(data, []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}...) // source connection ID data = append(data, []byte{'f', 'o', 'o', 'b', 'a', 'r'}...) // token + data = append(data, []byte{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}...) hdr, pdata, rest, err := ParsePacket(data, 0) Expect(err).ToNot(HaveOccurred()) Expect(hdr.Type).To(Equal(protocol.PacketTypeRetry)) - Expect(hdr.OrigDestConnectionID).To(Equal(protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10})) + Expect(hdr.DestConnectionID).To(Equal(protocol.ConnectionID{6, 5, 4, 3, 2, 1})) + Expect(hdr.SrcConnectionID).To(Equal(protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10})) Expect(hdr.Token).To(Equal([]byte("foobar"))) Expect(pdata).To(Equal(data)) Expect(rest).To(BeEmpty()) }) + It("errors if the Retry packet is too short for the integrity tag", func() { + data := []byte{0xc0 | 0x3<<4 | (10 - 3) /* connection ID length */} + data = appendVersion(data, versionIETFFrames) + data = append(data, []byte{0, 0}...) // conn ID lens + data = append(data, []byte{'f', 'o', 'o', 'b', 'a', 'r'}...) // token + data = append(data, []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}...) + // this results in a token length of 0 + _, _, _, err := ParsePacket(data, 0) + Expect(err).To(MatchError(io.EOF)) + }) + It("errors if the token length is too large", func() { data := []byte{0xc0 ^ 0x1} data = appendVersion(data, versionIETFFrames) diff --git a/server.go b/server.go index 7638d037..93ebf755 100644 --- a/server.go +++ b/server.go @@ -508,7 +508,6 @@ func (s *baseServer) sendRetry(remoteAddr net.Addr, hdr *wire.Header) error { replyHdr.Version = hdr.Version replyHdr.SrcConnectionID = connID replyHdr.DestConnectionID = hdr.SrcConnectionID - replyHdr.OrigDestConnectionID = hdr.DestConnectionID replyHdr.Token = token s.logger.Debugf("Changing connection ID to %s.", connID) s.logger.Debugf("-> Sending Retry") @@ -517,6 +516,9 @@ func (s *baseServer) sendRetry(remoteAddr net.Addr, hdr *wire.Header) error { if err := replyHdr.Write(buf, hdr.Version); err != nil { return err } + // append the Retry integrity tag + tag := handshake.GetRetryIntegrityTag(buf.Bytes(), hdr.DestConnectionID) + buf.Write(tag[:]) if _, err := s.conn.WriteTo(buf.Bytes(), remoteAddr); err != nil { s.logger.Debugf("Error sending Retry: %s", err) } diff --git a/server_test.go b/server_test.go index 5858fe9b..9f1064ad 100644 --- a/server_test.go +++ b/server_test.go @@ -297,8 +297,8 @@ var _ = Describe("Server", func() { Expect(replyHdr.Type).To(Equal(protocol.PacketTypeRetry)) Expect(replyHdr.SrcConnectionID).ToNot(Equal(hdr.DestConnectionID)) Expect(replyHdr.DestConnectionID).To(Equal(hdr.SrcConnectionID)) - Expect(replyHdr.OrigDestConnectionID).To(Equal(hdr.DestConnectionID)) Expect(replyHdr.Token).ToNot(BeEmpty()) + Expect(write.data[len(write.data)-16:]).To(Equal(handshake.GetRetryIntegrityTag(write.data[:len(write.data)-16], hdr.DestConnectionID)[:])) }) It("creates a session, if no Token is required", func() { diff --git a/session.go b/session.go index e34c8973..c95ce798 100644 --- a/session.go +++ b/session.go @@ -692,7 +692,7 @@ func (s *session) handleSinglePacket(p *receivedPacket, hdr *wire.Header) bool / }() if hdr.Type == protocol.PacketTypeRetry { - return s.handleRetryPacket(hdr) + return s.handleRetryPacket(hdr, p.data) } // The server can change the source connection ID with the first Handshake packet. @@ -738,7 +738,7 @@ func (s *session) handleSinglePacket(p *receivedPacket, hdr *wire.Header) bool / return true } -func (s *session) handleRetryPacket(hdr *wire.Header) bool /* was this a valid Retry */ { +func (s *session) handleRetryPacket(hdr *wire.Header, data []byte) bool /* was this a valid Retry */ { if s.perspective == protocol.PerspectiveServer { s.logger.Debugf("Ignoring Retry.") return false @@ -748,12 +748,14 @@ func (s *session) handleRetryPacket(hdr *wire.Header) bool /* was this a valid R return false } (&wire.ExtendedHeader{Header: *hdr}).Log(s.logger) - if !hdr.OrigDestConnectionID.Equal(s.handshakeDestConnID) { - s.logger.Debugf("Ignoring spoofed Retry. Original Destination Connection ID: %s, expected: %s", hdr.OrigDestConnectionID, s.handshakeDestConnID) + destConnID := s.connIDManager.Get() + if hdr.SrcConnectionID.Equal(destConnID) { + s.logger.Debugf("Ignoring Retry, since the server didn't change the Source Connection ID.") return false } - if hdr.SrcConnectionID.Equal(s.handshakeDestConnID) { - s.logger.Debugf("Ignoring Retry, since the server didn't change the Source Connection ID.") + tag := handshake.GetRetryIntegrityTag(data[:len(data)-16], destConnID) + if !bytes.Equal(data[len(data)-16:], tag[:]) { + s.logger.Debugf("Ignoring spoofed Retry. Integrity Tag doesn't match.") return false } // If a token is already set, this means that we already received a Retry from the server. diff --git a/session_test.go b/session_test.go index 9a842e1a..61b8672c 100644 --- a/session_test.go +++ b/session_test.go @@ -1750,51 +1750,49 @@ var _ = Describe("Client Session", func() { }) Context("handling Retry", func() { - var validRetryHdr *wire.ExtendedHeader + origDestConnID := protocol.ConnectionID{8, 7, 6, 5, 4, 3, 2, 1} + + var retryHdr *wire.ExtendedHeader JustBeforeEach(func() { - validRetryHdr = &wire.ExtendedHeader{ + retryHdr = &wire.ExtendedHeader{ Header: wire.Header{ - IsLongHeader: true, - Type: protocol.PacketTypeRetry, - SrcConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, - DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - OrigDestConnectionID: protocol.ConnectionID{8, 7, 6, 5, 4, 3, 2, 1}, - Token: []byte("foobar"), - Version: sess.version, + IsLongHeader: true, + Type: protocol.PacketTypeRetry, + SrcConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, + DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, + Token: []byte("foobar"), + Version: sess.version, }, } }) + getRetryTag := func(hdr *wire.ExtendedHeader) []byte { + buf := &bytes.Buffer{} + hdr.Write(buf, sess.version) + return handshake.GetRetryIntegrityTag(buf.Bytes(), origDestConnID)[:] + } + It("handles Retry packets", func() { cryptoSetup.EXPECT().ChangeConnectionID(protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}) packer.EXPECT().SetToken([]byte("foobar")) - Expect(sess.handlePacketImpl(getPacket(validRetryHdr, nil))).To(BeTrue()) + Expect(sess.handlePacketImpl(getPacket(retryHdr, getRetryTag(retryHdr)))).To(BeTrue()) }) It("ignores Retry packets after receiving a regular packet", func() { sess.receivedFirstPacket = true - Expect(sess.handlePacketImpl(getPacket(validRetryHdr, nil))).To(BeFalse()) + Expect(sess.handlePacketImpl(getPacket(retryHdr, getRetryTag(retryHdr)))).To(BeFalse()) }) It("ignores Retry packets if the server didn't change the connection ID", func() { - validRetryHdr.SrcConnectionID = destConnID - Expect(sess.handlePacketImpl(getPacket(validRetryHdr, nil))).To(BeFalse()) + retryHdr.SrcConnectionID = destConnID + Expect(sess.handlePacketImpl(getPacket(retryHdr, getRetryTag(retryHdr)))).To(BeFalse()) }) - It("ignores Retry packets with the wrong original destination connection ID", func() { - hdr := &wire.ExtendedHeader{ - Header: wire.Header{ - IsLongHeader: true, - Type: protocol.PacketTypeRetry, - SrcConnectionID: protocol.ConnectionID{0xde, 0xad, 0xbe, 0xef}, - DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - OrigDestConnectionID: protocol.ConnectionID{1, 2, 3, 4}, - Token: []byte("foobar"), - }, - PacketNumberLen: protocol.PacketNumberLen3, - } - Expect(sess.handlePacketImpl(getPacket(hdr, nil))).To(BeFalse()) + It("ignores Retry packets with the a wrong Integrity tag", func() { + tag := getRetryTag(retryHdr) + tag[0]++ + Expect(sess.handlePacketImpl(getPacket(retryHdr, tag))).To(BeFalse()) }) })