diff --git a/pkg/sentry/socket/netstack/netstack.go b/pkg/sentry/socket/netstack/netstack.go index cb63e46a1..aa41d2e85 100644 --- a/pkg/sentry/socket/netstack/netstack.go +++ b/pkg/sentry/socket/netstack/netstack.go @@ -97,7 +97,9 @@ var Metrics = tcpip.Stats{ Bytes: mustCreateMetric("/netstack/nic/disabled_rx/bytes", "Number of bytes received on disabled NICs."), }, Neighbor: tcpip.NICNeighborStats{ - UnreachableEntryLookups: mustCreateMetric("/netstack/nic/neighbor/unreachable_entry_loopups", "Number of lookups performed on a neighbor entry in Unreachable state."), + UnreachableEntryLookups: mustCreateMetric("/netstack/nic/neighbor/unreachable_entry_loopups", "Number of lookups performed on a neighbor entry in Unreachable state."), + DroppedConfirmationForNoninitiatedNeighbor: mustCreateMetric("/netstack/nic/neighbor/dropped_confirmation_for_noninitiated_neighbor", "Number of advertisements received that don't match an entry in the neighbor cache."), + DroppedInvalidLinkAddressConfirmations: mustCreateMetric("/netstack/nic/neighbor/dropped_invalid_link_address_confirmations", "Number of advertisements dropped because they have empty source link-layer addresses"), }, }, ICMP: tcpip.ICMPStats{ diff --git a/pkg/tcpip/stack/neighbor_cache.go b/pkg/tcpip/stack/neighbor_cache.go index 19d518ef0..b0468d8d2 100644 --- a/pkg/tcpip/stack/neighbor_cache.go +++ b/pkg/tcpip/stack/neighbor_cache.go @@ -286,10 +286,12 @@ func (n *neighborCache) handleConfirmation(addr tcpip.Address, linkAddr tcpip.Li entry.mu.Lock() entry.handleConfirmationLocked(linkAddr, flags) entry.mu.Unlock() + } else { + // The confirmation SHOULD be silently discarded if the recipient did not + // initiate any communication with the target. This is indicated if there is + // no matching entry for the remote address. + n.nic.stats.neighbor.droppedConfirmationForNoninitiatedNeighbor.Increment() } - // The confirmation SHOULD be silently discarded if the recipient did not - // initiate any communication with the target. This is indicated if there is - // no matching entry for the remote address. } // handleUpperLevelConfirmation processes a confirmation of reachablity from diff --git a/pkg/tcpip/stack/neighbor_cache_test.go b/pkg/tcpip/stack/neighbor_cache_test.go index 48131e2e8..af419ae07 100644 --- a/pkg/tcpip/stack/neighbor_cache_test.go +++ b/pkg/tcpip/stack/neighbor_cache_test.go @@ -84,15 +84,18 @@ func newTestNeighborResolver(nudDisp NUDDispatcher, config NUDConfigurations, cl entries: newTestEntryStore(), delay: typicalLatency, } + stack := &Stack{ + clock: clock, + nudDisp: nudDisp, + nudConfigs: config, + randomGenerator: rng, + stats: tcpip.Stats{}.FillIn(), + } + linkRes.neigh.init(&nic{ - stack: &Stack{ - clock: clock, - nudDisp: nudDisp, - nudConfigs: config, - randomGenerator: rng, - }, + stack: stack, id: 1, - stats: makeNICStats(tcpip.NICStats{}.FillIn()), + stats: makeNICStats(stack.stats.NICs), }, linkRes) return linkRes } @@ -1547,6 +1550,63 @@ func TestNeighborCacheRetryResolution(t *testing.T) { } } +func TestNeighborCacheIgnoreUnexpectedAdvertisement(t *testing.T) { + config := DefaultNUDConfigurations() + + nudDisp := testNUDDispatcher{} + clock := faketime.NewManualClock() + linkRes := newTestNeighborResolver(&nudDisp, config, clock) + + addr := toAddress(1) + linkAddr := toLinkAddress(1) + + // Receiving confirmation for a neighbor that isn't in the cache should + // increment the counter. + linkRes.neigh.handleConfirmation(addr, linkAddr, ReachabilityConfirmationFlags{ + Solicited: true, + Override: false, + IsRouter: false, + }) + + if got, want := linkRes.neigh.nic.stack.Stats().NICs.Neighbor.DroppedConfirmationForNoninitiatedNeighbor.Value(), uint64(1); got != want { + t.Errorf("got DroppedConfirmationForNoninitiatedNeighbor.Value() = %d, want = %d", got, want) + } +} + +func TestNeighborCacheIgnoreInvalidLinkAddress(t *testing.T) { + config := DefaultNUDConfigurations() + + nudDisp := testNUDDispatcher{} + clock := faketime.NewManualClock() + linkRes := newTestNeighborResolver(&nudDisp, config, clock) + + entry, ok := linkRes.entries.entry(0) + if !ok { + t.Fatal("got linkRes.entries.entry(0) = _, false, want = true ") + } + + _, _, err := linkRes.neigh.entry(entry.Addr, "", nil) + if _, ok := err.(*tcpip.ErrWouldBlock); !ok { + t.Fatalf("linkRes.neigh.entry(%s, \"\", nil): %s", entry.Addr, err) + } + + // Receiving confirmation with an empty link address should increment the + // counter. + linkRes.neigh.handleConfirmation(entry.Addr, "" /* linkAddr */, ReachabilityConfirmationFlags{ + Solicited: true, + Override: false, + IsRouter: false, + }) + + stats := linkRes.neigh.nic.stack.Stats() + if got, want := stats.NICs.Neighbor.DroppedInvalidLinkAddressConfirmations.Value(), uint64(1); got != want { + t.Errorf("got DroppedInvalidLinkAddressConfirmations.Value() = %d, want = %d", got, want) + } + if got, want := stats.NICs.Neighbor.DroppedConfirmationForNoninitiatedNeighbor.Value(), uint64(0); got != want { + t.Errorf("got DroppedConfirmationForNoninitiatedNeighbor.Value() = %d, want = %d", got, want) + } +} + func BenchmarkCacheClear(b *testing.B) { b.StopTimer() config := DefaultNUDConfigurations() diff --git a/pkg/tcpip/stack/neighbor_entry.go b/pkg/tcpip/stack/neighbor_entry.go index 11c069ed9..da7837af9 100644 --- a/pkg/tcpip/stack/neighbor_entry.go +++ b/pkg/tcpip/stack/neighbor_entry.go @@ -506,6 +506,7 @@ func (e *neighborEntry) handleConfirmationLocked(linkAddr tcpip.LinkAddress, fla // "If the link layer has addresses and no Target Link-Layer Address // option is included, the receiving node SHOULD silently discard the // received advertisement." - RFC 4861 section 7.2.5 + e.cache.nic.stats.neighbor.droppedInvalidLinkAddressConfirmations.Increment() break } diff --git a/pkg/tcpip/stack/nic_stats.go b/pkg/tcpip/stack/nic_stats.go index e94ea572f..aa336545b 100644 --- a/pkg/tcpip/stack/nic_stats.go +++ b/pkg/tcpip/stack/nic_stats.go @@ -40,11 +40,15 @@ func (m *multiCounterNICPacketStats) init(a, b *tcpip.NICPacketStats) { // LINT.IfChange(multiCounterNICNeighborStats) type multiCounterNICNeighborStats struct { - unreachableEntryLookups tcpip.MultiCounterStat + unreachableEntryLookups tcpip.MultiCounterStat + droppedConfirmationForNoninitiatedNeighbor tcpip.MultiCounterStat + droppedInvalidLinkAddressConfirmations tcpip.MultiCounterStat } func (m *multiCounterNICNeighborStats) init(a, b *tcpip.NICNeighborStats) { m.unreachableEntryLookups.Init(a.UnreachableEntryLookups, b.UnreachableEntryLookups) + m.droppedConfirmationForNoninitiatedNeighbor.Init(a.DroppedConfirmationForNoninitiatedNeighbor, b.DroppedConfirmationForNoninitiatedNeighbor) + m.droppedInvalidLinkAddressConfirmations.Init(a.DroppedInvalidLinkAddressConfirmations, b.DroppedInvalidLinkAddressConfirmations) } // LINT.ThenChange(../tcpip.go:NICNeighborStats) diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index cd981ba60..a43970901 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -2036,6 +2036,16 @@ type NICNeighborStats struct { // entry in Unreachable state. UnreachableEntryLookups *StatCounter + // DroppedConfirmationForNoninitiatedNeighbor counts the number of neighbor + // responses that were dropped because they didn't match an entry in the + // cache. + DroppedConfirmationForNoninitiatedNeighbor *StatCounter + + // DroppedInvalidLinkAddressConfirmations counts the number of neighbor + // responses that were ignored because they had an invalid source link-layer + // address. + DroppedInvalidLinkAddressConfirmations *StatCounter + // LINT.ThenChange(stack/nic_stats.go:multiCounterNICNeighborStats) }