Merge pull request #2480 from lucas-clemente/qlog-dropped-retry

qlog dropped Retry packets

qlog/types.go

@@ -242,20 +242,26 @@ func (t keyUpdateTrigger) String() string {
 type PacketDropReason uint8
 
 const (
-	// PacketDropKeyUnavailable: when a packet is dropped because keys are unavailable
+	// PacketDropKeyUnavailable is used when a packet is dropped because keys are unavailable
 	PacketDropKeyUnavailable PacketDropReason = iota
-	// PacketDropUnknownConnectionID: when a packet is dropped because the connection ID is unknown
+	// PacketDropUnknownConnectionID is used when a packet is dropped because the connection ID is unknown
 	PacketDropUnknownConnectionID
-	// PacketDropHeaderParseError: when a packet is dropped because header parsing failed
+	// PacketDropHeaderParseError is used when a packet is dropped because header parsing failed
 	PacketDropHeaderParseError
-	// PacketDropPayloadDecryptError: when a packet is dropped because decrypting the payload failed
+	// PacketDropPayloadDecryptError is used when a packet is dropped because decrypting the payload failed
 	PacketDropPayloadDecryptError
-	// PacketDropProtocolViolation: when a packet is dropped due to a protocol violation
+	// PacketDropProtocolViolation is used when a packet is dropped due to a protocol violation
 	PacketDropProtocolViolation
-	// PacketDropDOSPrevention: when a packet is dropped to mitigate a DoS attack
+	// PacketDropDOSPrevention is used when a packet is dropped to mitigate a DoS attack
 	PacketDropDOSPrevention
-	// PacketDropUnsupportedVersion: when a packet is dropped because the version is not supported
+	// PacketDropUnsupportedVersion is used when a packet is dropped because the version is not supported
 	PacketDropUnsupportedVersion
+	// PacketDropUnexpectedPacket is used when an unexpected packet is received
+	PacketDropUnexpectedPacket
+	// PacketDropUnexpectedSourceConnectionID is used when a packet with an unexpected source connection ID is received
+	PacketDropUnexpectedSourceConnectionID
+	// PacketDropUnexpectedVersion is used when a packet with an unexpected version is received
+	PacketDropUnexpectedVersion
 )
 
 func (r PacketDropReason) String() string {
@@ -274,6 +280,12 @@ func (r PacketDropReason) String() string {
 		return "dos_prevention"
 	case PacketDropUnsupportedVersion:
 		return "unsupported_version"
+	case PacketDropUnexpectedPacket:
+		return "unexpected_packet"
+	case PacketDropUnexpectedSourceConnectionID:
+		return "unexpected_source_connection_id"
+	case PacketDropUnexpectedVersion:
+		return "unexpected_version"
 	default:
 		panic("unknown packet drop reason")
 	}

qlog/types_test.go

@@ -71,6 +71,9 @@ var _ = Describe("Types", func() {
 		Expect(PacketDropProtocolViolation.String()).To(Equal("protocol_violation"))
 		Expect(PacketDropDOSPrevention.String()).To(Equal("dos_prevention"))
 		Expect(PacketDropUnsupportedVersion.String()).To(Equal("unsupported_version"))
+		Expect(PacketDropUnexpectedPacket.String()).To(Equal("unexpected_packet"))
+		Expect(PacketDropUnexpectedSourceConnectionID.String()).To(Equal("unexpected_source_connection_id"))
+		Expect(PacketDropUnexpectedVersion.String()).To(Equal("unexpected_version"))
 	})
 
 	Context("transport errors", func() {

session.go

@@ -807,31 +807,45 @@ func (s *session) handleSinglePacket(p *receivedPacket, hdr *wire.Header) bool /
 }
 
 func (s *session) handleRetryPacket(hdr *wire.Header, data []byte) bool /* was this a valid Retry */ {
+	(&wire.ExtendedHeader{Header: *hdr}).Log(s.logger)
 	if s.perspective == protocol.PerspectiveServer {
+		if s.qlogger != nil {
+			s.qlogger.DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(data)), qlog.PacketDropUnexpectedPacket)
+		}
 		s.logger.Debugf("Ignoring Retry.")
 		return false
 	}
 	if s.receivedFirstPacket {
+		if s.qlogger != nil {
+			s.qlogger.DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(data)), qlog.PacketDropUnexpectedPacket)
+		}
 		s.logger.Debugf("Ignoring Retry, since we already received a packet.")
 		return false
 	}
-	(&wire.ExtendedHeader{Header: *hdr}).Log(s.logger)
 	destConnID := s.connIDManager.Get()
 	if hdr.SrcConnectionID.Equal(destConnID) {
+		if s.qlogger != nil {
+			s.qlogger.DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(data)), qlog.PacketDropUnexpectedPacket)
+		}
 		s.logger.Debugf("Ignoring Retry, since the server didn't change the Source Connection ID.")
 		return false
 	}
-	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.
 	// Ignore this Retry packet.
 	if s.receivedRetry {
 		s.logger.Debugf("Ignoring Retry, since a Retry was already received.")
 		return false
 	}
+
+	tag := handshake.GetRetryIntegrityTag(data[:len(data)-16], destConnID)
+	if !bytes.Equal(data[len(data)-16:], tag[:]) {
+		if s.qlogger != nil {
+			s.qlogger.DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(data)), qlog.PacketDropPayloadDecryptError)
+		}
+		s.logger.Debugf("Ignoring spoofed Retry. Integrity Tag doesn't match.")
+		return false
+	}
+
 	s.logger.Debugf("<- Received Retry")
 	s.logger.Debugf("Switching destination connection ID to: %s", hdr.SrcConnectionID)
 	if s.qlogger != nil {

session_test.go

@@ -578,10 +578,14 @@ var _ = Describe("Session", func() {
 
 		It("drops Retry packets", func() {
 			p := getPacket(&wire.ExtendedHeader{Header: wire.Header{
-				IsLongHeader: true,
-				Type:         protocol.PacketTypeRetry,
-			}}, nil)
-			qlogger.EXPECT().DroppedPacket(qlog.PacketTypeNotDetermined, protocol.ByteCount(len(p.data)), gomock.Any())
+				IsLongHeader:     true,
+				Type:             protocol.PacketTypeRetry,
+				DestConnectionID: destConnID,
+				SrcConnectionID:  srcConnID,
+				Version:          sess.version,
+				Token:            []byte("foobar"),
+			}}, make([]byte, 16) /* Retry integrity tag */)
+			qlogger.EXPECT().DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(p.data)), qlog.PacketDropUnexpectedPacket)
 			Expect(sess.handlePacketImpl(p)).To(BeFalse())
 		})
 
@@ -2054,18 +2058,24 @@ var _ = Describe("Client Session", func() {
 
 		It("ignores Retry packets after receiving a regular packet", func() {
 			sess.receivedFirstPacket = true
-			Expect(sess.handlePacketImpl(getPacket(retryHdr, getRetryTag(retryHdr)))).To(BeFalse())
+			p := getPacket(retryHdr, getRetryTag(retryHdr))
+			qlogger.EXPECT().DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(p.data)), qlog.PacketDropUnexpectedPacket)
+			Expect(sess.handlePacketImpl(p)).To(BeFalse())
 		})
 
 		It("ignores Retry packets if the server didn't change the connection ID", func() {
 			retryHdr.SrcConnectionID = destConnID
-			Expect(sess.handlePacketImpl(getPacket(retryHdr, getRetryTag(retryHdr)))).To(BeFalse())
+			p := getPacket(retryHdr, getRetryTag(retryHdr))
+			qlogger.EXPECT().DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(p.data)), qlog.PacketDropUnexpectedPacket)
+			Expect(sess.handlePacketImpl(p)).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())
+			p := getPacket(retryHdr, tag)
+			qlogger.EXPECT().DroppedPacket(qlog.PacketTypeRetry, protocol.ByteCount(len(p.data)), qlog.PacketDropPayloadDecryptError)
+			Expect(sess.handlePacketImpl(p)).To(BeFalse())
 		})
 	})