Allow partial packets in ICMP errors when NATing

An ICMP error may not hold the full packet that triggered the ICMP
response. As long as the IP header and the transport header is
parsable, we should be able to successfully NAT as that is all that
we need to identify the connection.

PiperOrigin-RevId: 406966048

1 files changed, 156 insertions, 114 deletions

diff --git a/pkg/tcpip/tests/integration/iptables_test.go b/pkg/tcpip/tests/integration/iptables_test.go
index 7fe3b29d9..b2383576c 100644
--- a/pkg/tcpip/tests/integration/iptables_test.go
+++ b/pkg/tcpip/tests/integration/iptables_test.go
@@ -1781,8 +1781,11 @@ func TestNAT(t *testing.T) {
 }
 
 func TestNATICMPError(t *testing.T) {
-	const srcPort = 1234
-	const dstPort = 5432
+	const (
+		srcPort  = 1234
+		dstPort  = 5432
+		dataSize = 4
+	)
 
 	type icmpTypeTest struct {
 		name           string
@@ -1836,8 +1839,7 @@ func TestNATICMPError(t *testing.T) {
 			netProto:  ipv4.ProtocolNumber,
 			host1Addr: utils.Host1IPv4Addr.AddressWithPrefix.Address,
 			icmpError: func(t *testing.T, original buffer.View, icmpType uint8) buffer.View {
-				totalLen := header.IPv4MinimumSize + header.ICMPv4MinimumSize + len(original)
-				hdr := buffer.NewPrependable(totalLen)
+				hdr := buffer.NewPrependable(header.IPv4MinimumSize + header.ICMPv4MinimumSize + len(original))
 				if n := copy(hdr.Prepend(len(original)), original); n != len(original) {
 					t.Fatalf("got copy(...) = %d, want = %d", n, len(original))
 				}
@@ -1845,8 +1847,9 @@ func TestNATICMPError(t *testing.T) {
 				icmp.SetType(header.ICMPv4Type(icmpType))
 				icmp.SetChecksum(0)
 				icmp.SetChecksum(header.ICMPv4Checksum(icmp, 0))
-				ipHdr(hdr.Prepend(header.IPv4MinimumSize),
-					totalLen,
+				ipHdr(
+					hdr.Prepend(header.IPv4MinimumSize),
+					hdr.UsedLength(),
 					header.ICMPv4ProtocolNumber,
 					utils.Host1IPv4Addr.AddressWithPrefix.Address,
 					utils.RouterNIC1IPv4Addr.AddressWithPrefix.Address,
@@ -1875,9 +1878,9 @@ func TestNATICMPError(t *testing.T) {
 					name:  "UDP",
 					proto: header.UDPProtocolNumber,
 					buf: func() buffer.View {
-						totalLen := header.IPv4MinimumSize + header.UDPMinimumSize
-						hdr := buffer.NewPrependable(totalLen)
-						udp := header.UDP(hdr.Prepend(header.UDPMinimumSize))
+						udpSize := header.UDPMinimumSize + dataSize
+						hdr := buffer.NewPrependable(header.IPv4MinimumSize + udpSize)
+						udp := header.UDP(hdr.Prepend(udpSize))
 						udp.SetSourcePort(srcPort)
 						udp.SetDestinationPort(dstPort)
 						udp.SetChecksum(0)
@@ -1887,8 +1890,9 @@ func TestNATICMPError(t *testing.T) {
 							utils.RouterNIC2IPv4Addr.AddressWithPrefix.Address,
 							uint16(len(udp)),
 						)))
-						ipHdr(hdr.Prepend(header.IPv4MinimumSize),
-							totalLen,
+						ipHdr(
+							hdr.Prepend(header.IPv4MinimumSize),
+							hdr.UsedLength(),
 							header.UDPProtocolNumber,
 							utils.Host2IPv4Addr.AddressWithPrefix.Address,
 							utils.RouterNIC2IPv4Addr.AddressWithPrefix.Address,
@@ -1910,9 +1914,9 @@ func TestNATICMPError(t *testing.T) {
 					name:  "TCP",
 					proto: header.TCPProtocolNumber,
 					buf: func() buffer.View {
-						totalLen := header.IPv4MinimumSize + header.TCPMinimumSize
-						hdr := buffer.NewPrependable(totalLen)
-						tcp := header.TCP(hdr.Prepend(header.TCPMinimumSize))
+						tcpSize := header.TCPMinimumSize + dataSize
+						hdr := buffer.NewPrependable(header.IPv4MinimumSize + tcpSize)
+						tcp := header.TCP(hdr.Prepend(tcpSize))
 						tcp.SetSourcePort(srcPort)
 						tcp.SetDestinationPort(dstPort)
 						tcp.SetDataOffset(header.TCPMinimumSize)
@@ -1923,8 +1927,9 @@ func TestNATICMPError(t *testing.T) {
 							utils.RouterNIC2IPv4Addr.AddressWithPrefix.Address,
 							uint16(len(tcp)),
 						)))
-						ipHdr(hdr.Prepend(header.IPv4MinimumSize),
-							totalLen,
+						ipHdr(
+							hdr.Prepend(header.IPv4MinimumSize),
+							hdr.UsedLength(),
 							header.TCPProtocolNumber,
 							utils.Host2IPv4Addr.AddressWithPrefix.Address,
 							utils.RouterNIC2IPv4Addr.AddressWithPrefix.Address,
@@ -1989,7 +1994,8 @@ func TestNATICMPError(t *testing.T) {
 					Src:    utils.Host1IPv6Addr.AddressWithPrefix.Address,
 					Dst:    utils.RouterNIC1IPv6Addr.AddressWithPrefix.Address,
 				}))
-				ip6Hdr(hdr.Prepend(header.IPv6MinimumSize),
+				ip6Hdr(
+					hdr.Prepend(header.IPv6MinimumSize),
 					payloadLen,
 					header.ICMPv6ProtocolNumber,
 					utils.Host1IPv6Addr.AddressWithPrefix.Address,
@@ -2016,8 +2022,9 @@ func TestNATICMPError(t *testing.T) {
 					name:  "UDP",
 					proto: header.UDPProtocolNumber,
 					buf: func() buffer.View {
-						hdr := buffer.NewPrependable(header.IPv6MinimumSize + header.UDPMinimumSize)
-						udp := header.UDP(hdr.Prepend(header.UDPMinimumSize))
+						udpSize := header.UDPMinimumSize + dataSize
+						hdr := buffer.NewPrependable(header.IPv6MinimumSize + udpSize)
+						udp := header.UDP(hdr.Prepend(udpSize))
 						udp.SetSourcePort(srcPort)
 						udp.SetDestinationPort(dstPort)
 						udp.SetChecksum(0)
@@ -2027,8 +2034,9 @@ func TestNATICMPError(t *testing.T) {
 							utils.RouterNIC2IPv6Addr.AddressWithPrefix.Address,
 							uint16(len(udp)),
 						)))
-						ip6Hdr(hdr.Prepend(header.IPv6MinimumSize),
-							header.UDPMinimumSize,
+						ip6Hdr(
+							hdr.Prepend(header.IPv6MinimumSize),
+							len(udp),
 							header.UDPProtocolNumber,
 							utils.Host2IPv6Addr.AddressWithPrefix.Address,
 							utils.RouterNIC2IPv6Addr.AddressWithPrefix.Address,
@@ -2050,8 +2058,9 @@ func TestNATICMPError(t *testing.T) {
 					name:  "TCP",
 					proto: header.TCPProtocolNumber,
 					buf: func() buffer.View {
-						hdr := buffer.NewPrependable(header.IPv6MinimumSize + header.TCPMinimumSize)
-						tcp := header.TCP(hdr.Prepend(header.TCPMinimumSize))
+						tcpSize := header.TCPMinimumSize + dataSize
+						hdr := buffer.NewPrependable(header.IPv6MinimumSize + tcpSize)
+						tcp := header.TCP(hdr.Prepend(tcpSize))
 						tcp.SetSourcePort(srcPort)
 						tcp.SetDestinationPort(dstPort)
 						tcp.SetDataOffset(header.TCPMinimumSize)
@@ -2062,8 +2071,9 @@ func TestNATICMPError(t *testing.T) {
 							utils.RouterNIC2IPv6Addr.AddressWithPrefix.Address,
 							uint16(len(tcp)),
 						)))
-						ip6Hdr(hdr.Prepend(header.IPv6MinimumSize),
-							header.TCPMinimumSize,
+						ip6Hdr(
+							hdr.Prepend(header.IPv6MinimumSize),
+							len(tcp),
 							header.TCPProtocolNumber,
 							utils.Host2IPv6Addr.AddressWithPrefix.Address,
 							utils.RouterNIC2IPv6Addr.AddressWithPrefix.Address,
@@ -2117,109 +2127,141 @@ func TestNATICMPError(t *testing.T) {
 		},
 	}
 
+	trimTests := []struct {
+		name            string
+		trimLen         int
+		expectNATedICMP bool
+	}{
+		{
+			name:            "Trim nothing",
+			trimLen:         0,
+			expectNATedICMP: true,
+		},
+		{
+			name:            "Trim data",
+			trimLen:         dataSize,
+			expectNATedICMP: true,
+		},
+		{
+			name:            "Trim data and transport header",
+			trimLen:         dataSize + 1,
+			expectNATedICMP: false,
+		},
+	}
+
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			for _, transportType := range test.transportTypes {
 				t.Run(transportType.name, func(t *testing.T) {
 					for _, icmpType := range test.icmpTypes {
 						t.Run(icmpType.name, func(t *testing.T) {
-							s := stack.New(stack.Options{
-								NetworkProtocols:   []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol},
-								TransportProtocols: []stack.TransportProtocolFactory{udp.NewProtocol, tcp.NewProtocol},
-							})
-
-							ep1 := channel.New(1, header.IPv6MinimumMTU, "")
-							ep2 := channel.New(1, header.IPv6MinimumMTU, "")
-							utils.SetupRouterStack(t, s, ep1, ep2)
-
-							ipv6 := test.netProto == ipv6.ProtocolNumber
-							ipt := s.IPTables()
-
-							table := stack.Table{
-								Rules: []stack.Rule{
-									// Prerouting
-									{
-										Filter: stack.IPHeaderFilter{
-											Protocol:       transportType.proto,
-											CheckProtocol:  true,
-											InputInterface: utils.RouterNIC2Name,
+							for _, trimTest := range trimTests {
+								t.Run(trimTest.name, func(t *testing.T) {
+									s := stack.New(stack.Options{
+										NetworkProtocols:   []stack.NetworkProtocolFactory{ipv4.NewProtocol, ipv6.NewProtocol},
+										TransportProtocols: []stack.TransportProtocolFactory{udp.NewProtocol, tcp.NewProtocol},
+									})
+
+									ep1 := channel.New(1, header.IPv6MinimumMTU, "")
+									ep2 := channel.New(1, header.IPv6MinimumMTU, "")
+									utils.SetupRouterStack(t, s, ep1, ep2)
+
+									ipv6 := test.netProto == ipv6.ProtocolNumber
+									ipt := s.IPTables()
+
+									table := stack.Table{
+										Rules: []stack.Rule{
+											// Prerouting
+											{
+												Filter: stack.IPHeaderFilter{
+													Protocol:       transportType.proto,
+													CheckProtocol:  true,
+													InputInterface: utils.RouterNIC2Name,
+												},
+												Target: &stack.DNATTarget{NetworkProtocol: test.netProto, Addr: test.host1Addr, Port: dstPort},
+											},
+											{
+												Target: &stack.AcceptTarget{},
+											},
+
+											// Input
+											{
+												Target: &stack.AcceptTarget{},
+											},
+
+											// Forward
+											{
+												Target: &stack.AcceptTarget{},
+											},
+
+											// Output
+											{
+												Target: &stack.AcceptTarget{},
+											},
+
+											// Postrouting
+											{
+												Filter: stack.IPHeaderFilter{
+													Protocol:        transportType.proto,
+													CheckProtocol:   true,
+													OutputInterface: utils.RouterNIC1Name,
+												},
+												Target: &stack.MasqueradeTarget{NetworkProtocol: test.netProto},
+											},
+											{
+												Target: &stack.AcceptTarget{},
+											},
 										},
-										Target: &stack.DNATTarget{NetworkProtocol: test.netProto, Addr: test.host1Addr, Port: dstPort},
-									},
-									{
-										Target: &stack.AcceptTarget{},
-									},
-
-									// Input
-									{
-										Target: &stack.AcceptTarget{},
-									},
-
-									// Forward
-									{
-										Target: &stack.AcceptTarget{},
-									},
-
-									// Output
-									{
-										Target: &stack.AcceptTarget{},
-									},
-
-									// Postrouting
-									{
-										Filter: stack.IPHeaderFilter{
-											Protocol:        transportType.proto,
-											CheckProtocol:   true,
-											OutputInterface: utils.RouterNIC1Name,
+										BuiltinChains: [stack.NumHooks]int{
+											stack.Prerouting:  0,
+											stack.Input:       2,
+											stack.Forward:     3,
+											stack.Output:      4,
+											stack.Postrouting: 5,
 										},
-										Target: &stack.MasqueradeTarget{NetworkProtocol: test.netProto},
-									},
-									{
-										Target: &stack.AcceptTarget{},
-									},
-								},
-								BuiltinChains: [stack.NumHooks]int{
-									stack.Prerouting:  0,
-									stack.Input:       2,
-									stack.Forward:     3,
-									stack.Output:      4,
-									stack.Postrouting: 5,
-								},
-							}
+									}
 
-							if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil {
-								t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err)
-							}
+									if err := ipt.ReplaceTable(stack.NATID, table, ipv6); err != nil {
+										t.Fatalf("ipt.ReplaceTable(%d, _, %t): %s", stack.NATID, ipv6, err)
+									}
 
-							ep2.InjectInbound(test.netProto, stack.NewPacketBuffer(stack.PacketBufferOptions{
-								Data: append(buffer.View(nil), transportType.buf...).ToVectorisedView(),
-							}))
+									buf := transportType.buf
 
-							{
-								pkt, ok := ep1.Read()
-								if !ok {
-									t.Fatal("expected to read a packet on ep1")
-								}
-								pktView := stack.PayloadSince(pkt.Pkt.NetworkHeader())
-								transportType.checkNATed(t, pktView)
-								if t.Failed() {
-									t.FailNow()
-								}
+									ep2.InjectInbound(test.netProto, stack.NewPacketBuffer(stack.PacketBufferOptions{
+										Data: append(buffer.View(nil), buf...).ToVectorisedView(),
+									}))
 
-								ep1.InjectInbound(test.netProto, stack.NewPacketBuffer(stack.PacketBufferOptions{
-									Data: test.icmpError(t, pktView, icmpType.val).ToVectorisedView(),
-								}))
-							}
+									{
+										pkt, ok := ep1.Read()
+										if !ok {
+											t.Fatal("expected to read a packet on ep1")
+										}
+										pktView := stack.PayloadSince(pkt.Pkt.NetworkHeader())
+										transportType.checkNATed(t, pktView)
+										if t.Failed() {
+											t.FailNow()
+										}
+
+										pktView = pktView[:len(pktView)-trimTest.trimLen]
+										buf = buf[:len(buf)-trimTest.trimLen]
+
+										ep1.InjectInbound(test.netProto, stack.NewPacketBuffer(stack.PacketBufferOptions{
+											Data: test.icmpError(t, pktView, icmpType.val).ToVectorisedView(),
+										}))
+									}
 
-							pkt, ok := ep2.Read()
-							if ok != icmpType.expectResponse {
-								t.Fatalf("got ep2.Read() = (%#v, %t), want = (_, %t)", pkt, ok, icmpType.expectResponse)
-							}
-							if !icmpType.expectResponse {
-								return
+									pkt, ok := ep2.Read()
+									expectResponse := icmpType.expectResponse && trimTest.expectNATedICMP
+									if ok != expectResponse {
+										t.Fatalf("got ep2.Read() = (%#v, %t), want = (_, %t)", pkt, ok, expectResponse)
+									}
+									if !expectResponse {
+										return
+									}
+									test.decrementTTL(buf)
+									test.checkNATedError(t, stack.PayloadSince(pkt.Pkt.NetworkHeader()), buf, icmpType.val)
+								})
 							}
-							test.decrementTTL(transportType.buf)
-							test.checkNATedError(t, stack.PayloadSince(pkt.Pkt.NetworkHeader()), transportType.buf, icmpType.val)
 						})
 					}
 				})