Re: Duplicate IP address, and permissions problems on Windows

[Daniel Lenski <dlenski@gmail.com>]

On Thu, Apr 8, 2021 at 9:59 AM David Woodhouse <dwmw2@infradead.org> wrote:
> Hm, your description doesn't match the code I see at that link.
>
> You're using GetAdaptersAddresses() which gives you the UP/DOWN status
> as well as the addresses, and you iterate over those. The loop is
>
>  ∀ adapter, ∀ Unicast address on that adapter:
>      Check if it's our Legacy IP or IPv6 address.
>
> That isn't O(n²), is it? It's still O(n) of the total number of unicast
> addresses in the system?

It's O(n²) the number of unicast addresses, because there's an extra layer…

  ∀ adapter, ∀ Unicast address on that adapter (iterating via
GetAdaptersAddresses)
      1. Check if it's using our Legacy IP or IPv6 address.
      2. If yes, then check if the other adapter is UP or non-UP
      3. If non-UP, then…
          ∀ Unicast address on the system (iterating via
GetUnicastIpAddressTable(), since the other one maddeningly lacks an
API to delete addresses)
          2. If non-UP, then steal/delete/reclaim the desired address from it.

> Once you've found an address which needs to be removed, you're *then*
> using GetUnicastIpAddressTable() and searching through the results to
> find the appropriate MIB_UNICASTIPADDRESS_ROW that you need to pass to
> DeleteUnicastIpAddressEntry().
>
> Does *every* field in the MIB_UNICASTIPADDRESS_ROW have to be filled
> in, or is it just the Address, InterfaceLuid and InterfaceIndex? Can't
> we get those from the table we get back from GetAdaptersAddresses()?

Yes, I already tried precisely this in
https://gitlab.com/openconnect/openconnect/-/commit/b3dbabda7b68cf86fc72e2d5158b0707f74d61f0,
and it doesn't work. I could faff around with it more, but even if I
got it to work, it's clearly not how Microsoft wants us to do it, and
liable to break.

    /* Create a "fake" MIB_UNICASTIPADDRESS_ROW based on the
IP_ADAPTER_UNICAST_ADDRESS and IP_ADAPTER_ADDRESSES data structures
which we already have. */

> Alternatively, can't we start with GetUnicastIpAddressTable() as my
> original code did, and if we want to check whether an interface is down
> before we steal the address from it, use GetIfEntry2() to find out?
>
> Using GetIfEntry2() is probably a saner way to find the InterfaceIndex
> for the Wintun itself, which I was dredging the registry for manually.
>
>
> I'd like to be consistent about clearing the 'conflicting' addresses
> and setting the address on the Wintun interface. Whatever we do in
> OpenConnect for Legacy IP we should also do for IPv6. It looks like
> you're clearing the conflicting addresses for both families, but we
> still aren't *setting* the IPv6 address from the C code?

Let's save the OpenConnect-specific decisions, but…

It seems to me that we've identifying a couple of tasks that many
users of Wintun would need, and which are (in my opinion) quite
tedious to implement robustly in Windows:

1. Identifying the “interface index” of the newly-created adapter (for
use with 'netsh', etc.).
2. Reclaiming desired Layer3 (IP) addresses from other non-UP adapters
to which they may already be assigned.

If the Wintun developers are amenable to it, these both seem like they
could be useful additions to Wintun itself.

Dan