diff options
| -rw-r--r-- | pkg/tcpip/network/arp/arp.go | 2 | ||||
| -rw-r--r-- | pkg/tcpip/network/ipv6/icmp.go | 36 | ||||
| -rw-r--r-- | pkg/tcpip/network/ipv6/ipv6.go | 4 | ||||
| -rw-r--r-- | pkg/tcpip/stack/registration.go | 6 |
4 files changed, 29 insertions, 19 deletions
diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go index 98ded525c..ae0461a6d 100644 --- a/pkg/tcpip/network/arp/arp.go +++ b/pkg/tcpip/network/arp/arp.go @@ -232,7 +232,7 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) { linkAddr := tcpip.LinkAddress(h.HardwareAddressSender()) e.mu.Lock() - e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{}) + e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{HolderLinkAddress: linkAddr}) e.mu.Unlock() // The solicited, override, and isRouter flags are not available for ARP; diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index 1cebe75ec..6344a3e09 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -382,6 +382,10 @@ 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 { + // 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 + // We would get an error if the address no longer exists or the address // is no longer tentative (DAD resolved between the call to // hasTentativeAddr and this point). Both of these are valid scenarios: @@ -393,7 +397,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); err.(type) { + switch err := e.dupTentativeAddrDetected(targetAddr, holderLinkAddress); err.(type) { case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState: default: panic(fmt.Sprintf("unexpected error handling duplicate tentative address: %s", err)) @@ -561,10 +565,24 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r // 5, NDP messages cannot be fragmented. Also note that in the common case // NDP datagrams are very small and AsView() will not incur allocations. na := header.NDPNeighborAdvert(payload.AsView()) + + it, err := na.Options().Iter(false /* check */) + if err != nil { + // If we have a malformed NDP NA option, drop the packet. + received.invalid.Increment() + return + } + + targetLinkAddr, ok := getTargetLinkAddr(it) + if !ok { + received.invalid.Increment() + return + } + targetAddr := na.TargetAddress() e.dad.mu.Lock() - e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{}) + e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{HolderLinkAddress: targetLinkAddr}) e.dad.mu.Unlock() if e.hasTentativeAddr(targetAddr) { @@ -584,7 +602,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); err.(type) { + switch err := e.dupTentativeAddrDetected(targetAddr, targetLinkAddr); err.(type) { case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState: return default: @@ -592,13 +610,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r } } - it, err := na.Options().Iter(false /* check */) - if err != nil { - // If we have a malformed NDP NA option, drop the packet. - received.invalid.Increment() - return - } - // At this point we know that the target address is not tentative on the // NIC. However, the target address may still be assigned to the NIC but not // tentative (it could be permanent). Such a scenario is beyond the scope of @@ -608,11 +619,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r // TODO(b/143147598): Handle the scenario described above. Also inform the // netstack integration that a duplicate address was detected outside of // DAD. - targetLinkAddr, ok := getTargetLinkAddr(it) - if !ok { - received.invalid.Increment() - return - } // As per RFC 4861 section 7.1.2: // A node MUST silently discard any received Neighbor Advertisement diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go index d9ab240b6..46b6cc41a 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) tcpip.Error { +func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr tcpip.LinkAddress) tcpip.Error { e.mu.Lock() defer e.mu.Unlock() @@ -363,7 +363,7 @@ func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address) tcpip.Error { // 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{}); err != nil { + if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{HolderLinkAddress: holderLinkAddr}); err != nil { return err } diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go index 426342fd2..85f0f471a 100644 --- a/pkg/tcpip/stack/registration.go +++ b/pkg/tcpip/stack/registration.go @@ -884,7 +884,11 @@ func (*DADAborted) isDADResult() {} var _ DADResult = (*DADDupAddrDetected)(nil) // DADDupAddrDetected indicates DAD detected a duplicate address. -type DADDupAddrDetected struct{} +type DADDupAddrDetected struct { + // HolderLinkAddress is the link address of the node that holds the duplicate + // address. + HolderLinkAddress tcpip.LinkAddress +} func (*DADDupAddrDetected) isDADResult() {} |