diff --git a/src/crypto/PSAOperationalKeystore.cpp b/src/crypto/PSAOperationalKeystore.cpp index c8a5e3a3b3..b8c3f4fb31 100644 --- a/src/crypto/PSAOperationalKeystore.cpp +++ b/src/crypto/PSAOperationalKeystore.cpp @@ -27,7 +27,9 @@ namespace chip { namespace Crypto { namespace { -// Tags for our operational keypair storage copied from PersistentStorageOperationalKeystore.cpp +// Tags and version of operational keypair storage copied from PersistentStorageOperationalKeystore.cpp +// These values must be compatible with values in the PersistentStorageOperationalKeystore.cpp file to perform a proper keys +// migration. constexpr TLV::Tag kOpKeyVersionTag = TLV::ContextTag(0); constexpr TLV::Tag kOpKeyDataTag = TLV::ContextTag(1); constexpr uint16_t kOpKeyVersion = 1; @@ -111,6 +113,9 @@ bool PSAOperationalKeystore::HasOpKeypairForFabric(FabricIndex fabricIndex) cons { VerifyOrReturnError(IsValidFabricIndex(fabricIndex), false); + // check if the key exists in the PersistentStorageOperationalKeystore exist, and if so move it to the PSA ITS secure storage. + VerifyOrReturnError(CHIP_NO_ERROR == MoveOpKeysFromKVSToITS(fabricIndex), false); + if (mPendingFabricIndex == fabricIndex) { return mIsPendingKeypairActive; @@ -150,7 +155,7 @@ CHIP_ERROR PSAOperationalKeystore::PersistentP256Keypair::Deserialize(P256Serial psa_status_t status = PSA_SUCCESS; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_key_id_t keyId = 0; - Encoding::BufferWriter bbuf(mPublicKey, mPublicKey.Length()); + VerifyOrReturnError(input.Length() == mPublicKey.Length() + kP256_PrivateKey_Length, CHIP_ERROR_INVALID_ARGUMENT); Destroy(); @@ -165,12 +170,7 @@ CHIP_ERROR PSAOperationalKeystore::PersistentP256Keypair::Deserialize(P256Serial status = psa_import_key(&attributes, input.ConstBytes() + mPublicKey.Length(), kP256_PrivateKey_Length, &keyId); VerifyOrExit(status == PSA_SUCCESS, error = CHIP_ERROR_INTERNAL); - bbuf.Put(input.ConstBytes(), mPublicKey.Length()); - if (!bbuf.Fit()) - { - psa_destroy_key(keyId); - error = CHIP_ERROR_NO_MEMORY; - } + memcpy(mPublicKey.Bytes(), input.ConstBytes(), mPublicKey.Length()); exit: psa_reset_key_attributes(&attributes); @@ -221,6 +221,8 @@ CHIP_ERROR PSAOperationalKeystore::SignWithOpKeypair(FabricIndex fabricIndex, co Crypto::P256ECDSASignature & outSignature) const { VerifyOrReturnError(IsValidFabricIndex(fabricIndex), CHIP_ERROR_INVALID_FABRIC_INDEX); + // Check if the key already exists and need to be migrated to PSA ITS + VerifyOrReturnError(HasOpKeypairForFabric(fabricIndex), CHIP_ERROR_INVALID_FABRIC_INDEX); if (mPendingFabricIndex == fabricIndex) { @@ -252,25 +254,18 @@ void PSAOperationalKeystore::ReleasePendingKeypair() mIsPendingKeypairActive = false; } -CHIP_ERROR PSAOperationalKeystore::MoveOpKeysFromKVSToITS(PersistentStorageDelegate * storage) +CHIP_ERROR PSAOperationalKeystore::MoveOpKeysFromKVSToITS(FabricIndex fabricIndex) const { Crypto::SensitiveDataBuffer buf; uint16_t keySize = static_cast(buf.Capacity()); - for (FabricIndex index = 0; index < CHIP_CONFIG_MAX_FABRICS; index++) + if (CHIP_NO_ERROR == + mStorage->SyncGetKeyValue(DefaultStorageKeyAllocator::FabricOpKey(fabricIndex).KeyName(), buf.Bytes(), keySize)) { - CHIP_ERROR err = storage->SyncGetKeyValue(DefaultStorageKeyAllocator::FabricOpKey(index).KeyName(), buf.Bytes(), keySize); - if (CHIP_NO_ERROR == err) + PersistentP256Keypair keypair(fabricIndex); + // Do not allow overwriting the existing key and just remove it from KVS + if (!keypair.Exists()) { - PersistentP256Keypair keyPair(index); - - // Do not allow overwriting the existing key and just remove it from KVS - if (keyPair.Exists()) - { - ReturnErrorOnFailure(storage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricOpKey(index).KeyName())); - continue; - } - // Read the operational private and public keys from the TLV entry buf.SetLength(static_cast(keySize)); TLV::ContiguousBufferTLVReader reader; @@ -291,13 +286,18 @@ CHIP_ERROR PSAOperationalKeystore::MoveOpKeysFromKVSToITS(PersistentStorageDeleg if (keyData.size() == kP256_PrivateKey_Length + kP256_PublicKey_Length) { + // Migrate the operational keys P256SerializedKeypair serializedKeypair; ReturnErrorOnFailure(serializedKeypair.SetLength(kP256_PrivateKey_Length + kP256_PublicKey_Length)); memcpy(serializedKeypair.Bytes(), keyData.data(), kP256_PrivateKey_Length + kP256_PublicKey_Length); - ReturnErrorOnFailure(keyPair.Deserialize(serializedKeypair)); - ReturnErrorOnFailure(storage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricOpKey(index).KeyName())); + ReturnErrorOnFailure(keypair.Deserialize(serializedKeypair)); + ReturnErrorOnFailure(mStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricOpKey(fabricIndex).KeyName())); } } + else + { + ReturnErrorOnFailure(mStorage->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricOpKey(fabricIndex).KeyName())); + } } return CHIP_NO_ERROR; diff --git a/src/crypto/PSAOperationalKeystore.h b/src/crypto/PSAOperationalKeystore.h index 6bd5539a43..87bfb530b8 100644 --- a/src/crypto/PSAOperationalKeystore.h +++ b/src/crypto/PSAOperationalKeystore.h @@ -28,18 +28,22 @@ class PSAOperationalKeystore final : public OperationalKeystore { public: /** - * @brief Initialize the PSA Operational Keystore and move all sensitive keys from storage to ITS secure storage. + * @brief Initialize the PSA Operational Keystore to save the persistent storage delegate. * - * After moving all sensitive keys to ITS, the old one stored in the storage will be removed and not available anymore. + * The PersistentStorageDelegate object will be needed to perform operational keys migration from the Persistent Storage + * Operational Keystore database to PSA ITS secure storage. * - * @param storage Pointer to persistent storage delegate to use. - * @retval CHIP_NO_ERROR on success - * @retval CHIP_ERROR_INCORRECT_STATE if already initialized + * NOTE: The migration of the operational keys will occur on each HasOpKeypairForFabric method call for specific fabric index. + * + * @param storage Pointer to persistent storage delegate to use. Must outlive this instance. + * @retval CHIP_NO_ERROR on success. + * @retval CHIP_ERROR_INCORRECT_STATE if already initialized. */ CHIP_ERROR Init(PersistentStorageDelegate * storage) { - VerifyOrReturnError(storage != nullptr, CHIP_ERROR_INVALID_ARGUMENT); - return MoveOpKeysFromKVSToITS(storage); + VerifyOrReturnError(mStorage == nullptr, CHIP_ERROR_INCORRECT_STATE); + mStorage = storage; + return CHIP_NO_ERROR; } bool HasPendingOpKeypair() const override; @@ -77,9 +81,10 @@ class PSAOperationalKeystore final : public OperationalKeystore PersistentP256Keypair * mPendingKeypair = nullptr; FabricIndex mPendingFabricIndex = kUndefinedFabricIndex; bool mIsPendingKeypairActive = false; + PersistentStorageDelegate * mStorage = nullptr; private: - CHIP_ERROR MoveOpKeysFromKVSToITS(PersistentStorageDelegate * storage); + CHIP_ERROR MoveOpKeysFromKVSToITS(FabricIndex fabricIndex) const; }; } // namespace Crypto diff --git a/src/crypto/PersistentStorageOperationalKeystore.cpp b/src/crypto/PersistentStorageOperationalKeystore.cpp index 80eee2d974..9747070a92 100644 --- a/src/crypto/PersistentStorageOperationalKeystore.cpp +++ b/src/crypto/PersistentStorageOperationalKeystore.cpp @@ -34,11 +34,15 @@ using namespace chip::Crypto; namespace { // Tags for our operational keypair storage. +// If the tags changes, change also the copy in the PSAOperationalKeystore.cpp to keep migration of the operational keys to +// PSA ITS properly constexpr TLV::Tag kOpKeyVersionTag = TLV::ContextTag(0); constexpr TLV::Tag kOpKeyDataTag = TLV::ContextTag(1); // If this version grows beyond UINT16_MAX, adjust OpKeypairTLVMaxSize // accordingly. +// If this version changes, change also the copy in the PSAOperationalKeystore.cpp to keep migration of the operational keys to +// PSA ITS properly constexpr uint16_t kOpKeyVersion = 1; constexpr size_t OpKeyTLVMaxSize()