From 8116a0b52c37c9320fbefaaa1e12efc64c260d7c Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Thu, 19 May 2022 13:36:17 +0530 Subject: [PATCH 1/3] [ESP32] Handle NVS key with length > 15 --- .../ESP32/KeyValueStoreManagerImpl.cpp | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp index 66a93c4c155b4f..0e8ae21681efa5 100644 --- a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp +++ b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp @@ -30,11 +30,38 @@ #include "nvs.h" #include "nvs_flash.h" #include +#include namespace chip { namespace DeviceLayer { namespace PersistedStorage { +namespace { +// Implementation Note: esp-idf nvs implementation cannot handle key length > 15, +// Below implementation tries to handle that case by hashing the key +// If key length is > 15 then take the SHA1 of the key and convert the first 8 bytes to hex string. +// Not sure how likely we would run into a conflict as we are only using 8 bytes out of 20 + +CHIP_ERROR HashIfLongKey(const char * key, char * keyHash) +{ + VerifyOrReturnError(strlen(key) > (NVS_KEY_NAME_MAX_SIZE - 1), CHIP_ERROR_INCORRECT_STATE); + + uint8_t hashBuffer[chip::Crypto::kSHA1_Hash_Length]; + ReturnErrorOnFailure(chip::Crypto::Hash_SHA1((const uint8_t *)key, strlen(key), hashBuffer)); + + int i = 0, j = 0; + while (i < NVS_KEY_NAME_MAX_SIZE) + { + sprintf(keyHash + i, "%02x", hashBuffer[j]); + i += 2; + j += 1; + } + keyHash[NVS_KEY_NAME_MAX_SIZE - 1] = 0; + ChipLogDetail(DeviceLayer, "Using hash:%s for nvs key:%s", keyHash, key); + return CHIP_NO_ERROR; +} +} // namespace + KeyValueStoreManagerImpl KeyValueStoreManagerImpl::sInstance; CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t value_size, size_t * read_bytes_size, @@ -48,6 +75,10 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t Internal::ScopedNvsHandle handle; ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READONLY)); + + char keyHash[NVS_KEY_NAME_MAX_SIZE]; + VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + ReturnMappedErrorOnFailure(nvs_get_blob(handle, key, value, &value_size)); if (read_bytes_size) @@ -65,6 +96,9 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, Internal::ScopedNvsHandle handle; ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READWRITE)); + char keyHash[NVS_KEY_NAME_MAX_SIZE]; + VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + ReturnMappedErrorOnFailure(nvs_set_blob(handle, key, value, value_size)); // Commit the value to the persistent store. @@ -79,6 +113,9 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key) ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READWRITE)); + char keyHash[NVS_KEY_NAME_MAX_SIZE]; + VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + ReturnMappedErrorOnFailure(nvs_erase_key(handle, key)); // Commit the value to the persistent store. From 2a8689aad8f99b466fbfed4d3098fcac9b6079f4 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 19 May 2022 08:23:19 +0000 Subject: [PATCH 2/3] Restyled by clang-format --- src/platform/ESP32/KeyValueStoreManagerImpl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp index 0e8ae21681efa5..844e4c068b9cdc 100644 --- a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp +++ b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp @@ -29,8 +29,8 @@ #include "nvs.h" #include "nvs_flash.h" -#include #include +#include namespace chip { namespace DeviceLayer { @@ -47,7 +47,7 @@ CHIP_ERROR HashIfLongKey(const char * key, char * keyHash) VerifyOrReturnError(strlen(key) > (NVS_KEY_NAME_MAX_SIZE - 1), CHIP_ERROR_INCORRECT_STATE); uint8_t hashBuffer[chip::Crypto::kSHA1_Hash_Length]; - ReturnErrorOnFailure(chip::Crypto::Hash_SHA1((const uint8_t *)key, strlen(key), hashBuffer)); + ReturnErrorOnFailure(chip::Crypto::Hash_SHA1((const uint8_t *) key, strlen(key), hashBuffer)); int i = 0, j = 0; while (i < NVS_KEY_NAME_MAX_SIZE) From 0c3ff663f9bef8c473975bc397a06786ea78761d Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Mon, 23 May 2022 12:42:23 +0530 Subject: [PATCH 3/3] Using helper API BytesToHex() and now function returns bool value --- .../ESP32/KeyValueStoreManagerImpl.cpp | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp index 844e4c068b9cdc..75d66e99513492 100644 --- a/src/platform/ESP32/KeyValueStoreManagerImpl.cpp +++ b/src/platform/ESP32/KeyValueStoreManagerImpl.cpp @@ -30,6 +30,7 @@ #include "nvs.h" #include "nvs_flash.h" #include +#include #include namespace chip { @@ -39,26 +40,26 @@ namespace PersistedStorage { namespace { // Implementation Note: esp-idf nvs implementation cannot handle key length > 15, // Below implementation tries to handle that case by hashing the key -// If key length is > 15 then take the SHA1 of the key and convert the first 8 bytes to hex string. +// If key length is > 15 then take the SHA1 of the key and convert the first 7.5 bytes to hex string. // Not sure how likely we would run into a conflict as we are only using 8 bytes out of 20 - -CHIP_ERROR HashIfLongKey(const char * key, char * keyHash) +// +// key returned by below function will not collide with any existing "normal" keys because those always have a "/" in +// the first few chars and the output of this code never will. +// +// Returns true if key is hashed, false otherwise. +bool HashIfLongKey(const char * key, char * keyHash) { - VerifyOrReturnError(strlen(key) > (NVS_KEY_NAME_MAX_SIZE - 1), CHIP_ERROR_INCORRECT_STATE); + ReturnErrorCodeIf(strlen(key) < NVS_KEY_NAME_MAX_SIZE, false); uint8_t hashBuffer[chip::Crypto::kSHA1_Hash_Length]; - ReturnErrorOnFailure(chip::Crypto::Hash_SHA1((const uint8_t *) key, strlen(key), hashBuffer)); + ReturnErrorCodeIf(Crypto::Hash_SHA1(Uint8::from_const_char(key), strlen(key), hashBuffer) != CHIP_NO_ERROR, false); - int i = 0, j = 0; - while (i < NVS_KEY_NAME_MAX_SIZE) - { - sprintf(keyHash + i, "%02x", hashBuffer[j]); - i += 2; - j += 1; - } + BitFlags flags(Encoding::HexFlags::kNone); + Encoding::BytesToHex(hashBuffer, NVS_KEY_NAME_MAX_SIZE / 2, keyHash, NVS_KEY_NAME_MAX_SIZE, flags); keyHash[NVS_KEY_NAME_MAX_SIZE - 1] = 0; + ChipLogDetail(DeviceLayer, "Using hash:%s for nvs key:%s", keyHash, key); - return CHIP_NO_ERROR; + return true; } } // namespace @@ -77,7 +78,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Get(const char * key, void * value, size_t ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READONLY)); char keyHash[NVS_KEY_NAME_MAX_SIZE]; - VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + VerifyOrdo(HashIfLongKey(key, keyHash) == false, key = keyHash); ReturnMappedErrorOnFailure(nvs_get_blob(handle, key, value, &value_size)); @@ -97,7 +98,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Put(const char * key, const void * value, ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READWRITE)); char keyHash[NVS_KEY_NAME_MAX_SIZE]; - VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + VerifyOrdo(HashIfLongKey(key, keyHash) == false, key = keyHash); ReturnMappedErrorOnFailure(nvs_set_blob(handle, key, value, value_size)); @@ -114,7 +115,7 @@ CHIP_ERROR KeyValueStoreManagerImpl::_Delete(const char * key) ReturnErrorOnFailure(handle.Open(kNamespace, NVS_READWRITE)); char keyHash[NVS_KEY_NAME_MAX_SIZE]; - VerifyOrdo(HashIfLongKey(key, keyHash) != CHIP_NO_ERROR, key = keyHash); + VerifyOrdo(HashIfLongKey(key, keyHash) == false, key = keyHash); ReturnMappedErrorOnFailure(nvs_erase_key(handle, key));