Only genenerate the random fallback MAC address once (project-chip#32694

)

* Set buf.size() correctly in GetPrimaryMACAddress()

When we fall back to returning the WiFi MAC on a device that supports Thread,
the returned MAC is shorter than kPrimaryMACAddressLength.

* Only genenerate the random fallback MAC address once

This avoids changing our hostname every time we restart advertising, in
scenarios where a real physical MAC is unavailable (e.g. on iOS).

examples/chef/common/clusters/wake-on-lan/WakeOnLanManager.cpp

@@ -41,7 +41,7 @@ std::string getMacAddress()
     }
 
     char macStr[chip::DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength * 2 + 1] = { 0 }; // added null char
-    if (BytesToHex(&macBuffer[0], sizeof(macBuffer), &macStr[0], sizeof(macBuffer) * 2u, chip::Encoding::HexFlags::kUppercase) !=
+    if (BytesToHex(mac.data(), mac.size(), &macStr[0], sizeof(macBuffer) * 2u, chip::Encoding::HexFlags::kUppercase) !=
         CHIP_NO_ERROR)
     {
         ChipLogProgress(Zcl, "WakeOnLanManager::getMacAddress hex conversion failed");

examples/tv-app/tv-common/clusters/wake-on-lan/WakeOnLanManager.cpp

@@ -39,7 +39,7 @@ std::string getMacAddress()
     }
 
     char macStr[chip::DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength * 2 + 1] = { 0 }; // added null char
-    if (BytesToHex(&macBuffer[0], sizeof(macBuffer), &macStr[0], sizeof(macBuffer) * 2u, chip::Encoding::HexFlags::kUppercase) !=
+    if (BytesToHex(mac.data(), mac.size(), &macStr[0], sizeof(macBuffer) * 2u, chip::Encoding::HexFlags::kUppercase) !=
         CHIP_NO_ERROR)
     {
         ChipLogProgress(Zcl, "WakeOnLanManager::getMacAddress hex conversion failed");

src/app/server/Dnssd.cpp

@@ -38,6 +38,11 @@
 #include <setup_payload/SetupPayload.h>
 #include <system/TimeSource.h>
 
+#include <algorithm>
+
+using namespace chip;
+using namespace chip::DeviceLayer;
+
 namespace chip {
 namespace app {
 namespace {
@@ -165,6 +170,21 @@ void DnssdServer::AddICDKeyToAdvertisement(AdvertisingParams & advParams)
 }
 #endif
 
+void DnssdServer::GetPrimaryOrFallbackMACAddress(chip::MutableByteSpan mac)
+{
+    if (ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR)
+    {
+        // Only generate a fallback "MAC" once, so we don't keep constantly changing our host name.
+        if (std::all_of(std::begin(mFallbackMAC), std::end(mFallbackMAC), [](uint8_t v) { return v == 0; }))
+        {
+            ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one.");
+            Crypto::DRBG_get_bytes(mFallbackMAC, sizeof(mFallbackMAC));
+        }
+        VerifyOrDie(mac.size() == sizeof(mFallbackMAC)); // kPrimaryMACAddressLength
+        memcpy(mac.data(), mFallbackMAC, sizeof(mFallbackMAC));
+    }
+}
+
 /// Set MDNS operational advertisement
 CHIP_ERROR DnssdServer::AdvertiseOperational()
 {
@@ -179,11 +199,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
 
         uint8_t macBuffer[DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength];
         MutableByteSpan mac(macBuffer);
-        if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR)
-        {
-            ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one.");
-            Crypto::DRBG_get_bytes(macBuffer, sizeof(macBuffer));
-        }
+        GetPrimaryOrFallbackMACAddress(mac);
 
         auto advertiseParameters = chip::Dnssd::OperationalAdvertisingParameters()
                                        .SetPeerId(fabricInfo.GetPeerId())
@@ -224,11 +240,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi
 
     uint8_t macBuffer[DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength];
     MutableByteSpan mac(macBuffer);
-    if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR)
-    {
-        ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one.");
-        Crypto::DRBG_get_bytes(macBuffer, sizeof(macBuffer));
-    }
+    GetPrimaryOrFallbackMACAddress(mac);
     advertiseParameters.SetMac(mac);
 
     uint16_t value;

src/app/server/Dnssd.h

@@ -146,6 +146,11 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
     /// Set MDNS commissionable node advertisement
     CHIP_ERROR AdvertiseCommissionableNode(chip::Dnssd::CommissioningMode mode);
 
+    // Our randomly-generated fallback "MAC address", in case we don't have a real one.
+    uint8_t mFallbackMAC[chip::DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength] = { 0 };
+
+    void GetPrimaryOrFallbackMACAddress(chip::MutableByteSpan mac);
+
     //
     // Check if we have any valid operational credentials present in the fabric table and return true
     // if we do.

src/include/platform/ConfigurationManager.h

@@ -80,17 +80,26 @@ class ConfigurationManager
         kMinRotatingDeviceIDUniqueIDLength = 16,
         kRotatingDeviceIDUniqueIDLength    = CHIP_DEVICE_CONFIG_ROTATING_DEVICE_ID_UNIQUE_ID_LENGTH,
 #endif
+        kEthernetMACAddressLength = 6,
+        kThreadMACAddressLength   = 8,
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD
-        kPrimaryMACAddressLength = 8,
+        kPrimaryMACAddressLength = kThreadMACAddressLength,
 #else
-        kPrimaryMACAddressLength = 6,
+        kPrimaryMACAddressLength = kEthernetMACAddressLength,
 #endif
         kMaxMACAddressLength  = 8,
         kMaxLanguageTagLength = 5 // ISO 639-1 standard language codes
     };
 
-    virtual CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf)                           = 0;
-    virtual CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf)                             = 0;
+    // Copies the primary MAC into a mutable span, which must be of size kPrimaryMACAddressLength.
+    // Upon success, the span will be reduced to the size of the MAC address being returned, which
+    // can be less than kPrimaryMACAddressLength on a device that supports Thread.
+    virtual CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) = 0;
+
+    // Copies the primary WiFi MAC into a buffer of size kEthernetMACAddressLength
+    virtual CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) = 0;
+
+    // Copies the primary Thread (802.15.4) MAC into a buffer of size kThreadMACAddressLength
     virtual CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf)                           = 0;
     virtual CHIP_ERROR GetSoftwareVersionString(char * buf, size_t bufSize)                = 0;
     virtual CHIP_ERROR GetSoftwareVersion(uint32_t & softwareVer)                          = 0;

src/include/platform/internal/GenericConfigurationManagerImpl.ipp

@@ -374,19 +374,21 @@ CHIP_ERROR GenericConfigurationManagerImpl<ConfigClass>::GetPrimaryMACAddress(Mu
     if (buf.size() != ConfigurationManager::kPrimaryMACAddressLength)
         return CHIP_ERROR_INVALID_ARGUMENT;
 
-    memset(buf.data(), 0, buf.size());
+    memset(buf.data(), 0, buf.size()); // zero the whole buffer, in case the caller ignores buf.size()
 
 #if CHIP_DEVICE_CONFIG_ENABLE_THREAD
     if (chip::DeviceLayer::ThreadStackMgr().GetPrimary802154MACAddress(buf.data()) == CHIP_NO_ERROR)
     {
         ChipLogDetail(DeviceLayer, "Using Thread extended MAC for hostname.");
+        buf.reduce_size(kThreadMACAddressLength);
         return CHIP_NO_ERROR;
     }
 #endif
 
     if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryWiFiMACAddress(buf.data()) == CHIP_NO_ERROR)
     {
-        ChipLogDetail(DeviceLayer, "Using wifi MAC for hostname");
+        ChipLogDetail(DeviceLayer, "Using WiFi MAC for hostname");
+        buf.reduce_size(kEthernetMACAddressLength);
         return CHIP_NO_ERROR;
     }