Send ICMP errors when the network is unreachable

Before this change, we would silently drop packets when unable to determine a
route to the destination host. This change brings us into line with RFC 792
(IPv4) and RFC 4443 (IPv6), both of which specify that gateways should return
an ICMP error to the sender when unable to reach the destination.

Startblock:
  has LGTM from asfez
  and then
  add reviewer ghanan
PiperOrigin-RevId: 372214051

3 files changed, 207 insertions, 57 deletions

diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go
index 3f2093f00..c8ed1ce79 100644
--- a/pkg/tcpip/network/ipv4/icmp.go
+++ b/pkg/tcpip/network/ipv4/icmp.go
@@ -446,6 +446,23 @@ func (r *icmpReasonParamProblem) isForwarding() bool {
 	return r.forwarding
 }
 
+// icmpReasonNetworkUnreachable is an error in which the network specified in
+// the internet destination field of the datagram is unreachable.
+type icmpReasonNetworkUnreachable struct{}
+
+func (*icmpReasonNetworkUnreachable) isICMPReason() {}
+func (*icmpReasonNetworkUnreachable) isForwarding() bool {
+	// If we hit a Net Unreachable error, then we know we are operating as
+	// a router. As per RFC 792 page 5, Destination Unreachable Message,
+	//
+	//  If, according to the information in the gateway's routing tables,
+	//  the network specified in the internet destination field of a
+	//  datagram is unreachable, e.g., the distance to the network is
+	//  infinity, the gateway may send a destination unreachable message to
+	//  the internet source host of the datagram.
+	return true
+}
+
 // returnError takes an error descriptor and generates the appropriate ICMP
 // error packet for IPv4 and sends it back to the remote device that sent
 // the problematic packet. It incorporates as much of that packet as
@@ -614,6 +631,10 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip
 		icmpHdr.SetType(header.ICMPv4DstUnreachable)
 		icmpHdr.SetCode(header.ICMPv4ProtoUnreachable)
 		counter = sent.dstUnreachable
+	case *icmpReasonNetworkUnreachable:
+		icmpHdr.SetType(header.ICMPv4DstUnreachable)
+		icmpHdr.SetCode(header.ICMPv4NetUnreachable)
+		counter = sent.dstUnreachable
 	case *icmpReasonTTLExceeded:
 		icmpHdr.SetType(header.ICMPv4TimeExceeded)
 		icmpHdr.SetCode(header.ICMPv4TTLExceeded)
diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index a0bc06465..b11e56c6a 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -29,6 +29,7 @@ import (
 	"gvisor.dev/gvisor/pkg/tcpip/header/parse"
 	"gvisor.dev/gvisor/pkg/tcpip/network/hash"
 	"gvisor.dev/gvisor/pkg/tcpip/network/internal/fragmentation"
+	"gvisor.dev/gvisor/pkg/tcpip/network/internal/ip"
 	"gvisor.dev/gvisor/pkg/tcpip/stack"
 )
 
@@ -599,22 +600,25 @@ func (e *endpoint) WriteHeaderIncludedPacket(r *stack.Route, pkt *stack.PacketBu
 }
 
 // forwardPacket attempts to forward a packet to its final destination.
-func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
+func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) ip.ForwardingError {
 	h := header.IPv4(pkt.NetworkHeader().View())
 
 	dstAddr := h.DestinationAddress()
-	if header.IsV4LinkLocalUnicastAddress(h.SourceAddress()) || header.IsV4LinkLocalUnicastAddress(dstAddr) || header.IsV4LinkLocalMulticastAddress(dstAddr) {
-		// As per RFC 3927 section 7,
-		//
-		//   A router MUST NOT forward a packet with an IPv4 Link-Local source or
-		//   destination address, irrespective of the router's default route
-		//   configuration or routes obtained from dynamic routing protocols.
-		//
-		//   A router which receives a packet with an IPv4 Link-Local source or
-		//   destination address MUST NOT forward the packet.  This prevents
-		//   forwarding of packets back onto the network segment from which they
-		//   originated, or to any other segment.
-		return nil
+	// As per RFC 3927 section 7,
+	//
+	//   A router MUST NOT forward a packet with an IPv4 Link-Local source or
+	//   destination address, irrespective of the router's default route
+	//   configuration or routes obtained from dynamic routing protocols.
+	//
+	//   A router which receives a packet with an IPv4 Link-Local source or
+	//   destination address MUST NOT forward the packet.  This prevents
+	//   forwarding of packets back onto the network segment from which they
+	//   originated, or to any other segment.
+	if header.IsV4LinkLocalUnicastAddress(h.SourceAddress()) {
+		return &ip.ErrLinkLocalSourceAddress{}
+	}
+	if header.IsV4LinkLocalUnicastAddress(dstAddr) || header.IsV4LinkLocalMulticastAddress(dstAddr) {
+		return &ip.ErrLinkLocalDestinationAddress{}
 	}
 
 	ttl := h.TTL()
@@ -624,7 +628,12 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 		//  If the gateway processing a datagram finds the time to live field
 		//  is zero it must discard the datagram.  The gateway may also notify
 		//  the source host via the time exceeded message.
-		return e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt)
+		//
+		// We return the original error rather than the result of returning
+		// the ICMP packet because the original error is more relevant to
+		// the caller.
+		_ = e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt)
+		return &ip.ErrTTLExceeded{}
 	}
 
 	if opts := h.Options(); len(opts) != 0 {
@@ -635,10 +644,8 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 					pointer:    optProblem.Pointer,
 					forwarding: true,
 				}, pkt)
-				e.protocol.stack.Stats().MalformedRcvdPackets.Increment()
-				e.stats.ip.MalformedPacketsReceived.Increment()
 			}
-			return nil // option problems are not reported locally.
+			return &ip.ErrIPOptProblem{}
 		}
 		copied := copy(opts, newOpts)
 		if copied != len(newOpts) {
@@ -662,8 +669,16 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 	}
 
 	r, err := e.protocol.stack.FindRoute(0, "", dstAddr, ProtocolNumber, false /* multicastLoop */)
-	if err != nil {
-		return err
+	switch err.(type) {
+	case nil:
+	case *tcpip.ErrNoRoute, *tcpip.ErrNetworkUnreachable:
+		// We return the original error rather than the result of returning
+		// the ICMP packet because the original error is more relevant to
+		// the caller.
+		_ = e.protocol.returnError(&icmpReasonNetworkUnreachable{}, pkt)
+		return &ip.ErrNoRoute{}
+	default:
+		return &ip.ErrOther{Err: err}
 	}
 	defer r.Release()
 
@@ -680,10 +695,13 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 	//   spent, the field must be decremented by 1.
 	newHdr.SetTTL(ttl - 1)
 
-	return r.WriteHeaderIncludedPacket(stack.NewPacketBuffer(stack.PacketBufferOptions{
+	if err := r.WriteHeaderIncludedPacket(stack.NewPacketBuffer(stack.PacketBufferOptions{
 		ReserveHeaderBytes: int(r.MaxHeaderLength()),
 		Data:               buffer.View(newHdr).ToVectorisedView(),
-	}))
+	})); err != nil {
+		return &ip.ErrOther{Err: err}
+	}
+	return nil
 }
 
 // HandlePacket is called by the link layer when new ipv4 packets arrive for
@@ -798,7 +816,24 @@ func (e *endpoint) handleValidatedPacket(h header.IPv4, pkt *stack.PacketBuffer)
 			stats.ip.InvalidDestinationAddressesReceived.Increment()
 			return
 		}
-		_ = e.forwardPacket(pkt)
+		switch err := e.forwardPacket(pkt); err.(type) {
+		case nil:
+			return
+		case *ip.ErrLinkLocalSourceAddress:
+			stats.ip.Forwarding.LinkLocalSource.Increment()
+		case *ip.ErrLinkLocalDestinationAddress:
+			stats.ip.Forwarding.LinkLocalDestination.Increment()
+		case *ip.ErrTTLExceeded:
+			stats.ip.Forwarding.ExhaustedTTL.Increment()
+		case *ip.ErrNoRoute:
+			stats.ip.Forwarding.Unrouteable.Increment()
+		case *ip.ErrIPOptProblem:
+			e.protocol.stack.Stats().MalformedRcvdPackets.Increment()
+			stats.ip.MalformedPacketsReceived.Increment()
+		default:
+			panic(fmt.Sprintf("unexpected error %s while trying to forward packet: %#v", err, pkt))
+		}
+		stats.ip.Forwarding.Errors.Increment()
 		return
 	}
 
diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go
index 7d413c455..7a7cad04a 100644
--- a/pkg/tcpip/network/ipv4/ipv4_test.go
+++ b/pkg/tcpip/network/ipv4/ipv4_test.go
@@ -131,48 +131,69 @@ func TestForwarding(t *testing.T) {
 	}
 	remoteIPv4Addr1 := tcpip.Address(net.ParseIP("10.0.0.2").To4())
 	remoteIPv4Addr2 := tcpip.Address(net.ParseIP("11.0.0.2").To4())
+	unreachableIPv4Addr := tcpip.Address(net.ParseIP("12.0.0.2").To4())
+	multicastIPv4Addr := tcpip.Address(net.ParseIP("225.0.0.0").To4())
+	linkLocalIPv4Addr := tcpip.Address(net.ParseIP("169.254.0.0").To4())
 
 	tests := []struct {
-		name             string
-		TTL              uint8
-		expectErrorICMP  bool
-		options          header.IPv4Options
-		forwardedOptions header.IPv4Options
-		icmpType         header.ICMPv4Type
-		icmpCode         header.ICMPv4Code
+		name                         string
+		TTL                          uint8
+		sourceAddr                   tcpip.Address
+		destAddr                     tcpip.Address
+		expectErrorICMP              bool
+		expectPacketForwarded        bool
+		options                      header.IPv4Options
+		forwardedOptions             header.IPv4Options
+		icmpType                     header.ICMPv4Type
+		icmpCode                     header.ICMPv4Code
+		expectPacketUnrouteableError bool
+		expectLinkLocalSourceError   bool
+		expectLinkLocalDestError     bool
 	}{
 		{
 			name:            "TTL of zero",
 			TTL:             0,
+			sourceAddr:      remoteIPv4Addr1,
+			destAddr:        remoteIPv4Addr2,
 			expectErrorICMP: true,
 			icmpType:        header.ICMPv4TimeExceeded,
 			icmpCode:        header.ICMPv4TTLExceeded,
 		},
 		{
-			name:            "TTL of one",
-			TTL:             1,
-			expectErrorICMP: false,
+			name:                  "TTL of one",
+			TTL:                   1,
+			sourceAddr:            remoteIPv4Addr1,
+			destAddr:              remoteIPv4Addr2,
+			expectPacketForwarded: true,
 		},
 		{
-			name:            "TTL of two",
-			TTL:             2,
-			expectErrorICMP: false,
+			name:                  "TTL of two",
+			TTL:                   2,
+			sourceAddr:            remoteIPv4Addr1,
+			destAddr:              remoteIPv4Addr2,
+			expectPacketForwarded: true,
 		},
 		{
-			name:            "Max TTL",
-			TTL:             math.MaxUint8,
-			expectErrorICMP: false,
+			name:                  "Max TTL",
+			TTL:                   math.MaxUint8,
+			sourceAddr:            remoteIPv4Addr1,
+			destAddr:              remoteIPv4Addr2,
+			expectPacketForwarded: true,
 		},
 		{
-			name:             "four EOL options",
-			TTL:              2,
-			expectErrorICMP:  false,
-			options:          header.IPv4Options{0, 0, 0, 0},
-			forwardedOptions: header.IPv4Options{0, 0, 0, 0},
+			name:                  "four EOL options",
+			TTL:                   2,
+			sourceAddr:            remoteIPv4Addr1,
+			destAddr:              remoteIPv4Addr2,
+			expectPacketForwarded: true,
+			options:               header.IPv4Options{0, 0, 0, 0},
+			forwardedOptions:      header.IPv4Options{0, 0, 0, 0},
 		},
 		{
-			name: "TS type 1 full",
-			TTL:  2,
+			name:       "TS type 1 full",
+			TTL:        2,
+			sourceAddr: remoteIPv4Addr1,
+			destAddr:   remoteIPv4Addr2,
 			options: header.IPv4Options{
 				68, 12, 13, 0xF1,
 				192, 168, 1, 12,
@@ -183,8 +204,10 @@ func TestForwarding(t *testing.T) {
 			icmpCode:        header.ICMPv4UnusedCode,
 		},
 		{
-			name: "TS type 0",
-			TTL:  2,
+			name:       "TS type 0",
+			TTL:        2,
+			sourceAddr: remoteIPv4Addr1,
+			destAddr:   remoteIPv4Addr2,
 			options: header.IPv4Options{
 				68, 24, 21, 0x00,
 				1, 2, 3, 4,
@@ -201,10 +224,13 @@ func TestForwarding(t *testing.T) {
 				13, 14, 15, 16,
 				0x00, 0xad, 0x1c, 0x40, // time we expect from fakeclock
 			},
+			expectPacketForwarded: true,
 		},
 		{
-			name: "end of options list",
-			TTL:  2,
+			name:       "end of options list",
+			TTL:        2,
+			sourceAddr: remoteIPv4Addr1,
+			destAddr:   remoteIPv4Addr2,
 			options: header.IPv4Options{
 				68, 12, 13, 0x11,
 				192, 168, 1, 12,
@@ -220,6 +246,37 @@ func TestForwarding(t *testing.T) {
 				0, 0, 0, // 7 bytes unknown option removed.
 				0, 0, 0, 0,
 			},
+			expectPacketForwarded: true,
+		},
+		{
+			name:                         "Network unreachable",
+			TTL:                          2,
+			sourceAddr:                   remoteIPv4Addr1,
+			destAddr:                     unreachableIPv4Addr,
+			expectErrorICMP:              true,
+			icmpType:                     header.ICMPv4DstUnreachable,
+			icmpCode:                     header.ICMPv4NetUnreachable,
+			expectPacketUnrouteableError: true,
+		},
+		{
+			name:                         "Multicast destination",
+			TTL:                          2,
+			destAddr:                     multicastIPv4Addr,
+			expectPacketUnrouteableError: true,
+		},
+		{
+			name:                     "Link local destination",
+			TTL:                      2,
+			sourceAddr:               remoteIPv4Addr1,
+			destAddr:                 linkLocalIPv4Addr,
+			expectLinkLocalDestError: true,
+		},
+		{
+			name:                       "Link local source",
+			TTL:                        2,
+			sourceAddr:                 linkLocalIPv4Addr,
+			destAddr:                   remoteIPv4Addr2,
+			expectLinkLocalSourceError: true,
 		},
 	}
 	for _, test := range tests {
@@ -287,8 +344,8 @@ func TestForwarding(t *testing.T) {
 				TotalLength: totalLen,
 				Protocol:    uint8(header.ICMPv4ProtocolNumber),
 				TTL:         test.TTL,
-				SrcAddr:     remoteIPv4Addr1,
-				DstAddr:     remoteIPv4Addr2,
+				SrcAddr:     test.sourceAddr,
+				DstAddr:     test.destAddr,
 			})
 			if len(test.options) != 0 {
 				ip.SetHeaderLength(uint8(ipHeaderLength))
@@ -305,15 +362,15 @@ func TestForwarding(t *testing.T) {
 			})
 			e1.InjectInbound(header.IPv4ProtocolNumber, requestPkt)
 
+			reply, ok := e1.Read()
 			if test.expectErrorICMP {
-				reply, ok := e1.Read()
 				if !ok {
 					t.Fatalf("expected ICMP packet type %d through incoming NIC", test.icmpType)
 				}
 
 				checker.IPv4(t, header.IPv4(stack.PayloadSince(reply.Pkt.NetworkHeader())),
 					checker.SrcAddr(ipv4Addr1.Address),
-					checker.DstAddr(remoteIPv4Addr1),
+					checker.DstAddr(test.sourceAddr),
 					checker.TTL(ipv4.DefaultTTL),
 					checker.ICMPv4(
 						checker.ICMPv4Checksum(),
@@ -326,15 +383,19 @@ func TestForwarding(t *testing.T) {
 				if n := e2.Drain(); n != 0 {
 					t.Fatalf("got e2.Drain() = %d, want = 0", n)
 				}
-			} else {
-				reply, ok := e2.Read()
+			} else if ok {
+				t.Fatalf("expected no ICMP packet through incoming NIC, instead found: %#v", reply)
+			}
+
+			reply, ok = e2.Read()
+			if test.expectPacketForwarded {
 				if !ok {
 					t.Fatal("expected ICMP Echo packet through outgoing NIC")
 				}
 
 				checker.IPv4(t, header.IPv4(stack.PayloadSince(reply.Pkt.NetworkHeader())),
-					checker.SrcAddr(remoteIPv4Addr1),
-					checker.DstAddr(remoteIPv4Addr2),
+					checker.SrcAddr(test.sourceAddr),
+					checker.DstAddr(test.destAddr),
 					checker.TTL(test.TTL-1),
 					checker.IPv4Options(test.forwardedOptions),
 					checker.ICMPv4(
@@ -348,6 +409,39 @@ func TestForwarding(t *testing.T) {
 				if n := e1.Drain(); n != 0 {
 					t.Fatalf("got e1.Drain() = %d, want = 0", n)
 				}
+			} else if ok {
+				t.Fatalf("expected no ICMP Echo packet through outgoing NIC, instead found: %#v", reply)
+			}
+
+			boolToInt := func(val bool) uint64 {
+				if val {
+					return 1
+				}
+				return 0
+			}
+
+			if got, want := s.Stats().IP.Forwarding.LinkLocalSource.Value(), boolToInt(test.expectLinkLocalSourceError); got != want {
+				t.Errorf("got s.Stats().IP.Forwarding.LinkLocalSource.Value() = %d, want = %d", got, want)
+			}
+
+			if got, want := s.Stats().IP.Forwarding.LinkLocalDestination.Value(), boolToInt(test.expectLinkLocalDestError); got != want {
+				t.Errorf("got s.Stats().IP.Forwarding.LinkLocalDestination.Value() = %d, want = %d", got, want)
+			}
+
+			if got, want := s.Stats().IP.MalformedPacketsReceived.Value(), boolToInt(test.icmpType == header.ICMPv4ParamProblem); got != want {
+				t.Errorf("got s.Stats().IP.MalformedPacketsReceived.Value() = %d, want = %d", got, want)
+			}
+
+			if got, want := s.Stats().IP.Forwarding.ExhaustedTTL.Value(), boolToInt(test.TTL <= 0); got != want {
+				t.Errorf("got s.Stats().IP.Forwarding.ExhaustedTTL.Value() = %d, want = %d", got, want)
+			}
+
+			if got, want := s.Stats().IP.Forwarding.Unrouteable.Value(), boolToInt(test.expectPacketUnrouteableError); got != want {
+				t.Errorf("got s.Stats().IP.Forwarding.Unrouteable.Value() = %d, want = %d", got, want)
+			}
+
+			if got, want := s.Stats().IP.Forwarding.Errors.Value(), boolToInt(!test.expectPacketForwarded); got != want {
+				t.Errorf("got s.Stats().IP.Forwarding.Errors.Value() = %d, want = %d", got, want)
 			}
 		})
 	}