Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ICD] Improve ICDClientStorage #36036

Merged
merged 13 commits into from
Oct 12, 2024
Merged

[ICD] Improve ICDClientStorage #36036

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5d153dc
Improve ICDClientStorage
yunhanw-google Oct 11, 2024
d2a3f09
Update DefaultICDClientStorage.cpp
yunhanw-google Oct 11, 2024
d6ee3df
Update DefaultICDClientStorage.cpp
yunhanw-google Oct 11, 2024
35e29e7
Update DefaultICDClientStorage.cpp
yunhanw-google Oct 11, 2024
daeb4a1
Update DefaultICDClientStorage.cpp
yunhanw-google Oct 11, 2024
ccc22b8
Update DefaultICDClientStorage.cpp
yunhanw-google Oct 11, 2024
b15fd63
address comments
yunhanw-google Oct 11, 2024
acbecaa
Restyled by whitespace
restyled-commits Oct 11, 2024
685ba45
Restyled by clang-format
restyled-commits Oct 11, 2024
512f5c8
address comments
yunhanw-google Oct 11, 2024
51fa4b6
address comments and add more tests
yunhanw-google Oct 11, 2024
2fd1c29
Restyled by whitespace
restyled-commits Oct 12, 2024
eb3e1f6
Restyled by clang-format
restyled-commits Oct 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 56 additions & 7 deletions src/app/icd/client/DefaultICDClientStorage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ CHIP_ERROR DefaultICDClientStorage::UpdateFabricList(FabricIndex fabricIndex)

mFabricList.push_back(fabricIndex);

return StoreFabricList();
}

CHIP_ERROR DefaultICDClientStorage::StoreFabricList()
{
Platform::ScopedMemoryBuffer<uint8_t> backingBuffer;
size_t counter = mFabricList.size();
size_t total = kFabricIndexTlvSize * counter + kArrayOverHead;
Expand All @@ -68,7 +73,7 @@ CHIP_ERROR DefaultICDClientStorage::UpdateFabricList(FabricIndex fabricIndex)
const auto len = writer.GetLengthWritten();
VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_BUFFER_TOO_SMALL);

writer.Finalize(backingBuffer);
ReturnErrorOnFailure(writer.Finalize(backingBuffer));
return mpClientInfoStore->SyncSetKeyValue(DefaultStorageKeyAllocator::ICDFabricList().KeyName(), backingBuffer.Get(),
static_cast<uint16_t>(len));
}
Expand Down Expand Up @@ -211,6 +216,10 @@ CHIP_ERROR DefaultICDClientStorage::Load(FabricIndex fabricIndex, std::vector<IC
{
size_t count = 0;
ReturnErrorOnFailure(LoadCounter(fabricIndex, count, clientInfoSize));
if (count == 0)
{
return CHIP_NO_ERROR;
}
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
size_t len = clientInfoSize * count + kArrayOverHead;
Platform::ScopedMemoryBuffer<uint8_t> backingBuffer;
VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_BUFFER_TOO_SMALL);
Expand Down Expand Up @@ -344,8 +353,21 @@ CHIP_ERROR DefaultICDClientStorage::SerializeToTlv(TLV::TLVWriter & writer, cons
return writer.EndContainer(arrayType);
}

CHIP_ERROR DefaultICDClientStorage::CheckFabricExistence(FabricIndex fabricIndex)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
{
for (auto & fabric_idx : mFabricList)
{
if (fabric_idx == fabricIndex)
{
return CHIP_NO_ERROR;
}
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
}
return CHIP_ERROR_INVALID_FABRIC_INDEX;
}

CHIP_ERROR DefaultICDClientStorage::StoreEntry(const ICDClientInfo & clientInfo)
{
ReturnErrorOnFailure(CheckFabricExistence(clientInfo.peer_node.GetFabricIndex()));
std::vector<ICDClientInfo> clientInfoVector;
size_t clientInfoSize = MaxICDClientInfoSize();
ReturnErrorOnFailure(Load(clientInfo.peer_node.GetFabricIndex(), clientInfoVector, clientInfoSize));
Expand All @@ -359,7 +381,6 @@ CHIP_ERROR DefaultICDClientStorage::StoreEntry(const ICDClientInfo & clientInfo)
break;
}
}

clientInfoVector.push_back(clientInfo);
size_t total = clientInfoSize * clientInfoVector.size() + kArrayOverHead;
Platform::ScopedMemoryBuffer<uint8_t> backingBuffer;
Expand All @@ -371,7 +392,7 @@ CHIP_ERROR DefaultICDClientStorage::StoreEntry(const ICDClientInfo & clientInfo)
const auto len = writer.GetLengthWritten();
VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_BUFFER_TOO_SMALL);

writer.Finalize(backingBuffer);
ReturnErrorOnFailure(writer.Finalize(backingBuffer));
ReturnErrorOnFailure(mpClientInfoStore->SyncSetKeyValue(
DefaultStorageKeyAllocator::ICDClientInfoKey(clientInfo.peer_node.GetFabricIndex()).KeyName(), backingBuffer.Get(),
static_cast<uint16_t>(len)));
Expand Down Expand Up @@ -416,17 +437,26 @@ CHIP_ERROR DefaultICDClientStorage::UpdateEntryCountForFabric(FabricIndex fabric

const auto len = writer.GetLengthWritten();
VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_BUFFER_TOO_SMALL);
writer.Finalize(backingBuffer);
ReturnErrorOnFailure(writer.Finalize(backingBuffer));

return mpClientInfoStore->SyncSetKeyValue(DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricIndex).KeyName(),
backingBuffer.Get(), static_cast<uint16_t>(len));
}

CHIP_ERROR DefaultICDClientStorage::DeleteEntry(const ScopedNodeId & peerNode)
{
if (CheckFabricExistence(peerNode.GetFabricIndex()) != CHIP_NO_ERROR)
{
return CHIP_NO_ERROR;
}
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

size_t clientInfoSize = 0;
std::vector<ICDClientInfo> clientInfoVector;
ReturnErrorOnFailure(Load(peerNode.GetFabricIndex(), clientInfoVector, clientInfoSize));
if (clientInfoVector.size() == 0)
{
return CHIP_NO_ERROR;
}
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

for (auto it = clientInfoVector.begin(); it != clientInfoVector.end(); it++)
{
Expand All @@ -440,7 +470,6 @@ CHIP_ERROR DefaultICDClientStorage::DeleteEntry(const ScopedNodeId & peerNode)

ReturnErrorOnFailure(
mpClientInfoStore->SyncDeleteKeyValue(DefaultStorageKeyAllocator::ICDClientInfoKey(peerNode.GetFabricIndex()).KeyName()));

size_t total = clientInfoSize * clientInfoVector.size() + kArrayOverHead;
Platform::ScopedMemoryBuffer<uint8_t> backingBuffer;
ReturnErrorCodeIf(!backingBuffer.Calloc(total), CHIP_ERROR_NO_MEMORY);
Expand All @@ -451,7 +480,7 @@ CHIP_ERROR DefaultICDClientStorage::DeleteEntry(const ScopedNodeId & peerNode)
const auto len = writer.GetLengthWritten();
VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_BUFFER_TOO_SMALL);

writer.Finalize(backingBuffer);
ReturnErrorOnFailure(writer.Finalize(backingBuffer));
ReturnErrorOnFailure(
mpClientInfoStore->SyncSetKeyValue(DefaultStorageKeyAllocator::ICDClientInfoKey(peerNode.GetFabricIndex()).KeyName(),
backingBuffer.Get(), static_cast<uint16_t>(len)));
Expand All @@ -461,6 +490,11 @@ CHIP_ERROR DefaultICDClientStorage::DeleteEntry(const ScopedNodeId & peerNode)

CHIP_ERROR DefaultICDClientStorage::DeleteAllEntries(FabricIndex fabricIndex)
{
if (CheckFabricExistence(fabricIndex) != CHIP_NO_ERROR)
{
return CHIP_NO_ERROR;
}
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

size_t clientInfoSize = 0;
std::vector<ICDClientInfo> clientInfoVector;
ReturnErrorOnFailure(Load(fabricIndex, clientInfoVector, clientInfoSize));
Expand All @@ -471,7 +505,22 @@ CHIP_ERROR DefaultICDClientStorage::DeleteAllEntries(FabricIndex fabricIndex)
}
ReturnErrorOnFailure(
mpClientInfoStore->SyncDeleteKeyValue(DefaultStorageKeyAllocator::ICDClientInfoKey(fabricIndex).KeyName()));
return mpClientInfoStore->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricIndex).KeyName());
ReturnErrorOnFailure(mpClientInfoStore->SyncDeleteKeyValue(DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricIndex).KeyName()));

for (auto fabric = mFabricList.begin(); fabric != mFabricList.end(); fabric++)
{
if (*fabric == fabricIndex)
{
fabric = mFabricList.erase(fabric);
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

if (mFabricList.size() == 0)
{
return mpClientInfoStore->SyncDeleteKeyValue(DefaultStorageKeyAllocator::ICDFabricList().KeyName());
}
return StoreFabricList();
}

CHIP_ERROR DefaultICDClientStorage::ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Expand Down
15 changes: 15 additions & 0 deletions src/app/icd/client/DefaultICDClientStorage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@ class DefaultICDClientStorage : public ICDClientStorage
CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Protocols::SecureChannel::CounterType & counter) override;

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
size_t GetFabricListSize()
{
return mFabricList.size();
}

PersistentStorageDelegate * GetClientInfoStore()
{
return mpClientInfoStore;
}
#endif // CONFIG_BUILD_FOR_HOST_UNIT_TEST

protected:
enum class ClientInfoTag : uint8_t
{
Expand Down Expand Up @@ -173,9 +185,12 @@ class DefaultICDClientStorage : public ICDClientStorage

private:
friend class ICDClientInfoIteratorImpl;
CHIP_ERROR StoreFabricList();
CHIP_ERROR LoadFabricList();
CHIP_ERROR LoadCounter(FabricIndex fabricIndex, size_t & count, size_t & clientInfoSize);

CHIP_ERROR CheckFabricExistence(FabricIndex fabricIndex);

CHIP_ERROR IncreaseEntryCountForFabric(FabricIndex fabricIndex);
CHIP_ERROR DecreaseEntryCountForFabric(FabricIndex fabricIndex);
CHIP_ERROR UpdateEntryCountForFabric(FabricIndex fabricIndex, bool increase);
Expand Down
158 changes: 158 additions & 0 deletions src/app/tests/TestDefaultICDClientStorage.cpp
View file
Open in desktop
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ TEST_F(TestDefaultICDClientStorage, TestClientInfoCount)

iterator->Release();

EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDFabricList().KeyName()));
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(
DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricId).KeyName()));
EXPECT_TRUE(
manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDClientInfoKey(fabricId).KeyName()));

// Delete all and verify iterator counts 0
EXPECT_EQ(manager.DeleteAllEntries(fabricId), CHIP_NO_ERROR);
iterator = manager.IterateICDClientInfo();
Expand All @@ -116,6 +122,14 @@ TEST_F(TestDefaultICDClientStorage, TestClientInfoCount)
}
iterator->Release();
EXPECT_EQ(count, 0u);
EXPECT_EQ(manager.GetFabricListSize(), 0u);
EXPECT_FALSE(manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDFabricList().KeyName()));
EXPECT_FALSE(manager.GetClientInfoStore()->SyncDoesKeyExist(
DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricId).KeyName()));
EXPECT_FALSE(
manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDClientInfoKey(fabricId).KeyName()));

EXPECT_EQ(manager.DeleteAllEntries(fabricId), CHIP_NO_ERROR);
}

{
Expand Down Expand Up @@ -174,6 +188,15 @@ TEST_F(TestDefaultICDClientStorage, TestClientInfoCountMultipleFabric)
EXPECT_EQ(manager.DeleteEntry(ScopedNodeId(nodeId3, fabricId2)), CHIP_NO_ERROR);
EXPECT_EQ(iterator->Count(), 0u);

EXPECT_EQ(manager.GetFabricListSize(), 2u);
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDFabricList().KeyName()));
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(
DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricId1).KeyName()));
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDClientInfoKey(fabricId1).KeyName()));
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(
DefaultStorageKeyAllocator::FabricICDClientInfoCounter(fabricId2).KeyName()));
EXPECT_TRUE(manager.GetClientInfoStore()->SyncDoesKeyExist(DefaultStorageKeyAllocator::ICDClientInfoKey(fabricId2).KeyName()));

// Verify ClientInfos manually count correctly
size_t count = 0;
ICDClientInfo clientInfo;
Expand All @@ -183,6 +206,59 @@ TEST_F(TestDefaultICDClientStorage, TestClientInfoCountMultipleFabric)
}

EXPECT_FALSE(count);

EXPECT_EQ(manager.DeleteEntry(ScopedNodeId(nodeId3, fabricId2)), CHIP_NO_ERROR);
}

TEST_F(TestDefaultICDClientStorage, TestClientInfoCountMultipleFabricWithRemovingFabric)
{

FabricIndex fabricId1 = 1;
FabricIndex fabricId2 = 2;
NodeId nodeId1 = 6666;
NodeId nodeId2 = 6667;
NodeId nodeId3 = 6668;
DefaultICDClientStorage manager;
TestPersistentStorageDelegate clientInfoStorage;
TestSessionKeystoreImpl keystore;
EXPECT_EQ(manager.Init(&clientInfoStorage, &keystore), CHIP_NO_ERROR);
EXPECT_EQ(manager.UpdateFabricList(fabricId1), CHIP_NO_ERROR);
EXPECT_EQ(manager.UpdateFabricList(fabricId2), CHIP_NO_ERROR);

// Write some ClientInfos and see the counts are correct
ICDClientInfo clientInfo1;
clientInfo1.peer_node = ScopedNodeId(nodeId1, fabricId1);
ICDClientInfo clientInfo2;
clientInfo2.peer_node = ScopedNodeId(nodeId2, fabricId1);
ICDClientInfo clientInfo3;
clientInfo3.peer_node = ScopedNodeId(nodeId3, fabricId2);

EXPECT_EQ(manager.SetKey(clientInfo1, ByteSpan(kKeyBuffer1)), CHIP_NO_ERROR);
EXPECT_EQ(manager.StoreEntry(clientInfo1), CHIP_NO_ERROR);

EXPECT_EQ(manager.SetKey(clientInfo2, ByteSpan(kKeyBuffer2)), CHIP_NO_ERROR);
EXPECT_EQ(manager.StoreEntry(clientInfo2), CHIP_NO_ERROR);

EXPECT_EQ(manager.SetKey(clientInfo3, ByteSpan(kKeyBuffer3)), CHIP_NO_ERROR);
EXPECT_EQ(manager.StoreEntry(clientInfo3), CHIP_NO_ERROR);
// Make sure iterator counts correctly
auto * iterator = manager.IterateICDClientInfo();
EXPECT_EQ(iterator->Count(), 3u);
iterator->Release();

EXPECT_EQ(manager.DeleteAllEntries(fabricId1), CHIP_NO_ERROR);

iterator = manager.IterateICDClientInfo();
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved
ASSERT_NE(iterator, nullptr);
DefaultICDClientStorage::ICDClientInfoIteratorWrapper clientInfoIteratorWrapper(iterator);
EXPECT_EQ(iterator->Count(), 1u);

EXPECT_EQ(manager.DeleteAllEntries(fabricId2), CHIP_NO_ERROR);
EXPECT_EQ(iterator->Count(), 0u);

EXPECT_EQ(manager.StoreEntry(clientInfo1), CHIP_ERROR_INVALID_FABRIC_INDEX);
EXPECT_EQ(manager.StoreEntry(clientInfo2), CHIP_ERROR_INVALID_FABRIC_INDEX);
EXPECT_EQ(manager.StoreEntry(clientInfo3), CHIP_ERROR_INVALID_FABRIC_INDEX);
}

TEST_F(TestDefaultICDClientStorage, TestProcessCheckInPayload)
Expand Down Expand Up @@ -225,3 +301,85 @@ TEST_F(TestDefaultICDClientStorage, TestProcessCheckInPayload)
ByteSpan payload1{ buffer->Start(), buffer->DataLength() };
EXPECT_EQ(manager.ProcessCheckInPayload(payload1, decodeClientInfo, checkInCounter), CHIP_ERROR_NOT_FOUND);
}

TEST_F(TestDefaultICDClientStorage, TestProcessCheckInPayloadWithRemovedKey)
{
FabricIndex fabricId = 1;
NodeId nodeId = 6666;
TestPersistentStorageDelegate clientInfoStorage;
TestSessionKeystoreImpl keystore;

DefaultICDClientStorage manager;
EXPECT_EQ(manager.Init(&clientInfoStorage, &keystore), CHIP_NO_ERROR);
EXPECT_EQ(manager.UpdateFabricList(fabricId), CHIP_NO_ERROR);
// Populate clientInfo
ICDClientInfo clientInfo;
clientInfo.peer_node = ScopedNodeId(nodeId, fabricId);

EXPECT_EQ(manager.SetKey(clientInfo, ByteSpan(kKeyBuffer1)), CHIP_NO_ERROR);
EXPECT_EQ(manager.StoreEntry(clientInfo), CHIP_NO_ERROR);

uint32_t counter = 1;
System::PacketBufferHandle buffer = MessagePacketBuffer::New(chip::Protocols::SecureChannel::CheckinMessage::kMinPayloadSize);
MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() };
EXPECT_EQ(chip::Protocols::SecureChannel::CheckinMessage::GenerateCheckinMessagePayload(
clientInfo.aes_key_handle, clientInfo.hmac_key_handle, counter, ByteSpan(), output),
CHIP_NO_ERROR);

buffer->SetDataLength(static_cast<uint16_t>(output.size()));
ICDClientInfo decodeClientInfo;
uint32_t checkInCounter = 0;
ByteSpan payload{ buffer->Start(), buffer->DataLength() };
EXPECT_EQ(manager.ProcessCheckInPayload(payload, decodeClientInfo, checkInCounter), CHIP_NO_ERROR);
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

// Use a removed key in the storage for encoding
manager.RemoveKey(clientInfo), CHIP_NO_ERROR;
EXPECT_EQ(chip::Protocols::SecureChannel::CheckinMessage::GenerateCheckinMessagePayload(
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved
clientInfo.aes_key_handle, clientInfo.hmac_key_handle, counter, ByteSpan(), output),
CHIP_NO_ERROR);

buffer->SetDataLength(static_cast<uint16_t>(output.size()));
ByteSpan payload1{ buffer->Start(), buffer->DataLength() };
EXPECT_EQ(manager.ProcessCheckInPayload(payload1, decodeClientInfo, checkInCounter), CHIP_ERROR_NOT_FOUND);
}

TEST_F(TestDefaultICDClientStorage, TestProcessCheckInPayloadWithEmptyIcdStorage)
{
FabricIndex fabricId = 1;
NodeId nodeId = 6666;
TestPersistentStorageDelegate clientInfoStorage;
TestSessionKeystoreImpl keystore;

DefaultICDClientStorage manager;
EXPECT_EQ(manager.Init(&clientInfoStorage, &keystore), CHIP_NO_ERROR);
EXPECT_EQ(manager.UpdateFabricList(fabricId), CHIP_NO_ERROR);
// Populate clientInfo
ICDClientInfo clientInfo;
clientInfo.peer_node = ScopedNodeId(nodeId, fabricId);

EXPECT_EQ(manager.SetKey(clientInfo, ByteSpan(kKeyBuffer1)), CHIP_NO_ERROR);
EXPECT_EQ(manager.StoreEntry(clientInfo), CHIP_NO_ERROR);

uint32_t counter = 1;
System::PacketBufferHandle buffer = MessagePacketBuffer::New(chip::Protocols::SecureChannel::CheckinMessage::kMinPayloadSize);
MutableByteSpan output{ buffer->Start(), buffer->MaxDataLength() };
EXPECT_EQ(chip::Protocols::SecureChannel::CheckinMessage::GenerateCheckinMessagePayload(
clientInfo.aes_key_handle, clientInfo.hmac_key_handle, counter, ByteSpan(), output),
CHIP_NO_ERROR);

buffer->SetDataLength(static_cast<uint16_t>(output.size()));
ICDClientInfo decodeClientInfo;
uint32_t checkInCounter = 0;
ByteSpan payload{ buffer->Start(), buffer->DataLength() };
EXPECT_EQ(manager.ProcessCheckInPayload(payload, decodeClientInfo, checkInCounter), CHIP_NO_ERROR);

manager.DeleteAllEntries(fabricId);

EXPECT_EQ(chip::Protocols::SecureChannel::CheckinMessage::GenerateCheckinMessagePayload(
clientInfo.aes_key_handle, clientInfo.hmac_key_handle, counter, ByteSpan(), output),
CHIP_NO_ERROR);

buffer->SetDataLength(static_cast<uint16_t>(output.size()));
ByteSpan payload1{ buffer->Start(), buffer->DataLength() };
EXPECT_EQ(manager.ProcessCheckInPayload(payload1, decodeClientInfo, checkInCounter), CHIP_ERROR_NOT_FOUND);
}
Loading