From a654e7600a4c05a56216e71cefb64e354c69f17a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 4 Jul 2018 12:46:11 +0700 Subject: [PATCH] move cutting of packets at the payload length to the multiplexer --- client.go | 5 ---- client_multiplexer.go | 8 +++++++ client_multiplexer_test.go | 47 ++++++++++++++++++++++++++++++++++++++ client_test.go | 42 ---------------------------------- 4 files changed, 55 insertions(+), 47 deletions(-) diff --git a/client.go b/client.go index 4e157885b..86465222e 100644 --- a/client.go +++ b/client.go @@ -412,11 +412,6 @@ func (c *client) handleIETFQUICPacket(p *receivedPacket) error { default: return fmt.Errorf("Received unsupported packet type: %s", p.header.Type) } - if protocol.ByteCount(len(p.data)) < p.header.PayloadLen { - return fmt.Errorf("packet payload (%d bytes) is smaller than the expected payload length (%d bytes)", len(p.data), p.header.PayloadLen) - } - p.data = p.data[:int(p.header.PayloadLen)] - // TODO(#1312): implement parsing of compound packets } // this is the first packet we are receiving diff --git a/client_multiplexer.go b/client_multiplexer.go index 5a19b3384..4ae83f40f 100644 --- a/client_multiplexer.go +++ b/client_multiplexer.go @@ -129,6 +129,14 @@ func (m *clientMultiplexer) handlePacket(addr net.Addr, data []byte, p *connMana hdr.Raw = data[:len(data)-r.Len()] packetData := data[len(data)-r.Len():] + if hdr.IsLongHeader { + if protocol.ByteCount(len(packetData)) < hdr.PayloadLen { + return fmt.Errorf("packet payload (%d bytes) is smaller than the expected payload length (%d bytes)", len(packetData), hdr.PayloadLen) + } + packetData = packetData[:int(hdr.PayloadLen)] + // TODO(#1312): implement parsing of compound packets + } + client.handlePacket(&receivedPacket{ remoteAddr: addr, header: hdr, diff --git a/client_multiplexer_test.go b/client_multiplexer_test.go index ab3fe5e89..9ba4dbab3 100644 --- a/client_multiplexer_test.go +++ b/client_multiplexer_test.go @@ -112,6 +112,53 @@ var _ = Describe("Client Multiplexer", func() { Expect(err).To(MatchError("received a packet with an unexpected connection ID 0x0102030405060708")) }) + It("errors on packets that are smaller than the Payload Length in the packet header", func() { + connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} + hdr := &wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeHandshake, + PayloadLen: 1000, + DestConnectionID: connID, + PacketNumberLen: protocol.PacketNumberLen1, + Version: versionIETFFrames, + } + buf := &bytes.Buffer{} + Expect(hdr.Write(buf, protocol.PerspectiveServer, versionIETFFrames)).To(Succeed()) + buf.Write(bytes.Repeat([]byte{0}, 500)) + + sess := NewMockQuicSession(mockCtrl) + sess.EXPECT().GetVersion().Return(versionIETFFrames) + manager := NewMockPacketHandlerManager(mockCtrl) + manager.EXPECT().Get(connID).Return(sess, true) + err := getClientMultiplexer().(*clientMultiplexer).handlePacket(nil, buf.Bytes(), &connManager{manager: manager, connIDLen: 8}) + Expect(err).To(MatchError("packet payload (500 bytes) is smaller than the expected payload length (1000 bytes)")) + }) + + It("errors on packets that are smaller than the Payload Length in the packet header", func() { + connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} + hdr := &wire.Header{ + IsLongHeader: true, + Type: protocol.PacketTypeHandshake, + PayloadLen: 456, + DestConnectionID: connID, + PacketNumberLen: protocol.PacketNumberLen1, + Version: versionIETFFrames, + } + buf := &bytes.Buffer{} + Expect(hdr.Write(buf, protocol.PerspectiveServer, versionIETFFrames)).To(Succeed()) + buf.Write(bytes.Repeat([]byte{0}, 500)) + + sess := NewMockQuicSession(mockCtrl) + sess.EXPECT().GetVersion().Return(versionIETFFrames) + sess.EXPECT().handlePacket(gomock.Any()).Do(func(p *receivedPacket) { + Expect(p.data).To(HaveLen(456)) + }) + manager := NewMockPacketHandlerManager(mockCtrl) + manager.EXPECT().Get(connID).Return(sess, true) + err := getClientMultiplexer().(*clientMultiplexer).handlePacket(nil, buf.Bytes(), &connManager{manager: manager, connIDLen: 8}) + Expect(err).ToNot(HaveOccurred()) + }) + It("closes the packet handlers when reading from the conn fails", func() { conn := newMockPacketConn() conn.readErr = errors.New("test error") diff --git a/client_test.go b/client_test.go index 9af37f73f..5ccf40321 100644 --- a/client_test.go +++ b/client_test.go @@ -671,48 +671,6 @@ var _ = Describe("Client", func() { Expect(cl.GetVersion()).To(Equal(cl.version)) }) - It("errors on packets that are smaller than the Payload Length in the packet header", func() { - cl.session = NewMockQuicSession(mockCtrl) // don't EXPECT any handlePacket calls - hdr := &wire.Header{ - IsLongHeader: true, - Type: protocol.PacketTypeHandshake, - PayloadLen: 1000, - SrcConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - DestConnectionID: protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8}, - PacketNumberLen: protocol.PacketNumberLen1, - Version: versionIETFFrames, - } - err := cl.handlePacketImpl(&receivedPacket{ - remoteAddr: addr, - header: hdr, - data: make([]byte, 456), - }) - Expect(err).To(MatchError("received a packet with an unexpected connection ID (0x0102030405060708, expected 0x0000000000001337)")) - }) - - It("cuts packets at the payload length", func() { - sess := NewMockQuicSession(mockCtrl) - sess.EXPECT().handlePacket(gomock.Any()).Do(func(packet *receivedPacket) { - Expect(packet.data).To(HaveLen(123)) - }) - cl.session = sess - hdr := &wire.Header{ - IsLongHeader: true, - Type: protocol.PacketTypeHandshake, - PayloadLen: 123, - SrcConnectionID: connID, - DestConnectionID: connID, - PacketNumberLen: protocol.PacketNumberLen1, - Version: versionIETFFrames, - } - err := cl.handlePacketImpl(&receivedPacket{ - remoteAddr: addr, - header: hdr, - data: make([]byte, 456), - }) - Expect(err).ToNot(HaveOccurred()) - }) - It("ignores packets with the wrong Long Header Type", func() { hdr := &wire.Header{ IsLongHeader: true,