From 71623e40680c79e82390016a10376e73ff2370db Mon Sep 17 00:00:00 2001 From: Peter Johnston Date: Fri, 29 Jan 2021 08:34:45 -0800 Subject: [PATCH] Discard invalid Neighbor Advertisements ...per RFC 4861 s7.1.2. Startblock: has LGTM from sbalana and then add reviewer ghanan PiperOrigin-RevId: 354539026 --- pkg/tcpip/network/ipv6/icmp.go | 11 +++ pkg/tcpip/network/ipv6/ndp_test.go | 130 +++++++++++++++++++++++++++-- 2 files changed, 132 insertions(+), 9 deletions(-) diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index 43f8a825e..7298bd061 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -445,6 +445,17 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool) { return } + // As per RFC 4861 section 7.1.2: + // A node MUST silently discard any received Neighbor Advertisement + // messages that do not satisfy all of the following validity checks: + // ... + // - If the IP Destination Address is a multicast address the + // Solicited flag is zero. + if header.IsV6MulticastAddress(dstAddr) && na.SolicitedFlag() { + received.invalid.Increment() + return + } + // If the NA message has the target link layer option, update the link // address cache with the link address for the target of the message. if e.nud == nil { diff --git a/pkg/tcpip/network/ipv6/ndp_test.go b/pkg/tcpip/network/ipv6/ndp_test.go index 9d34ea436..1d38b8b05 100644 --- a/pkg/tcpip/network/ipv6/ndp_test.go +++ b/pkg/tcpip/network/ipv6/ndp_test.go @@ -167,10 +167,10 @@ type linkResolutionResult struct { ok bool } -// TestNeighorSolicitationWithSourceLinkLayerOption tests that receiving a +// TestNeighborSolicitationWithSourceLinkLayerOption tests that receiving a // valid NDP NS message with the Source Link Layer Address option results in a // new entry in the link address cache for the sender of the message. -func TestNeighorSolicitationWithSourceLinkLayerOption(t *testing.T) { +func TestNeighborSolicitationWithSourceLinkLayerOption(t *testing.T) { const nicID = 1 tests := []struct { @@ -265,11 +265,11 @@ func TestNeighorSolicitationWithSourceLinkLayerOption(t *testing.T) { } } -// TestNeighorSolicitationWithSourceLinkLayerOptionUsingNeighborCache tests +// TestNeighborSolicitationWithSourceLinkLayerOptionUsingNeighborCache tests // that receiving a valid NDP NS message with the Source Link Layer Address // option results in a new entry in the link address cache for the sender of // the message. -func TestNeighorSolicitationWithSourceLinkLayerOptionUsingNeighborCache(t *testing.T) { +func TestNeighborSolicitationWithSourceLinkLayerOptionUsingNeighborCache(t *testing.T) { const nicID = 1 tests := []struct { @@ -382,7 +382,7 @@ func TestNeighorSolicitationWithSourceLinkLayerOptionUsingNeighborCache(t *testi } } -func TestNeighorSolicitationResponse(t *testing.T) { +func TestNeighborSolicitationResponse(t *testing.T) { const nicID = 1 nicAddr := lladdr0 remoteAddr := lladdr1 @@ -721,10 +721,10 @@ func TestNeighorSolicitationResponse(t *testing.T) { } } -// TestNeighorAdvertisementWithTargetLinkLayerOption tests that receiving a +// TestNeighborAdvertisementWithTargetLinkLayerOption tests that receiving a // valid NDP NA message with the Target Link Layer Address option results in a // new entry in the link address cache for the target of the message. -func TestNeighorAdvertisementWithTargetLinkLayerOption(t *testing.T) { +func TestNeighborAdvertisementWithTargetLinkLayerOption(t *testing.T) { const nicID = 1 tests := []struct { @@ -826,11 +826,11 @@ func TestNeighorAdvertisementWithTargetLinkLayerOption(t *testing.T) { } } -// TestNeighorAdvertisementWithTargetLinkLayerOptionUsingNeighborCache tests +// TestNeighborAdvertisementWithTargetLinkLayerOptionUsingNeighborCache tests // that receiving a valid NDP NA message with the Target Link Layer Address // option does not result in a new entry in the neighbor cache for the target // of the message. -func TestNeighorAdvertisementWithTargetLinkLayerOptionUsingNeighborCache(t *testing.T) { +func TestNeighborAdvertisementWithTargetLinkLayerOptionUsingNeighborCache(t *testing.T) { const nicID = 1 tests := []struct { @@ -1172,6 +1172,118 @@ func TestNDPValidation(t *testing.T) { } +// TestNeighborAdvertisementValidation tests that the NIC validates received +// Neighbor Advertisements. +// +// In particular, if the IP Destination Address is a multicast address, and the +// Solicited flag is not zero, the Neighbor Advertisement is invalid and should +// be discarded. +func TestNeighborAdvertisementValidation(t *testing.T) { + tests := []struct { + name string + ipDstAddr tcpip.Address + solicitedFlag bool + valid bool + }{ + { + name: "Multicast IP destination address with Solicited flag set", + ipDstAddr: header.IPv6AllNodesMulticastAddress, + solicitedFlag: true, + valid: false, + }, + { + name: "Multicast IP destination address with Solicited flag unset", + ipDstAddr: header.IPv6AllNodesMulticastAddress, + solicitedFlag: false, + valid: true, + }, + { + name: "Unicast IP destination address with Solicited flag set", + ipDstAddr: lladdr0, + solicitedFlag: true, + valid: true, + }, + { + name: "Unicast IP destination address with Solicited flag unset", + ipDstAddr: lladdr0, + solicitedFlag: false, + valid: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := stack.New(stack.Options{ + NetworkProtocols: []stack.NetworkProtocolFactory{NewProtocol}, + UseNeighborCache: true, + }) + e := channel.New(0, header.IPv6MinimumMTU, linkAddr0) + e.LinkEPCapabilities |= stack.CapabilityResolutionRequired + if err := s.CreateNIC(nicID, e); err != nil { + t.Fatalf("CreateNIC(%d, _) = %s", nicID, err) + } + if err := s.AddAddress(nicID, ProtocolNumber, lladdr0); err != nil { + t.Fatalf("AddAddress(%d, %d, %s) = %s", nicID, ProtocolNumber, lladdr0, err) + } + + ndpNASize := header.ICMPv6NeighborAdvertMinimumSize + hdr := buffer.NewPrependable(header.IPv6MinimumSize + ndpNASize) + pkt := header.ICMPv6(hdr.Prepend(ndpNASize)) + pkt.SetType(header.ICMPv6NeighborAdvert) + na := header.NDPNeighborAdvert(pkt.MessageBody()) + na.SetTargetAddress(lladdr1) + na.SetSolicitedFlag(test.solicitedFlag) + pkt.SetChecksum(header.ICMPv6Checksum(pkt, lladdr1, test.ipDstAddr, buffer.VectorisedView{})) + payloadLength := hdr.UsedLength() + ip := header.IPv6(hdr.Prepend(header.IPv6MinimumSize)) + ip.Encode(&header.IPv6Fields{ + PayloadLength: uint16(payloadLength), + TransportProtocol: header.ICMPv6ProtocolNumber, + HopLimit: 255, + SrcAddr: lladdr1, + DstAddr: test.ipDstAddr, + }) + + stats := s.Stats().ICMP.V6.PacketsReceived + invalid := stats.Invalid + rxNA := stats.NeighborAdvert + + if got := rxNA.Value(); got != 0 { + t.Fatalf("got rxNA = %d, want = 0", got) + } + if got := invalid.Value(); got != 0 { + t.Fatalf("got invalid = %d, want = 0", got) + } + + e.InjectInbound(header.IPv6ProtocolNumber, stack.NewPacketBuffer(stack.PacketBufferOptions{ + Data: hdr.View().ToVectorisedView(), + })) + + if got := rxNA.Value(); got != 1 { + t.Fatalf("got rxNA = %d, want = 1", got) + } + var wantInvalid uint64 = 1 + if test.valid { + wantInvalid = 0 + } + if got := invalid.Value(); got != wantInvalid { + t.Fatalf("got invalid = %d, want = %d", got, wantInvalid) + } + // As per RFC 4861 section 7.2.5: + // When a valid Neighbor Advertisement is received ... + // If no entry exists, the advertisement SHOULD be silently discarded. + // There is no need to create an entry if none exists, since the + // recipient has apparently not initiated any communication with the + // target. + if neighbors, err := s.Neighbors(nicID); err != nil { + t.Fatalf("s.Neighbors(%d): %s", nicID, err) + } else if len(neighbors) != 0 { + t.Fatalf("got len(neighbors) = %d, want = 0; neighbors = %#v", len(neighbors), neighbors) + } + }) + } +} + // TestRouterAdvertValidation tests that when the NIC is configured to handle // NDP Router Advertisement packets, it validates the Router Advertisement // properly before handling them.