Merge pull request #2569 from lucas-clemente/drop-duplicate-packets

drop duplicate packets
This commit is contained in:
Marten Seemann 2020-05-29 16:39:46 +07:00 committed by GitHub
commit 8d00ec135e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 170 additions and 4 deletions

View file

@ -63,6 +63,7 @@ type sentPacketTracker interface {
// ReceivedPacketHandler handles ACKs needed to send for incoming packets
type ReceivedPacketHandler interface {
IsPotentiallyDuplicate(protocol.PacketNumber, protocol.EncryptionLevel) bool
ReceivedPacket(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel, rcvTime time.Time, shouldInstigateAck bool) error
DropPackets(protocol.EncryptionLevel)

View file

@ -137,3 +137,21 @@ func (h *receivedPacketHandler) GetAckFrame(encLevel protocol.EncryptionLevel) *
}
return ack
}
func (h *receivedPacketHandler) IsPotentiallyDuplicate(pn protocol.PacketNumber, encLevel protocol.EncryptionLevel) bool {
switch encLevel {
case protocol.EncryptionInitial:
if h.initialPackets != nil {
return h.initialPackets.IsPotentiallyDuplicate(pn)
}
case protocol.EncryptionHandshake:
if h.handshakePackets != nil {
return h.handshakePackets.IsPotentiallyDuplicate(pn)
}
case protocol.Encryption0RTT, protocol.Encryption1RTT:
if h.appDataPackets != nil {
return h.appDataPackets.IsPotentiallyDuplicate(pn)
}
}
panic("unexpected encryption level")
}

View file

@ -134,4 +134,26 @@ var _ = Describe("Received Packet Handler", func() {
Expect(ack.LowestAcked()).To(Equal(protocol.PacketNumber(2)))
Expect(ack.LargestAcked()).To(Equal(protocol.PacketNumber(4)))
})
It("says if packets are duplicates", func() {
sendTime := time.Now()
sentPackets.EXPECT().GetLowestPacketNotConfirmedAcked().AnyTimes()
// Initial
Expect(handler.IsPotentiallyDuplicate(3, protocol.EncryptionInitial)).To(BeFalse())
Expect(handler.ReceivedPacket(3, protocol.EncryptionInitial, sendTime, true)).To(Succeed())
Expect(handler.IsPotentiallyDuplicate(3, protocol.EncryptionInitial)).To(BeTrue())
// Handshake
Expect(handler.IsPotentiallyDuplicate(3, protocol.EncryptionHandshake)).To(BeFalse())
Expect(handler.ReceivedPacket(3, protocol.EncryptionHandshake, sendTime, true)).To(Succeed())
Expect(handler.IsPotentiallyDuplicate(3, protocol.EncryptionHandshake)).To(BeTrue())
// 0-RTT
Expect(handler.IsPotentiallyDuplicate(3, protocol.Encryption0RTT)).To(BeFalse())
Expect(handler.ReceivedPacket(3, protocol.Encryption0RTT, sendTime, true)).To(Succeed())
Expect(handler.IsPotentiallyDuplicate(3, protocol.Encryption0RTT)).To(BeTrue())
// 1-RTT
Expect(handler.IsPotentiallyDuplicate(3, protocol.Encryption1RTT)).To(BeTrue())
Expect(handler.IsPotentiallyDuplicate(4, protocol.Encryption1RTT)).To(BeFalse())
Expect(handler.ReceivedPacket(4, protocol.Encryption1RTT, sendTime, true)).To(Succeed())
Expect(handler.IsPotentiallyDuplicate(4, protocol.Encryption1RTT)).To(BeTrue())
})
})

View file

@ -128,3 +128,18 @@ func (h *receivedPacketHistory) GetHighestAckRange() wire.AckRange {
}
return ackRange
}
func (h *receivedPacketHistory) IsPotentiallyDuplicate(p protocol.PacketNumber) bool {
if p < h.deletedBelow {
return true
}
for el := h.ranges.Back(); el != nil; el = el.Prev() {
if p > el.Value.End {
return false
}
if p <= el.Value.End && p >= el.Value.Start {
return true
}
}
return false
}

View file

@ -246,4 +246,55 @@ var _ = Describe("receivedPacketHistory", func() {
Expect(hist.GetHighestAckRange()).To(Equal(wire.AckRange{Smallest: 6, Largest: 7}))
})
})
Context("duplicate detection", func() {
It("doesn't declare the first packet a duplicate", func() {
Expect(hist.IsPotentiallyDuplicate(5)).To(BeFalse())
})
It("detects a duplicate in a range", func() {
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(6)
Expect(hist.IsPotentiallyDuplicate(3)).To(BeFalse())
Expect(hist.IsPotentiallyDuplicate(4)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(5)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(6)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(7)).To(BeFalse())
})
It("detects a duplicate in multiple ranges", func() {
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(8)
hist.ReceivedPacket(9)
Expect(hist.IsPotentiallyDuplicate(3)).To(BeFalse())
Expect(hist.IsPotentiallyDuplicate(4)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(5)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(6)).To(BeFalse())
Expect(hist.IsPotentiallyDuplicate(7)).To(BeFalse())
Expect(hist.IsPotentiallyDuplicate(8)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(9)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(10)).To(BeFalse())
})
It("says a packet is a potentially duplicate if the ranges were already deleted", func() {
hist.ReceivedPacket(4)
hist.ReceivedPacket(5)
hist.ReceivedPacket(8)
hist.ReceivedPacket(9)
hist.ReceivedPacket(11)
hist.DeleteBelow(8)
Expect(hist.IsPotentiallyDuplicate(3)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(4)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(5)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(6)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(7)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(8)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(9)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(10)).To(BeFalse())
Expect(hist.IsPotentiallyDuplicate(11)).To(BeTrue())
Expect(hist.IsPotentiallyDuplicate(12)).To(BeFalse())
})
})
})

View file

@ -188,3 +188,7 @@ func (h *receivedPacketTracker) GetAckFrame() *wire.AckFrame {
}
func (h *receivedPacketTracker) GetAlarmTimeout() time.Time { return h.ackAlarm }
func (h *receivedPacketTracker) IsPotentiallyDuplicate(pn protocol.PacketNumber) bool {
return h.packetHistory.IsPotentiallyDuplicate(pn)
}

View file

@ -76,6 +76,20 @@ func (mr *MockReceivedPacketHandlerMockRecorder) GetAlarmTimeout() *gomock.Call
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAlarmTimeout", reflect.TypeOf((*MockReceivedPacketHandler)(nil).GetAlarmTimeout))
}
// IsPotentiallyDuplicate mocks base method
func (m *MockReceivedPacketHandler) IsPotentiallyDuplicate(arg0 protocol.PacketNumber, arg1 protocol.EncryptionLevel) bool {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "IsPotentiallyDuplicate", arg0, arg1)
ret0, _ := ret[0].(bool)
return ret0
}
// IsPotentiallyDuplicate indicates an expected call of IsPotentiallyDuplicate
func (mr *MockReceivedPacketHandlerMockRecorder) IsPotentiallyDuplicate(arg0, arg1 interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsPotentiallyDuplicate", reflect.TypeOf((*MockReceivedPacketHandler)(nil).IsPotentiallyDuplicate), arg0, arg1)
}
// ReceivedPacket mocks base method
func (m *MockReceivedPacketHandler) ReceivedPacket(arg0 protocol.PacketNumber, arg1 protocol.EncryptionLevel, arg2 time.Time, arg3 bool) error {
m.ctrl.T.Helper()

View file

@ -262,6 +262,8 @@ const (
PacketDropUnexpectedSourceConnectionID
// PacketDropUnexpectedVersion is used when a packet with an unexpected version is received
PacketDropUnexpectedVersion
// PacketDropDuplicate is used when a duplicate packet is received
PacketDropDuplicate
)
func (r PacketDropReason) String() string {
@ -286,6 +288,8 @@ func (r PacketDropReason) String() string {
return "unexpected_source_connection_id"
case PacketDropUnexpectedVersion:
return "unexpected_version"
case PacketDropDuplicate:
return "duplicate"
default:
panic("unknown packet drop reason")
}

View file

@ -816,6 +816,14 @@ func (s *session) handleSinglePacket(p *receivedPacket, hdr *wire.Header) bool /
packet.hdr.Log(s.logger)
}
if s.receivedPacketHandler.IsPotentiallyDuplicate(packet.packetNumber, packet.encryptionLevel) {
s.logger.Debugf("Dropping (potentially) duplicate packet.")
if s.qlogger != nil {
s.qlogger.DroppedPacket(qlog.PacketTypeFromHeader(hdr), protocol.ByteCount(len(p.data)), qlog.PacketDropDuplicate)
}
return false
}
if err := s.handleUnpackedPacket(packet, p.rcvTime, protocol.ByteCount(len(p.data))); err != nil {
s.closeLocal(err)
return false

View file

@ -666,7 +666,10 @@ var _ = Describe("Session", func() {
data: []byte{0}, // one PADDING frame
}, nil)
rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl)
rph.EXPECT().ReceivedPacket(protocol.PacketNumber(0x1337), protocol.EncryptionInitial, rcvTime, false)
gomock.InOrder(
rph.EXPECT().IsPotentiallyDuplicate(protocol.PacketNumber(0x1337), protocol.EncryptionInitial),
rph.EXPECT().ReceivedPacket(protocol.PacketNumber(0x1337), protocol.EncryptionInitial, rcvTime, false),
)
sess.receivedPacketHandler = rph
packet.rcvTime = rcvTime
qlogger.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
@ -691,7 +694,10 @@ var _ = Describe("Session", func() {
data: buf.Bytes(),
}, nil)
rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl)
rph.EXPECT().ReceivedPacket(protocol.PacketNumber(0x1337), protocol.Encryption1RTT, rcvTime, true)
gomock.InOrder(
rph.EXPECT().IsPotentiallyDuplicate(protocol.PacketNumber(0x1337), protocol.Encryption1RTT),
rph.EXPECT().ReceivedPacket(protocol.PacketNumber(0x1337), protocol.Encryption1RTT, rcvTime, true),
)
sess.receivedPacketHandler = rph
packet.rcvTime = rcvTime
qlogger.EXPECT().StartedConnection(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
@ -699,6 +705,26 @@ var _ = Describe("Session", func() {
Expect(sess.handlePacketImpl(packet)).To(BeTrue())
})
It("drops duplicate packets", func() {
hdr := &wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: srcConnID},
PacketNumber: 0x37,
PacketNumberLen: protocol.PacketNumberLen1,
}
packet := getPacket(hdr, nil)
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).Return(&unpackedPacket{
packetNumber: 0x1337,
encryptionLevel: protocol.Encryption1RTT,
hdr: hdr,
data: []byte("foobar"),
}, nil)
rph := mockackhandler.NewMockReceivedPacketHandler(mockCtrl)
rph.EXPECT().IsPotentiallyDuplicate(protocol.PacketNumber(0x1337), protocol.Encryption1RTT).Return(true)
sess.receivedPacketHandler = rph
qlogger.EXPECT().DroppedPacket(qlog.PacketType1RTT, protocol.ByteCount(len(packet.data)), qlog.PacketDropDuplicate)
Expect(sess.handlePacketImpl(packet)).To(BeFalse())
})
It("drops a packet when unpacking fails", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, handshake.ErrDecryptionFailed)
streamManager.EXPECT().CloseWithError(gomock.Any())
@ -784,8 +810,9 @@ var _ = Describe("Session", func() {
It("rejects packets with empty payload", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).Return(&unpackedPacket{
hdr: &wire.ExtendedHeader{},
data: []byte{}, // no payload
hdr: &wire.ExtendedHeader{},
data: []byte{}, // no payload
encryptionLevel: protocol.Encryption1RTT,
}, nil)
streamManager.EXPECT().CloseWithError(gomock.Any())
cryptoSetup.EXPECT().Close()
@ -933,6 +960,7 @@ var _ = Describe("Session", func() {
return &unpackedPacket{
encryptionLevel: protocol.EncryptionHandshake,
data: []byte{0},
packetNumber: 1,
hdr: &wire.ExtendedHeader{Header: wire.Header{SrcConnectionID: destConnID}},
}, nil
})
@ -942,6 +970,7 @@ var _ = Describe("Session", func() {
return &unpackedPacket{
encryptionLevel: protocol.EncryptionHandshake,
data: []byte{0},
packetNumber: 2,
hdr: &wire.ExtendedHeader{Header: wire.Header{SrcConnectionID: destConnID}},
}, nil
})