gr1ffon/quic-go
1
0
Fork 0
forked from quic-go/quic-go
Code Pull Requests Activity

don't close the session when unpacking a packet fails

Browse Source
This commit is contained in:
Marten Seemann
2019-06-30 18:35:30 +07:00
parent 5929a83210
commit 6af7df436a
2 changed files with 32 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
session.go
View File

@@ -598,12 +598,12 @@ func (s *session) handleSinglePacket(p *receivedPacket, hdr *wire.Header) bool /
// Try again later.
wasQueued = true
s.tryQueueingUndecryptablePacket(p)
case handshake.ErrDecryptionFailed:
case wire.ErrInvalidReservedBits:
s.closeLocal(qerr.Error(qerr.ProtocolViolation, err.Error()))
default:
// This might be a packet injected by an attacker.
// Drop it.
s.logger.Debugf("Dropping packet that could not be unpacked.")
default:
s.closeLocal(err)
s.logger.Debugf("Dropping packet that could not be unpacked. Error: %s", err)
}
return false
}

32
session_test.go
View File

@@ -565,9 +565,8 @@ var _ = Describe("Session", func() {
Eventually(sess.Context().Done()).Should(BeClosed())
})
It("closes the session when unpacking fails for any other reason than a decryption error", func() {
testErr := errors.New("test error")
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(nil, testErr)
It("closes the session when unpacking fails because the reserved bits were incorrect", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(nil, wire.ErrInvalidReservedBits)
streamManager.EXPECT().CloseWithError(gomock.Any())
cryptoSetup.EXPECT().Close()
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
@@ -575,7 +574,9 @@ var _ = Describe("Session", func() {
go func() {
defer GinkgoRecover()
cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
Expect(sess.run()).To(MatchError(testErr))
err := sess.run()
Expect(err).To(HaveOccurred())
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.ProtocolViolation))
close(done)
}()
sessionRunner.EXPECT().Retire(gomock.Any())
@@ -586,6 +587,29 @@ var _ = Describe("Session", func() {
Eventually(sess.Context().Done()).Should(BeClosed())
})
It("ignores packets when unpacking fails for any other reason", func() {
testErr := errors.New("test err")
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(nil, testErr)
streamManager.EXPECT().CloseWithError(gomock.Any())
cryptoSetup.EXPECT().Close()
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
runErr := make(chan error)
go func() {
defer GinkgoRecover()
cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
runErr <- sess.run()
}()
sessionRunner.EXPECT().Retire(gomock.Any())
sess.handlePacket(getPacket(&wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: sess.srcConnID},
PacketNumberLen: protocol.PacketNumberLen1,
}, nil))
Consistently(runErr).ShouldNot(Receive())
// make the go routine return
sess.Close()
Eventually(sess.Context().Done()).Should(BeClosed())
})
It("rejects packets with empty payload", func() {
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any()).Return(&unpackedPacket{
hdr: &wire.ExtendedHeader{},