[BUG] Windows Network Interfaces Not Filtered

[odellcraig]

Describe the bug In the WIN32 environment the getLocalhostIpAddresses() function in Network.c does not match the Unix/Main path. Specifically the interfaces returned by the OS are not filtered in the same way. In the Unix path, the loopback interfaces and any interface that is not Up/Running is filtered, whereas the interfaces in the WIN32 path are not and therefore a list of IP addresses including ones like 127.0.0.1, ::1, and self-assigned 169.254.x.x addresses are return from this function which causes problems downstream. Here is the specific code in question Network.c lines 28 - 128

    retWinStatus = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, NULL, NULL, &sizeAAPointer);
    CHK(retWinStatus == ERROR_BUFFER_OVERFLOW, STATUS_GET_LOCAL_IP_ADDRESSES_FAILED);

    adapterAddresses = (PIP_ADAPTER_ADDRESSES) MEMALLOC(sizeAAPointer);

    retWinStatus = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, NULL, adapterAddresses, &sizeAAPointer);
    CHK(retWinStatus == ERROR_SUCCESS, STATUS_GET_LOCAL_IP_ADDRESSES_FAILED);

    for (aa = adapterAddresses; aa != NULL && ipCount < destIpListLen; aa = aa->Next) {
        char ifa_name[BUFSIZ];
        memset(ifa_name, 0, BUFSIZ);
        WideCharToMultiByte(CP_ACP, 0, aa->FriendlyName, wcslen(aa->FriendlyName), ifa_name, BUFSIZ, NULL, NULL);

        for (ua = aa->FirstUnicastAddress; ua != NULL; ua = ua->Next) {
            if (filter != NULL) {
                DLOGI("Callback set to allow network interface filtering");
                // The callback evaluates to a FALSE if the application is interested in black listing an interface
                if (filter(customData, ifa_name) == FALSE) {
                    filterSet = FALSE;
                } else {
                    filterSet = TRUE;
                }
            }

            // If filter is set, ensure the details are collected for the interface
            if (filterSet == TRUE) {
                int family = ua->Address.lpSockaddr->sa_family;

                if (family == AF_INET) {
                    destIpList[ipCount].family = KVS_IP_FAMILY_TYPE_IPV4;
                    destIpList[ipCount].port = 0;

                    pIpv4Addr = (struct sockaddr_in*) (ua->Address.lpSockaddr);
                    MEMCPY(destIpList[ipCount].address, &pIpv4Addr->sin_addr, IPV4_ADDRESS_LENGTH);
                } else {
                    destIpList[ipCount].family = KVS_IP_FAMILY_TYPE_IPV6;
                    destIpList[ipCount].port = 0;

                    pIpv6Addr = (struct sockaddr_in6*) (ua->Address.lpSockaddr);
                    // Ignore unspecified addres: the other peer can't use this address
                    // Ignore link local: not very useful and will add work unnecessarily
                    // Ignore site local: https://tools.ietf.org/html/rfc8445#section-5.1.1.1
                    if (IN6_IS_ADDR_UNSPECIFIED(&pIpv6Addr->sin6_addr) || IN6_IS_ADDR_LINKLOCAL(&pIpv6Addr->sin6_addr) ||
                        IN6_IS_ADDR_SITELOCAL(&pIpv6Addr->sin6_addr)) {
                        continue;
                    }
                    MEMCPY(destIpList[ipCount].address, &pIpv6Addr->sin6_addr, IPV6_ADDRESS_LENGTH);
                }

                // in case of overfilling destIpList
                ipCount++;
            }
        }
    }
#else
    CHK(getifaddrs(&ifaddr) != -1, STATUS_GET_LOCAL_IP_ADDRESSES_FAILED);
    for (ifa = ifaddr; ifa != NULL && ipCount < destIpListLen; ifa = ifa->ifa_next) {
        if (ifa->ifa_addr != NULL && (ifa->ifa_flags & IFF_LOOPBACK) == 0 && // ignore loopback interface
            (ifa->ifa_flags & IFF_RUNNING) > 0 &&                            // interface has to be allocated
            (ifa->ifa_addr->sa_family == AF_INET || ifa->ifa_addr->sa_family == AF_INET6)) {
            // mark vpn interface
            destIpList[ipCount].isPointToPoint = ((ifa->ifa_flags & IFF_POINTOPOINT) != 0);

            if (filter != NULL) {
                // The callback evaluates to a FALSE if the application is interested in black listing an interface
                if (filter(customData, ifa->ifa_name) == FALSE) {
                    filterSet = FALSE;
                } else {
                    filterSet = TRUE;
                }
            }

            // If filter is set, ensure the details are collected for the interface
            if (filterSet == TRUE) {
                if (ifa->ifa_addr->sa_family == AF_INET) {
                    destIpList[ipCount].family = KVS_IP_FAMILY_TYPE_IPV4;
                    destIpList[ipCount].port = 0;
                    pIpv4Addr = (struct sockaddr_in*) ifa->ifa_addr;
                    MEMCPY(destIpList[ipCount].address, &pIpv4Addr->sin_addr, IPV4_ADDRESS_LENGTH);

                } else {
                    destIpList[ipCount].family = KVS_IP_FAMILY_TYPE_IPV6;
                    destIpList[ipCount].port = 0;
                    pIpv6Addr = (struct sockaddr_in6*) ifa->ifa_addr;
                    // Ignore unspecified addres: the other peer can't use this address
                    // Ignore link local: not very useful and will add work unnecessarily
                    // Ignore site local: https://tools.ietf.org/html/rfc8445#section-5.1.1.1
                    if (IN6_IS_ADDR_UNSPECIFIED(&pIpv6Addr->sin6_addr) || IN6_IS_ADDR_LINKLOCAL(&pIpv6Addr->sin6_addr) ||
                        IN6_IS_ADDR_SITELOCAL(&pIpv6Addr->sin6_addr)) {
                        continue;
                    }
                    MEMCPY(destIpList[ipCount].address, &pIpv6Addr->sin6_addr, IPV6_ADDRESS_LENGTH);
                }

                // in case of overfilling destIpList
                ipCount++;
            }
        }
    }
#endif

SDK version number master branch last updated 18 months ago

Open source building If it is a build issue, include 3rd party library version and steps to how you are building it

To Reproduce Steps to reproduce the behavior:

1. Build the exes on Windows 10
2. Run the kvsWebrtcClientViewer.exe
3. Logs show attempts to bind to 169.254.x.x addresses and loopback addresses
4. Exe crashes due because it is unable to bind the socket

Expected behavior I would expect this code path to operate in the same way as the Unix path (#else) and filter out all interfaces that are down and loopback interfaces)

Screenshots Here is the analogous code where loopback and not running interfaces are checked for loopback and running:

Desktop (please complete the following information):

• OS: Windows 10
• Browser N/A
• Version master

[MushMal]

@odellcraig - great suggestion! Do you mind sending a pull request for this?

[odellcraig]

@MushMal will do. It will probably be a couple weeks, but I'll take a look at your contribution process and submit a PR.

[github-actions[bot]]

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

[github-actions[bot]]

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.

[github-actions[bot]]

This is a very old issue. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to open a new one.