Merge release-20210208.0-71-g2a2cb29e1 (automated)

4 files changed, 161 insertions, 63 deletions

diff --git a/pkg/tcpip/network/internal/ip/stats.go b/pkg/tcpip/network/internal/ip/stats.go
index 898f8b356..5f7e60c5c 100644
--- a/pkg/tcpip/network/internal/ip/stats.go
+++ b/pkg/tcpip/network/internal/ip/stats.go
@@ -68,11 +68,17 @@ type MultiCounterIPStats struct {
 	// Output chain.
 	IPTablesOutputDropped tcpip.MultiCounterStat
 
-	// OptionTSReceived is the number of Timestamp options seen.
-	OptionTSReceived tcpip.MultiCounterStat
+	// TODO(https://gvisor.dev/issues/5529): Move the IPv4-only option stats out
 
-	// OptionRRReceived is the number of Record Route options seen.
-	OptionRRReceived tcpip.MultiCounterStat
+	// of IPStats.
+	// OptionTimestampReceived is the number of Timestamp options seen.
+	OptionTimestampReceived tcpip.MultiCounterStat
+
+	// OptionRecordRouteReceived is the number of Record Route options seen.
+	OptionRecordRouteReceived tcpip.MultiCounterStat
+
+	// OptionRouterAlertReceived is the number of Router Alert options seen.
+	OptionRouterAlertReceived tcpip.MultiCounterStat
 
 	// OptionUnknownReceived is the number of unknown IP options seen.
 	OptionUnknownReceived tcpip.MultiCounterStat
@@ -92,9 +98,10 @@ func (m *MultiCounterIPStats) Init(a, b *tcpip.IPStats) {
 	m.IPTablesPreroutingDropped.Init(a.IPTablesPreroutingDropped, b.IPTablesPreroutingDropped)
 	m.IPTablesInputDropped.Init(a.IPTablesInputDropped, b.IPTablesInputDropped)
 	m.IPTablesOutputDropped.Init(a.IPTablesOutputDropped, b.IPTablesOutputDropped)
-	m.OptionTSReceived.Init(a.OptionTSReceived, b.OptionTSReceived)
-	m.OptionRRReceived.Init(a.OptionRRReceived, b.OptionRRReceived)
+	m.OptionTimestampReceived.Init(a.OptionTimestampReceived, b.OptionTimestampReceived)
+	m.OptionRecordRouteReceived.Init(a.OptionRecordRouteReceived, b.OptionRecordRouteReceived)
+	m.OptionRouterAlertReceived.Init(a.OptionRouterAlertReceived, b.OptionRouterAlertReceived)
 	m.OptionUnknownReceived.Init(a.OptionUnknownReceived, b.OptionUnknownReceived)
 }
 
-// LINT.ThenChange(:MultiCounterIPStats, ../../tcpip.go:IPStats)
+// LINT.ThenChange(:MultiCounterIPStats, ../../../tcpip.go:IPStats)
diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go
index b44304cee..bd0eabad1 100644
--- a/pkg/tcpip/network/ipv4/icmp.go
+++ b/pkg/tcpip/network/ipv4/icmp.go
@@ -214,7 +214,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer) {
 			op = &optionUsageReceive{}
 		}
 		var optProblem *header.IPv4OptParameterProblem
-		newOptions, optProblem = e.processIPOptions(pkt, opts, op)
+		newOptions, _, optProblem = e.processIPOptions(pkt, opts, op)
 		if optProblem != nil {
 			if optProblem.NeedICMP {
 				_ = e.protocol.returnError(&icmpReasonParamProblem{
diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go
index 12632aceb..0a15ae897 100644
--- a/pkg/tcpip/network/ipv4/igmp.go
+++ b/pkg/tcpip/network/ipv4/igmp.go
@@ -145,10 +145,57 @@ func (igmp *igmpState) init(ep *endpoint) {
 	})
 }
 
+// Precondition: igmp.ep.mu must be locked.
+func (igmp *igmpState) isSourceIPValidLocked(src tcpip.Address, messageType header.IGMPType) bool {
+	if messageType == header.IGMPMembershipQuery {
+		// RFC 2236 does not require the IGMP implementation to check the source IP
+		// for Membership Query messages.
+		return true
+	}
+
+	// As per RFC 2236 section 10,
+	//
+	//   Ignore the Report if you cannot identify the source address of the
+	//   packet as belonging to a subnet assigned to the interface on which the
+	//   packet was received.
+	//
+	//   Ignore the Leave message if you cannot identify the source address of
+	//   the packet as belonging to a subnet assigned to the interface on which
+	//   the packet was received.
+	//
+	// Note: this rule applies to both V1 and V2 Membership Reports.
+	var isSourceIPValid bool
+	igmp.ep.mu.addressableEndpointState.ForEachPrimaryEndpoint(func(addressEndpoint stack.AddressEndpoint) bool {
+		if subnet := addressEndpoint.Subnet(); subnet.Contains(src) {
+			isSourceIPValid = true
+			return false
+		}
+		return true
+	})
+
+	return isSourceIPValid
+}
+
+// Precondition: igmp.ep.mu must be locked.
+func (igmp *igmpState) isPacketValidLocked(pkt *stack.PacketBuffer, messageType header.IGMPType, hasRouterAlertOption bool) bool {
+	// We can safely assume that the IP header is valid if we got this far.
+	iph := header.IPv4(pkt.NetworkHeader().View())
+
+	// As per RFC 2236 section 2,
+	//
+	//   All IGMP messages described in this document are sent with IP TTL 1, and
+	//   contain the IP Router Alert option [RFC 2113] in their IP header.
+	if !hasRouterAlertOption || iph.TTL() != header.IGMPTTL {
+		return false
+	}
+
+	return igmp.isSourceIPValidLocked(iph.SourceAddress(), messageType)
+}
+
 // handleIGMP handles an IGMP packet.
 //
 // Precondition: igmp.ep.mu must be locked.
-func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer) {
+func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer, hasRouterAlertOption bool) {
 	received := igmp.ep.stats.igmp.packetsReceived
 	headerView, ok := pkt.Data.PullUp(header.IGMPMinimumSize)
 	if !ok {
@@ -168,30 +215,38 @@ func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer) {
 		return
 	}
 
+	isValid := func(minimumSize int) bool {
+		return len(headerView) >= minimumSize && igmp.isPacketValidLocked(pkt, h.Type(), hasRouterAlertOption)
+	}
+
 	switch h.Type() {
 	case header.IGMPMembershipQuery:
 		received.membershipQuery.Increment()
-		if len(headerView) < header.IGMPQueryMinimumSize {
+		if !isValid(header.IGMPQueryMinimumSize) {
 			received.invalid.Increment()
 			return
 		}
 		igmp.handleMembershipQuery(h.GroupAddress(), h.MaxRespTime())
 	case header.IGMPv1MembershipReport:
 		received.v1MembershipReport.Increment()
-		if len(headerView) < header.IGMPReportMinimumSize {
+		if !isValid(header.IGMPReportMinimumSize) {
 			received.invalid.Increment()
 			return
 		}
 		igmp.handleMembershipReport(h.GroupAddress())
 	case header.IGMPv2MembershipReport:
 		received.v2MembershipReport.Increment()
-		if len(headerView) < header.IGMPReportMinimumSize {
+		if !isValid(header.IGMPReportMinimumSize) {
 			received.invalid.Increment()
 			return
 		}
 		igmp.handleMembershipReport(h.GroupAddress())
 	case header.IGMPLeaveGroup:
 		received.leaveGroup.Increment()
+		if !isValid(header.IGMPLeaveMessageMinimumSize) {
+			received.invalid.Increment()
+			return
+		}
 		// As per RFC 2236 Section 6, Page 7: "IGMP messages other than Query or
 		// Report, are ignored in all states"
 
diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index 250e4846a..4a429ea6c 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -562,7 +562,7 @@ func (e *endpoint) forwardPacket(pkt *stack.PacketBuffer) tcpip.Error {
 	}
 
 	if opts := h.Options(); len(opts) != 0 {
-		newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{})
+		newOpts, _, optProblem := e.processIPOptions(pkt, opts, &optionUsageForward{})
 		if optProblem != nil {
 			if optProblem.NeedICMP {
 				_ = e.protocol.returnError(&icmpReasonParamProblem{
@@ -776,7 +776,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 			// 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 := e.processIPOptions(pkt, opts, &optionUsageVerify{}); optProblem != nil {
 				if optProblem.NeedICMP {
 					_ = e.protocol.returnError(&icmpReasonParamProblem{
 						pointer: optProblem.Pointer,
@@ -846,8 +846,9 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 		return
 	}
 	// ICMP handles options itself but do it here for all remaining destinations.
+	var hasRouterAlertOption bool
 	if opts := h.Options(); len(opts) != 0 {
-		newOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{})
+		newOpts, processedOpts, optProblem := e.processIPOptions(pkt, opts, &optionUsageReceive{})
 		if optProblem != nil {
 			if optProblem.NeedICMP {
 				_ = e.protocol.returnError(&icmpReasonParamProblem{
@@ -858,6 +859,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 			}
 			return
 		}
+		hasRouterAlertOption = processedOpts.routerAlert
 		copied := copy(opts, newOpts)
 		if copied != len(newOpts) {
 			panic(fmt.Sprintf("copied %d bytes of new options, expected %d bytes", copied, len(newOpts)))
@@ -869,7 +871,7 @@ func (e *endpoint) handlePacket(pkt *stack.PacketBuffer) {
 	}
 	if p == header.IGMPProtocolNumber {
 		e.mu.Lock()
-		e.mu.igmp.handleIGMP(pkt)
+		e.mu.igmp.handleIGMP(pkt, hasRouterAlertOption)
 		e.mu.Unlock()
 		return
 	}
@@ -1291,9 +1293,12 @@ type optionActions struct {
 	// timestamp controls what to do with a Timestamp option.
 	timestamp optionAction
 
-	// recordroute controls what to do with a Record Route option.
+	// recordRoute controls what to do with a Record Route option.
 	recordRoute optionAction
 
+	// routerAlert controls what to do with a Router Alert option.
+	routerAlert optionAction
+
 	// unknown controls what to do with an unknown option.
 	unknown optionAction
 }
@@ -1314,6 +1319,7 @@ func (*optionUsageVerify) actions() optionActions {
 	return optionActions{
 		timestamp:   optionVerify,
 		recordRoute: optionVerify,
+		routerAlert: optionVerify,
 		unknown:     optionRemove,
 	}
 }
@@ -1327,6 +1333,7 @@ func (*optionUsageReceive) actions() optionActions {
 	return optionActions{
 		timestamp:   optionProcess,
 		recordRoute: optionProcess,
+		routerAlert: optionVerify,
 		unknown:     optionPass,
 	}
 }
@@ -1341,6 +1348,7 @@ func (*optionUsageForward) actions() optionActions {
 	return optionActions{
 		timestamp:   optionProcess,
 		recordRoute: optionProcess,
+		routerAlert: optionVerify,
 		unknown:     optionPass,
 	}
 }
@@ -1354,6 +1362,7 @@ func (*optionUsageEcho) actions() optionActions {
 	return optionActions{
 		timestamp:   optionProcess,
 		recordRoute: optionProcess,
+		routerAlert: optionVerify,
 		unknown:     optionRemove,
 	}
 }
@@ -1551,45 +1560,64 @@ func handleRecordRoute(rrOpt header.IPv4OptionRecordRoute, localAddress tcpip.Ad
 	return nil
 }
 
+// handleRouterAlert performs sanity checks on a Router Alert option.
+func handleRouterAlert(raOpt header.IPv4OptionRouterAlert) *header.IPv4OptParameterProblem {
+	// Only the zero value is acceptable, as per RFC 2113, section 2.1:
+	//   Value:  A two octet code with the following values:
+	//     0 - Router shall examine packet
+	//     1-65535 - Reserved
+	if raOpt.Value() != header.IPv4OptionRouterAlertValue {
+		return &header.IPv4OptParameterProblem{
+			Pointer:  header.IPv4OptionRouterAlertValueOffset,
+			NeedICMP: true,
+		}
+	}
+	return nil
+}
+
+type optionTracker struct {
+	timestamp   bool
+	recordRoute bool
+	routerAlert bool
+}
+
 // processIPOptions parses the IPv4 options and produces a new set of options
 // suitable for use in the next step of packet processing as informed by usage.
 // The original will not be touched.
 //
-// Returns
-// - The location of an error if there was one (or 0 if no error)
-// - If there is an error, information as to what it was was.
-// - The replacement option set.
-func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Options, usage optionsUsage) (header.IPv4Options, *header.IPv4OptParameterProblem) {
+// If there were no errors during parsing, the new set of options is returned as
+// a new buffer.
+func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Options, usage optionsUsage) (header.IPv4Options, optionTracker, *header.IPv4OptParameterProblem) {
 	stats := e.stats.ip
 	opts := header.IPv4Options(orig)
 	optIter := opts.MakeIterator()
 
-	// Each option other than NOP must only appear (RFC 791 section 3.1, at the
-	// definition of every type). Keep track of each of the possible types in
-	// the 8 bit 'type' field.
+	// Except NOP, each option must only appear at most once (RFC 791 section 3.1,
+	// at the definition of every type).
+	// Keep track of each option we find to enable duplicate option detection.
 	var seenOptions [math.MaxUint8 + 1]bool
 
-	// TODO(gvisor.dev/issue/4586):
-	// This will need tweaking  when we start really forwarding packets
-	// as we may need to get two addresses, for rx and tx interfaces.
-	// We will also have to take usage into account.
+	// TODO(https://gvisor.dev/issue/4586): This will need tweaking when we start
+	// really forwarding packets as we may need to get two addresses, for rx and
+	// tx interfaces. We will also have to take usage into account.
 	prefixedAddress, ok := e.protocol.stack.GetMainNICAddress(e.nic.ID(), ProtocolNumber)
 	localAddress := prefixedAddress.Address
 	if !ok {
 		h := header.IPv4(pkt.NetworkHeader().View())
 		dstAddr := h.DestinationAddress()
 		if pkt.NetworkPacketInfo.LocalAddressBroadcast || header.IsV4MulticastAddress(dstAddr) {
-			return nil, &header.IPv4OptParameterProblem{
+			return nil, optionTracker{}, &header.IPv4OptParameterProblem{
 				NeedICMP: false,
 			}
 		}
 		localAddress = dstAddr
 	}
 
+	var optionsProcessed optionTracker
 	for {
 		option, done, optProblem := optIter.Next()
 		if done || optProblem != nil {
-			return optIter.Finalize(), optProblem
+			return optIter.Finalize(), optionsProcessed, optProblem
 		}
 		optType := option.Type()
 		if optType == header.IPv4OptionNOPType {
@@ -1598,53 +1626,61 @@ func (e *endpoint) processIPOptions(pkt *stack.PacketBuffer, orig header.IPv4Opt
 		}
 		if optType == header.IPv4OptionListEndType {
 			optIter.PushNOPOrEnd(optType)
-			return optIter.Finalize(), nil
+			return optIter.Finalize(), optionsProcessed, nil
 		}
 
 		// check for repeating options (multiple NOPs are OK)
 		if seenOptions[optType] {
-			return nil, &header.IPv4OptParameterProblem{
+			return nil, optionTracker{}, &header.IPv4OptParameterProblem{
 				Pointer:  optIter.ErrCursor,
 				NeedICMP: true,
 			}
 		}
 		seenOptions[optType] = true
 
-		optLen := int(option.Size())
-		switch option := option.(type) {
-		case *header.IPv4OptionTimestamp:
-			stats.OptionTSReceived.Increment()
-			if usage.actions().timestamp != optionRemove {
-				clock := e.protocol.stack.Clock()
-				newBuffer := optIter.RemainingBuffer()[:len(*option)]
-				_ = copy(newBuffer, option.Contents())
-				if optProblem := handleTimestamp(header.IPv4OptionTimestamp(newBuffer), localAddress, clock, usage); optProblem != nil {
-					optProblem.Pointer += optIter.ErrCursor
-					return nil, optProblem
+		optLen, optProblem := func() (int, *header.IPv4OptParameterProblem) {
+			switch option := option.(type) {
+			case *header.IPv4OptionTimestamp:
+				stats.OptionTimestampReceived.Increment()
+				optionsProcessed.timestamp = true
+				if usage.actions().timestamp != optionRemove {
+					clock := e.protocol.stack.Clock()
+					newBuffer := optIter.InitReplacement(option)
+					optProblem := handleTimestamp(header.IPv4OptionTimestamp(newBuffer), localAddress, clock, usage)
+					return len(newBuffer), optProblem
 				}
-				optIter.ConsumeBuffer(optLen)
-			}
 
-		case *header.IPv4OptionRecordRoute:
-			stats.OptionRRReceived.Increment()
-			if usage.actions().recordRoute != optionRemove {
-				newBuffer := optIter.RemainingBuffer()[:len(*option)]
-				_ = copy(newBuffer, option.Contents())
-				if optProblem := handleRecordRoute(header.IPv4OptionRecordRoute(newBuffer), localAddress, usage); optProblem != nil {
-					optProblem.Pointer += optIter.ErrCursor
-					return nil, optProblem
+			case *header.IPv4OptionRecordRoute:
+				stats.OptionRecordRouteReceived.Increment()
+				optionsProcessed.recordRoute = true
+				if usage.actions().recordRoute != optionRemove {
+					newBuffer := optIter.InitReplacement(option)
+					optProblem := handleRecordRoute(header.IPv4OptionRecordRoute(newBuffer), localAddress, usage)
+					return len(newBuffer), optProblem
 				}
-				optIter.ConsumeBuffer(optLen)
-			}
 
-		default:
-			stats.OptionUnknownReceived.Increment()
-			if usage.actions().unknown == optionPass {
-				newBuffer := optIter.RemainingBuffer()[:optLen]
-				// Arguments already heavily checked.. ignore result.
-				_ = copy(newBuffer, option.Contents())
-				optIter.ConsumeBuffer(optLen)
+			case *header.IPv4OptionRouterAlert:
+				stats.OptionRouterAlertReceived.Increment()
+				optionsProcessed.routerAlert = true
+				if usage.actions().routerAlert != optionRemove {
+					newBuffer := optIter.InitReplacement(option)
+					optProblem := handleRouterAlert(header.IPv4OptionRouterAlert(newBuffer))
+					return len(newBuffer), optProblem
+				}
+
+			default:
+				stats.OptionUnknownReceived.Increment()
+				if usage.actions().unknown == optionPass {
+					return len(optIter.InitReplacement(option)), nil
+				}
 			}
+			return 0, nil
+		}()
+
+		if optProblem != nil {
+			optProblem.Pointer += optIter.ErrCursor
+			return nil, optionTracker{}, optProblem
 		}
+		optIter.ConsumeBuffer(optLen)
 	}
 }