From 75d4ffedae1e383ce92530e7de2d9d3ed767fd76 Mon Sep 17 00:00:00 2001 From: Markus Becker Date: Mon, 8 Nov 2021 20:43:51 +0100 Subject: [PATCH] Use null-terminated SerialNumber (#11462) * Aligned with FirmwareRevisionString, ProductionRevisionString, ProductName, VendorName which are also null-terminated * Adapt SerialNumber test accordingly --- examples/common/pigweed/rpc_services/Device.h | 4 ++-- src/app/server/Dnssd.cpp | 6 ++---- src/include/platform/ConfigurationManager.h | 2 +- .../GenericConfigurationManagerImpl.cpp | 18 +++++++++++------- .../internal/GenericConfigurationManagerImpl.h | 2 +- src/platform/Linux/bluez/Helper.cpp | 5 ++--- src/platform/fake/ConfigurationManagerImpl.h | 2 +- src/platform/tests/TestConfigurationMgr.cpp | 9 ++++----- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/examples/common/pigweed/rpc_services/Device.h b/examples/common/pigweed/rpc_services/Device.h index 32b143a584b365..81b719e63b60e8 100644 --- a/examples/common/pigweed/rpc_services/Device.h +++ b/examples/common/pigweed/rpc_services/Device.h @@ -103,8 +103,8 @@ class Device : public generated::Device response.pairing_info.discriminator = static_cast(discriminator); response.has_pairing_info = true; } - size_t serial_size; - DeviceLayer::ConfigurationMgr().GetSerialNumber(response.serial_number, sizeof(response.serial_number), serial_size); + + DeviceLayer::ConfigurationMgr().GetSerialNumber(response.serial_number, sizeof(response.serial_number)); return pw::OkStatus(); } diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp index 7c20d03e5fc4d9..1bc2dbd598fcb8 100644 --- a/src/app/server/Dnssd.cpp +++ b/src/app/server/Dnssd.cpp @@ -491,15 +491,13 @@ void DnssdServer::StartServer(chip::Dnssd::CommissioningMode mode) CHIP_ERROR DnssdServer::GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize) { char serialNumber[chip::DeviceLayer::ConfigurationManager::kMaxSerialNumberLength + 1]; - size_t serialNumberSize = 0; uint16_t lifetimeCounter = 0; size_t rotatingDeviceIdValueOutputSize = 0; - ReturnErrorOnFailure( - chip::DeviceLayer::ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber), serialNumberSize)); + ReturnErrorOnFailure(chip::DeviceLayer::ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber))); ReturnErrorOnFailure(chip::DeviceLayer::ConfigurationMgr().GetLifetimeCounter(lifetimeCounter)); return AdditionalDataPayloadGenerator().generateRotatingDeviceIdAsHexString( - lifetimeCounter, serialNumber, serialNumberSize, rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize, + lifetimeCounter, serialNumber, strlen(serialNumber), rotatingDeviceIdHexBuffer, rotatingDeviceIdHexBufferSize, rotatingDeviceIdValueOutputSize); } #endif diff --git a/src/include/platform/ConfigurationManager.h b/src/include/platform/ConfigurationManager.h index f0c0758b8f1877..579250666a6254 100644 --- a/src/include/platform/ConfigurationManager.h +++ b/src/include/platform/ConfigurationManager.h @@ -74,7 +74,7 @@ class ConfigurationManager virtual CHIP_ERROR GetProductId(uint16_t & productId) = 0; virtual CHIP_ERROR GetProductRevisionString(char * buf, size_t bufSize) = 0; virtual CHIP_ERROR GetProductRevision(uint16_t & productRev) = 0; - virtual CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) = 0; + virtual CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize) = 0; virtual CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) = 0; virtual CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) = 0; virtual CHIP_ERROR GetPrimary802154MACAddress(uint8_t * buf) = 0; diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp index 464fd9c5fd5a45..ae9068179d7e11 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.cpp +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.cpp @@ -79,22 +79,27 @@ CHIP_ERROR GenericConfigurationManagerImpl::GetFirmwareRevisionString } template -CHIP_ERROR GenericConfigurationManagerImpl::GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) +CHIP_ERROR GenericConfigurationManagerImpl::GetSerialNumber(char * buf, size_t bufSize) { CHIP_ERROR err; - err = Impl()->ReadConfigValueStr(ImplClass::kConfigKey_SerialNum, buf, bufSize, serialNumLen); + size_t serialNumLen = 0; // without counting null-terminator + err = Impl()->ReadConfigValueStr(ImplClass::kConfigKey_SerialNum, buf, bufSize, serialNumLen); + #ifdef CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER if (CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER[0] != 0 && err == CHIP_DEVICE_ERROR_CONFIG_NOT_FOUND) { - VerifyOrExit(sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) <= bufSize, err = CHIP_ERROR_BUFFER_TOO_SMALL); + ReturnErrorCodeIf(sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) > bufSize, CHIP_ERROR_BUFFER_TOO_SMALL); memcpy(buf, CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER, sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER)); serialNumLen = sizeof(CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER) - 1; err = CHIP_NO_ERROR; } #endif // CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER - SuccessOrExit(err); -exit: + ReturnErrorOnFailure(err); + + ReturnErrorCodeIf(serialNumLen >= bufSize, CHIP_ERROR_BUFFER_TOO_SMALL); + ReturnErrorCodeIf(buf[serialNumLen] != 0, CHIP_ERROR_INVALID_STRING_LENGTH); + return err; } @@ -469,8 +474,7 @@ void GenericConfigurationManagerImpl::LogDeviceConfig() { char serialNum[ConfigurationManager::kMaxSerialNumberLength + 1]; - size_t serialNumLen; - err = GetSerialNumber(serialNum, sizeof(serialNum), serialNumLen); + err = GetSerialNumber(serialNum, sizeof(serialNum)); ChipLogProgress(DeviceLayer, " Serial Number: %s", (err == CHIP_NO_ERROR) ? serialNum : "(not set)"); } diff --git a/src/include/platform/internal/GenericConfigurationManagerImpl.h b/src/include/platform/internal/GenericConfigurationManagerImpl.h index e2dbcdb3173fbc..380b9540d0f86f 100644 --- a/src/include/platform/internal/GenericConfigurationManagerImpl.h +++ b/src/include/platform/internal/GenericConfigurationManagerImpl.h @@ -63,7 +63,7 @@ class GenericConfigurationManagerImpl : public ConfigurationManager CHIP_ERROR StoreProductRevision(uint16_t productRev) override; CHIP_ERROR GetFirmwareRevisionString(char * buf, size_t bufSize) override; CHIP_ERROR GetFirmwareRevision(uint16_t & firmwareRev) override; - CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) override; + CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize) override; CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override; CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override; CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override; diff --git a/src/platform/Linux/bluez/Helper.cpp b/src/platform/Linux/bluez/Helper.cpp index 53be1c2a558537..4da5da3021be03 100644 --- a/src/platform/Linux/bluez/Helper.cpp +++ b/src/platform/Linux/bluez/Helper.cpp @@ -1191,12 +1191,11 @@ static void UpdateAdditionalDataCharacteristic(BluezGattCharacteristic1 * charac chip::System::PacketBufferHandle bufferHandle; char serialNumber[ConfigurationManager::kMaxSerialNumberLength + 1]; - size_t serialNumberSize = 0; uint16_t lifetimeCounter = 0; BitFlags additionalDataFields; #if CHIP_ENABLE_ROTATING_DEVICE_ID - err = ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber), serialNumberSize); + err = ConfigurationMgr().GetSerialNumber(serialNumber, sizeof(serialNumber)); SuccessOrExit(err); err = ConfigurationMgr().GetLifetimeCounter(lifetimeCounter); SuccessOrExit(err); @@ -1204,7 +1203,7 @@ static void UpdateAdditionalDataCharacteristic(BluezGattCharacteristic1 * charac additionalDataFields.Set(AdditionalDataFields::RotatingDeviceId); #endif - err = AdditionalDataPayloadGenerator().generateAdditionalDataPayload(lifetimeCounter, serialNumber, serialNumberSize, + err = AdditionalDataPayloadGenerator().generateAdditionalDataPayload(lifetimeCounter, serialNumber, strlen(serialNumber), bufferHandle, additionalDataFields); SuccessOrExit(err); diff --git a/src/platform/fake/ConfigurationManagerImpl.h b/src/platform/fake/ConfigurationManagerImpl.h index 3a78d29a011f49..af8ac5c6c2bbfb 100644 --- a/src/platform/fake/ConfigurationManagerImpl.h +++ b/src/platform/fake/ConfigurationManagerImpl.h @@ -40,7 +40,7 @@ class ConfigurationManagerImpl final : public ConfigurationManager CHIP_ERROR StoreProductRevision(uint16_t productRev) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetFirmwareRevisionString(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetFirmwareRevision(uint16_t & firmwareRev) override { return CHIP_ERROR_NOT_IMPLEMENTED; } - CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize, size_t & serialNumLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; } + CHIP_ERROR GetSerialNumber(char * buf, size_t bufSize) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR StoreSerialNumber(const char * serialNum, size_t serialNumLen) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetPrimaryMACAddress(MutableByteSpan buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; } CHIP_ERROR GetPrimaryWiFiMACAddress(uint8_t * buf) override { return CHIP_ERROR_NOT_IMPLEMENTED; } diff --git a/src/platform/tests/TestConfigurationMgr.cpp b/src/platform/tests/TestConfigurationMgr.cpp index 7f47f1d73037f8..d8d883f0937e60 100644 --- a/src/platform/tests/TestConfigurationMgr.cpp +++ b/src/platform/tests/TestConfigurationMgr.cpp @@ -66,25 +66,24 @@ static void TestConfigurationMgr_SerialNumber(nlTestSuite * inSuite, void * inCo CHIP_ERROR err = CHIP_NO_ERROR; char buf[64]; - size_t serialNumberLen = 0; const char * serialNumber = "89051AAZZ236"; err = ConfigurationMgr().StoreSerialNumber(serialNumber, strlen(serialNumber)); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - err = ConfigurationMgr().GetSerialNumber(buf, 64, serialNumberLen); + err = ConfigurationMgr().GetSerialNumber(buf, 64); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, serialNumberLen == strlen(serialNumber)); + NL_TEST_ASSERT(inSuite, strlen(buf) == 12); NL_TEST_ASSERT(inSuite, strcmp(buf, serialNumber) == 0); err = ConfigurationMgr().StoreSerialNumber(serialNumber, 5); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - err = ConfigurationMgr().GetSerialNumber(buf, 64, serialNumberLen); + err = ConfigurationMgr().GetSerialNumber(buf, 64); NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, serialNumberLen == 5); + NL_TEST_ASSERT(inSuite, strlen(buf) == 5); NL_TEST_ASSERT(inSuite, strcmp(buf, "89051") == 0); }