From ec853a2cacfb87f747ffa572f74e30720e4915c7 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 15 Feb 2022 17:18:32 -0500 Subject: [PATCH 1/5] Fix mac address get for dnssd --- src/app/server/Dnssd.cpp | 18 ++++++++++++++++-- .../GenericConfigurationManagerImpl.cpp | 11 ++++------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 1d079593b1d51d..0f99ca02f1facf 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -256,7 +256,14 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() { uint8_t macBuffer[DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength]; MutableByteSpan mac(macBuffer); - chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac); + if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR) + { + ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one."); + for (unsigned i = 0; i < sizeof(macBuffer); i++) + { + macBuffer[i] = chip::Crypto::GetRandU8(); + } + } const auto advertiseParameters = chip::Dnssd::OperationalAdvertisingParameters() .SetPeerId(fabricInfo.GetPeerId()) @@ -293,7 +300,14 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi uint8_t macBuffer[DeviceLayer::ConfigurationManager::kPrimaryMACAddressLength]; MutableByteSpan mac(macBuffer); - chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac); + if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR) + { + ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one."); + for (unsigned i = 0; i < sizeof(macBuffer); i++) + { + macBuffer[i] = chip::Crypto::GetRandU8(); + } + } advertiseParameters.SetMac(mac); uint16_t value; diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp index 91f14a9cee7105..1ec4aa122e8add 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp @@ -136,18 +136,15 @@ CHIP_ERROR GenericConfigurationManagerImpl::GetPrimaryMACAddress(Mu ChipLogDetail(DeviceLayer, "Using Thread extended MAC for hostname."); return CHIP_NO_ERROR; } -#else - if (DeviceLayer::ConfigurationMgr().GetPrimaryWiFiMACAddress(buf.data()) == CHIP_NO_ERROR) +#endif + + if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryWiFiMACAddress(buf.data()) == CHIP_NO_ERROR) { ChipLogDetail(DeviceLayer, "Using wifi MAC for hostname"); return CHIP_NO_ERROR; } -#endif - ChipLogError(DeviceLayer, "MAC is not known, using a default."); - uint8_t temp[ConfigurationManager::kMaxMACAddressLength] = { 0xEE, 0xAA, 0xBA, 0xDA, 0xBA, 0xD0, 0xDD, 0xCA }; - memcpy(buf.data(), temp, buf.size()); - return CHIP_NO_ERROR; + return CHIP_ERROR_NOT_FOUND; } template From 51fa7dab36a5ae4b1bea5b2c245ad70f1f0d101b Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 15 Feb 2022 18:16:46 -0500 Subject: [PATCH 2/5] Stop asserting that the primary mac getting returns a default - it should not --- src/platform/tests/TestConfigurationMgr.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index 26cebebed47767..3c4c7dc0804407 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -187,8 +187,7 @@ static void TestConfigurationMgr_Breadcrumb(nlTestSuite * inSuite, void * inCont static void TestConfigurationMgr_GetPrimaryMACAddress(nlTestSuite * inSuite, void * inContext) { - CHIP_ERROR err = CHIP_NO_ERROR; - const uint8_t defaultMacAddress[8] = { 0xEE, 0xAA, 0xBA, 0xDA, 0xBA, 0xD0, 0xDD, 0xCA }; + CHIP_ERROR err = CHIP_NO_ERROR; uint8_t macBuffer8Bytes[8]; uint8_t macBuffer6Bytes[6]; MutableByteSpan mac8Bytes(macBuffer8Bytes); @@ -202,11 +201,6 @@ static void TestConfigurationMgr_GetPrimaryMACAddress(nlTestSuite * inSuite, voi else { NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - - // Verify default MAC address value - NL_TEST_ASSERT(inSuite, - strncmp(reinterpret_cast(mac8Bytes.data()), reinterpret_cast(defaultMacAddress), - mac8Bytes.size()) == 0); } err = ConfigurationMgr().GetPrimaryMACAddress(mac6Bytes); From c5ed00703ed506cea9f2ba1d531d6ad80b7b11a9 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 15 Feb 2022 18:41:40 -0500 Subject: [PATCH 3/5] remove one more use of the default mac --- src/platform/tests/TestConfigurationMgr.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index 3c4c7dc0804407..71b2b72a4e2808 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -211,13 +211,6 @@ static void TestConfigurationMgr_GetPrimaryMACAddress(nlTestSuite * inSuite, voi else { NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - -#ifndef __MBED__ - // Verify default MAC address value - NL_TEST_ASSERT(inSuite, - strncmp(reinterpret_cast(mac6Bytes.data()), reinterpret_cast(defaultMacAddress), - mac6Bytes.size()) == 0); -#endif } } From fd11b72b474bb52960175cfbb4d43b87580c0948 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 16 Feb 2022 09:39:23 -0500 Subject: [PATCH 4/5] Use DRBG_get_bytes to get a random mac --- src/app/server/Dnssd.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 0f99ca02f1facf..f16a94809a7555 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -259,10 +259,7 @@ CHIP_ERROR DnssdServer::AdvertiseOperational() if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR) { ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one."); - for (unsigned i = 0; i < sizeof(macBuffer); i++) - { - macBuffer[i] = chip::Crypto::GetRandU8(); - } + Crypto::DRBG_get_bytes(macBuffer, sizeof(macBuffer)); } const auto advertiseParameters = chip::Dnssd::OperationalAdvertisingParameters() @@ -303,10 +300,7 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi if (chip::DeviceLayer::ConfigurationMgr().GetPrimaryMACAddress(mac) != CHIP_NO_ERROR) { ChipLogError(Discovery, "Failed to get primary mac address of device. Generating a random one."); - for (unsigned i = 0; i < sizeof(macBuffer); i++) - { - macBuffer[i] = chip::Crypto::GetRandU8(); - } + Crypto::DRBG_get_bytes(macBuffer, sizeof(macBuffer)); } advertiseParameters.SetMac(mac); From 7fcf7088fa6b5504f912b031a7b7be1672de9188 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Wed, 16 Feb 2022 12:27:41 -0500 Subject: [PATCH 5/5] Remove the NO_ERROR assumption for getting primary mac. Emulators have no active IP interface to get a mac from --- src/platform/tests/TestConfigurationMgr.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index 71b2b72a4e2808..9057a446a287ef 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -198,20 +198,17 @@ static void TestConfigurationMgr_GetPrimaryMACAddress(nlTestSuite * inSuite, voi { NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); } - else - { - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - } err = ConfigurationMgr().GetPrimaryMACAddress(mac6Bytes); if (mac6Bytes.size() != ConfigurationManager::kPrimaryMACAddressLength) { NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT); } - else - { - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - } + + // NOTICE for above: + // no validation for CHIP_NO_ERROR: + // - there is no guarantee in CI that a valid IP address exists, + // expecially if running in emulators (zephyr and qemu) } /**