Skip to content

Commit

Permalink
Remove GroupKeyMap entries for removed KeySet
Browse files Browse the repository at this point in the history
Per Core spec 11.2.9.4, If there exist any entries for the accessing
fabric within the GroupKeyMap attribute that refer to the
GroupKeySetID just removed, then these entries SHALL be removed from
that list.

Fixes project-chip#23862
  • Loading branch information
cecille committed Dec 1, 2022
1 parent 0493fc3 commit f3ade11
Show file tree
Hide file tree
Showing 4 changed files with 684 additions and 103 deletions.
76 changes: 76 additions & 0 deletions src/app/tests/suites/TestGroupKeyManagementCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,79 @@ tests:
value: 0x01a2
response:
error: NOT_FOUND

- label: "KeySet Write 1"
command: "KeySetWrite"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a1,
GroupKeySecurityPolicy: 0,
EpochKey0: "\xa0\xa1\xa2\xa3\xa4\xa5\xa6\xa7\xa8\xa9\xaa\xab\xac\xad\xae\xaf",
EpochStartTime0: 1110000,
EpochKey1: "\xb0\xb1\xb2\xb3\xb4\xb5\xb6\xb7\xb8\xb9\xba\xbb\xbc\xbd\xbe\xbf",
EpochStartTime1: 1110001,
EpochKey2: "\xc0\xc1\xc2\xc3\xc4\xc5\xc6\xc7\xc8\xc9\xca\xcb\xcc\xcd\xce\xcf",
EpochStartTime2: 1110002,
}

- label: "KeySet Write 2"
command: "KeySetWrite"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a2,
GroupKeySecurityPolicy: 1,
EpochKey0: "\xd0\xd1\xd2\xd3\xd4\xd5\xd6\xd7\xd8\xd9\xda\xdb\xdc\xdd\xde\xdf",
EpochStartTime0: 2110000,
EpochKey1: "\xe0\xe1\xe2\xe3\xe4\xe5\xe6\xe7\xe8\xe9\xea\xeb\xec\xed\xee\xef",
EpochStartTime1: 2110001,
EpochKey2: "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7\xf8\xf9\xfa\xfb\xfc\xfd\xfe\xff",
EpochStartTime2: 2110002,
}

- label: "Map Group 1 and Group 2 to KeySet 1 and group 2 to KeySet 2"
command: "writeAttribute"
attribute: "GroupKeyMap"
arguments:
value:
[
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a1 },
]

- label: "Remove keyset 1"
command: "KeySetRemove"
arguments:
values:
- name: "GroupKeySetID"
value: 0x01a1

- label: "TH verifies GroupKeyMap entries for KeySet 1 have been removed"
cluster: "Group Key Management"
endpoint: 0
command: "readAttribute"
attribute: "GroupKeyMap"
response:
value: [
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
]
- label: "Remove keyset 2"
command: "KeySetRemove"
arguments:
values:
- name: "GroupKeySetID"
value: 0x01a2

- label: "TH verifies GroupKeyMap entries for KeySet 2 have been removed"
cluster: "Group Key Management"
endpoint: 0
command: "readAttribute"
attribute: "GroupKeyMap"
response:
value: []
51 changes: 50 additions & 1 deletion src/credentials/GroupDataProviderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,37 @@ struct KeyMapData : public GroupDataProvider::GroupKey, LinkedData
id = static_cast<uint16_t>(max_id + 1);
return false;
}

// returns index if the find_idx is found, otherwise std::numeric_limits<size_t>::max
size_t Find(PersistentStorageDelegate * storage, const FabricData & fabric, const KeysetId find_id)
{
fabric_index = fabric.fabric_index;
id = fabric.first_map;
max_id = 0;
index = 0;
first = true;

while (index < fabric.map_count)
{
if (CHIP_NO_ERROR != Load(storage))
{
break;
}
if (keyset_id == find_id)
{
// Match found
return index;
}
max_id = std::max(id, max_id);
first = false;
prev = id;
id = next;
index++;
}

id = static_cast<uint16_t>(max_id + 1);
return std::numeric_limits<size_t>::max();
}
};

struct EndpointData : GroupDataProvider::GroupEndpoint, PersistentData<kPersistentBufferMax>
Expand Down Expand Up @@ -1566,7 +1597,25 @@ CHIP_ERROR GroupDataProviderImpl::RemoveKeySet(chip::FabricIndex fabric_index, u
fabric.keyset_count--;
}
// Update fabric info
return fabric.Save(mStorage);
ReturnErrorOnFailure(fabric.Save(mStorage));

// Removing a key set also removes the associated group mappings
KeyMapData map;
uint16_t original_count = fabric.map_count;
for (uint16_t i = 0; i < original_count; ++i)
{
fabric.Load(mStorage);
size_t idx = map.Find(mStorage, fabric, target_id);
if (idx == std::numeric_limits<size_t>::max())
{
break;
}
// NOTE: It's unclear what should happen here if we have removed the key set
// and possibly some mappings before failing. For now, ignoring errors, but
// open to suggestsions for the correct behavior.
RemoveGroupKeyAt(fabric_index, idx);
}
return CHIP_NO_ERROR;
}

GroupDataProvider::KeySetIterator * GroupDataProviderImpl::IterateKeySets(chip::FabricIndex fabric_index)
Expand Down
Loading

0 comments on commit f3ade11

Please sign in to comment.