Clear ITS keys on factory reset #436
Conversation
Added a kconfig option to enable clearing keys stored in the PSA Internal Trusted Storage. If the kconfig is selected then during the factory reset process all keys stored in the PSA ITS will be destroyed. This mechanism ensures that all PSA ITS crypto materials that belong to the Matter stack will be removed.
Prevent from removing DAC Private Key from ITS during factory reset.
ArekBalysNordic
requested review from
LuDuda,
kkasperczyk-no,
Damian-Nordic and
markaj-nordic
| @@ -433,6 +433,14 @@ config CHIP_FACTORY_RESET_ERASE_NVS | |||
| configuration is supposed to be cleared on a factory reset, including | |||
| device-specific entries. | |||
|
|
|||
| config CHIP_FACTORY_RESET_ERASE_PSA_ITS | |||
There was a problem hiding this comment.
I don't know the full background of this change, but do we want to make it configurable? It seems like we may have problem if these data will not be removed, so maybe we should just do it always? Also shouldn't we enable it for our platform or set default to y for everyone?
| for (uint32_t keyID = static_cast<uint32_t>(chip::Crypto::KeyIdBase::Minimum); | ||
| keyID <= static_cast<uint32_t>(chip::Crypto::KeyIdBase::Maximum); keyID++) | ||
| { | ||
| #ifdef CONFIG_CHIP_CRYPTO_PSA_MIGRATE_DAC_PRIV_KEY |
There was a problem hiding this comment.
I would run this if unconditionally.
| @@ -210,6 +214,22 @@ void ConfigurationManagerImpl::DoFactoryReset(intptr_t arg) | |||
| ConnectivityMgr().ErasePersistentInfo(); | |||
| #endif | |||
|
|
|||
| #ifdef CONFIG_CHIP_FACTORY_RESET_ERASE_PSA_ITS | |||
| // Ensure that all persistent PSA crypto materials are removed. | |||
| for (uint32_t keyID = static_cast<uint32_t>(chip::Crypto::KeyIdBase::Minimum); | |||
There was a problem hiding this comment.
| for (uint32_t keyID = static_cast<uint32_t>(chip::Crypto::KeyIdBase::Minimum); | |
| for (psa_key_id_t keyID = chip::to_underlying(Crypto::KeyIdBase::Minimum); |
etc. to not cast via uint32_t.
Though now that I think about it, this will invoke psa_destroy_key for 255 possible fabrics even though we have only 5 configured. Maybe it's better to run this loop only for the [minimum, minimum + MAX_FABRICS) range? Or it doesn't generate any latency?
|
We decided to close this PR and check whether it is a real problem or not. |
Added a kconfig option to enable clearing keys stored in the PSA Internal Trusted Storage. If the kconfig is selected then during the factory reset process all keys stored in the PSA ITS will be destroyed.
This mechanism ensures that all PSA ITS crypto materials that belong to the Matter stack will be removed.
manifest-pr-skip
Commit c6d0842 will be pushed to Upstream after the first review here, and then this PR will be continued.