move the check for empty payload to the unpacker

This commit is contained in:
Marten Seemann 2022-08-27 11:57:00 +03:00
parent fd1b3a23c4
commit e3723a0ef1
4 changed files with 31 additions and 38 deletions

View file

@ -1123,13 +1123,6 @@ func (s *connection) handleUnpackedPacket(
rcvTime time.Time,
packetSize protocol.ByteCount, // only for logging
) error {
if len(packet.data) == 0 {
return &qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "empty packet",
}
}
if !s.receivedFirstPacket {
s.receivedFirstPacket = true
if !s.versionNegotiated && s.tracer != nil {

View file

@ -1013,36 +1013,6 @@ var _ = Describe("Connection", func() {
Eventually(conn.Context().Done()).Should(BeClosed())
})
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
encryptionLevel: protocol.Encryption1RTT,
}, nil)
streamManager.EXPECT().CloseWithError(gomock.Any())
cryptoSetup.EXPECT().Close()
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&coalescedPacket{buffer: getPacketBuffer()}, nil)
done := make(chan struct{})
go func() {
defer GinkgoRecover()
cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
Expect(conn.run()).To(MatchError(&qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "empty packet",
}))
close(done)
}()
expectReplaceWithClosed()
mconn.EXPECT().Write(gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.handlePacket(getPacket(&wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: srcConnID},
PacketNumberLen: protocol.PacketNumberLen1,
}, nil))
Eventually(done).Should(BeClosed())
})
It("ignores packets with a different source connection ID", func() {
hdr1 := &wire.ExtendedHeader{
Header: wire.Header{

View file

@ -7,6 +7,7 @@ import (
"github.com/lucas-clemente/quic-go/internal/handshake"
"github.com/lucas-clemente/quic-go/internal/protocol"
"github.com/lucas-clemente/quic-go/internal/qerr"
"github.com/lucas-clemente/quic-go/internal/wire"
)
@ -102,6 +103,13 @@ func (u *packetUnpacker) Unpack(hdr *wire.Header, rcvTime time.Time, data []byte
}
}
if len(decrypted) == 0 {
return nil, &qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "empty packet",
}
}
return &unpackedPacket{
hdr: extHdr,
encryptionLevel: encLevel,

View file

@ -155,7 +155,7 @@ var _ = Describe("Packet Unpacker", func() {
Expect(packet.data).To(Equal([]byte("decrypted")))
})
It("returns the error when getting the sealer fails", func() {
It("returns the error when getting the opener fails", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: connID},
PacketNumber: 0x1337,
@ -167,6 +167,28 @@ var _ = Describe("Packet Unpacker", func() {
Expect(err).To(MatchError(handshake.ErrKeysNotYetAvailable))
})
It("errors on empty packets", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{DestConnectionID: connID},
KeyPhase: protocol.KeyPhaseOne,
PacketNumberLen: protocol.PacketNumberLen4,
}
hdr, hdrRaw := getHeader(extHdr)
opener := mocks.NewMockShortHeaderOpener(mockCtrl)
now := time.Now()
gomock.InOrder(
cs.EXPECT().Get1RTTOpener().Return(opener, nil),
opener.EXPECT().DecryptHeader(gomock.Any(), gomock.Any(), gomock.Any()),
opener.EXPECT().DecodePacketNumber(gomock.Any(), gomock.Any()).Return(protocol.PacketNumber(321)),
opener.EXPECT().Open(gomock.Any(), payload, now, protocol.PacketNumber(321), protocol.KeyPhaseOne, hdrRaw).Return([]byte(""), nil),
)
_, err := unpacker.Unpack(hdr, now, append(hdrRaw, payload...))
Expect(err).To(MatchError(&qerr.TransportError{
ErrorCode: qerr.ProtocolViolation,
ErrorMessage: "empty packet",
}))
})
It("returns the error when unpacking fails", func() {
extHdr := &wire.ExtendedHeader{
Header: wire.Header{