From e27aa3200b452214490e98b2a7fbee70f82cca5a Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 11 Mar 2022 16:23:25 -0500 Subject: [PATCH] Don't store ICAC when we don't have one. (#16132) The ESP32 storage backend fails out on null-with-0-length being stored, so we are failing to commission ESP32 devices. CI did not catch this because our Linux/Darwin backends allow null-with-0-length. --- src/credentials/FabricTable.cpp | 37 +++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/credentials/FabricTable.cpp b/src/credentials/FabricTable.cpp index 0735f6579b1e97..d93df4b9b103ea 100644 --- a/src/credentials/FabricTable.cpp +++ b/src/credentials/FabricTable.cpp @@ -68,10 +68,23 @@ CHIP_ERROR FabricInfo::CommitToStorage(PersistentStorageDelegate * storage) ReturnErrorOnFailure( storage->SyncSetKeyValue(keyAlloc.FabricRCAC(mFabric), mRootCert.data(), static_cast(mRootCert.size()))); - // 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()))); + // Workaround for the fact that some storage backends do not allow storing + // a nullptr with 0 length. See + // https://github.com/project-chip/connectedhomeip/issues/16030. + if (!mICACert.empty()) + { + ReturnErrorOnFailure( + storage->SyncSetKeyValue(keyAlloc.FabricICAC(mFabric), mICACert.data(), static_cast(mICACert.size()))); + } + else + { + // Make sure there is no stale data. + CHIP_ERROR err = storage->SyncDeleteKeyValue(keyAlloc.FabricICAC(mFabric)); + if (err != CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + ReturnErrorOnFailure(err); + } + } ReturnErrorOnFailure( storage->SyncSetKeyValue(keyAlloc.FabricNOC(mFabric), mNOCCert.data(), static_cast(mNOCCert.size()))); @@ -149,11 +162,17 @@ CHIP_ERROR FabricInfo::LoadFromStorage(PersistentStorageDelegate * storage) { 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)); + uint16_t size = sizeof(buf); + CHIP_ERROR err = storage->SyncGetKeyValue(keyAlloc.FabricICAC(mFabric), buf, size); + if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + // That's OK; that just means no ICAC. + size = 0; + } + else + { + ReturnErrorOnFailure(err); + } ReturnErrorOnFailure(SetICACert(ByteSpan(buf, size))); }