From 13908301a9a4bed77feb7bf96777bb71cbce6112 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 10 Mar 2022 14:37:02 -0500 Subject: [PATCH] Change fabric table persistence to not use a single blob per fabric. (#16032) * Change fabric table persistence to not use a single blob per fabric. This will let us do a better job of loading things on demand as needed, in followups, so we don't have to keep the certs in RAM all the time. * Address review comments --- src/credentials/FabricTable.cpp | 300 ++++++++++++------- src/credentials/FabricTable.h | 20 +- src/lib/support/DefaultStorageKeyAllocator.h | 7 +- 3 files changed, 200 insertions(+), 127 deletions(-) diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index 8eece2a614b0b5..0735f6579b1e97 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #if CHIP_CRYPTO_HSM #include @@ -41,145 +42,208 @@ CHIP_ERROR FabricInfo::SetFabricLabel(const CharSpan & fabricLabel) return CHIP_NO_ERROR; } +namespace { +// Tags for our metadata storage. +constexpr TLV::Tag kVendorIdTag = TLV::ContextTag(0); +constexpr TLV::Tag kFabricLabelTag = TLV::ContextTag(1); + +// Tags for our operational keypair storage. +constexpr TLV::Tag kOpKeyVersionTag = TLV::ContextTag(0); +constexpr TLV::Tag kOpKeyDataTag = TLV::ContextTag(1); + +// If this version grows beyond UINT16_MAX, adjust OpKeypairTLVMaxSize +// accordingly. +constexpr uint16_t kOpKeyVersion = 1; +} // anonymous namespace + CHIP_ERROR FabricInfo::CommitToStorage(PersistentStorageDelegate * storage) { - CHIP_ERROR err = CHIP_NO_ERROR; + DefaultStorageKeyAllocator keyAlloc; - char key[kKeySize]; - ReturnErrorOnFailure(GenerateKey(mFabric, key, sizeof(key))); + VerifyOrReturnError(mRootCert.size() <= kMaxCHIPCertLength && mICACert.size() <= kMaxCHIPCertLength && + mNOCCert.size() <= kMaxCHIPCertLength, + CHIP_ERROR_BUFFER_TOO_SMALL); + static_assert(kMaxCHIPCertLength <= UINT16_MAX, "Casting to uint16_t won't be safe"); - StorableFabricInfo * info = chip::Platform::New(); - ReturnErrorCodeIf(info == nullptr, CHIP_ERROR_NO_MEMORY); + ReturnErrorOnFailure( + storage->SyncSetKeyValue(keyAlloc.FabricRCAC(mFabric), mRootCert.data(), static_cast(mRootCert.size()))); - info->mFabricIndex = mFabric; - info->mVendorId = Encoding::LittleEndian::HostSwap16(mVendorId); + // If we stop storing ICA certs when empty, update LoadFromStorage + // accordingly to check for CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND. + ReturnErrorOnFailure( + storage->SyncSetKeyValue(keyAlloc.FabricICAC(mFabric), mICACert.data(), static_cast(mICACert.size()))); - size_t stringLength = strnlen(mFabricLabel, kFabricLabelMaxLengthInBytes); - memcpy(info->mFabricLabel, mFabricLabel, stringLength); - info->mFabricLabel[stringLength] = '\0'; // Set null terminator + ReturnErrorOnFailure( + storage->SyncSetKeyValue(keyAlloc.FabricNOC(mFabric), mNOCCert.data(), static_cast(mNOCCert.size()))); - if (mOperationalKey != nullptr) { - SuccessOrExit(err = mOperationalKey->Serialize(info->mOperationalKey)); - } - else - { - P256Keypair keypair; - SuccessOrExit(err = keypair.Initialize()); - SuccessOrExit(err = keypair.Serialize(info->mOperationalKey)); - } + Crypto::P256SerializedKeypair serializedOpKey; + if (mOperationalKey != nullptr) + { + ReturnErrorOnFailure(mOperationalKey->Serialize(serializedOpKey)); + } + else + { + // Could we just not store it instead? What would deserialize need + // to do then? + P256Keypair keypair; + ReturnErrorOnFailure(keypair.Initialize()); + ReturnErrorOnFailure(keypair.Serialize(serializedOpKey)); + } - if (mRootCert.empty()) - { - info->mRootCertLen = 0; - } - else - { - VerifyOrExit(CanCastTo(mRootCert.size()), err = CHIP_ERROR_INVALID_ARGUMENT); - info->mRootCertLen = Encoding::LittleEndian::HostSwap16(static_cast(mRootCert.size())); - memcpy(info->mRootCert, mRootCert.data(), mRootCert.size()); - } + uint8_t buf[OpKeyTLVMaxSize()]; + TLV::TLVWriter writer; + writer.Init(buf); - if (mICACert.empty()) - { - info->mICACertLen = 0; - } - else - { - VerifyOrExit(CanCastTo(mICACert.size()), err = CHIP_ERROR_INVALID_ARGUMENT); - info->mICACertLen = Encoding::LittleEndian::HostSwap16(static_cast(mICACert.size())); - memcpy(info->mICACert, mICACert.data(), mICACert.size()); + TLV::TLVType outerType; + ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outerType)); + + ReturnErrorOnFailure(writer.Put(kOpKeyVersionTag, kOpKeyVersion)); + + ReturnErrorOnFailure(writer.Put(kOpKeyDataTag, ByteSpan(serializedOpKey.Bytes(), serializedOpKey.Length()))); + + ReturnErrorOnFailure(writer.EndContainer(outerType)); + + const auto opKeyLength = writer.GetLengthWritten(); + VerifyOrReturnError(CanCastTo(opKeyLength), CHIP_ERROR_BUFFER_TOO_SMALL); + ReturnErrorOnFailure(storage->SyncSetKeyValue(keyAlloc.FabricOpKey(mFabric), buf, static_cast(opKeyLength))); } - if (mNOCCert.empty()) { - info->mNOCCertLen = 0; + uint8_t buf[MetadataTLVMaxSize()]; + TLV::TLVWriter writer; + writer.Init(buf); + + TLV::TLVType outerType; + ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outerType)); + + ReturnErrorOnFailure(writer.Put(kVendorIdTag, mVendorId)); + + ReturnErrorOnFailure(writer.PutString(kFabricLabelTag, CharSpan::fromCharString(mFabricLabel))); + + ReturnErrorOnFailure(writer.EndContainer(outerType)); + + const auto metadataLength = writer.GetLengthWritten(); + VerifyOrReturnError(CanCastTo(metadataLength), CHIP_ERROR_BUFFER_TOO_SMALL); + ReturnErrorOnFailure( + storage->SyncSetKeyValue(keyAlloc.FabricMetadata(mFabric), buf, static_cast(metadataLength))); } - else + + return CHIP_NO_ERROR; +} + +CHIP_ERROR FabricInfo::LoadFromStorage(PersistentStorageDelegate * storage) +{ + DefaultStorageKeyAllocator keyAlloc; + + ChipLogProgress(Inet, "Loading from storage for fabric index %u", mFabric); + + // Scopes for "size" so we don't forget to re-initialize it between gets, + // since each get modifies it. { - VerifyOrExit(CanCastTo(mNOCCert.size()), err = CHIP_ERROR_INVALID_ARGUMENT); - info->mNOCCertLen = Encoding::LittleEndian::HostSwap16(static_cast(mNOCCert.size())); - memcpy(info->mNOCCert, mNOCCert.data(), mNOCCert.size()); + uint8_t buf[Credentials::kMaxCHIPCertLength]; + uint16_t size = sizeof(buf); + ReturnErrorOnFailure(storage->SyncGetKeyValue(keyAlloc.FabricRCAC(mFabric), buf, size)); + ReturnErrorOnFailure(SetRootCert(ByteSpan(buf, size))); } - err = storage->SyncSetKeyValue(key, info, sizeof(StorableFabricInfo)); - if (err != CHIP_NO_ERROR) { - ChipLogError(Discovery, "Error occurred calling SyncSetKeyValue: %s", chip::ErrorStr(err)); + uint8_t buf[Credentials::kMaxCHIPCertLength]; + uint16_t size = sizeof(buf); + // For now we always store an ICA cert buffer (possibly empty). If we + // stop doing that, check for + // CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND here. + ReturnErrorOnFailure(storage->SyncGetKeyValue(keyAlloc.FabricICAC(mFabric), buf, size)); + ReturnErrorOnFailure(SetICACert(ByteSpan(buf, size))); } -exit: - if (info != nullptr) { - chip::Platform::Delete(info); + uint8_t buf[Credentials::kMaxCHIPCertLength]; + uint16_t size = sizeof(buf); + ReturnErrorOnFailure(storage->SyncGetKeyValue(keyAlloc.FabricNOC(mFabric), buf, size)); + ByteSpan nocCert(buf, size); + NodeId nodeId; + ReturnErrorOnFailure(ExtractNodeIdFabricIdFromOpCert(nocCert, &nodeId, &mFabricId)); + // The compressed fabric ID doesn't change for a fabric over time. + // Computing it here will save computational overhead when it's accessed by other + // parts of the code. + ReturnErrorOnFailure(GeneratePeerId(mFabricId, nodeId, &mOperationalId)); + ReturnErrorOnFailure(SetNOCCert(nocCert)); } - return err; -} - -CHIP_ERROR FabricInfo::LoadFromStorage(PersistentStorageDelegate * storage) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - char key[kKeySize]; - ReturnErrorOnFailure(GenerateKey(mFabric, key, sizeof(key))); - - StorableFabricInfo * info = chip::Platform::New(); - ReturnErrorCodeIf(info == nullptr, CHIP_ERROR_NO_MEMORY); - uint16_t infoSize = sizeof(StorableFabricInfo); + { + uint8_t buf[OpKeyTLVMaxSize()]; + uint16_t size = sizeof(buf); + ReturnErrorOnFailure(storage->SyncGetKeyValue(keyAlloc.FabricOpKey(mFabric), buf, size)); + TLV::ContiguousBufferTLVReader reader; + reader.Init(buf, size); - FabricIndex id; - uint16_t rootCertLen, icaCertLen, nocCertLen; - size_t stringLength; - NodeId nodeId; + ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag())); + TLV::TLVType containerType; + ReturnErrorOnFailure(reader.EnterContainer(containerType)); - SuccessOrExit(err = storage->SyncGetKeyValue(key, info, infoSize)); + ReturnErrorOnFailure(reader.Next(kOpKeyVersionTag)); + uint16_t opKeyVersion; + ReturnErrorOnFailure(reader.Get(opKeyVersion)); + VerifyOrReturnError(opKeyVersion == kOpKeyVersion, CHIP_ERROR_VERSION_MISMATCH); - id = info->mFabricIndex; - mVendorId = Encoding::LittleEndian::HostSwap16(info->mVendorId); - rootCertLen = Encoding::LittleEndian::HostSwap16(info->mRootCertLen); - icaCertLen = Encoding::LittleEndian::HostSwap16(info->mICACertLen); - nocCertLen = Encoding::LittleEndian::HostSwap16(info->mNOCCertLen); + ReturnErrorOnFailure(reader.Next(kOpKeyDataTag)); + ByteSpan keyData; + ReturnErrorOnFailure(reader.GetByteView(keyData)); - stringLength = strnlen(info->mFabricLabel, kFabricLabelMaxLengthInBytes); - memcpy(mFabricLabel, info->mFabricLabel, stringLength); - mFabricLabel[stringLength] = '\0'; // Set null terminator + // Unfortunately, we have to copy the data into a P256SerializedKeypair. + Crypto::P256SerializedKeypair serializedOpKey; + VerifyOrReturnError(keyData.size() <= serializedOpKey.Capacity(), CHIP_ERROR_BUFFER_TOO_SMALL); - VerifyOrExit(mFabric == id, err = CHIP_ERROR_INCORRECT_STATE); + memcpy(serializedOpKey.Bytes(), keyData.data(), keyData.size()); + serializedOpKey.SetLength(keyData.size()); - if (mOperationalKey == nullptr) - { + if (mOperationalKey == nullptr) + { #ifdef ENABLE_HSM_CASE_OPS_KEY - mOperationalKey = chip::Platform::New(); - mOperationalKey->SetKeyId(CASE_OPS_KEY); + mOperationalKey = chip::Platform::New(); + mOperationalKey->SetKeyId(CASE_OPS_KEY); #else - mOperationalKey = chip::Platform::New(); + mOperationalKey = chip::Platform::New(); #endif - } - VerifyOrExit(mOperationalKey != nullptr, err = CHIP_ERROR_NO_MEMORY); - SuccessOrExit(err = mOperationalKey->Deserialize(info->mOperationalKey)); + } + VerifyOrReturnError(mOperationalKey != nullptr, CHIP_ERROR_NO_MEMORY); + ReturnErrorOnFailure(mOperationalKey->Deserialize(serializedOpKey)); #ifdef ENABLE_HSM_CASE_OPS_KEY - // Set provisioned_key = true , so that key is not deleted from HSM. - mOperationalKey->provisioned_key = true; + // Set provisioned_key = true , so that key is not deleted from HSM. + mOperationalKey->provisioned_key = true; #endif - ChipLogProgress(Inet, "Loading certs from storage"); - SuccessOrExit(err = SetRootCert(ByteSpan(info->mRootCert, rootCertLen))); + ReturnErrorOnFailure(reader.ExitContainer(containerType)); + ReturnErrorOnFailure(reader.VerifyEndOfContainer()); + } - // The compressed fabric ID doesn't change for a fabric over time. - // Computing it here will save computational overhead when it's accessed by other - // parts of the code. - SuccessOrExit(err = ExtractNodeIdFabricIdFromOpCert(ByteSpan(info->mNOCCert, nocCertLen), &nodeId, &mFabricId)); - SuccessOrExit(err = GeneratePeerId(mFabricId, nodeId, &mOperationalId)); + { + uint8_t buf[MetadataTLVMaxSize()]; + uint16_t size = sizeof(buf); + ReturnErrorOnFailure(storage->SyncGetKeyValue(keyAlloc.FabricMetadata(mFabric), buf, size)); + TLV::ContiguousBufferTLVReader reader; + reader.Init(buf, size); - SuccessOrExit(err = SetICACert(ByteSpan(info->mICACert, icaCertLen))); - SuccessOrExit(err = SetNOCCert(ByteSpan(info->mNOCCert, nocCertLen))); + ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag())); + TLV::TLVType containerType; + ReturnErrorOnFailure(reader.EnterContainer(containerType)); -exit: - if (info != nullptr) - { - chip::Platform::Delete(info); + ReturnErrorOnFailure(reader.Next(kVendorIdTag)); + ReturnErrorOnFailure(reader.Get(mVendorId)); + + ReturnErrorOnFailure(reader.Next(kFabricLabelTag)); + CharSpan label; + ReturnErrorOnFailure(reader.Get(label)); + + VerifyOrReturnError(label.size() <= kFabricLabelMaxLengthInBytes, CHIP_ERROR_BUFFER_TOO_SMALL); + Platform::CopyString(mFabricLabel, label); + + ReturnErrorOnFailure(reader.ExitContainer(containerType)); + ReturnErrorOnFailure(reader.VerifyEndOfContainer()); } - return err; + + return CHIP_NO_ERROR; } CHIP_ERROR FabricInfo::GeneratePeerId(FabricId fabricId, NodeId nodeId, PeerId * compressedPeerId) const @@ -212,26 +276,30 @@ CHIP_ERROR FabricInfo::GeneratePeerId(FabricId fabricId, NodeId nodeId, PeerId * CHIP_ERROR FabricInfo::DeleteFromStorage(PersistentStorageDelegate * storage, FabricIndex fabricIndex) { - CHIP_ERROR err = CHIP_NO_ERROR; + DefaultStorageKeyAllocator keyAlloc; + + // Try to delete all the state even if one of the deletes fails. + typedef const char * (DefaultStorageKeyAllocator::*KeyGetter)(FabricIndex); + constexpr KeyGetter keyGetters[] = { &DefaultStorageKeyAllocator::FabricNOC, &DefaultStorageKeyAllocator::FabricICAC, + &DefaultStorageKeyAllocator::FabricRCAC, &DefaultStorageKeyAllocator::FabricMetadata, + &DefaultStorageKeyAllocator::FabricOpKey }; - char key[kKeySize]; - ReturnErrorOnFailure(GenerateKey(fabricIndex, key, sizeof(key))); + CHIP_ERROR prevDeleteErr = CHIP_NO_ERROR; - err = storage->SyncDeleteKeyValue(key); - if (err != CHIP_NO_ERROR) + for (auto & keyGetter : keyGetters) { - ChipLogDetail(Discovery, "Fabric %d is not yet configured", fabricIndex); + CHIP_ERROR deleteErr = storage->SyncDeleteKeyValue((keyAlloc.*keyGetter)(fabricIndex)); + // Keys not existing is not really an error condition. + if (prevDeleteErr == CHIP_NO_ERROR && deleteErr != CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + prevDeleteErr = deleteErr; + } } - return err; -} - -CHIP_ERROR FabricInfo::GenerateKey(FabricIndex id, char * key, size_t len) -{ - VerifyOrReturnError(len >= kKeySize, CHIP_ERROR_INVALID_ARGUMENT); - int keySize = snprintf(key, len, "%s%x", kFabricTableKeyPrefix, id); - VerifyOrReturnError(keySize > 0, CHIP_ERROR_INTERNAL); - VerifyOrReturnError(len > (size_t) keySize, CHIP_ERROR_INTERNAL); - return CHIP_NO_ERROR; + if (prevDeleteErr != CHIP_NO_ERROR) + { + ChipLogDetail(Discovery, "Error deleting part of fabric %d: %" CHIP_ERROR_FORMAT, fabricIndex, prevDeleteErr.Format()); + } + return prevDeleteErr; } CHIP_ERROR FabricInfo::SetOperationalKeypair(const P256Keypair * keyPair) diff --git a/src/credentials/FabricTable.h b/src/credentials/FabricTable.h index 8a9ea3b4d3b813..3939ead901ed56 100644 --- a/src/credentials/FabricTable.h +++ b/src/credentials/FabricTable.h @@ -32,6 +32,7 @@ #endif #include #include +#include #include #include #include @@ -49,11 +50,6 @@ static constexpr uint8_t kFabricLabelMaxLengthInBytes = 32; static_assert(kUndefinedFabricIndex < chip::kMinValidFabricIndex, "Undefined fabric index should not be valid"); -// KVS store is sensitive to length of key strings, based on the underlying -// platform. Keeping them short. -constexpr char kFabricTableKeyPrefix[] = "Fabric"; -constexpr char kFabricTableCountKey[] = "NumFabrics"; - /** * Defines state of a pairing established by a fabric. * Node ID is only settable using the device operational credentials. @@ -213,6 +209,16 @@ class DLL_EXPORT FabricInfo friend class FabricTable; private: + static constexpr size_t MetadataTLVMaxSize() + { + return TLV::EstimateStructOverhead(sizeof(VendorId), kFabricLabelMaxLengthInBytes); + } + + static constexpr size_t OpKeyTLVMaxSize() + { + return TLV::EstimateStructOverhead(sizeof(uint16_t), Crypto::P256SerializedKeypair::Capacity()); + } + PeerId mOperationalId; FabricIndex mFabric = kUndefinedFabricIndex; @@ -231,10 +237,6 @@ class DLL_EXPORT FabricInfo FabricId mFabricId = 0; - static constexpr size_t kKeySize = sizeof(kFabricTableKeyPrefix) + 2 * sizeof(FabricIndex); - - static CHIP_ERROR GenerateKey(FabricIndex id, char * key, size_t len); - CHIP_ERROR CommitToStorage(PersistentStorageDelegate * storage); CHIP_ERROR LoadFromStorage(PersistentStorageDelegate * storage); static CHIP_ERROR DeleteFromStorage(PersistentStorageDelegate * storage, FabricIndex fabricIndex); diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h index 865fb26aecbcab..b310f6451be665 100644 --- a/src/lib/support/DefaultStorageKeyAllocator.h +++ b/src/lib/support/DefaultStorageKeyAllocator.h @@ -34,8 +34,11 @@ class DefaultStorageKeyAllocator const char * KeyName() { return mKeyName; } // Fabric Table - - const char * FabricTable(chip::FabricIndex fabric) { return Format("f/%x/t", fabric); } + const char * FabricNOC(FabricIndex fabric) { return Format("f/%x/n", fabric); } + const char * FabricICAC(FabricIndex fabric) { return Format("f/%x/i", fabric); } + const char * FabricRCAC(FabricIndex fabric) { return Format("f/%x/r", fabric); } + const char * FabricMetadata(FabricIndex fabric) { return Format("f/%x/m", fabric); } + const char * FabricOpKey(FabricIndex fabric) { return Format("f/%x/o", fabric); } // Access Control List