6 files changed, 191 insertions, 111 deletions

diff --git a/pkg/tcpip/stack/linkaddrcache.go b/pkg/tcpip/stack/linkaddrcache.go
index cd2bb3417..4504db752 100644
--- a/pkg/tcpip/stack/linkaddrcache.go
+++ b/pkg/tcpip/stack/linkaddrcache.go
@@ -24,8 +24,6 @@ import (
 
 const linkAddrCacheSize = 512 // max cache entries
 
-var _ LinkAddressCache = (*linkAddrCache)(nil)
-
 // linkAddrCache is a fixed-sized cache mapping IP addresses to link addresses.
 //
 // The entries are stored in a ring buffer, oldest entry replaced first.
@@ -140,7 +138,7 @@ func (e *linkAddrEntry) changeStateLocked(ns entryState, expiration time.Time) {
 }
 
 // add adds a k -> v mapping to the cache.
-func (c *linkAddrCache) AddLinkAddress(k tcpip.Address, v tcpip.LinkAddress) {
+func (c *linkAddrCache) add(k tcpip.Address, v tcpip.LinkAddress) {
 	// Calculate expiration time before acquiring the lock, since expiration is
 	// relative to the time when information was learned, rather than when it
 	// happened to be inserted into the cache.
@@ -290,3 +288,67 @@ func newLinkAddrCache(nic *NIC, ageLimit, resolutionTimeout time.Duration, resol
 	c.mu.table = make(map[tcpip.Address]*linkAddrEntry, linkAddrCacheSize)
 	return c
 }
+
+var _ neighborTable = (*linkAddrCache)(nil)
+
+func (*linkAddrCache) neighbors() ([]NeighborEntry, tcpip.Error) {
+	return nil, &tcpip.ErrNotSupported{}
+}
+
+func (c *linkAddrCache) addStaticEntry(addr tcpip.Address, linkAddr tcpip.LinkAddress) {
+	c.add(addr, linkAddr)
+}
+
+func (*linkAddrCache) remove(addr tcpip.Address) tcpip.Error {
+	return &tcpip.ErrNotSupported{}
+}
+
+func (*linkAddrCache) removeAll() tcpip.Error {
+	return &tcpip.ErrNotSupported{}
+}
+
+func (c *linkAddrCache) handleProbe(addr tcpip.Address, linkAddr tcpip.LinkAddress, _ LinkAddressResolver) {
+	if len(linkAddr) != 0 {
+		// NUD allows probes without a link address but linkAddrCache
+		// is a simple neighbor table which does not implement NUD.
+		//
+		// As per RFC 4861 section 4.3,
+		//
+		//   Source link-layer address
+		//               The link-layer address for the sender. MUST NOT be
+		//               included when the source IP address is the
+		//               unspecified address. Otherwise, on link layers
+		//               that have addresses this option MUST be included in
+		//               multicast solicitations and SHOULD be included in
+		//               unicast solicitations.
+		c.add(addr, linkAddr)
+	}
+}
+
+func (c *linkAddrCache) handleConfirmation(addr tcpip.Address, linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags) {
+	if len(linkAddr) != 0 {
+		// NUD allows confirmations without a link address but linkAddrCache
+		// is a simple neighbor table which does not implement NUD.
+		//
+		// As per RFC 4861 section 4.4,
+		//
+		//   Target link-layer address
+		//               The link-layer address for the target, i.e., the
+		//               sender of the advertisement. This option MUST be
+		//               included on link layers that have addresses when
+		//               responding to multicast solicitations. When
+		//               responding to a unicast Neighbor Solicitation this
+		//               option SHOULD be included.
+		c.add(addr, linkAddr)
+	}
+}
+
+func (c *linkAddrCache) handleUpperLevelConfirmation(tcpip.Address) {}
+
+func (*linkAddrCache) nudConfig() (NUDConfigurations, tcpip.Error) {
+	return NUDConfigurations{}, &tcpip.ErrNotSupported{}
+}
+
+func (*linkAddrCache) setNUDConfig(NUDConfigurations) tcpip.Error {
+	return &tcpip.ErrNotSupported{}
+}
diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go
index 88a3ff776..64b8046f5 100644
--- a/pkg/tcpip/stack/neighbor_cache.go
+++ b/pkg/tcpip/stack/neighbor_cache.go
@@ -42,8 +42,6 @@ type NeighborStats struct {
 //  2. Static entries are explicitly added by a user and have no expiration.
 //     Their state is always Static. The amount of static entries stored in the
 //     cache is unbounded.
-//
-// neighborCache implements NUDHandler.
 type neighborCache struct {
 	nic   *NIC
 	state *NUDState
@@ -62,8 +60,6 @@ type neighborCache struct {
 	}
 }
 
-var _ NUDHandler = (*neighborCache)(nil)
-
 // getOrCreateEntry retrieves a cache entry associated with addr. The
 // returned entry is always refreshed in the cache (it is reachable via the
 // map, and its place is bumped in LRU).
@@ -263,27 +259,45 @@ func (n *neighborCache) setConfig(config NUDConfigurations) {
 	n.state.SetConfig(config)
 }
 
-// HandleProbe implements NUDHandler.HandleProbe by following the logic defined
-// in RFC 4861 section 7.2.3. Validation of the probe is expected to be handled
-// by the caller.
-func (n *neighborCache) HandleProbe(remoteAddr tcpip.Address, protocol tcpip.NetworkProtocolNumber, remoteLinkAddr tcpip.LinkAddress, linkRes LinkAddressResolver) {
+var _ neighborTable = (*neighborCache)(nil)
+
+func (n *neighborCache) neighbors() ([]NeighborEntry, tcpip.Error) {
+	return n.entries(), nil
+}
+
+func (n *neighborCache) get(addr tcpip.Address, linkRes LinkAddressResolver, localAddr tcpip.Address, onResolve func(LinkResolutionResult)) (tcpip.LinkAddress, <-chan struct{}, tcpip.Error) {
+	entry, ch, err := n.entry(addr, localAddr, linkRes, onResolve)
+	return entry.LinkAddr, ch, err
+}
+
+func (n *neighborCache) remove(addr tcpip.Address) tcpip.Error {
+	if !n.removeEntry(addr) {
+		return &tcpip.ErrBadAddress{}
+	}
+
+	return nil
+}
+
+func (n *neighborCache) removeAll() tcpip.Error {
+	n.clear()
+	return nil
+}
+
+// handleProbe handles a neighbor probe as defined by RFC 4861 section 7.2.3.
+//
+// Validation of the probe is expected to be handled by the caller.
+func (n *neighborCache) handleProbe(remoteAddr tcpip.Address, remoteLinkAddr tcpip.LinkAddress, linkRes LinkAddressResolver) {
 	entry := n.getOrCreateEntry(remoteAddr, linkRes)
 	entry.mu.Lock()
 	entry.handleProbeLocked(remoteLinkAddr)
 	entry.mu.Unlock()
 }
 
-// HandleConfirmation implements NUDHandler.HandleConfirmation by following the
-// logic defined in RFC 4861 section 7.2.5.
+// handleConfirmation handles a neighbor confirmation as defined by
+// RFC 4861 section 7.2.5.
 //
-// TODO(gvisor.dev/issue/2277): To protect against ARP poisoning and other
-// attacks against NDP functions, Secure Neighbor Discovery (SEND) Protocol
-// should be deployed where preventing access to the broadcast segment might
-// not be possible. SEND uses RSA key pairs to produce cryptographically
-// generated addresses, as defined in RFC 3972, Cryptographically Generated
-// Addresses (CGA). This ensures that the claimed source of an NDP message is
-// the owner of the claimed address.
-func (n *neighborCache) HandleConfirmation(addr tcpip.Address, linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags) {
+// Validation of the confirmation is expected to be handled by the caller.
+func (n *neighborCache) handleConfirmation(addr tcpip.Address, linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags) {
 	n.mu.RLock()
 	entry, ok := n.cache[addr]
 	n.mu.RUnlock()
@@ -309,3 +323,12 @@ func (n *neighborCache) handleUpperLevelConfirmation(addr tcpip.Address) {
 		entry.mu.Unlock()
 	}
 }
+
+func (n *neighborCache) nudConfig() (NUDConfigurations, tcpip.Error) {
+	return n.config(), nil
+}
+
+func (n *neighborCache) setNUDConfig(c NUDConfigurations) tcpip.Error {
+	n.setConfig(c)
+	return nil
+}
diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go
index 0707c3ce2..c813b0da5 100644
--- a/pkg/tcpip/stack/nic.go
+++ b/pkg/tcpip/stack/nic.go
@@ -16,7 +16,6 @@ package stack
 
 import (
 	"fmt"
-	"math/rand"
 	"reflect"
 	"sync/atomic"
 
@@ -25,6 +24,21 @@ import (
 	"gvisor.dev/gvisor/pkg/tcpip/header"
 )
 
+type neighborTable interface {
+	neighbors() ([]NeighborEntry, tcpip.Error)
+	addStaticEntry(tcpip.Address, tcpip.LinkAddress)
+	get(addr tcpip.Address, linkRes LinkAddressResolver, localAddr tcpip.Address, onResolve func(LinkResolutionResult)) (tcpip.LinkAddress, <-chan struct{}, tcpip.Error)
+	remove(tcpip.Address) tcpip.Error
+	removeAll() tcpip.Error
+
+	handleProbe(tcpip.Address, tcpip.LinkAddress, LinkAddressResolver)
+	handleConfirmation(tcpip.Address, tcpip.LinkAddress, ReachabilityConfirmationFlags)
+	handleUpperLevelConfirmation(tcpip.Address)
+
+	nudConfig() (NUDConfigurations, tcpip.Error)
+	setNUDConfig(NUDConfigurations) tcpip.Error
+}
+
 var _ NetworkInterface = (*NIC)(nil)
 
 // NIC represents a "network interface card" to which the networking stack is
@@ -38,7 +52,6 @@ type NIC struct {
 	context NICContext
 
 	stats NICStats
-	neigh *neighborCache
 
 	// The network endpoints themselves may be modified by calling the interface's
 	// methods, but the map reference and entries must be constant.
@@ -54,7 +67,7 @@ type NIC struct {
 	// complete.
 	linkResQueue packetsPendingLinkResolution
 
-	linkAddrCache *linkAddrCache
+	neighborTable neighborTable
 
 	mu struct {
 		sync.RWMutex
@@ -143,26 +156,20 @@ func newNIC(stack *Stack, id tcpip.NICID, name string, ep LinkEndpoint, ctx NICC
 		linkAddrResolvers: make(map[tcpip.NetworkProtocolNumber]LinkAddressResolver),
 	}
 	nic.linkResQueue.init(nic)
-	nic.linkAddrCache = newLinkAddrCache(nic, ageLimit, resolutionTimeout, resolutionAttempts)
 	nic.mu.packetEPs = make(map[tcpip.NetworkProtocolNumber]*packetEndpointList)
 
-	// Check for Neighbor Unreachability Detection support.
-	var nud NUDHandler
-	if ep.Capabilities()&CapabilityResolutionRequired != 0 && stack.useNeighborCache {
-		rng := rand.New(rand.NewSource(stack.clock.NowNanoseconds()))
-		nic.neigh = &neighborCache{
-			nic:   nic,
-			state: NewNUDState(stack.nudConfigs, rng),
-			cache: make(map[tcpip.Address]*neighborEntry, neighborCacheSize),
-		}
+	resolutionRequired := ep.Capabilities()&CapabilityResolutionRequired != 0
 
-		// An interface value that holds a nil pointer but non-nil type is not the
-		// same as the nil interface. Because of this, nud must only be assignd if
-		// nic.neigh is non-nil since a nil reference to a neighborCache is not
-		// valid.
-		//
-		// See https://golang.org/doc/faq#nil_error for more information.
-		nud = nic.neigh
+	if resolutionRequired {
+		if stack.useNeighborCache {
+			nic.neighborTable = &neighborCache{
+				nic:   nic,
+				state: NewNUDState(stack.nudConfigs, stack.randomGenerator),
+				cache: make(map[tcpip.Address]*neighborEntry, neighborCacheSize),
+			}
+		} else {
+			nic.neighborTable = newLinkAddrCache(nic, ageLimit, resolutionTimeout, resolutionAttempts)
+		}
 	}
 
 	// Register supported packet and network endpoint protocols.
@@ -173,11 +180,13 @@ func newNIC(stack *Stack, id tcpip.NICID, name string, ep LinkEndpoint, ctx NICC
 		netNum := netProto.Number()
 		nic.mu.packetEPs[netNum] = new(packetEndpointList)
 
-		netEP := netProto.NewEndpoint(nic, nic.linkAddrCache, nud, nic)
+		netEP := netProto.NewEndpoint(nic, nic)
 		nic.networkEndpoints[netNum] = netEP
 
-		if r, ok := netEP.(LinkAddressResolver); ok {
-			nic.linkAddrResolvers[r.LinkAddressProtocol()] = r
+		if resolutionRequired {
+			if r, ok := netEP.(LinkAddressResolver); ok {
+				nic.linkAddrResolvers[r.LinkAddressProtocol()] = r
+			}
 		}
 	}
 
@@ -596,8 +605,8 @@ func (n *NIC) removeAddress(addr tcpip.Address) tcpip.Error {
 }
 
 func (n *NIC) confirmReachable(addr tcpip.Address) {
-	if n := n.neigh; n != nil {
-		n.handleUpperLevelConfirmation(addr)
+	if n.neighborTable != nil {
+		n.neighborTable.handleUpperLevelConfirmation(addr)
 	}
 }
 
@@ -617,49 +626,44 @@ func (n *NIC) getLinkAddress(addr, localAddr tcpip.Address, protocol tcpip.Netwo
 }
 
 func (n *NIC) getNeighborLinkAddress(addr, localAddr tcpip.Address, linkRes LinkAddressResolver, onResolve func(LinkResolutionResult)) (tcpip.LinkAddress, <-chan struct{}, tcpip.Error) {
-	if n.neigh != nil {
-		entry, ch, err := n.neigh.entry(addr, localAddr, linkRes, onResolve)
-		return entry.LinkAddr, ch, err
+	if n.neighborTable != nil {
+		return n.neighborTable.get(addr, linkRes, localAddr, onResolve)
 	}
 
-	return n.linkAddrCache.get(addr, linkRes, localAddr, onResolve)
+	return "", nil, &tcpip.ErrNotSupported{}
 }
 
 func (n *NIC) neighbors() ([]NeighborEntry, tcpip.Error) {
-	if n.neigh == nil {
-		return nil, &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		return n.neighborTable.neighbors()
 	}
 
-	return n.neigh.entries(), nil
+	return nil, &tcpip.ErrNotSupported{}
 }
 
 func (n *NIC) addStaticNeighbor(addr tcpip.Address, linkAddress tcpip.LinkAddress) tcpip.Error {
-	if n.neigh == nil {
-		return &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		n.neighborTable.addStaticEntry(addr, linkAddress)
+		return nil
 	}
 
-	n.neigh.addStaticEntry(addr, linkAddress)
-	return nil
+	return &tcpip.ErrNotSupported{}
 }
 
 func (n *NIC) removeNeighbor(addr tcpip.Address) tcpip.Error {
-	if n.neigh == nil {
-		return &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		return n.neighborTable.remove(addr)
 	}
 
-	if !n.neigh.removeEntry(addr) {
-		return &tcpip.ErrBadAddress{}
-	}
-	return nil
+	return &tcpip.ErrNotSupported{}
 }
 
 func (n *NIC) clearNeighbors() tcpip.Error {
-	if n.neigh == nil {
-		return &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		return n.neighborTable.removeAll()
 	}
 
-	n.neigh.clear()
-	return nil
+	return &tcpip.ErrNotSupported{}
 }
 
 // joinGroup adds a new endpoint for the given multicast address, if none
@@ -944,10 +948,11 @@ func (n *NIC) Name() string {
 
 // nudConfigs gets the NUD configurations for n.
 func (n *NIC) nudConfigs() (NUDConfigurations, tcpip.Error) {
-	if n.neigh == nil {
-		return NUDConfigurations{}, &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		return n.neighborTable.nudConfig()
 	}
-	return n.neigh.config(), nil
+
+	return NUDConfigurations{}, &tcpip.ErrNotSupported{}
 }
 
 // setNUDConfigs sets the NUD configurations for n.
@@ -955,12 +960,12 @@ func (n *NIC) nudConfigs() (NUDConfigurations, tcpip.Error) {
 // Note, if c contains invalid NUD configuration values, it will be fixed to
 // use default values for the erroneous values.
 func (n *NIC) setNUDConfigs(c NUDConfigurations) tcpip.Error {
-	if n.neigh == nil {
-		return &tcpip.ErrNotSupported{}
+	if n.neighborTable != nil {
+		c.resetInvalidFields()
+		return n.neighborTable.setNUDConfig(c)
 	}
-	c.resetInvalidFields()
-	n.neigh.setConfig(c)
-	return nil
+
+	return &tcpip.ErrNotSupported{}
 }
 
 func (n *NIC) registerPacketEndpoint(netProto tcpip.NetworkProtocolNumber, ep PacketEndpoint) tcpip.Error {
@@ -996,3 +1001,17 @@ func (n *NIC) isValidForOutgoing(ep AssignableAddressEndpoint) bool {
 	n.mu.RUnlock()
 	return n.Enabled() && ep.IsAssigned(spoofing)
 }
+
+// HandleNeighborProbe implements NetworkInterface.
+func (n *NIC) HandleNeighborProbe(addr tcpip.Address, linkAddr tcpip.LinkAddress, linkRes LinkAddressResolver) {
+	if n.neighborTable != nil {
+		n.neighborTable.handleProbe(addr, linkAddr, linkRes)
+	}
+}
+
+// HandleNeighborConfirmation implements NetworkInterface.
+func (n *NIC) HandleNeighborConfirmation(addr tcpip.Address, linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags) {
+	if n.neighborTable != nil {
+		n.neighborTable.handleConfirmation(addr, linkAddr, flags)
+	}
+}
diff --git a/pkg/tcpip/stack/nud.go b/pkg/tcpip/stack/nud.go
index 77926e289..5a94e9ac6 100644
--- a/pkg/tcpip/stack/nud.go
+++ b/pkg/tcpip/stack/nud.go
@@ -161,21 +161,6 @@ type ReachabilityConfirmationFlags struct {
 	IsRouter bool
 }
 
-// NUDHandler communicates external events to the Neighbor Unreachability
-// Detection state machine, which is implemented per-interface. This is used by
-// network endpoints to inform the Neighbor Cache of probes and confirmations.
-type NUDHandler interface {
-	// HandleProbe processes an incoming neighbor probe (e.g. ARP request or
-	// Neighbor Solicitation for ARP or NDP, respectively). Validation of the
-	// probe needs to be performed before calling this function since the
-	// Neighbor Cache doesn't have access to view the NIC's assigned addresses.
-	HandleProbe(remoteAddr tcpip.Address, protocol tcpip.NetworkProtocolNumber, remoteLinkAddr tcpip.LinkAddress, linkRes LinkAddressResolver)
-
-	// HandleConfirmation processes an incoming neighbor confirmation (e.g. ARP
-	// reply or Neighbor Advertisement for ARP or NDP, respectively).
-	HandleConfirmation(addr tcpip.Address, linkAddr tcpip.LinkAddress, flags ReachabilityConfirmationFlags)
-}
-
 // NUDConfigurations is the NUD configurations for the netstack. This is used
 // by the neighbor cache to operate the NUD state machine on each device in the
 // local network.
diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go
index 64b5627e1..c652c2bd7 100644
--- a/pkg/tcpip/stack/registration.go
+++ b/pkg/tcpip/stack/registration.go
@@ -530,6 +530,17 @@ type NetworkInterface interface {
 	// offload is enabled. If it will be used for something else, syscall filters
 	// may need to be updated.
 	WritePackets(*Route, *GSO, PacketBufferList, tcpip.NetworkProtocolNumber) (int, tcpip.Error)
+
+	// HandleNeighborProbe processes an incoming neighbor probe (e.g. ARP
+	// request or NDP Neighbor Solicitation).
+	//
+	// HandleNeighborProbe assumes that the probe is valid for the network
+	// interface the probe was received on.
+	HandleNeighborProbe(tcpip.Address, tcpip.LinkAddress, LinkAddressResolver)
+
+	// HandleNeighborConfirmation processes an incoming neighbor confirmation
+	// (e.g. ARP reply or NDP Neighbor Advertisement).
+	HandleNeighborConfirmation(tcpip.Address, tcpip.LinkAddress, ReachabilityConfirmationFlags)
 }
 
 // LinkResolvableNetworkEndpoint handles link resolution events.
@@ -649,7 +660,7 @@ type NetworkProtocol interface {
 	ParseAddresses(v buffer.View) (src, dst tcpip.Address)
 
 	// NewEndpoint creates a new endpoint of this protocol.
-	NewEndpoint(nic NetworkInterface, linkAddrCache LinkAddressCache, nud NUDHandler, dispatcher TransportDispatcher) NetworkEndpoint
+	NewEndpoint(nic NetworkInterface, dispatcher TransportDispatcher) NetworkEndpoint
 
 	// SetOption allows enabling/disabling protocol specific features.
 	// SetOption returns an error if the option is not supported or the
@@ -843,12 +854,6 @@ type LinkAddressResolver interface {
 	LinkAddressProtocol() tcpip.NetworkProtocolNumber
 }
 
-// A LinkAddressCache caches link addresses.
-type LinkAddressCache interface {
-	// AddLinkAddress adds a link address to the cache.
-	AddLinkAddress(addr tcpip.Address, linkAddr tcpip.LinkAddress)
-}
-
 // RawFactory produces endpoints for writing various types of raw packets.
 type RawFactory interface {
 	// NewUnassociatedEndpoint produces endpoints for writing packets not
diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go
index 66bf22823..73db6e031 100644
--- a/pkg/tcpip/stack/stack.go
+++ b/pkg/tcpip/stack/stack.go
@@ -1526,20 +1526,6 @@ func (s *Stack) SetSpoofing(nicID tcpip.NICID, enable bool) tcpip.Error {
 	return nil
 }
 
-// AddLinkAddress adds a link address for the neighbor on the specified NIC.
-func (s *Stack) AddLinkAddress(nicID tcpip.NICID, neighbor tcpip.Address, linkAddr tcpip.LinkAddress) tcpip.Error {
-	s.mu.RLock()
-	defer s.mu.RUnlock()
-
-	nic, ok := s.nics[nicID]
-	if !ok {
-		return &tcpip.ErrUnknownNICID{}
-	}
-
-	nic.linkAddrCache.AddLinkAddress(neighbor, linkAddr)
-	return nil
-}
-
 // LinkResolutionResult is the result of a link address resolution attempt.
 type LinkResolutionResult struct {
 	LinkAddress tcpip.LinkAddress