From 899e1694d7bb754bd053d34858cfb22c670c306f Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 10 Apr 2020 14:35:37 +0700 Subject: [PATCH 1/2] add a test for dropped packets due to header parsing errors --- session_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/session_test.go b/session_test.go index b53836064..1ef2b22fd 100644 --- a/session_test.go +++ b/session_test.go @@ -585,6 +585,20 @@ var _ = Describe("Session", func() { Expect(sess.handlePacketImpl(p)).To(BeFalse()) }) + It("drops packets for which header decryption fails", func() { + p := getPacket(&wire.ExtendedHeader{ + Header: wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeHandshake, + Version: sess.version, + }, + PacketNumberLen: protocol.PacketNumberLen2, + }, nil) + p.data[0] ^= 0x40 // unset the QUIC bit + qlogger.EXPECT().DroppedPacket(qlog.PacketTypeNotDetermined, protocol.ByteCount(len(p.data)), qlog.PacketDropHeaderParseError) + Expect(sess.handlePacketImpl(p)).To(BeFalse()) + }) + It("informs the ReceivedPacketHandler about non-ack-eliciting packets", func() { hdr := &wire.ExtendedHeader{ Header: wire.Header{DestConnectionID: srcConnID}, From f58eb4738328516505e6175f93bcacf4aec12b80 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 10 Apr 2020 14:39:02 +0700 Subject: [PATCH 2/2] qlog when packets are dropped due to unsupported QUIC version --- internal/wire/header.go | 8 ++++---- internal/wire/header_test.go | 2 +- server.go | 2 +- session.go | 6 +++++- session_test.go | 13 +++++++++++++ 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/internal/wire/header.go b/internal/wire/header.go index 8c68b8910..0ec54bade 100644 --- a/internal/wire/header.go +++ b/internal/wire/header.go @@ -42,7 +42,7 @@ func IsVersionNegotiationPacket(b []byte) bool { return b[0]&0x80 > 0 && b[1] == 0 && b[2] == 0 && b[3] == 0 && b[4] == 0 } -var errUnsupportedVersion = errors.New("unsupported version") +var ErrUnsupportedVersion = errors.New("unsupported version") // The Header is the version independent part of the header type Header struct { @@ -69,8 +69,8 @@ type Header struct { func ParsePacket(data []byte, shortHeaderConnIDLen int) (*Header, []byte /* packet data */, []byte /* rest */, error) { hdr, err := parseHeader(bytes.NewReader(data), shortHeaderConnIDLen) if err != nil { - if err == errUnsupportedVersion { - return hdr, nil, nil, nil + if err == ErrUnsupportedVersion { + return hdr, nil, nil, ErrUnsupportedVersion } return nil, nil, nil, err } @@ -160,7 +160,7 @@ func (h *Header) parseLongHeader(b *bytes.Reader) error { } // If we don't understand the version, we have no idea how to interpret the rest of the bytes if !protocol.IsSupportedVersion(protocol.SupportedVersions, h.Version) { - return errUnsupportedVersion + return ErrUnsupportedVersion } switch (h.typeByte & 0x30) >> 4 { diff --git a/internal/wire/header_test.go b/internal/wire/header_test.go index 48c0ea7ac..12f0f521e 100644 --- a/internal/wire/header_test.go +++ b/internal/wire/header_test.go @@ -213,7 +213,7 @@ var _ = Describe("Header Parsing", func() { 'f', 'o', 'o', 'b', 'a', 'r', // unspecified bytes } hdr, _, rest, err := ParsePacket(data, 0) - Expect(err).ToNot(HaveOccurred()) + Expect(err).To(MatchError(ErrUnsupportedVersion)) Expect(hdr.IsLongHeader).To(BeTrue()) Expect(hdr.Version).To(Equal(protocol.VersionNumber(0xdeadbeef))) Expect(hdr.DestConnectionID).To(Equal(protocol.ConnectionID{0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8})) diff --git a/server.go b/server.go index 788a3fb4c..96b59cbb9 100644 --- a/server.go +++ b/server.go @@ -296,7 +296,7 @@ func (s *baseServer) handlePacketImpl(p *receivedPacket) bool /* was the packet // If we're creating a new session, the packet will be passed to the session. // The header will then be parsed again. hdr, _, _, err := wire.ParsePacket(p.data, s.config.ConnectionIDLength) - if err != nil { + if err != nil && err != wire.ErrUnsupportedVersion { s.logger.Debugf("Error parsing packet: %s", err) return false } diff --git a/session.go b/session.go index b452f8c31..ef250b6eb 100644 --- a/session.go +++ b/session.go @@ -710,7 +710,11 @@ func (s *session) handlePacketImpl(rp *receivedPacket) bool { hdr, packetData, rest, err := wire.ParsePacket(p.data, s.srcConnIDLen) if err != nil { if s.qlogger != nil { - s.qlogger.DroppedPacket(qlog.PacketTypeNotDetermined, protocol.ByteCount(len(data)), qlog.PacketDropHeaderParseError) + dropReason := qlog.PacketDropHeaderParseError + if err == wire.ErrUnsupportedVersion { + dropReason = qlog.PacketDropUnsupportedVersion + } + s.qlogger.DroppedPacket(qlog.PacketTypeNotDetermined, protocol.ByteCount(len(data)), dropReason) } s.logger.Debugf("error parsing packet: %s", err) break diff --git a/session_test.go b/session_test.go index 1ef2b22fd..7a8831007 100644 --- a/session_test.go +++ b/session_test.go @@ -599,6 +599,19 @@ var _ = Describe("Session", func() { Expect(sess.handlePacketImpl(p)).To(BeFalse()) }) + It("drops packets for which the version is unsupported", func() { + p := getPacket(&wire.ExtendedHeader{ + Header: wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeHandshake, + Version: sess.version + 1, + }, + PacketNumberLen: protocol.PacketNumberLen2, + }, nil) + qlogger.EXPECT().DroppedPacket(qlog.PacketTypeNotDetermined, protocol.ByteCount(len(p.data)), qlog.PacketDropUnsupportedVersion) + Expect(sess.handlePacketImpl(p)).To(BeFalse()) + }) + It("informs the ReceivedPacketHandler about non-ack-eliciting packets", func() { hdr := &wire.ExtendedHeader{ Header: wire.Header{DestConnectionID: srcConnID},