diff --git a/ackhandler/interfaces.go b/ackhandler/interfaces.go index edfa2122f..716c88f73 100644 --- a/ackhandler/interfaces.go +++ b/ackhandler/interfaces.go @@ -19,6 +19,7 @@ type SentPacketHandler interface { GetLargestObserved() protocol.PacketNumber CongestionAllowsSending() bool + CheckForError() error TimeToFirstRTO() time.Duration } diff --git a/ackhandler/sent_packet_handler.go b/ackhandler/sent_packet_handler.go index ff2f7b929..cc027b7a5 100644 --- a/ackhandler/sent_packet_handler.go +++ b/ackhandler/sent_packet_handler.go @@ -17,8 +17,10 @@ var ( // ErrEntropy occurs when an ACK with incorrect entropy is received ErrEntropy = qerr.Error(qerr.InvalidAckData, "wrong entropy") // ErrMapAccess occurs when a NACK contains invalid NACK ranges - ErrMapAccess = qerr.Error(qerr.InvalidAckData, "Packet does not exist in PacketHistory") - errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package") + ErrMapAccess = qerr.Error(qerr.InvalidAckData, "Packet does not exist in PacketHistory") + // ErrTooManyTrackedSentPackets occurs when the sentPacketHandler has to keep track of too many packets + ErrTooManyTrackedSentPackets = errors.New("To many outstanding not-acked and not retransmitted packets.") + errAckForUnsentPacket = qerr.Error(qerr.InvalidAckData, "Received ACK for an unsent package") ) var ( @@ -103,6 +105,8 @@ func (h *sentPacketHandler) queuePacketForRetransmission(packet *Packet) { h.bytesInFlight -= packet.Length h.retransmissionQueue = append(h.retransmissionQueue, packet) packet.Retransmitted = true + + // TODO: delete from packetHistory once we drop support for version smaller than QUIC 33 } func (h *sentPacketHandler) SentPacket(packet *Packet) error { @@ -278,6 +282,14 @@ func (h *sentPacketHandler) CongestionAllowsSending() bool { return h.BytesInFlight() <= h.congestion.GetCongestionWindow() } +func (h *sentPacketHandler) CheckForError() error { + length := len(h.retransmissionQueue) + len(h.packetHistory) + if uint32(length) > protocol.MaxTrackedSentPackets { + return ErrTooManyTrackedSentPackets + } + return nil +} + func (h *sentPacketHandler) getRTO() time.Duration { rto := h.congestion.RetransmissionDelay() if rto == 0 { diff --git a/ackhandler/sent_packet_handler_test.go b/ackhandler/sent_packet_handler_test.go index 93e0ce345..44a6c67d1 100644 --- a/ackhandler/sent_packet_handler_test.go +++ b/ackhandler/sent_packet_handler_test.go @@ -162,6 +162,20 @@ var _ = Describe("SentPacketHandler", func() { }) }) + Context("DOS mitigation", func() { + It("checks the size of the packet history, for unacked packets", func() { + for i := uint32(1); i < protocol.MaxTrackedSentPackets+10; i++ { + packet := Packet{PacketNumber: protocol.PacketNumber(i), Frames: []frames.Frame{&streamFrame}, Length: 1} + err := handler.SentPacket(&packet) + Expect(err).ToNot(HaveOccurred()) + } + err := handler.CheckForError() + Expect(err).To(MatchError(ErrTooManyTrackedSentPackets)) + }) + + // TODO: add a test that the length of the retransmission queue is considered, even if packets have already been ACKed. Relevant once we drop support for QUIC 33 and earlier + }) + Context("ACK entropy calculations", func() { var packets []*Packet var entropy EntropyAccumulator diff --git a/packet_packer_test.go b/packet_packer_test.go index 6443d6955..d0b1b74b5 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -22,6 +22,7 @@ func (h *mockSentPacketHandler) HasPacketForRetransmission() bool func (h *mockSentPacketHandler) BytesInFlight() protocol.ByteCount { return 0 } func (h *mockSentPacketHandler) GetLargestObserved() protocol.PacketNumber { return 1 } func (h *mockSentPacketHandler) CongestionAllowsSending() bool { panic("not implemented") } +func (h *mockSentPacketHandler) CheckForError() error { panic("not implemented") } func (h *mockSentPacketHandler) TimeToFirstRTO() time.Duration { panic("not implemented") } func newMockSentPacketHandler() ackhandler.SentPacketHandler { diff --git a/protocol/server_parameters.go b/protocol/server_parameters.go index 703f0392f..e95c16eab 100644 --- a/protocol/server_parameters.go +++ b/protocol/server_parameters.go @@ -51,3 +51,8 @@ const RetransmissionThreshold uint8 = 3 // STKExpiryTimeSec is the valid time of a source address token in seconds const STKExpiryTimeSec = 24 * 60 * 60 + +// MaxTrackedSentPackets is maximum number of sent packets saved for either later retransmission or entropy calculation +// TODO: find a reasonable value here +// TODO: decrease this value after dropping support for QUIC 33 and earlier +const MaxTrackedSentPackets uint32 = 2000 diff --git a/session.go b/session.go index db7b7e385..0465016d5 100644 --- a/session.go +++ b/session.go @@ -463,6 +463,11 @@ func (s *Session) maybeSendPacket() error { func (s *Session) sendPacket() error { s.smallPacketDelayedOccurranceTime = time.Time{} // zero + err := s.sentPacketHandler.CheckForError() + if err != nil { + return err + } + if !s.sentPacketHandler.CongestionAllowsSending() { return nil } diff --git a/session_test.go b/session_test.go index ed2e5c896..f2e152e6b 100644 --- a/session_test.go +++ b/session_test.go @@ -576,6 +576,18 @@ var _ = Describe("Session", func() { close(done) }, 0.5) + It("errors when the SentPacketHandler has too many packets tracked", func() { + streamFrame := frames.StreamFrame{StreamID: 5, Data: []byte("foobar")} + for i := uint32(1); i < protocol.MaxTrackedSentPackets+10; i++ { + packet := ackhandler.Packet{PacketNumber: protocol.PacketNumber(i), Frames: []frames.Frame{&streamFrame}, Length: 1} + err := session.sentPacketHandler.SentPacket(&packet) + Expect(err).ToNot(HaveOccurred()) + } + // now session.sentPacketHandler.CheckForError will return an error + err := session.sendPacket() + Expect(err).To(MatchError(ackhandler.ErrTooManyTrackedSentPackets)) + }) + It("stores up to MaxSessionUnprocessedPackets packets", func(done Done) { // Nothing here should block for i := 0; i < protocol.MaxSessionUnprocessedPackets+10; i++ {