From 68065d1ceb589b7ea1d3e4b3b067fb8772e30760 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Tue, 16 Mar 2021 11:07:02 -0700 Subject: Detect looped-back NDP DAD messages ...as per RFC 7527. If a looped-back DAD message is received, do not fail DAD since our own DAD message does not indicate that a neighbor has the address assigned. Test: ndp_test.TestDADResolveLoopback PiperOrigin-RevId: 363224288 --- pkg/tcpip/network/ipv6/icmp.go | 55 +++++++++++++++-------- pkg/tcpip/network/ipv6/ipv6.go | 89 ++++++++++++++++++++++++++++---------- pkg/tcpip/network/ipv6/mld_test.go | 18 +++++++- pkg/tcpip/network/ipv6/ndp.go | 14 +++--- pkg/tcpip/network/ipv6/ndp_test.go | 72 +++++++++++++++++------------- 5 files changed, 169 insertions(+), 79 deletions(-) (limited to 'pkg/tcpip/network/ipv6') diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index 8059e0690..2afa856dc 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -369,6 +369,18 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r return } + var it header.NDPOptionIterator + { + var err error + it, err = ns.Options().Iter(false /* check */) + if err != nil { + // Options are not valid as per the wire format, silently drop the + // packet. + received.invalid.Increment() + return + } + } + if e.hasTentativeAddr(targetAddr) { // If the target address is tentative and the source of the packet is a // unicast (specified) address, then the source of the packet is @@ -382,6 +394,22 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r // stack know so it can handle such a scenario and do nothing further with // the NS. if srcAddr == header.IPv6Any { + var nonce []byte + for { + opt, done, err := it.Next() + if err != nil { + received.invalid.Increment() + return + } + if done { + break + } + if n, ok := opt.(header.NDPNonceOption); ok { + nonce = n.Nonce() + break + } + } + // Since this is a DAD message we know the sender does not actually hold // the target address so there is no "holder". var holderLinkAddress tcpip.LinkAddress @@ -397,7 +425,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r // // TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate // address is detected for an assigned address. - switch err := e.dupTentativeAddrDetected(targetAddr, holderLinkAddress); err.(type) { + switch err := e.dupTentativeAddrDetected(targetAddr, holderLinkAddress, nonce); err.(type) { case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState: default: panic(fmt.Sprintf("unexpected error handling duplicate tentative address: %s", err)) @@ -418,21 +446,10 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r return } - var sourceLinkAddr tcpip.LinkAddress - { - it, err := ns.Options().Iter(false /* check */) - if err != nil { - // Options are not valid as per the wire format, silently drop the - // packet. - received.invalid.Increment() - return - } - - sourceLinkAddr, ok = getSourceLinkAddr(it) - if !ok { - received.invalid.Increment() - return - } + sourceLinkAddr, ok := getSourceLinkAddr(it) + if !ok { + received.invalid.Increment() + return } // As per RFC 4861 section 4.3, the Source Link-Layer Address Option MUST @@ -586,6 +603,10 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r e.dad.mu.Unlock() if e.hasTentativeAddr(targetAddr) { + // We only send a nonce value in DAD messages to check for loopedback + // messages so we use the empty nonce value here. + var nonce []byte + // We just got an NA from a node that owns an address we are performing // DAD on, implying the address is not unique. In this case we let the // stack know so it can handle such a scenario and do nothing furthur with @@ -602,7 +623,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r // // TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate // address is detected for an assigned address. - switch err := e.dupTentativeAddrDetected(targetAddr, targetLinkAddr); err.(type) { + switch err := e.dupTentativeAddrDetected(targetAddr, targetLinkAddr, nonce); err.(type) { case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState: return default: diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index 46b6cc41a..350493958 100644 --- a/pkg/tcpip/network/ipv6/ipv6.go +++ b/pkg/tcpip/network/ipv6/ipv6.go @@ -348,7 +348,7 @@ func (e *endpoint) hasTentativeAddr(addr tcpip.Address) bool { // dupTentativeAddrDetected removes the tentative address if it exists. If the // address was generated via SLAAC, an attempt is made to generate a new // address. -func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr tcpip.LinkAddress) tcpip.Error { +func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr tcpip.LinkAddress, nonce []byte) tcpip.Error { e.mu.Lock() defer e.mu.Unlock() @@ -361,27 +361,48 @@ func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr t return &tcpip.ErrInvalidEndpointState{} } - // If the address is a SLAAC address, do not invalidate its SLAAC prefix as an - // attempt will be made to generate a new address for it. - if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{HolderLinkAddress: holderLinkAddr}); err != nil { - return err - } + switch result := e.mu.ndp.dad.ExtendIfNonceEqualLocked(addr, nonce); result { + case ip.Extended: + // The nonce we got back was the same we sent so we know the message + // indicating a duplicate address was likely ours so do not consider + // the address duplicate here. + return nil + case ip.AlreadyExtended: + // See Extended. + // + // Our DAD message was looped back already. + return nil + case ip.NoDADStateFound: + panic(fmt.Sprintf("expected DAD state for tentative address %s", addr)) + case ip.NonceDisabled: + // If nonce is disabled then we have no way to know if the packet was + // looped-back so we have to assume it indicates a duplicate address. + fallthrough + case ip.NonceNotEqual: + // If the address is a SLAAC address, do not invalidate its SLAAC prefix as an + // attempt will be made to generate a new address for it. + if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{HolderLinkAddress: holderLinkAddr}); err != nil { + return err + } - prefix := addressEndpoint.Subnet() + prefix := addressEndpoint.Subnet() - switch t := addressEndpoint.ConfigType(); t { - case stack.AddressConfigStatic: - case stack.AddressConfigSlaac: - e.mu.ndp.regenerateSLAACAddr(prefix) - case stack.AddressConfigSlaacTemp: - // Do not reset the generation attempts counter for the prefix as the - // temporary address is being regenerated in response to a DAD conflict. - e.mu.ndp.regenerateTempSLAACAddr(prefix, false /* resetGenAttempts */) + switch t := addressEndpoint.ConfigType(); t { + case stack.AddressConfigStatic: + case stack.AddressConfigSlaac: + e.mu.ndp.regenerateSLAACAddr(prefix) + case stack.AddressConfigSlaacTemp: + // Do not reset the generation attempts counter for the prefix as the + // temporary address is being regenerated in response to a DAD conflict. + e.mu.ndp.regenerateTempSLAACAddr(prefix, false /* resetGenAttempts */) + default: + panic(fmt.Sprintf("unrecognized address config type = %d", t)) + } + + return nil default: - panic(fmt.Sprintf("unrecognized address config type = %d", t)) + panic(fmt.Sprintf("unhandled result = %d", result)) } - - return nil } // transitionForwarding transitions the endpoint's forwarding status to @@ -1797,16 +1818,36 @@ func (p *protocol) NewEndpoint(nic stack.NetworkInterface, dispatcher stack.Tran dispatcher: dispatcher, protocol: p, } + + // NDP options must be 8 octet aligned and the first 2 bytes are used for + // the type and length fields leaving 6 octets as the minimum size for a + // nonce option without padding. + const nonceSize = 6 + + // As per RFC 7527 section 4.1, + // + // If any probe is looped back within RetransTimer milliseconds after + // having sent DupAddrDetectTransmits NS(DAD) messages, the interface + // continues with another MAX_MULTICAST_SOLICIT number of NS(DAD) + // messages transmitted RetransTimer milliseconds apart. + // + // Value taken from RFC 4861 section 10. + const maxMulticastSolicit = 3 + dadOptions := ip.DADOptions{ + Clock: p.stack.Clock(), + SecureRNG: p.stack.SecureRNG(), + NonceSize: nonceSize, + ExtendDADTransmits: maxMulticastSolicit, + Protocol: &e.mu.ndp, + NICID: nic.ID(), + } + e.mu.Lock() e.mu.addressableEndpointState.Init(e) - e.mu.ndp.init(e) + e.mu.ndp.init(e, dadOptions) e.mu.mld.init(e) e.dad.mu.Lock() - e.dad.mu.dad.Init(&e.dad.mu, p.options.DADConfigs, ip.DADOptions{ - Clock: p.stack.Clock(), - Protocol: &e.mu.ndp, - NICID: nic.ID(), - }) + e.dad.mu.dad.Init(&e.dad.mu, p.options.DADConfigs, dadOptions) e.dad.mu.Unlock() e.mu.Unlock() diff --git a/pkg/tcpip/network/ipv6/mld_test.go b/pkg/tcpip/network/ipv6/mld_test.go index 9a425e50a..85a8f9944 100644 --- a/pkg/tcpip/network/ipv6/mld_test.go +++ b/pkg/tcpip/network/ipv6/mld_test.go @@ -15,6 +15,7 @@ package ipv6_test import ( + "bytes" "testing" "time" @@ -119,11 +120,26 @@ func TestSendQueuedMLDReports(t *testing.T) { }, } + nonce := [...]byte{ + 1, 2, 3, 4, 5, 6, + } + + const maxNSMessages = 2 + secureRNGBytes := make([]byte, len(nonce)*maxNSMessages) + for b := secureRNGBytes[:]; len(b) > 0; b = b[len(nonce):] { + if n := copy(b, nonce[:]); n != len(nonce) { + t.Fatalf("got copy(...) = %d, want = %d", n, len(nonce)) + } + } + for _, test := range tests { t.Run(test.name, func(t *testing.T) { dadResolutionTime := test.retransmitTimer * time.Duration(test.dadTransmits) clock := faketime.NewManualClock() + var secureRNG bytes.Reader + secureRNG.Reset(secureRNGBytes[:]) s := stack.New(stack.Options{ + SecureRNG: &secureRNG, NetworkProtocols: []stack.NetworkProtocolFactory{ipv6.NewProtocolWithOptions(ipv6.Options{ DADConfigs: stack.DADConfigurations{ DupAddrDetectTransmits: test.dadTransmits, @@ -154,7 +170,7 @@ func TestSendQueuedMLDReports(t *testing.T) { checker.TTL(header.NDPHopLimit), checker.NDPNS( checker.NDPNSTargetAddress(addr), - checker.NDPNSOptions(nil), + checker.NDPNSOptions([]header.NDPOption{header.NDPNonceOption(nonce[:])}), )) } } diff --git a/pkg/tcpip/network/ipv6/ndp.go b/pkg/tcpip/network/ipv6/ndp.go index d9b728878..536493f87 100644 --- a/pkg/tcpip/network/ipv6/ndp.go +++ b/pkg/tcpip/network/ipv6/ndp.go @@ -1789,18 +1789,14 @@ func (ndp *ndpState) stopSolicitingRouters() { ndp.rtrSolicitTimer = timer{} } -func (ndp *ndpState) init(ep *endpoint) { +func (ndp *ndpState) init(ep *endpoint, dadOptions ip.DADOptions) { if ndp.defaultRouters != nil { panic("attempted to initialize NDP state twice") } ndp.ep = ep ndp.configs = ep.protocol.options.NDPConfigs - ndp.dad.Init(&ndp.ep.mu, ep.protocol.options.DADConfigs, ip.DADOptions{ - Clock: ep.protocol.stack.Clock(), - Protocol: ndp, - NICID: ep.nic.ID(), - }) + ndp.dad.Init(&ndp.ep.mu, ep.protocol.options.DADConfigs, dadOptions) ndp.defaultRouters = make(map[tcpip.Address]defaultRouterState) ndp.onLinkPrefixes = make(map[tcpip.Subnet]onLinkPrefixState) ndp.slaacPrefixes = make(map[tcpip.Subnet]slaacPrefixState) @@ -1811,9 +1807,11 @@ func (ndp *ndpState) init(ep *endpoint) { } } -func (ndp *ndpState) SendDADMessage(addr tcpip.Address) tcpip.Error { +func (ndp *ndpState) SendDADMessage(addr tcpip.Address, nonce []byte) tcpip.Error { snmc := header.SolicitedNodeAddr(addr) - return ndp.ep.sendNDPNS(header.IPv6Any, snmc, addr, header.EthernetAddressFromMulticastIPv6Address(snmc), nil /* opts */) + return ndp.ep.sendNDPNS(header.IPv6Any, snmc, addr, header.EthernetAddressFromMulticastIPv6Address(snmc), header.NDPOptionsSerializer{ + header.NDPNonceOption(nonce), + }) } func (e *endpoint) sendNDPNS(srcAddr, dstAddr, targetAddr tcpip.Address, remoteLinkAddr tcpip.LinkAddress, opts header.NDPOptionsSerializer) tcpip.Error { diff --git a/pkg/tcpip/network/ipv6/ndp_test.go b/pkg/tcpip/network/ipv6/ndp_test.go index 6e850fd46..52b9a200c 100644 --- a/pkg/tcpip/network/ipv6/ndp_test.go +++ b/pkg/tcpip/network/ipv6/ndp_test.go @@ -15,6 +15,7 @@ package ipv6 import ( + "bytes" "context" "strings" "testing" @@ -1264,8 +1265,21 @@ func TestCheckDuplicateAddress(t *testing.T) { DupAddrDetectTransmits: 1, RetransmitTimer: time.Second, } + + nonces := [...][]byte{ + {1, 2, 3, 4, 5, 6}, + {7, 8, 9, 10, 11, 12}, + } + + var secureRNGBytes []byte + for _, n := range nonces { + secureRNGBytes = append(secureRNGBytes, n...) + } + var secureRNG bytes.Reader + secureRNG.Reset(secureRNGBytes[:]) s := stack.New(stack.Options{ - Clock: clock, + SecureRNG: &secureRNG, + Clock: clock, NetworkProtocols: []stack.NetworkProtocolFactory{NewProtocolWithOptions(Options{ DADConfigs: dadConfigs, })}, @@ -1278,10 +1292,36 @@ func TestCheckDuplicateAddress(t *testing.T) { t.Fatalf("CreateNIC(%d, _) = %s", nicID, err) } - dadPacketsSent := 1 + dadPacketsSent := 0 + snmc := header.SolicitedNodeAddr(lladdr0) + remoteLinkAddr := header.EthernetAddressFromMulticastIPv6Address(snmc) + checkDADMsg := func() { + p, ok := e.ReadContext(context.Background()) + if !ok { + t.Fatalf("expected %d-th DAD message", dadPacketsSent) + } + + if p.Proto != header.IPv6ProtocolNumber { + t.Errorf("(i=%d) got p.Proto = %d, want = %d", dadPacketsSent, p.Proto, header.IPv6ProtocolNumber) + } + + if p.Route.RemoteLinkAddress != remoteLinkAddr { + t.Errorf("(i=%d) got p.Route.RemoteLinkAddress = %s, want = %s", dadPacketsSent, p.Route.RemoteLinkAddress, remoteLinkAddr) + } + + checker.IPv6(t, stack.PayloadSince(p.Pkt.NetworkHeader()), + checker.SrcAddr(header.IPv6Any), + checker.DstAddr(snmc), + checker.TTL(header.NDPHopLimit), + checker.NDPNS( + checker.NDPNSTargetAddress(lladdr0), + checker.NDPNSOptions([]header.NDPOption{header.NDPNonceOption(nonces[dadPacketsSent])}), + )) + } if err := s.AddAddress(nicID, ProtocolNumber, lladdr0); err != nil { t.Fatalf("AddAddress(%d, %d, %s) = %s", nicID, ProtocolNumber, lladdr0, err) } + checkDADMsg() // Start DAD for the address we just added. // @@ -1297,6 +1337,7 @@ func TestCheckDuplicateAddress(t *testing.T) { } else if res != stack.DADStarting { t.Fatalf("got s.CheckDuplicateAddress(%d, %d, %s, _) = %d, want = %d", nicID, ProtocolNumber, lladdr0, res, stack.DADStarting) } + checkDADMsg() // Remove the address and make sure our DAD request was not stopped. if err := s.RemoveAddress(nicID, lladdr0); err != nil { @@ -1328,33 +1369,6 @@ func TestCheckDuplicateAddress(t *testing.T) { default: } - snmc := header.SolicitedNodeAddr(lladdr0) - remoteLinkAddr := header.EthernetAddressFromMulticastIPv6Address(snmc) - - for i := 0; i < dadPacketsSent; i++ { - p, ok := e.Read() - if !ok { - t.Fatalf("expected %d-th DAD message", i) - } - - if p.Proto != header.IPv6ProtocolNumber { - t.Errorf("(i=%d) got p.Proto = %d, want = %d", i, p.Proto, header.IPv6ProtocolNumber) - } - - if p.Route.RemoteLinkAddress != remoteLinkAddr { - t.Errorf("(i=%d) got p.Route.RemoteLinkAddress = %s, want = %s", i, p.Route.RemoteLinkAddress, remoteLinkAddr) - } - - checker.IPv6(t, stack.PayloadSince(p.Pkt.NetworkHeader()), - checker.SrcAddr(header.IPv6Any), - checker.DstAddr(snmc), - checker.TTL(header.NDPHopLimit), - checker.NDPNS( - checker.NDPNSTargetAddress(lladdr0), - checker.NDPNSOptions(nil), - )) - } - // Should have no more packets. if p, ok := e.Read(); ok { t.Errorf("got unexpected packet = %#v", p) -- cgit v1.2.3