From 3b353ff0ef09caf53037a68a055418c7028557e7 Mon Sep 17 00:00:00 2001 From: Julian Elischer Date: Mon, 9 Nov 2020 21:57:30 -0800 Subject: Additions to ICMP and IPv4 parsers Teach ICMP.Parser/ToBytes to handle some non echo ICMP packets. Teach IPv4.Parser that fragments only have a payload, not an upper layer. Fix IPv4 and IPv6 reassembly tests to handle the change. Fixes #4758 PiperOrigin-RevId: 341549665 --- test/packetimpact/testbench/layers.go | 73 +++++++++++++++------- .../tests/ipv4_fragment_reassembly_test.go | 9 ++- .../tests/ipv6_fragment_reassembly_test.go | 3 +- 3 files changed, 56 insertions(+), 29 deletions(-) (limited to 'test/packetimpact') diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go index af7a2ba4e..fc45d2085 100644 --- a/test/packetimpact/testbench/layers.go +++ b/test/packetimpact/testbench/layers.go @@ -402,6 +402,10 @@ func parseIPv4(b []byte) (Layer, layerParser) { DstAddr: Address(h.DestinationAddress()), } var nextParser layerParser + // If it is a fragment, don't treat it as having a transport protocol. + if h.FragmentOffset() != 0 || h.More() { + return &ipv4, parsePayload + } switch h.TransportProtocol() { case header.TCPProtocolNumber: nextParser = parseTCP @@ -709,12 +713,17 @@ func parseIPv6FragmentExtHdr(b []byte) (Layer, layerParser) { nextHeader := b[0] var extHdr header.IPv6FragmentExtHdr copy(extHdr[:], b[2:]) - return &IPv6FragmentExtHdr{ + fragLayer := IPv6FragmentExtHdr{ NextHeader: IPv6ExtHdrIdent(header.IPv6ExtensionHeaderIdentifier(nextHeader)), FragmentOffset: Uint16(extHdr.FragmentOffset()), MoreFragments: Bool(extHdr.More()), Identification: Uint32(extHdr.ID()), - }, nextIPv6PayloadParser(nextHeader) + } + // If it is a fragment, we can't interpret it. + if extHdr.FragmentOffset() != 0 || extHdr.More() { + return &fragLayer, parsePayload + } + return &fragLayer, nextIPv6PayloadParser(nextHeader) } func (l *IPv6HopByHopOptionsExtHdr) length() int { @@ -861,26 +870,15 @@ func (l *ICMPv6) merge(other Layer) error { return mergeLayer(l, other) } -// ICMPv4Type is a helper routine that allocates a new header.ICMPv4Type value -// to store t and returns a pointer to it. -func ICMPv4Type(t header.ICMPv4Type) *header.ICMPv4Type { - return &t -} - -// ICMPv4Code is a helper routine that allocates a new header.ICMPv4Code value -// to store t and returns a pointer to it. -func ICMPv4Code(t header.ICMPv4Code) *header.ICMPv4Code { - return &t -} - // ICMPv4 can construct and match an ICMPv4 encapsulation. type ICMPv4 struct { LayerBase Type *header.ICMPv4Type Code *header.ICMPv4Code Checksum *uint16 - Ident *uint16 - Sequence *uint16 + Ident *uint16 // Only in Echo Request/Reply. + Sequence *uint16 // Only in Echo Request/Reply. + Pointer *uint8 // Only in Parameter Problem. Payload []byte } @@ -888,6 +886,18 @@ func (l *ICMPv4) String() string { return stringLayer(l) } +// ICMPv4Type is a helper routine that allocates a new header.ICMPv4Type value +// to store t and returns a pointer to it. +func ICMPv4Type(t header.ICMPv4Type) *header.ICMPv4Type { + return &t +} + +// ICMPv4Code is a helper routine that allocates a new header.ICMPv4Code value +// to store t and returns a pointer to it. +func ICMPv4Code(t header.ICMPv4Code) *header.ICMPv4Code { + return &t +} + // ToBytes implements Layer.ToBytes. func (l *ICMPv4) ToBytes() ([]byte, error) { b := make([]byte, header.ICMPv4MinimumSize+len(l.Payload)) @@ -901,11 +911,19 @@ func (l *ICMPv4) ToBytes() ([]byte, error) { if copied := copy(h.Payload(), l.Payload); copied != len(l.Payload) { panic(fmt.Sprintf("wrong number of bytes copied into h.Payload(): got = %d, want = %d", len(h.Payload()), len(l.Payload))) } - if l.Ident != nil { - h.SetIdent(*l.Ident) - } - if l.Sequence != nil { - h.SetSequence(*l.Sequence) + typ := h.Type() + switch typ { + case header.ICMPv4EchoReply, header.ICMPv4Echo: + if l.Ident != nil { + h.SetIdent(*l.Ident) + } + if l.Sequence != nil { + h.SetSequence(*l.Sequence) + } + case header.ICMPv4ParamProblem: + if l.Pointer != nil { + h.SetPointer(*l.Pointer) + } } // The checksum must be handled last because the ICMPv4 header fields are @@ -932,14 +950,21 @@ func (l *ICMPv4) ToBytes() ([]byte, error) { // parser for the encapsulated payload. func parseICMPv4(b []byte) (Layer, layerParser) { h := header.ICMPv4(b) + + msgType := h.Type() icmpv4 := ICMPv4{ - Type: ICMPv4Type(h.Type()), + Type: ICMPv4Type(msgType), Code: ICMPv4Code(h.Code()), Checksum: Uint16(h.Checksum()), - Ident: Uint16(h.Ident()), - Sequence: Uint16(h.Sequence()), Payload: h.Payload(), } + switch msgType { + case header.ICMPv4EchoReply, header.ICMPv4Echo: + icmpv4.Ident = Uint16(h.Ident()) + icmpv4.Sequence = Uint16(h.Sequence()) + case header.ICMPv4ParamProblem: + icmpv4.Pointer = Uint8(h.Pointer()) + } return &icmpv4, nil } diff --git a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go b/test/packetimpact/tests/ipv4_fragment_reassembly_test.go index 65c0df140..40f899065 100644 --- a/test/packetimpact/tests/ipv4_fragment_reassembly_test.go +++ b/test/packetimpact/tests/ipv4_fragment_reassembly_test.go @@ -105,11 +105,13 @@ func TestIPv4FragmentReassembly(t *testing.T) { var bytesReceived int reassembledPayload := make([]byte, test.ipPayloadLen) + // We are sending a packet fragmented into smaller parts but the + // response may also be large enough to require fragmentation. + // Therefore we only look for payload for an IPv4 packet not ICMP. for { incomingFrame, err := conn.ExpectFrame(t, testbench.Layers{ &testbench.Ether{}, &testbench.IPv4{}, - &testbench.ICMPv4{}, }, time.Second) if err != nil { // Either an unexpected frame was received, or none at all. @@ -121,9 +123,10 @@ func TestIPv4FragmentReassembly(t *testing.T) { if !test.expectReply { t.Fatalf("unexpected reply received:\n%s", incomingFrame) } - ipPayload, err := incomingFrame[2 /* ICMPv4 */].ToBytes() + // We only asked for Ethernet and IPv4 so the rest should be payload. + ipPayload, err := incomingFrame[2 /* Payload */].ToBytes() if err != nil { - t.Fatalf("failed to parse ICMPv4 header: incomingPacket[2].ToBytes() = (_, %s)", err) + t.Fatalf("failed to parse payload: incomingPacket[2].ToBytes() = (_, %s)", err) } offset := *incomingFrame[1 /* IPv4 */].(*testbench.IPv4).FragmentOffset if copied := copy(reassembledPayload[offset:], ipPayload); copied != len(ipPayload) { diff --git a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go index 4a29de688..eb56a53f7 100644 --- a/test/packetimpact/tests/ipv6_fragment_reassembly_test.go +++ b/test/packetimpact/tests/ipv6_fragment_reassembly_test.go @@ -117,7 +117,6 @@ func TestIPv6FragmentReassembly(t *testing.T) { &testbench.Ether{}, &testbench.IPv6{}, &testbench.IPv6FragmentExtHdr{}, - &testbench.ICMPv6{}, }, time.Second) if err != nil { // Either an unexpected frame was received, or none at all. @@ -129,7 +128,7 @@ func TestIPv6FragmentReassembly(t *testing.T) { if !test.expectReply { t.Fatalf("unexpected reply received:\n%s", incomingFrame) } - ipPayload, err := incomingFrame[3 /* ICMPv6 */].ToBytes() + ipPayload, err := incomingFrame[3 /* Payload */].ToBytes() if err != nil { t.Fatalf("failed to parse ICMPv6 header: incomingPacket[3].ToBytes() = (_, %s)", err) } -- cgit v1.2.3