[Inet] Explicit error return from FromSockAddr (#13049)

#### Problem

A review raised questions about failure cases of `IPAddress::FromSockAddr`
(see link in the issue for details).

Fixes  #10800 _IPAddress::FromSockAddr falls through to return Any — is this correct?_

#### Change overview

- Replace `FromSockAddr(SockAddr)` with `GetIPAddressFromSockAddr()`
  with an explicit error return.
- Change its main use, `InterfaceIterator::GetAddress()`, to also
  have an explicit error return.

#### Testing

CI (`GetAddress` is unit tested).

examples/chip-tool/commands/common/Command.cpp

@@ -119,16 +119,24 @@ static bool ParseAddressWithInterface(const char * addressString, Command::Addre
         return false;
     }
 
-    address->address = ::chip::Inet::IPAddress::FromSockAddr(*result->ai_addr);
     if (result->ai_family == AF_INET6)
     {
         struct sockaddr_in6 * addr = reinterpret_cast<struct sockaddr_in6 *>(result->ai_addr);
+        address->address           = ::chip::Inet::IPAddress::FromSockAddr(*addr);
         address->interfaceId       = ::chip::Inet::InterfaceId(addr->sin6_scope_id);
     }
-    else
+#if INET_CONFIG_ENABLE_IPV4
+    else if (result->ai_family == AF_INET)
     {
+        address->address     = ::chip::Inet::IPAddress::FromSockAddr(*reinterpret_cast<struct sockaddr_in *>(result->ai_addr));
         address->interfaceId = chip::Inet::InterfaceId::Null();
     }
+#endif // INET_CONFIG_ENABLE_IPV4
+    else
+    {
+        ChipLogError(chipTool, "Unsupported address: %s", addressString);
+        return false;
+    }
 
     return true;
 }

src/inet/IPAddress.cpp

@@ -218,15 +218,21 @@ struct in6_addr IPAddress::ToIPv6() const
     return ipAddr;
 }
 
-IPAddress IPAddress::FromSockAddr(const SockAddr & sockaddr)
+CHIP_ERROR IPAddress::GetIPAddressFromSockAddr(const SockAddr & sockaddr, IPAddress & outIPAddress)
 {
 #if INET_CONFIG_ENABLE_IPV4
     if (sockaddr.any.sa_family == AF_INET)
-        return FromSockAddr(sockaddr.in);
+    {
+        outIPAddress = FromSockAddr(sockaddr.in);
+        return CHIP_NO_ERROR;
+    }
 #endif // INET_CONFIG_ENABLE_IPV4
     if (sockaddr.any.sa_family == AF_INET6)
-        return FromSockAddr(sockaddr.in6);
-    return Any;
+    {
+        outIPAddress = FromSockAddr(sockaddr.in6);
+        return CHIP_NO_ERROR;
+    }
+    return INET_ERROR_WRONG_ADDRESS_TYPE;
 }
 
 #endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS || CHIP_SYSTEM_CONFIG_USE_NETWORK_FRAMEWORK

src/inet/IPAddress.h

@@ -37,6 +37,7 @@
 #include <lib/support/DLLUtil.h>
 
 #include <inet/InetConfig.h>
+#include <inet/InetError.h>
 
 #include "inet/IANAConstants.h"
 
@@ -512,8 +513,11 @@ class DLL_EXPORT IPAddress
     /**
      * Get the IP address from a SockAddr.
      */
-    static IPAddress FromSockAddr(const SockAddr & sockaddr);
-    static IPAddress FromSockAddr(const sockaddr & sockaddr) { return FromSockAddr(reinterpret_cast<const SockAddr &>(sockaddr)); }
+    static CHIP_ERROR GetIPAddressFromSockAddr(const SockAddr & sockaddr, IPAddress & outIPAddress);
+    static CHIP_ERROR GetIPAddressFromSockAddr(const sockaddr & sockaddr, IPAddress & outIPAddress)
+    {
+        return GetIPAddressFromSockAddr(reinterpret_cast<const SockAddr &>(sockaddr), outIPAddress);
+    }
     static IPAddress FromSockAddr(const sockaddr_in6 & sockaddr) { return IPAddress(sockaddr.sin6_addr); }
 #if INET_CONFIG_ENABLE_IPV4
     static IPAddress FromSockAddr(const sockaddr_in & sockaddr) { return IPAddress(sockaddr.sin_addr); }

src/inet/IPPrefix.h

@@ -46,6 +46,7 @@ class IPPrefix
 {
 public:
     IPPrefix() = default;
+    IPPrefix(const IPAddress & ipAddress, uint8_t length) : IPAddr(ipAddress), Length(length) {}
 
     /**
      *  Copy constructor for the IPPrefix class.

src/inet/InetInterface.cpp

@@ -214,25 +214,28 @@ bool InterfaceAddressIterator::Next()
     return false;
 }
 
-IPAddress InterfaceAddressIterator::GetAddress()
+CHIP_ERROR InterfaceAddressIterator::GetAddress(IPAddress & outIPAddress)
 {
-    if (HasCurrent())
+    if (!HasCurrent())
     {
-        struct netif * curIntf = mIntfIter.GetInterfaceId().GetPlatformInterface();
+        return CHIP_ERROR_SENTINEL;
+    }
 
-        if (mCurAddrIndex < LWIP_IPV6_NUM_ADDRESSES)
-        {
-            return IPAddress(*netif_ip6_addr(curIntf, mCurAddrIndex));
-        }
+    struct netif * curIntf = mIntfIter.GetInterfaceId().GetPlatformInterface();
+
+    if (mCurAddrIndex < LWIP_IPV6_NUM_ADDRESSES)
+    {
+        outIPAddress = IPAddress(*netif_ip6_addr(curIntf, mCurAddrIndex));
+        return CHIP_NO_ERROR;
+    }
 #if INET_CONFIG_ENABLE_IPV4 && LWIP_IPV4
-        else
-        {
-            return IPAddress(*netif_ip4_addr(curIntf));
-        }
-#endif // INET_CONFIG_ENABLE_IPV4 && LWIP_IPV4
+    else
+    {
+        outIPAddress = IPAddress(*netif_ip4_addr(curIntf));
+        return CHIP_NO_ERROR;
     }
-
-    return IPAddress::Any;
+#endif // INET_CONFIG_ENABLE_IPV4 && LWIP_IPV4
+    return CHIP_ERROR_INTERNAL;
 }
 
 uint8_t InterfaceAddressIterator::GetPrefixLength()
@@ -694,9 +697,9 @@ bool InterfaceAddressIterator::Next()
     }
 }
 
-IPAddress InterfaceAddressIterator::GetAddress()
+CHIP_ERROR InterfaceAddressIterator::GetAddress(IPAddress & outIPAddress)
 {
-    return HasCurrent() ? IPAddress::FromSockAddr(*mCurAddr->ifa_addr) : IPAddress::Any;
+    return HasCurrent() ? IPAddress::GetIPAddressFromSockAddr(*mCurAddr->ifa_addr, outIPAddress) : CHIP_ERROR_SENTINEL;
 }
 
 uint8_t InterfaceAddressIterator::GetPrefixLength()
@@ -945,9 +948,14 @@ bool InterfaceAddressIterator::Next()
     return false;
 }
 
-IPAddress InterfaceAddressIterator::GetAddress()
+CHIP_ERROR InterfaceAddressIterator::GetAddress(IPAddress & outIPAddress)
 {
-    return HasCurrent() ? IPAddress(mIpv6->unicast[mCurAddrIndex].address.in6_addr) : IPAddress::Any;
+    if (HasCurrent())
+    {
+        outIPAddress = IPAddress(mIpv6->unicast[mCurAddrIndex].address.in6_addr);
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_SENTINEL;
 }
 
 uint8_t InterfaceAddressIterator::GetPrefixLength()
@@ -1011,8 +1019,8 @@ InterfaceId InterfaceId::FromIPAddress(const IPAddress & addr)
 
     for (; addrIter.HasCurrent(); addrIter.Next())
     {
-        IPAddress curAddr = addrIter.GetAddress();
-        if (addr == curAddr)
+        IPAddress curAddr;
+        if ((addrIter.GetAddress(curAddr) == CHIP_NO_ERROR) && (addr == curAddr))
         {
             return addrIter.GetInterfaceId();
         }
@@ -1031,7 +1039,8 @@ bool InterfaceId::MatchLocalIPv6Subnet(const IPAddress & addr)
     for (; ifAddrIter.HasCurrent(); ifAddrIter.Next())
     {
         IPPrefix addrPrefix;
-        addrPrefix.IPAddr = ifAddrIter.GetAddress();
+        if (ifAddrIter.GetAddress(addrPrefix.IPAddr) != CHIP_NO_ERROR)
+            continue;
 #if INET_CONFIG_ENABLE_IPV4
         if (addrPrefix.IPAddr.IsIPv4())
             continue;
@@ -1046,19 +1055,6 @@ bool InterfaceId::MatchLocalIPv6Subnet(const IPAddress & addr)
     return false;
 }
 
-void InterfaceAddressIterator::GetAddressWithPrefix(IPPrefix & addrWithPrefix)
-{
-    if (HasCurrent())
-    {
-        addrWithPrefix.IPAddr = GetAddress();
-        addrWithPrefix.Length = GetPrefixLength();
-    }
-    else
-    {
-        addrWithPrefix = IPPrefix::Zero;
-    }
-}
-
 uint8_t NetmaskToPrefixLength(const uint8_t * netmask, uint16_t netmaskLen)
 {
     uint8_t prefixLen = 0;

src/inet/InetInterface.h

@@ -418,10 +418,13 @@ class DLL_EXPORT InterfaceAddressIterator
      *
      * @brief   Get the current interface address.
      *
-     * @return  the current interface address or \c IPAddress::Any if the iterator
-     *          is positioned beyond the end of the address list.
+     * @param[out] outIPAddress     The current interface address
+     *
+     * @return CHIP_NO_ERROR        if the result IPAddress is valid.
+     * @return CHIP_ERROR_SENTINEL  if the iterator is positioned beyond the end of the address list.
+     * @return other error from lower-level code
      */
-    IPAddress GetAddress();
+    CHIP_ERROR GetAddress(IPAddress & outIPAddress);
 
     /**
      * @fn      uint8_t InterfaceAddressIterator::GetPrefixLength(void)
@@ -442,14 +445,6 @@ class DLL_EXPORT InterfaceAddressIterator
      */
     uint8_t GetPrefixLength();
 
-    /**
-     * @fn       void InterfaceAddressIterator::GetAddressWithPrefix(IPPrefix & addrWithPrefix)
-     *
-     * @brief    Returns an IPPrefix containing the address and prefix length
-     *           for the current address.
-     */
-    void GetAddressWithPrefix(IPPrefix & addrWithPrefix);
-
     /**
      * @fn      InterfaceId InterfaceAddressIterator::GetInterfaceId(void)
      *

src/inet/TCPEndPointImplSockets.cpp

@@ -748,10 +748,8 @@ CHIP_ERROR TCPEndPointImplSockets::BindSrcAddrFromIntf(IPAddressType addrType, I
     bool ipAddrFound = false;
     for (InterfaceAddressIterator addrIter; addrIter.HasCurrent(); addrIter.Next())
     {
-        const IPAddress curAddr     = addrIter.GetAddress();
-        const InterfaceId curIntfId = addrIter.GetInterfaceId();
-
-        if (curIntfId == intfId)
+        IPAddress curAddr;
+        if ((addrIter.GetInterfaceId() == intfId) && (addrIter.GetAddress(curAddr) == CHIP_NO_ERROR))
         {
             // Search for an IPv4 address on the TargetInterface
 

src/inet/UDPEndPointImplSockets.cpp

@@ -758,9 +758,8 @@ CHIP_ERROR UDPEndPointImplSockets::IPv4JoinLeaveMulticastGroupImpl(InterfaceId a
 
     for (InterfaceAddressIterator lAddressIterator; lAddressIterator.HasCurrent(); lAddressIterator.Next())
     {
-        const IPAddress lCurrentAddress = lAddressIterator.GetAddress();
-
-        if (lAddressIterator.GetInterfaceId() == aInterfaceId)
+        IPAddress lCurrentAddress;
+        if ((lAddressIterator.GetInterfaceId() == aInterfaceId) && (lAddressIterator.GetAddress(lCurrentAddress) == CHIP_NO_ERROR))
         {
             if (lCurrentAddress.IsIPv4())
             {

src/inet/tests/TestInetEndPoint.cpp

@@ -130,7 +130,6 @@ static void TestInetInterface(nlTestSuite * inSuite, void * inContext)
     char intName[chip::Inet::InterfaceId::kMaxIfNameLength];
     InterfaceId intId;
     IPAddress addr;
-    IPPrefix addrWithPrefix;
     InterfaceType intType;
     // 64 bit IEEE MAC address
     const uint8_t kMaxHardwareAddressSize = 8;
@@ -209,8 +208,9 @@ static void TestInetInterface(nlTestSuite * inSuite, void * inContext)
     printf("    Addresses:\n");
     for (; addrIterator.HasCurrent(); addrIterator.Next())
     {
-        addr = addrIterator.GetAddress();
-        addrIterator.GetAddressWithPrefix(addrWithPrefix);
+        err = addrIterator.GetAddress(addr);
+        NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
+        IPPrefix addrWithPrefix(addr, addrIterator.GetPrefixLength());
         char addrStr[80];
         addrWithPrefix.IPAddr.ToString(addrStr);
         intId = addrIterator.GetInterfaceId();
@@ -231,8 +231,7 @@ static void TestInetInterface(nlTestSuite * inSuite, void * inContext)
                addrIterator.HasBroadcastAddress() ? "has" : "no");
     }
     NL_TEST_ASSERT(inSuite, !addrIterator.Next());
-    addrIterator.GetAddressWithPrefix(addrWithPrefix);
-    NL_TEST_ASSERT(inSuite, addrWithPrefix.IsZero());
+    NL_TEST_ASSERT(inSuite, addrIterator.GetAddress(addr) == CHIP_ERROR_SENTINEL);
     NL_TEST_ASSERT(inSuite, addrIterator.GetInterfaceId() == InterfaceId::Null());
     NL_TEST_ASSERT(inSuite, addrIterator.GetInterfaceName(intName, sizeof(intName)) == CHIP_ERROR_INCORRECT_STATE);
     NL_TEST_ASSERT(inSuite, !addrIterator.SupportsMulticast());

src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp

@@ -766,16 +766,21 @@ void AdvertiserMinMdns::AdvertiseRecords()
             continue;
         }
 
-        if (!ShouldAdvertiseOn(interfaceAddress.GetInterfaceId(), interfaceAddress.GetAddress()))
+        Inet::IPAddress ipAddress;
+        if (interfaceAddress.GetAddress(ipAddress) != CHIP_NO_ERROR)
+        {
+            continue;
+        }
+        if (!ShouldAdvertiseOn(interfaceAddress.GetInterfaceId(), ipAddress))
         {
             continue;
         }
 
         chip::Inet::IPPacketInfo packetInfo;
 
         packetInfo.Clear();
-        packetInfo.SrcAddress = interfaceAddress.GetAddress();
-        if (interfaceAddress.GetAddress().IsIPv4())
+        packetInfo.SrcAddress = ipAddress;
+        if (ipAddress.IsIPv4())
         {
             BroadcastIpAddresses::GetIpv4Into(packetInfo.DestAddress);
         }

src/lib/dnssd/minimal_mdns/responders/IP.cpp

@@ -23,19 +23,13 @@ namespace Minimal {
 
 void IPv4Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, ResponderDelegate * delegate)
 {
+    chip::Inet::IPAddress addr;
     for (chip::Inet::InterfaceAddressIterator it; it.HasCurrent(); it.Next())
     {
-        if (it.GetInterfaceId() != source->Interface)
-        {
-            continue;
-        }
-
-        chip::Inet::IPAddress addr = it.GetAddress();
-        if (!addr.IsIPv4())
+        if ((it.GetInterfaceId() == source->Interface) && (it.GetAddress(addr) == CHIP_NO_ERROR) && addr.IsIPv4())
         {
-            continue;
+            delegate->AddResponse(IPResourceRecord(GetQName(), addr));
         }
-        delegate->AddResponse(IPResourceRecord(GetQName(), addr));
     }
 }
 
@@ -48,13 +42,11 @@ void IPv6Responder::AddAllResponses(const chip::Inet::IPPacketInfo * source, Res
             continue;
         }
 
-        chip::Inet::IPAddress addr = it.GetAddress();
-        if (!addr.IsIPv6())
+        chip::Inet::IPAddress addr;
+        if ((it.GetInterfaceId() == source->Interface) && (it.GetAddress(addr) == CHIP_NO_ERROR) && addr.IsIPv6())
         {
-            continue;
+            delegate->AddResponse(IPResourceRecord(GetQName(), addr));
         }
-
-        delegate->AddResponse(IPResourceRecord(GetQName(), addr));
     }
 }
 

src/platform/Darwin/DnssdImpl.cpp

@@ -402,12 +402,14 @@ static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t i
     service.mPort          = sdCtx->port;
     service.mTextEntries   = sdCtx->textEntries.empty() ? nullptr : sdCtx->textEntries.data();
     service.mTextEntrySize = sdCtx->textEntries.empty() ? 0 : sdCtx->textEntries.size();
-    service.mAddress.SetValue(chip::Inet::IPAddress::FromSockAddr(*address));
+    chip::Inet::IPAddress ip;
+    CHIP_ERROR status = chip::Inet::IPAddress::GetIPAddressFromSockAddr(*address, ip);
+    service.mAddress.SetValue(ip);
     Platform::CopyString(service.mName, sdCtx->name);
     Platform::CopyString(service.mHostName, hostname);
     service.mInterface = Inet::InterfaceId(sdCtx->interfaceId);
 
-    sdCtx->callback(sdCtx->context, &service, CHIP_NO_ERROR);
+    sdCtx->callback(sdCtx->context, &service, status);
     MdnsContexts::GetInstance().Remove(sdCtx);
 }
 

src/platform/Linux/ConnectivityManagerImpl.cpp

@@ -946,8 +946,8 @@ CHIP_ERROR ConnectivityManagerImpl::ProvisionWiFiNetwork(const char * ssid, cons
                 if (it.IsUp() && CHIP_NO_ERROR == it.GetInterfaceName(ifName, sizeof(ifName)) &&
                     strncmp(ifName, sWiFiIfName, sizeof(ifName)) == 0)
                 {
-                    chip::Inet::IPAddress addr = it.GetAddress();
-                    if (addr.IsIPv4())
+                    chip::Inet::IPAddress addr;
+                    if ((it.GetAddress(addr) == CHIP_NO_ERROR) && addr.IsIPv4())
                     {
                         ChipDeviceEvent event;
                         event.Type                            = DeviceEventType::kInternetConnectivityChange;