From c9f23ccddaaa08f4c5cc2b8a9160eac1e2b4a2e9 Mon Sep 17 00:00:00 2001 From: jinran-google Date: Fri, 25 Mar 2022 12:22:32 +0800 Subject: [PATCH] [settings] set sensitive keys in platform settings initialization (#7496) This commit makes the core pass the sensitive keys to the platform settings initialization, so that the platform settings implementation can know which keys are sensitive keys during the initializing and do the migration when needed. --- examples/platforms/utils/settings_ram.c | 4 +++- include/openthread/instance.h | 2 +- include/openthread/platform/settings.h | 25 +++++++++-------------- src/core/common/settings.cpp | 7 +++---- src/core/common/settings.hpp | 2 +- src/core/common/settings_driver.hpp | 27 ++++++++----------------- src/lib/spinel/radio_spinel_impl.hpp | 2 +- src/posix/platform/settings.cpp | 14 +++++++++++-- tests/fuzz/fuzzer_platform.cpp | 4 +++- tests/unit/test_platform.cpp | 2 +- 10 files changed, 43 insertions(+), 46 deletions(-) diff --git a/examples/platforms/utils/settings_ram.c b/examples/platforms/utils/settings_ram.c index bd20c001c..7481b2d1f 100644 --- a/examples/platforms/utils/settings_ram.c +++ b/examples/platforms/utils/settings_ram.c @@ -57,9 +57,11 @@ struct settingsBlock } OT_TOOL_PACKED_END; // settings API -void otPlatSettingsInit(otInstance *aInstance) +void otPlatSettingsInit(otInstance *aInstance, const uint16_t *aSensitiveKeys, uint16_t aSensitiveKeysLength) { OT_UNUSED_VARIABLE(aInstance); + OT_UNUSED_VARIABLE(aSensitiveKeys); + OT_UNUSED_VARIABLE(aSensitiveKeysLength); sSettingsBufLength = 0; } diff --git a/include/openthread/instance.h b/include/openthread/instance.h index 7b3379330..0de3ec889 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -53,7 +53,7 @@ extern "C" { * @note This number versions both OpenThread platform and user APIs. * */ -#define OPENTHREAD_API_VERSION (198) +#define OPENTHREAD_API_VERSION (199) /** * @addtogroup api-instance diff --git a/include/openthread/platform/settings.h b/include/openthread/platform/settings.h index 8d0b2516b..826344e70 100644 --- a/include/openthread/platform/settings.h +++ b/include/openthread/platform/settings.h @@ -55,7 +55,7 @@ extern "C" { * This enumeration defines the keys of settings. * * Note: When adding a new settings key, if the settings corresponding to the key contains security sensitive - * information, the developer MUST add the key to the array `kCriticalKeys`. + * information, the developer MUST add the key to the array `kSensitiveKeys`. * */ enum @@ -80,10 +80,17 @@ enum /** * Performs any initialization for the settings subsystem, if necessary. * - * @param[in] aInstance The OpenThread instance structure. + * This function also sets the sensitive keys that should be stored in the secure area. + * + * Note that the memory pointed by @p aSensitiveKeys MUST not be released before @p aInstance is destroyed. + * + * @param[in] aInstance The OpenThread instance structure. + * @param[in] aSensitiveKeys A pointer to an array containing the list of sensitive keys. May be NULL only if + * @p aSensitiveKeysLength is 0, which means that there is no sensitive keys. + * @param[in] aSensitiveKeysLength The number of entries in the @p aSensitiveKeys array. * */ -void otPlatSettingsInit(otInstance *aInstance); +void otPlatSettingsInit(otInstance *aInstance, const uint16_t *aSensitiveKeys, uint16_t aSensitiveKeysLength); /** * Performs any de-initialization for the settings subsystem, if necessary. @@ -93,18 +100,6 @@ void otPlatSettingsInit(otInstance *aInstance); */ void otPlatSettingsDeinit(otInstance *aInstance); -/** - * This function sets the critical keys that should be stored in the secure area. - * - * Note that the memory pointed by @p aKeys MUST not be released before @p aInstance is destroyed. - * - * @param[in] aInstance The OpenThread instance structure. - * @param[in] aKeys A pointer to an array containing the list of critical keys. - * @param[in] aKeysLength The number of entries in the @p aKeys array. - * - */ -void otPlatSettingsSetCriticalKeys(otInstance *aInstance, const uint16_t *aKeys, uint16_t aKeysLength); - /// Fetches the value of a setting /** This function fetches the value of the setting identified * by aKey and write it to the memory pointed to by aValue. diff --git a/src/core/common/settings.cpp b/src/core/common/settings.cpp index 89b78f5a3..06f672ba7 100644 --- a/src/core/common/settings.cpp +++ b/src/core/common/settings.cpp @@ -187,8 +187,8 @@ const char *SettingsBase::KeyToString(Key aKey) //--------------------------------------------------------------------------------------------------------------------- // Settings -// This array contains critical keys that should be stored in the secure area. -const uint16_t Settings::kCriticalKeys[] = { +// This array contains sensitive keys that should be stored in the secure area. +const uint16_t Settings::kSensitiveKeys[] = { SettingsBase::kKeyActiveDataset, SettingsBase::kKeyPendingDataset, SettingsBase::kKeySrpEcdsaKey, @@ -196,8 +196,7 @@ const uint16_t Settings::kCriticalKeys[] = { void Settings::Init(void) { - Get().Init(); - Get().SetCriticalKeys(kCriticalKeys, GetArrayLength(kCriticalKeys)); + Get().Init(kSensitiveKeys, GetArrayLength(kSensitiveKeys)); } void Settings::Deinit(void) diff --git a/src/core/common/settings.hpp b/src/core/common/settings.hpp index 5197dd501..be53fa3eb 100644 --- a/src/core/common/settings.hpp +++ b/src/core/common/settings.hpp @@ -1118,7 +1118,7 @@ private: static void Log(Action aAction, Error aError, Key aKey, const void *aValue = nullptr); - static const uint16_t kCriticalKeys[]; + static const uint16_t kSensitiveKeys[]; }; } // namespace ot diff --git a/src/core/common/settings_driver.hpp b/src/core/common/settings_driver.hpp index 29a13aa31..f7851a228 100644 --- a/src/core/common/settings_driver.hpp +++ b/src/core/common/settings_driver.hpp @@ -66,13 +66,19 @@ public: /** * This method initializes the settings storage driver. * + * @param[in] aSensitiveKeys A pointer to an array containing the list of sensitive keys. + * @param[in] aSensitiveKeysLength The number of entries in the @p aSensitiveKeys array. + * */ - void Init(void) + void Init(const uint16_t *aSensitiveKeys, uint16_t aSensitiveKeysLength) { #if OPENTHREAD_CONFIG_PLATFORM_FLASH_API_ENABLE + OT_UNUSED_VARIABLE(aSensitiveKeys); + OT_UNUSED_VARIABLE(aSensitiveKeysLength); + mFlash.Init(); #else - otPlatSettingsInit(GetInstancePtr()); + otPlatSettingsInit(GetInstancePtr(), aSensitiveKeys, aSensitiveKeysLength); #endif } @@ -87,23 +93,6 @@ public: #endif } - /** - * This method sets the critical keys that should be stored in a secure area. - * - * @param[in] aKeys A pointer to an array containing the list of critical keys. - * @param[in] aKeysLength The number of entries in the @p aKeys array. - * - */ - void SetCriticalKeys(const uint16_t *aKeys, uint16_t aKeysLength) - { -#if OPENTHREAD_CONFIG_PLATFORM_FLASH_API_ENABLE - OT_UNUSED_VARIABLE(aKeys); - OT_UNUSED_VARIABLE(aKeysLength); -#else - otPlatSettingsSetCriticalKeys(GetInstancePtr(), aKeys, aKeysLength); -#endif - } - /** * This method adds a value to @p aKey. * diff --git a/src/lib/spinel/radio_spinel_impl.hpp b/src/lib/spinel/radio_spinel_impl.hpp index c8ec4f6a4..d5da5ba42 100644 --- a/src/lib/spinel/radio_spinel_impl.hpp +++ b/src/lib/spinel/radio_spinel_impl.hpp @@ -431,7 +431,7 @@ otError RadioSpinel::RestoreDatasetFromNcp(vo { otError error = OT_ERROR_NONE; - Instance::Get().template Get().Init(); + Instance::Get().template Get().Init(nullptr, 0); otLogInfoPlat("Trying to get saved dataset from NCP"); SuccessOrExit( diff --git a/src/posix/platform/settings.cpp b/src/posix/platform/settings.cpp index 140d762a3..d9809d301 100644 --- a/src/posix/platform/settings.cpp +++ b/src/posix/platform/settings.cpp @@ -167,10 +167,20 @@ static void swapDiscard(otInstance *aInstance, int aFd) VerifyOrDie(0 == unlink(swapFileName), OT_EXIT_ERROR_ERRNO); } -void otPlatSettingsInit(otInstance *aInstance) +void otPlatSettingsInit(otInstance *aInstance, const uint16_t *aSensitiveKeys, uint16_t aSensitiveKeysLength) { +#if !OPENTHREAD_POSIX_CONFIG_SECURE_SETTINGS_ENABLE + OT_UNUSED_VARIABLE(aSensitiveKeys); + OT_UNUSED_VARIABLE(aSensitiveKeysLength); +#endif + otError error = OT_ERROR_NONE; +#if OPENTHREAD_POSIX_CONFIG_SECURE_SETTINGS_ENABLE + sKeys = aSensitiveKeys; + sKeysLength = aSensitiveKeysLength; +#endif + // Don't touch the settings file the system runs in dry-run mode. VerifyOrExit(!IsSystemDryRun()); @@ -531,7 +541,7 @@ int main() data[i] = i; } - otPlatSettingsInit(instance); + otPlatSettingsInit(instance, nullptr, 0); // verify empty situation otPlatSettingsWipe(instance); diff --git a/tests/fuzz/fuzzer_platform.cpp b/tests/fuzz/fuzzer_platform.cpp index a8ae28b01..74499377e 100644 --- a/tests/fuzz/fuzzer_platform.cpp +++ b/tests/fuzz/fuzzer_platform.cpp @@ -441,9 +441,11 @@ otError otPlatEntropyGet(uint8_t *aOutput, uint16_t aOutputLength) return OT_ERROR_NONE; } -void otPlatSettingsInit(otInstance *aInstance) +void otPlatSettingsInit(otInstance *aInstance, const uint16_t *aSensitiveKeys, uint16_t aSensitiveKeysLength) { OT_UNUSED_VARIABLE(aInstance); + OT_UNUSED_VARIABLE(aSensitiveKeys); + OT_UNUSED_VARIABLE(aSensitiveKeysLength); } void otPlatSettingsDeinit(otInstance *aInstance) diff --git a/tests/unit/test_platform.cpp b/tests/unit/test_platform.cpp index ce6ab2a44..9d01cd3cf 100644 --- a/tests/unit/test_platform.cpp +++ b/tests/unit/test_platform.cpp @@ -330,7 +330,7 @@ OT_TOOL_WEAK void otPlatLog(otLogLevel, otLogRegion, const char *, ...) { } -OT_TOOL_WEAK void otPlatSettingsInit(otInstance *) +OT_TOOL_WEAK void otPlatSettingsInit(otInstance *, const uint16_t *, uint16_t) { }