Merge release-20210201.0-62-g3f802e718 (automated)

2 files changed, 131 insertions, 36 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.