Skip to content

Commit

Permalink
Only genenerate the random fallback MAC address once (#32694)
Browse files Browse the repository at this point in the history
* 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).
  • Loading branch information
ksperling-apple authored Mar 25, 2024
1 parent f5b2712 commit a9f1d35
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
32 changes: 22 additions & 10 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
{
Expand All @@ -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())
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions src/app/server/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 13 additions & 4 deletions src/include/platform/ConfigurationManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit a9f1d35

Please sign in to comment.