From 906f912b7c9484fd0028224a24055b887d4f84d2 Mon Sep 17 00:00:00 2001
From: Arthur Sfez <asfez@google.com>
Date: Wed, 28 Oct 2020 18:16:16 -0700
Subject: Add the payload directly to the ICMPv4 type

This makes handling inbound fragmented packets easier, because a fragmented
packet might not have an actual ICMP header but only a payload. After this
change, the ICMPv4 is the last layer you can get because the payload is
embedded in it.

Note that this makes it consistent with the ICMPv6 implementation.

While I'm here, I've also added the Ident and Sequence fields on the ICMPv4
type. Defaults are still zero.

PiperOrigin-RevId: 339577094
---
 test/packetimpact/testbench/layers.go              | 40 +++++++++++++++++-----
 .../tests/tcp_network_unreachable_test.go          |  4 ++-
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/test/packetimpact/testbench/layers.go b/test/packetimpact/testbench/layers.go
index a35562ca8..af7a2ba4e 100644
--- a/test/packetimpact/testbench/layers.go
+++ b/test/packetimpact/testbench/layers.go
@@ -879,6 +879,9 @@ type ICMPv4 struct {
 	Type     *header.ICMPv4Type
 	Code     *header.ICMPv4Code
 	Checksum *uint16
+	Ident    *uint16
+	Sequence *uint16
+	Payload  []byte
 }
 
 func (l *ICMPv4) String() string {
@@ -887,7 +890,7 @@ func (l *ICMPv4) String() string {
 
 // ToBytes implements Layer.ToBytes.
 func (l *ICMPv4) ToBytes() ([]byte, error) {
-	b := make([]byte, header.ICMPv4MinimumSize)
+	b := make([]byte, header.ICMPv4MinimumSize+len(l.Payload))
 	h := header.ICMPv4(b)
 	if l.Type != nil {
 		h.SetType(*l.Type)
@@ -895,15 +898,33 @@ func (l *ICMPv4) ToBytes() ([]byte, error) {
 	if l.Code != nil {
 		h.SetCode(*l.Code)
 	}
+	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)
+	}
+
+	// The checksum must be handled last because the ICMPv4 header fields are
+	// included in the computation.
 	if l.Checksum != nil {
 		h.SetChecksum(*l.Checksum)
-		return h, nil
-	}
-	payload, err := payload(l)
-	if err != nil {
-		return nil, err
+	} else {
+		// Compute the checksum based on the ICMPv4.Payload and also the subsequent
+		// layers.
+		payload, err := payload(l)
+		if err != nil {
+			return nil, err
+		}
+		var vv buffer.VectorisedView
+		vv.AppendView(buffer.View(l.Payload))
+		vv.Append(payload)
+		h.SetChecksum(header.ICMPv4Checksum(h, vv))
 	}
-	h.SetChecksum(header.ICMPv4Checksum(h, payload))
+
 	return h, nil
 }
 
@@ -915,8 +936,11 @@ func parseICMPv4(b []byte) (Layer, layerParser) {
 		Type:     ICMPv4Type(h.Type()),
 		Code:     ICMPv4Code(h.Code()),
 		Checksum: Uint16(h.Checksum()),
+		Ident:    Uint16(h.Ident()),
+		Sequence: Uint16(h.Sequence()),
+		Payload:  h.Payload(),
 	}
-	return &icmpv4, parsePayload
+	return &icmpv4, nil
 }
 
 func (l *ICMPv4) match(other Layer) bool {
diff --git a/test/packetimpact/tests/tcp_network_unreachable_test.go b/test/packetimpact/tests/tcp_network_unreachable_test.go
index 2f57dff19..8a1fe1279 100644
--- a/test/packetimpact/tests/tcp_network_unreachable_test.go
+++ b/test/packetimpact/tests/tcp_network_unreachable_test.go
@@ -74,7 +74,9 @@ func TestTCPSynSentUnreachable(t *testing.T) {
 	}
 	var icmpv4 testbench.ICMPv4 = testbench.ICMPv4{
 		Type: testbench.ICMPv4Type(header.ICMPv4DstUnreachable),
-		Code: testbench.ICMPv4Code(header.ICMPv4HostUnreachable)}
+		Code: testbench.ICMPv4Code(header.ICMPv4HostUnreachable),
+	}
+
 	layers = append(layers, &icmpv4, ip, tcp)
 	rawConn.SendFrameStateless(t, layers)
 
-- 
cgit v1.2.3