add IPv4 options processing for forwarding and reassembly

IPv4 forwarding and reassembly needs support for option processing
and regular processing also needs options to be processed before
being passed to the transport layer. This patch extends option processing
to those cases and provides additional testing. A small change to the ICMP
error generation API code was required to allow it to know when a packet was
being forwarded or not.

Updates #4586

PiperOrigin-RevId: 356446681

3 files changed, 241 insertions, 53 deletions

diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go
index 2b7bc0dd0..b44304cee 100644
--- a/pkg/tcpip/network/ipv4/icmp.go
+++ b/pkg/tcpip/network/ipv4/icmp.go
@@ -213,7 +213,8 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 		} else {
 			op = &optionUsageReceive{}
 		}
-		tmp, optProblem := e.processIPOptions(pkt, opts, op)
+		var optProblem *header.IPv4OptParameterProblem
+		newOptions, optProblem = e.processIPOptions(pkt, opts, op)
 		if optProblem != nil {
 			if optProblem.NeedICMP {
 				_ = e.protocol.returnError(&icmpReasonParamProblem{
@@ -224,7 +225,14 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 			}
 			return
 		}
-		newOptions = tmp
+		copied := copy(opts, newOptions)
+		if copied != len(newOptions) {
+			panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOptions)))
+		}
+		for i := copied; i < len(opts); i++ {
+			// Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero".
+			opts[i] = byte(header.IPv4OptionListEndType)
+		}
 	}
 
 	// TODO(b/112892170): Meaningfully handle all ICMP types.
@@ -375,6 +383,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 // icmpReason is a marker interface for IPv4 specific ICMP errors.
 type icmpReason interface {
 	isICMPReason()
+	isForwarding() bool
 }
 
 // icmpReasonPortUnreachable is an error where the transport protocol has no
@@ -382,12 +391,18 @@ type icmpReason interface {
 type icmpReasonPortUnreachable struct{}
 
 func (*icmpReasonPortUnreachable) isICMPReason() {}
+func (*icmpReasonPortUnreachable) isForwarding() bool {
+	return false
+}
 
 // icmpReasonProtoUnreachable is an error where the transport protocol is
 // not supported.
 type icmpReasonProtoUnreachable struct{}
 
 func (*icmpReasonProtoUnreachable) isICMPReason() {}
+func (*icmpReasonProtoUnreachable) isForwarding() bool {
+	return false
+}
 
 // icmpReasonTTLExceeded is an error where a packet's time to live exceeded in
 // transit to its final destination, as per RFC 792 page 6, Time Exceeded
@@ -395,6 +410,15 @@ func (*icmpReasonProtoUnreachable) isICMPReason() {}
 type icmpReasonTTLExceeded struct{}
 
 func (*icmpReasonTTLExceeded) isICMPReason() {}
+func (*icmpReasonTTLExceeded) isForwarding() bool {
+	// If we hit a TTL Exceeded error, then we know we are operating as a router.
+	// As per RFC 792 page 6, Time Exceeded Message,
+	//
+	//   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 true
+}
 
 // icmpReasonReassemblyTimeout is an error where insufficient fragments are
 // received to complete reassembly of a packet within a configured time after
@@ -402,14 +426,21 @@ func (*icmpReasonTTLExceeded) isICMPReason() {}
 type icmpReasonReassemblyTimeout struct{}
 
 func (*icmpReasonReassemblyTimeout) isICMPReason() {}
+func (*icmpReasonReassemblyTimeout) isForwarding() bool {
+	return false
+}
 
 // icmpReasonParamProblem is an error to use to request a Parameter Problem
 // message to be sent.
 type icmpReasonParamProblem struct {
-	pointer byte
+	pointer    byte
+	forwarding bool
 }
 
 func (*icmpReasonParamProblem) isICMPReason() {}
+func (r *icmpReasonParamProblem) isForwarding() bool {
+	return r.forwarding
+}
 
 // returnError takes an error descriptor and generates the appropriate ICMP
 // error packet for IPv4 and sends it back to the remote device that sent
@@ -448,26 +479,14 @@ func (p *protocol) returnError(reason icmpReason, pkt *stack.PacketBuffer) tcpip
 		return nil
 	}
 
-	// If we hit a TTL Exceeded error, then we know we are operating as a router.
-	// As per RFC 792 page 6, Time Exceeded Message,
-	//
-	//   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.
-	//
-	//   ...
-	//
-	//   Code 0 may be received from a gateway. ...
-	//
-	// Note, Code 0 is the TTL exceeded error.
-	//
 	// If we are operating as a router/gateway, don't use the packet's destination
 	// address as the response's source address as we should not not own the
 	// destination address of a packet we are forwarding.
 	localAddr := origIPHdrDst
-	if _, ok := reason.(*icmpReasonTTLExceeded); ok {
+	if reason.isForwarding() {
 		localAddr = ""
 	}
+
 	// Even if we were able to receive a packet from some remote, we may not have
 	// a route to it - the remote may be blocked via routing rules. We must always
 	// consult our routing table and find a route to the remote before sending any
diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index 7de438fe3..c5e4034ce 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -561,6 +561,34 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 		return e.protocol.returnError(&icmpReasonTTLExceeded{}, pkt)
 	}
 
+	if opts := h.Options(); len(opts) != 0 {
+		newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{})
+		if optProblem != nil {
+			if optProblem.NeedICMP {
+				_ = e.protocol.returnError(&icmpReasonParamProblem{
+					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.
+		}
+		copied := copy(opts, newOpts)
+		if copied != len(newOpts) {
+			panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts)))
+		}
+		// Since in forwarding we handle all options, including copying those we
+		// do not recognise, the options region should remain the same size which
+		// simplifies processing. As we MAY receive a packet with a lot of padded
+		// bytes after the "end of options list" byte, make sure we copy
+		// them as the legal padding value (0).
+		for i := copied; i < len(opts); i++ {
+			// Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero".
+			opts[i] = byte(header.IPv4OptionListEndType)
+		}
+	}
+
 	dstAddr := h.DestinationAddress()
 
 	// Check if the destination is owned by the stack.
@@ -711,8 +739,9 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 		}
 	}
 
-	// The destination address should be an address we own or a group we joined
-	// for us to receive the packet. Otherwise, attempt to forward the packet.
+	// Before we do any processing, note if the packet was received as some
+	// sort of broadcast. The destination address should be an address we own
+	// or a group we joined.
 	if addressEndpoint := e.AcquireAssignedAddress(dstAddr, e.nic.Promiscuous(), stack.CanBePrimaryEndpoint); addressEndpoint != nil {
 		subnet := addressEndpoint.AddressWithPrefix().Subnet()
 		addressEndpoint.DecRef()
@@ -722,7 +751,6 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 			stats.ip.InvalidDestinationAddressesReceived.Increment()
 			return
 		}
-
 		_ = e.forwardPacket(pkt)
 		return
 	}
@@ -744,6 +772,21 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 			stats.ip.MalformedFragmentsReceived.Increment()
 			return
 		}
+		if opts := h.Options(); len(opts) != 0 {
+			// If there are options we need to check them before we do assembly
+			// or we could be assembling errant packets. However we do not change the
+			// options as that could lead to double processing later.
+			if _, optProblem := e.processIPOptions(pkt, opts, &optionUsageVerify{}); optProblem != nil {
+				if optProblem.NeedICMP {
+					_ = e.protocol.returnError(&icmpReasonParamProblem{
+						pointer: optProblem.Pointer,
+					}, pkt)
+					e.protocol.stack.Stats().MalformedRcvdPackets.Increment()
+					e.stats.ip.MalformedPacketsReceived.Increment()
+				}
+				return
+			}
+		}
 		// The packet is a fragment, let's try to reassemble it.
 		start := h.FragmentOffset()
 		// Drop the fragment if the size of the reassembled payload would exceed the
@@ -802,17 +845,10 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 		e.handleICMP(pkt)
 		return
 	}
-	if p == header.IGMPProtocolNumber {
-		e.mu.Lock()
-		e.mu.igmp.handleIGMP(pkt)
-		e.mu.Unlock()
-		return
-	}
+	// ICMP handles options itself but do it here for all remaining destinations.
 	if opts := h.Options(); len(opts) != 0 {
-		// TODO(gvisor.dev/issue/4586):
-		// When we add forwarding support we should use the verified options
-		// rather than just throwing them away.
-		if _, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{}); optProblem != nil {
+		newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{})
+		if optProblem != nil {
 			if optProblem.NeedICMP {
 				_ = e.protocol.returnError(&icmpReasonParamProblem{
 					pointer: optProblem.Pointer,
@@ -822,6 +858,20 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 			}
 			return
 		}
+		copied := copy(opts, newOpts)
+		if copied != len(newOpts) {
+			panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts)))
+		}
+		for i := copied; i < len(opts); i++ {
+			// Pad with 0 (EOL). RFC 791 page 23 says "The padding is zero".
+			opts[i] = byte(header.IPv4OptionListEndType)
+		}
+	}
+	if p == header.IGMPProtocolNumber {
+		e.mu.Lock()
+		e.mu.igmp.handleIGMP(pkt)
+		e.mu.Unlock()
+		return
 	}
 
 	switch res := e.dispatcher.DeliverTransportPacket(p, pkt); res {
@@ -1254,23 +1304,49 @@ type optionsUsage interface {
 	actions() optionActions
 }
 
-// optionUsageReceive implements optionsUsage for received packets.
-type optionUsageReceive struct{}
+// optionUsageVerify implements optionsUsage for when we just want to check
+// fragments. Don't change anything, just check and reject if bad. No
+// replacement options are generated.
+type optionUsageVerify struct{}
 
 // actions implements optionsUsage.
-func (*optionUsageReceive) actions() optionActions {
+func (*optionUsageVerify) actions() optionActions {
 	return optionActions{
 		timestamp:   optionVerify,
 		recordRoute: optionVerify,
+		unknown:     optionRemove,
+	}
+}
+
+// optionUsageReceive implements optionsUsage for packets we will pass
+// to the transport layer (with the exception of Echo requests).
+type optionUsageReceive struct{}
+
+// actions implements optionsUsage.
+func (*optionUsageReceive) actions() optionActions {
+	return optionActions{
+		timestamp:   optionProcess,
+		recordRoute: optionProcess,
 		unknown:     optionPass,
 	}
 }
 
-// TODO(gvisor.dev/issue/4586): Add an entry here for forwarding when it
-// is enabled (Process, Process, Pass) and for fragmenting (Process, Process,
-// Pass for frag1, but Remove,Remove,Remove for all other frags).
+// optionUsageForward implements optionsUsage for packets about to be forwarded.
+// All options are passed on regardless of whether we recognise them, however
+// we do process the Timestamp and Record Route options.
+type optionUsageForward struct{}
+
+// actions implements optionsUsage.
+func (*optionUsageForward) actions() optionActions {
+	return optionActions{
+		timestamp:   optionProcess,
+		recordRoute: optionProcess,
+		unknown:     optionPass,
+	}
+}
 
 // optionUsageEcho implements optionsUsage for echo packet processing.
+// Only Timestamp and RecordRoute are processed and sent back.
 type optionUsageEcho struct{}
 
 // actions implements optionsUsage.
diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go
index a296bed79..c57d7f616 100644
--- a/pkg/tcpip/network/ipv4/ipv4_test.go
+++ b/pkg/tcpip/network/ipv4/ipv4_test.go
@@ -111,10 +111,11 @@ func TestExcludeBroadcast(t *testing.T) {
 
 func TestForwarding(t *testing.T) {
 	const (
-		nicID1         = 1
-		nicID2         = 2
-		randomSequence = 123
-		randomIdent    = 42
+		nicID1           = 1
+		nicID2           = 2
+		randomSequence   = 123
+		randomIdent      = 42
+		randomTimeOffset = 0x10203040
 	)
 
 	ipv4Addr1 := tcpip.AddressWithPrefix{
@@ -129,14 +130,20 @@ func TestForwarding(t *testing.T) {
 	remoteIPv4Addr2 := tcpip.Address(net.ParseIP("11.0.0.2").To4())
 
 	tests := []struct {
-		name            string
-		TTL             uint8
-		expectErrorICMP bool
+		name             string
+		TTL              uint8
+		expectErrorICMP  bool
+		options          header.IPv4Options
+		forwardedOptions header.IPv4Options
+		icmpType         header.ICMPv4Type
+		icmpCode         header.ICMPv4Code
 	}{
 		{
 			name:            "TTL of zero",
 			TTL:             0,
 			expectErrorICMP: true,
+			icmpType:        header.ICMPv4TimeExceeded,
+			icmpCode:        header.ICMPv4TTLExceeded,
 		},
 		{
 			name:            "TTL of one",
@@ -153,14 +160,78 @@ func TestForwarding(t *testing.T) {
 			TTL:             math.MaxUint8,
 			expectErrorICMP: false,
 		},
+		{
+			name:             "four EOL options",
+			TTL:              2,
+			expectErrorICMP:  false,
+			options:          header.IPv4Options{0, 0, 0, 0},
+			forwardedOptions: header.IPv4Options{0, 0, 0, 0},
+		},
+		{
+			name: "TS type 1 full",
+			TTL:  2,
+			options: header.IPv4Options{
+				68, 12, 13, 0xF1,
+				192, 168, 1, 12,
+				1, 2, 3, 4,
+			},
+			expectErrorICMP: true,
+			icmpType:        header.ICMPv4ParamProblem,
+			icmpCode:        header.ICMPv4UnusedCode,
+		},
+		{
+			name: "TS type 0",
+			TTL:  2,
+			options: header.IPv4Options{
+				68, 24, 21, 0x00,
+				1, 2, 3, 4,
+				5, 6, 7, 8,
+				9, 10, 11, 12,
+				13, 14, 15, 16,
+				0, 0, 0, 0,
+			},
+			forwardedOptions: header.IPv4Options{
+				68, 24, 25, 0x00,
+				1, 2, 3, 4,
+				5, 6, 7, 8,
+				9, 10, 11, 12,
+				13, 14, 15, 16,
+				0x00, 0xad, 0x1c, 0x40, // time we expect from fakeclock
+			},
+		},
+		{
+			name: "end of options list",
+			TTL:  2,
+			options: header.IPv4Options{
+				68, 12, 13, 0x11,
+				192, 168, 1, 12,
+				1, 2, 3, 4,
+				0, 10, 3, 99, // EOL followed by junk
+				1, 2, 3, 4,
+			},
+			forwardedOptions: header.IPv4Options{
+				68, 12, 13, 0x21,
+				192, 168, 1, 12,
+				1, 2, 3, 4,
+				0,       // End of Options hides following bytes.
+				0, 0, 0, // 7 bytes unknown option removed.
+				0, 0, 0, 0,
+			},
+		},
 	}
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			clock := faketime.NewManualClock()
 			s := stack.New(stack.Options{
 				NetworkProtocols:   []stack.NetworkProtocolFactory{ipv4.NewProtocol},
 				TransportProtocols: []stack.TransportProtocolFactory{icmp.NewProtocol4},
+				Clock:              clock,
 			})
+
+			// Advance the clock by some unimportant amount to make
+			// it give a more recognisable signature than 00,00,00,00.
+			clock.Advance(time.Millisecond * randomTimeOffset)
+
 			// We expect at most a single packet in response to our ICMP Echo Request.
 			e1 := channel.New(1, ipv4.MaxTotalSize, "")
 			if err := s.CreateNIC(nicID1, e1); err != nil {
@@ -195,7 +266,11 @@ func TestForwarding(t *testing.T) {
 				t.Fatalf("SetForwarding(%d, true): %s", header.IPv4ProtocolNumber, err)
 			}
 
-			totalLen := uint16(header.IPv4MinimumSize + header.ICMPv4MinimumSize)
+			ipHeaderLength := header.IPv4MinimumSize + len(test.options)
+			if ipHeaderLength > header.IPv4MaximumHeaderSize {
+				t.Fatalf("got ipHeaderLength = %d, want <= %d ", ipHeaderLength, header.IPv4MaximumHeaderSize)
+			}
+			totalLen := uint16(ipHeaderLength + header.ICMPv4MinimumSize)
 			hdr := buffer.NewPrependable(int(totalLen))
 			icmp := header.ICMPv4(hdr.Prepend(header.ICMPv4MinimumSize))
 			icmp.SetIdent(randomIdent)
@@ -204,7 +279,7 @@ func TestForwarding(t *testing.T) {
 			icmp.SetCode(header.ICMPv4UnusedCode)
 			icmp.SetChecksum(0)
 			icmp.SetChecksum(^header.Checksum(icmp, 0))
-			ip := header.IPv4(hdr.Prepend(header.IPv4MinimumSize))
+			ip := header.IPv4(hdr.Prepend(ipHeaderLength))
 			ip.Encode(&header.IPv4Fields{
 				TotalLength: totalLen,
 				Protocol:    uint8(header.ICMPv4ProtocolNumber),
@@ -212,6 +287,14 @@ func TestForwarding(t *testing.T) {
 				SrcAddr:     remoteIPv4Addr1,
 				DstAddr:     remoteIPv4Addr2,
 			})
+			if len(test.options) != 0 {
+				ip.SetHeaderLength(uint8(ipHeaderLength))
+				// Copy options manually. We do not use Encode for options so we can
+				// verify malformed options with handcrafted payloads.
+				if want, got := copy(ip.Options(), test.options), len(test.options); want != got {
+					t.Fatalf("got copy(ip.Options(), test.options) = %d, want = %d", got, want)
+				}
+			}
 			ip.SetChecksum(0)
 			ip.SetChecksum(^ip.CalculateChecksum())
 			requestPkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
@@ -222,7 +305,7 @@ func TestForwarding(t *testing.T) {
 			if test.expectErrorICMP {
 				reply, ok := e1.Read()
 				if !ok {
-					t.Fatal("expected ICMP TTL Exceeded packet through incoming NIC")
+					t.Fatalf("expected ICMP packet type %d through incoming NIC", test.icmpType)
 				}
 
 				checker.IPv4(t, header.IPv4(stack.PayloadSince(reply.Pkt.NetworkHeader())),
@@ -231,8 +314,8 @@ func TestForwarding(t *testing.T) {
 					checker.TTL(ipv4.DefaultTTL),
 					checker.ICMPv4(
 						checker.ICMPv4Checksum(),
-						checker.ICMPv4Type(header.ICMPv4TimeExceeded),
-						checker.ICMPv4Code(header.ICMPv4TTLExceeded),
+						checker.ICMPv4Type(test.icmpType),
+						checker.ICMPv4Code(test.icmpCode),
 						checker.ICMPv4Payload([]byte(hdr.View())),
 					),
 				)
@@ -250,6 +333,7 @@ func TestForwarding(t *testing.T) {
 					checker.SrcAddr(remoteIPv4Addr1),
 					checker.DstAddr(remoteIPv4Addr2),
 					checker.TTL(test.TTL-1),
+					checker.IPv4Options(test.forwardedOptions),
 					checker.ICMPv4(
 						checker.ICMPv4Checksum(),
 						checker.ICMPv4Type(header.ICMPv4Echo),
@@ -279,6 +363,7 @@ func TestIPv4Sanity(t *testing.T) {
 		// (offset 1). For compatibility we must do the same. Use this constant
 		// to indicate where this happens.
 		pointerOffsetForInvalidLength = 0
+		randomTimeOffset              = 0x10203040
 	)
 	var (
 		ipv4Addr = tcpip.AddressWithPrefix{
@@ -893,6 +978,10 @@ func TestIPv4Sanity(t *testing.T) {
 				TransportProtocols: []stack.TransportProtocolFactory{icmp.NewProtocol4},
 				Clock:              clock,
 			})
+			// Advance the clock by some unimportant amount to make
+			// it give a more recognisable signature than 00,00,00,00.
+			clock.Advance(time.Millisecond * randomTimeOffset)
+
 			// We expect at most a single packet in response to our ICMP Echo Request.
 			e := channel.New(1, ipv4.MaxTotalSize, "")
 			if err := s.CreateNIC(nicID, e); err != nil {
@@ -902,9 +991,6 @@ func TestIPv4Sanity(t *testing.T) {
 			if err := s.AddProtocolAddress(nicID, ipv4ProtoAddr); err != nil {
 				t.Fatalf("AddProtocolAddress(%d, %#v): %s", nicID, ipv4ProtoAddr, err)
 			}
-			// Advance the clock by some unimportant amount to make
-			// sure it's all set up.
-			clock.Advance(time.Millisecond * 0x10203040)
 
 			// Default routes for IPv4 so ICMP can find a route to the remote
 			// node when attempting to send the ICMP Echo Reply.
@@ -1739,12 +1825,19 @@ func TestInvalidFragments(t *testing.T) {
 
 				ip := header.IPv4(hdr.Prepend(pktSize))
 				ip.Encode(&f.ipv4fields)
+				if want, got := len(f.payload), copy(ip[header.IPv4MinimumSize:], f.payload); want != got {
+					t.Fatalf("copied %d bytes, expected %d bytes.", got, want)
+				}
 				// Encode sets this up correctly. If we want a different value for
 				// testing then we need to overwrite the good value.
 				if f.overrideIHL != 0 {
 					ip.SetHeaderLength(uint8(f.overrideIHL))
+					// If we are asked to add options (type not specified) then pad
+					// with 0 (EOL). RFC 791 page 23 says "The padding is zero".
+					for i := header.IPv4MinimumSize; i < f.overrideIHL; i++ {
+						ip[i] = byte(header.IPv4OptionListEndType)
+					}
 				}
-				copy(ip[header.IPv4MinimumSize:], f.payload)
 
 				if f.autoChecksum {
 					ip.SetChecksum(0)