Internal change.

PiperOrigin-RevId: 258424489

7 files changed, 112 insertions, 45 deletions

diff --git a/pkg/tcpip/header/icmpv4.go b/pkg/tcpip/header/icmpv4.go
index c081de61f..c52c0d851 100644
--- a/pkg/tcpip/header/icmpv4.go
+++ b/pkg/tcpip/header/icmpv4.go
@@ -24,15 +24,11 @@ import (
 type ICMPv4 []byte
 
 const (
-	// ICMPv4MinimumSize is the minimum size of a valid ICMP packet.
-	ICMPv4MinimumSize = 4
-
-	// ICMPv4EchoMinimumSize is the minimum size of a valid ICMP echo packet.
-	ICMPv4EchoMinimumSize = 6
+	// ICMPv4PayloadOffset defines the start of ICMP payload.
+	ICMPv4PayloadOffset = 4
 
-	// ICMPv4DstUnreachableMinimumSize is the minimum size of a valid ICMP
-	// destination unreachable packet.
-	ICMPv4DstUnreachableMinimumSize = ICMPv4MinimumSize + 4
+	// ICMPv4MinimumSize is the minimum size of a valid ICMP packet.
+	ICMPv4MinimumSize = 8
 
 	// ICMPv4ProtocolNumber is the ICMP transport protocol number.
 	ICMPv4ProtocolNumber tcpip.TransportProtocolNumber = 1
@@ -104,5 +100,5 @@ func (ICMPv4) SetDestinationPort(uint16) {
 
 // Payload implements Transport.Payload.
 func (b ICMPv4) Payload() []byte {
-	return b[ICMPv4MinimumSize:]
+	return b[ICMPv4PayloadOffset:]
 }
diff --git a/pkg/tcpip/network/ip_test.go b/pkg/tcpip/network/ip_test.go
index db65ee7cc..8ff428445 100644
--- a/pkg/tcpip/network/ip_test.go
+++ b/pkg/tcpip/network/ip_test.go
@@ -282,10 +282,10 @@ func TestIPv4ReceiveControl(t *testing.T) {
 		{"Truncated (10 bytes missing)", 0, 0, header.ICMPv4FragmentationNeeded, stack.ControlPacketTooBig, mtu, 10},
 		{"Truncated (missing IPv4 header)", 0, 0, header.ICMPv4FragmentationNeeded, stack.ControlPacketTooBig, mtu, header.IPv4MinimumSize + 8},
 		{"Truncated (missing 'extra info')", 0, 0, header.ICMPv4FragmentationNeeded, stack.ControlPacketTooBig, mtu, 4 + header.IPv4MinimumSize + 8},
-		{"Truncated (missing ICMP header)", 0, 0, header.ICMPv4FragmentationNeeded, stack.ControlPacketTooBig, mtu, header.ICMPv4DstUnreachableMinimumSize + header.IPv4MinimumSize + 8},
+		{"Truncated (missing ICMP header)", 0, 0, header.ICMPv4FragmentationNeeded, stack.ControlPacketTooBig, mtu, header.ICMPv4MinimumSize + header.IPv4MinimumSize + 8},
 		{"Port unreachable", 1, 0, header.ICMPv4PortUnreachable, stack.ControlPortUnreachable, 0, 0},
 		{"Non-zero fragment offset", 0, 100, header.ICMPv4PortUnreachable, stack.ControlPortUnreachable, 0, 0},
-		{"Zero-length packet", 0, 0, header.ICMPv4PortUnreachable, stack.ControlPortUnreachable, 0, 2*header.IPv4MinimumSize + header.ICMPv4DstUnreachableMinimumSize + 8},
+		{"Zero-length packet", 0, 0, header.ICMPv4PortUnreachable, stack.ControlPortUnreachable, 0, 2*header.IPv4MinimumSize + header.ICMPv4MinimumSize + 8},
 	}
 	r, err := buildIPv4Route(localIpv4Addr, "\x0a\x00\x00\xbb")
 	if err != nil {
@@ -301,7 +301,7 @@ func TestIPv4ReceiveControl(t *testing.T) {
 			}
 			defer ep.Close()
 
-			const dataOffset = header.IPv4MinimumSize*2 + header.ICMPv4MinimumSize + 4
+			const dataOffset = header.IPv4MinimumSize*2 + header.ICMPv4MinimumSize
 			view := buffer.NewView(dataOffset + 8)
 
 			// Create the outer IPv4 header.
@@ -319,10 +319,10 @@ func TestIPv4ReceiveControl(t *testing.T) {
 			icmp := header.ICMPv4(view[header.IPv4MinimumSize:])
 			icmp.SetType(header.ICMPv4DstUnreachable)
 			icmp.SetCode(c.code)
-			copy(view[header.IPv4MinimumSize+header.ICMPv4MinimumSize:], []byte{0xde, 0xad, 0xbe, 0xef})
+			copy(view[header.IPv4MinimumSize+header.ICMPv4PayloadOffset:], []byte{0xde, 0xad, 0xbe, 0xef})
 
 			// Create the inner IPv4 header.
-			ip = header.IPv4(view[header.IPv4MinimumSize+header.ICMPv4MinimumSize+4:])
+			ip = header.IPv4(view[header.IPv4MinimumSize+header.ICMPv4MinimumSize:])
 			ip.Encode(&header.IPv4Fields{
 				IHL:            header.IPv4MinimumSize,
 				TotalLength:    100,
diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go
index bc7f1c42a..fbef6947d 100644
--- a/pkg/tcpip/network/ipv4/icmp.go
+++ b/pkg/tcpip/network/ipv4/icmp.go
@@ -68,10 +68,6 @@ func (e *endpoint) handleICMP(r *stack.Route, netHeader buffer.View, vv buffer.V
 	switch h.Type() {
 	case header.ICMPv4Echo:
 		received.Echo.Increment()
-		if len(v) < header.ICMPv4EchoMinimumSize {
-			received.Invalid.Increment()
-			return
-		}
 
 		// Only send a reply if the checksum is valid.
 		wantChecksum := h.Checksum()
@@ -93,9 +89,9 @@ func (e *endpoint) handleICMP(r *stack.Route, netHeader buffer.View, vv buffer.V
 		e.dispatcher.DeliverTransportPacket(r, header.ICMPv4ProtocolNumber, netHeader, vv)
 
 		vv := vv.Clone(nil)
-		vv.TrimFront(header.ICMPv4EchoMinimumSize)
-		hdr := buffer.NewPrependable(int(r.MaxHeaderLength()) + header.ICMPv4EchoMinimumSize)
-		pkt := header.ICMPv4(hdr.Prepend(header.ICMPv4EchoMinimumSize))
+		vv.TrimFront(header.ICMPv4MinimumSize)
+		hdr := buffer.NewPrependable(int(r.MaxHeaderLength()) + header.ICMPv4MinimumSize)
+		pkt := header.ICMPv4(hdr.Prepend(header.ICMPv4MinimumSize))
 		copy(pkt, h)
 		pkt.SetType(header.ICMPv4EchoReply)
 		pkt.SetChecksum(^header.Checksum(pkt, header.ChecksumVV(vv, 0)))
@@ -108,25 +104,19 @@ func (e *endpoint) handleICMP(r *stack.Route, netHeader buffer.View, vv buffer.V
 
 	case header.ICMPv4EchoReply:
 		received.EchoReply.Increment()
-		if len(v) < header.ICMPv4EchoMinimumSize {
-			received.Invalid.Increment()
-			return
-		}
+
 		e.dispatcher.DeliverTransportPacket(r, header.ICMPv4ProtocolNumber, netHeader, vv)
 
 	case header.ICMPv4DstUnreachable:
 		received.DstUnreachable.Increment()
-		if len(v) < header.ICMPv4DstUnreachableMinimumSize {
-			received.Invalid.Increment()
-			return
-		}
-		vv.TrimFront(header.ICMPv4DstUnreachableMinimumSize)
+
+		vv.TrimFront(header.ICMPv4MinimumSize)
 		switch h.Code() {
 		case header.ICMPv4PortUnreachable:
 			e.handleControl(stack.ControlPortUnreachable, 0, vv)
 
 		case header.ICMPv4FragmentationNeeded:
-			mtu := uint32(binary.BigEndian.Uint16(v[header.ICMPv4DstUnreachableMinimumSize-2:]))
+			mtu := uint32(binary.BigEndian.Uint16(v[header.ICMPv4PayloadOffset+2:]))
 			e.handleControl(stack.ControlPacketTooBig, calculateMTU(mtu), vv)
 		}
 
diff --git a/pkg/tcpip/transport/icmp/endpoint.go b/pkg/tcpip/transport/icmp/endpoint.go
index ab9e80747..a80ceafd0 100644
--- a/pkg/tcpip/transport/icmp/endpoint.go
+++ b/pkg/tcpip/transport/icmp/endpoint.go
@@ -291,7 +291,7 @@ func (e *endpoint) Write(p tcpip.Payload, opts tcpip.WriteOptions) (uintptr, <-c
 
 	switch e.netProto {
 	case header.IPv4ProtocolNumber:
-		err = e.send4(route, v)
+		err = send4(route, e.id.LocalPort, v)
 
 	case header.IPv6ProtocolNumber:
 		err = send6(route, e.id.LocalPort, v)
@@ -352,20 +352,20 @@ func (e *endpoint) GetSockOpt(opt interface{}) *tcpip.Error {
 	}
 }
 
-func (e *endpoint) send4(r *stack.Route, data buffer.View) *tcpip.Error {
-	if len(data) < header.ICMPv4EchoMinimumSize {
+func send4(r *stack.Route, ident uint16, data buffer.View) *tcpip.Error {
+	if len(data) < header.ICMPv4MinimumSize {
 		return tcpip.ErrInvalidEndpointState
 	}
 
 	// Set the ident to the user-specified port. Sequence number should
 	// already be set by the user.
-	binary.BigEndian.PutUint16(data[header.ICMPv4MinimumSize:], e.id.LocalPort)
+	binary.BigEndian.PutUint16(data[header.ICMPv4PayloadOffset:], ident)
 
-	hdr := buffer.NewPrependable(header.ICMPv4EchoMinimumSize + int(r.MaxHeaderLength()))
+	hdr := buffer.NewPrependable(header.ICMPv4MinimumSize + int(r.MaxHeaderLength()))
 
-	icmpv4 := header.ICMPv4(hdr.Prepend(header.ICMPv4EchoMinimumSize))
+	icmpv4 := header.ICMPv4(hdr.Prepend(header.ICMPv4MinimumSize))
 	copy(icmpv4, data)
-	data = data[header.ICMPv4EchoMinimumSize:]
+	data = data[header.ICMPv4MinimumSize:]
 
 	// Linux performs these basic checks.
 	if icmpv4.Type() != header.ICMPv4Echo || icmpv4.Code() != 0 {
diff --git a/pkg/tcpip/transport/icmp/protocol.go b/pkg/tcpip/transport/icmp/protocol.go
index c89538131..7fdba5d56 100644
--- a/pkg/tcpip/transport/icmp/protocol.go
+++ b/pkg/tcpip/transport/icmp/protocol.go
@@ -90,19 +90,18 @@ func (p *protocol) NewRawEndpoint(stack *stack.Stack, netProto tcpip.NetworkProt
 func (p *protocol) MinimumPacketSize() int {
 	switch p.number {
 	case ProtocolNumber4:
-		return header.ICMPv4EchoMinimumSize
+		return header.ICMPv4MinimumSize
 	case ProtocolNumber6:
 		return header.ICMPv6EchoMinimumSize
 	}
 	panic(fmt.Sprint("unknown protocol number: ", p.number))
 }
 
-// ParsePorts returns the source and destination ports stored in the given icmp
-// packet.
+// ParsePorts in case of ICMP sets src to 0, dst to ICMP ID, and err to nil.
 func (p *protocol) ParsePorts(v buffer.View) (src, dst uint16, err *tcpip.Error) {
 	switch p.number {
 	case ProtocolNumber4:
-		return 0, binary.BigEndian.Uint16(v[header.ICMPv4MinimumSize:]), nil
+		return 0, binary.BigEndian.Uint16(v[header.ICMPv4PayloadOffset:]), nil
 	case ProtocolNumber6:
 		return 0, binary.BigEndian.Uint16(v[header.ICMPv6MinimumSize:]), nil
 	}
diff --git a/pkg/tcpip/transport/tcp/testing/context/context.go b/pkg/tcpip/transport/tcp/testing/context/context.go
index 630dd7925..bcc0f3e28 100644
--- a/pkg/tcpip/transport/tcp/testing/context/context.go
+++ b/pkg/tcpip/transport/tcp/testing/context/context.go
@@ -271,7 +271,7 @@ func (c *Context) GetPacketNonBlocking() []byte {
 // SendICMPPacket builds and sends an ICMPv4 packet via the link layer endpoint.
 func (c *Context) SendICMPPacket(typ header.ICMPv4Type, code uint8, p1, p2 []byte, maxTotalSize int) {
 	// Allocate a buffer data and headers.
-	buf := buffer.NewView(header.IPv4MinimumSize + header.ICMPv4MinimumSize + len(p1) + len(p2))
+	buf := buffer.NewView(header.IPv4MinimumSize + header.ICMPv4PayloadOffset + len(p1) + len(p2))
 	if len(buf) > maxTotalSize {
 		buf = buf[:maxTotalSize]
 	}
@@ -291,8 +291,8 @@ func (c *Context) SendICMPPacket(typ header.ICMPv4Type, code uint8, p1, p2 []byt
 	icmp.SetType(typ)
 	icmp.SetCode(code)
 
-	copy(icmp[header.ICMPv4MinimumSize:], p1)
-	copy(icmp[header.ICMPv4MinimumSize+len(p1):], p2)
+	copy(icmp[header.ICMPv4PayloadOffset:], p1)
+	copy(icmp[header.ICMPv4PayloadOffset+len(p1):], p2)
 
 	// Inject packet.
 	c.linkEP.Inject(ipv4.ProtocolNumber, buf.ToVectorisedView())
diff --git a/test/syscalls/linux/raw_socket_icmp.cc b/test/syscalls/linux/raw_socket_icmp.cc
index 8314588ce..ad19120d5 100644
--- a/test/syscalls/linux/raw_socket_icmp.cc
+++ b/test/syscalls/linux/raw_socket_icmp.cc
@@ -282,6 +282,88 @@ TEST_F(RawSocketICMPTest, RawAndPingSockets) {
       0);
 }
 
+// A raw ICMP socket should be able to send a malformed short ICMP Echo Request,
+// while ping socket should not.
+// Neither should be able to receieve a short malformed packet.
+TEST_F(RawSocketICMPTest, ShortEchoRawAndPingSockets) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_RAW)));
+
+  FileDescriptor ping_sock =
+      ASSERT_NO_ERRNO_AND_VALUE(Socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP));
+
+  struct icmphdr icmp;
+  icmp.type = ICMP_ECHO;
+  icmp.code = 0;
+  icmp.un.echo.sequence = 0;
+  icmp.un.echo.id = 6789;
+  icmp.checksum = 0;
+  icmp.checksum = Checksum(&icmp);
+
+  // Omit 2 bytes from ICMP packet.
+  constexpr int kShortICMPSize = sizeof(icmp) - 2;
+
+  // Sending a malformed short ICMP message to a ping socket should fail.
+  ASSERT_THAT(RetryEINTR(sendto)(ping_sock.get(), &icmp, kShortICMPSize, 0,
+                                 reinterpret_cast<struct sockaddr*>(&addr_),
+                                 sizeof(addr_)),
+              SyscallFailsWithErrno(EINVAL));
+
+  // Sending a malformed short ICMP message to a raw socket should not fail.
+  ASSERT_THAT(RetryEINTR(sendto)(s_, &icmp, kShortICMPSize, 0,
+                                 reinterpret_cast<struct sockaddr*>(&addr_),
+                                 sizeof(addr_)),
+              SyscallSucceedsWithValue(kShortICMPSize));
+
+  // Neither Ping nor Raw socket should have anything to read.
+  char recv_buf[kEmptyICMPSize];
+  EXPECT_THAT(RetryEINTR(recv)(ping_sock.get(), recv_buf, sizeof(recv_buf),
+                               MSG_DONTWAIT),
+              SyscallFailsWithErrno(EAGAIN));
+  EXPECT_THAT(RetryEINTR(recv)(s_, recv_buf, sizeof(recv_buf), MSG_DONTWAIT),
+              SyscallFailsWithErrno(EAGAIN));
+}
+
+// A raw ICMP socket should be able to send a malformed short ICMP Echo Reply,
+// while ping socket should not.
+// Neither should be able to receieve a short malformed packet.
+TEST_F(RawSocketICMPTest, ShortEchoReplyRawAndPingSockets) {
+  SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_RAW)));
+
+  FileDescriptor ping_sock =
+      ASSERT_NO_ERRNO_AND_VALUE(Socket(AF_INET, SOCK_DGRAM, IPPROTO_ICMP));
+
+  struct icmphdr icmp;
+  icmp.type = ICMP_ECHOREPLY;
+  icmp.code = 0;
+  icmp.un.echo.sequence = 0;
+  icmp.un.echo.id = 6789;
+  icmp.checksum = 0;
+  icmp.checksum = Checksum(&icmp);
+
+  // Omit 2 bytes from ICMP packet.
+  constexpr int kShortICMPSize = sizeof(icmp) - 2;
+
+  // Sending a malformed short ICMP message to a ping socket should fail.
+  ASSERT_THAT(RetryEINTR(sendto)(ping_sock.get(), &icmp, kShortICMPSize, 0,
+                                 reinterpret_cast<struct sockaddr*>(&addr_),
+                                 sizeof(addr_)),
+              SyscallFailsWithErrno(EINVAL));
+
+  // Sending a malformed short ICMP message to a raw socket should not fail.
+  ASSERT_THAT(RetryEINTR(sendto)(s_, &icmp, kShortICMPSize, 0,
+                                 reinterpret_cast<struct sockaddr*>(&addr_),
+                                 sizeof(addr_)),
+              SyscallSucceedsWithValue(kShortICMPSize));
+
+  // Neither Ping nor Raw socket should have anything to read.
+  char recv_buf[kEmptyICMPSize];
+  EXPECT_THAT(RetryEINTR(recv)(ping_sock.get(), recv_buf, sizeof(recv_buf),
+                               MSG_DONTWAIT),
+              SyscallFailsWithErrno(EAGAIN));
+  EXPECT_THAT(RetryEINTR(recv)(s_, recv_buf, sizeof(recv_buf), MSG_DONTWAIT),
+              SyscallFailsWithErrno(EAGAIN));
+}
+
 // Test that connect() sends packets to the right place.
 TEST_F(RawSocketICMPTest, SendAndReceiveViaConnect) {
   SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_RAW)));