Skip to content

Commit

Permalink
Fix non-fabric-filtered reads of Group Key Management attributes. (#2…
Browse files Browse the repository at this point in the history
…5319)

We were only returning the entries for the accessing fabric, not for
all fabrics.

Fixes #23322
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Sep 7, 2023
1 parent e60912a commit 1286350
Show file tree
Hide file tree
Showing 4 changed files with 1,713 additions and 266 deletions.
56 changes: 31 additions & 25 deletions src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,25 +155,28 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
}
CHIP_ERROR ReadGroupKeyMap(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
auto fabric_index = aEncoder.AccessingFabricIndex();
auto provider = GetGroupDataProvider();
auto provider = GetGroupDataProvider();
VerifyOrReturnError(nullptr != provider, CHIP_ERROR_INTERNAL);

CHIP_ERROR err = aEncoder.EncodeList([provider, fabric_index](const auto & encoder) -> CHIP_ERROR {
auto iter = provider->IterateGroupKeys(fabric_index);
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);

GroupDataProvider::GroupKey mapping;
while (iter->Next(mapping))
CHIP_ERROR err = aEncoder.EncodeList([provider](const auto & encoder) -> CHIP_ERROR {
for (auto & fabric : Server::GetInstance().GetFabricTable())
{
GroupKeyManagement::Structs::GroupKeyMapStruct::Type key = {
.groupId = mapping.group_id,
.groupKeySetID = mapping.keyset_id,
.fabricIndex = fabric_index,
};
encoder.Encode(key);
auto fabric_index = fabric.GetFabricIndex();
auto iter = provider->IterateGroupKeys(fabric_index);
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);

GroupDataProvider::GroupKey mapping;
while (iter->Next(mapping))
{
GroupKeyManagement::Structs::GroupKeyMapStruct::Type key = {
.groupId = mapping.group_id,
.groupKeySetID = mapping.keyset_id,
.fabricIndex = fabric_index,
};
encoder.Encode(key);
}
iter->Release();
}
iter->Release();
return CHIP_NO_ERROR;
});
return err;
Expand Down Expand Up @@ -240,20 +243,23 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface

CHIP_ERROR ReadGroupTable(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
auto fabric_index = aEncoder.AccessingFabricIndex();
auto provider = GetGroupDataProvider();
auto provider = GetGroupDataProvider();
VerifyOrReturnError(nullptr != provider, CHIP_ERROR_INTERNAL);

CHIP_ERROR err = aEncoder.EncodeList([provider, fabric_index](const auto & encoder) -> CHIP_ERROR {
auto iter = provider->IterateGroupInfo(fabric_index);
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);

GroupDataProvider::GroupInfo info;
while (iter->Next(info))
CHIP_ERROR err = aEncoder.EncodeList([provider](const auto & encoder) -> CHIP_ERROR {
for (auto & fabric : Server::GetInstance().GetFabricTable())
{
encoder.Encode(GroupTableCodec(provider, fabric_index, info));
auto fabric_index = fabric.GetFabricIndex();
auto iter = provider->IterateGroupInfo(fabric_index);
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);

GroupDataProvider::GroupInfo info;
while (iter->Next(info))
{
encoder.Encode(GroupTableCodec(provider, fabric_index, info));
}
iter->Release();
}
iter->Release();
return CHIP_NO_ERROR;
});
return err;
Expand Down
233 changes: 229 additions & 4 deletions src/app/tests/suites/TestGroupKeyManagementCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,41 @@ config:
nodeId: 0x12344321
cluster: "Group Key Management"
endpoint: 0
payload:
type: char_string
defaultValue: "MT:-24J0AFN00KA0648G00" # This value needs to be generated automatically

tests:
- label: "Wait for the commissioned device to be retrieved"
- label: "Wait for the commissioned device to be retrieved for alpha"
cluster: "DelayCommands"
command: "WaitForCommissionee"
arguments:
values:
- name: "nodeId"
value: nodeId

- label: "Open Commissioning Window from alpha"
cluster: "Administrator Commissioning"
command: "OpenBasicCommissioningWindow"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "CommissioningTimeout"
value: 180

- label: "Commission from beta"
identity: "beta"
cluster: "CommissionerCommands"
command: "PairWithCode"
arguments:
values:
- name: "nodeId"
value: nodeId
- name: "payload"
value: payload

- label: "Wait for the commissioned device to be retrieved for beta"
identity: "beta"
cluster: "DelayCommands"
command: "WaitForCommissionee"
arguments:
Expand Down Expand Up @@ -76,6 +108,26 @@ tests:
EpochStartTime2: 2110002,
}

- label: "KeySet Write 3"
identity: "beta"
command: "KeySetWrite"
arguments:
values:
- name: "GroupKeySet"
value:
{
GroupKeySetID: 0x01a3,
GroupKeySecurityPolicy: 1,
EpochKey0:
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
EpochStartTime0: 2110000,
EpochKey1: "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
EpochStartTime1: 2110001,
EpochKey2:
"\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f",
EpochStartTime2: 2110002,
}

- label: "KeySet Read"
command: "KeySetRead"
arguments:
Expand Down Expand Up @@ -129,10 +181,35 @@ tests:
response:
error: FAILURE

- label: "Write Group Keys"
- label: "Write Group Keys on alpha"
command: "writeAttribute"
attribute: "GroupKeyMap"
arguments:
value:
[
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
]

- label: "Write Group Keys on beta"
identity: "beta"
command: "writeAttribute"
attribute: "GroupKeyMap"
arguments:
value: [
# Note: the FabricIndex here does not matter; it's not sent on the wire.
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
{ FabricIndex: 1, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
]

- label: "Read Group Keys on alpha"
command: "readAttribute"
attribute: "GroupKeyMap"
response:
value:
[
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
Expand All @@ -141,16 +218,52 @@ tests:
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
]

- label: "Read Group Keys"
- label: "Read Group Keys on alpha without fabric filtering"
command: "readAttribute"
attribute: "GroupKeyMap"
fabricFiltered: false
response:
value:
[
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
]

- label: "Read Group Keys on beta"
identity: "beta"
command: "readAttribute"
attribute: "GroupKeyMap"
response:
value:
[
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
]

- label: "Read Group Keys on beta without fabric filtering"
identity: "beta"
command: "readAttribute"
attribute: "GroupKeyMap"
fabricFiltered: false
response:
value:
[
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
]

- label: "Add Group 1"
Expand Down Expand Up @@ -221,9 +334,115 @@ tests:
- name: "GroupID"
value: 0x0104

- label: "Read GroupTable"
- label: "Add Group 5"
identity: "beta"
cluster: "Groups"
endpoint: 1
command: "AddGroup"
arguments:
values:
- name: "GroupID"
value: 0x0105
- name: "GroupName"
value: "Group #5"
response:
values:
- name: "Status"
value: 0
- name: "GroupID"
value: 0x0105

- label: "Read GroupTable from alpha"
command: "readAttribute"
attribute: "GroupTable"
response:
value:
[
{
FabricIndex: 1,
GroupId: 0x0101,
Endpoints: [1],
GroupName: "Group #1",
},
{
FabricIndex: 1,
GroupId: 0x0102,
Endpoints: [1],
GroupName: "Group #2",
},
{
FabricIndex: 1,
GroupId: 0x0103,
Endpoints: [1],
GroupName: "Group #3",
},
{
FabricIndex: 1,
GroupId: 0x0104,
Endpoints: [1],
GroupName: "Group #4",
},
]

- label: "Read GroupTable from alpha without fabric filtering"
command: "readAttribute"
attribute: "GroupTable"
fabricFiltered: false
response:
value:
[
{
FabricIndex: 1,
GroupId: 0x0101,
Endpoints: [1],
GroupName: "Group #1",
},
{
FabricIndex: 1,
GroupId: 0x0102,
Endpoints: [1],
GroupName: "Group #2",
},
{
FabricIndex: 1,
GroupId: 0x0103,
Endpoints: [1],
GroupName: "Group #3",
},
{
FabricIndex: 1,
GroupId: 0x0104,
Endpoints: [1],
GroupName: "Group #4",
},
{
FabricIndex: 2,
GroupId: 0x0105,
Endpoints: [1],
GroupName: "Group #5",
},
]

- label: "Read GroupTable from beta"
identity: "beta"
command: "readAttribute"
attribute: "GroupTable"
response:
value:
[
{
FabricIndex: 2,
GroupId: 0x0105,
Endpoints: [1],
GroupName: "Group #5",
},
]

- label: "Read GroupTable from beta without fabric filtering"
identity: "beta"
command: "readAttribute"
attribute: "GroupTable"
fabricFiltered: false
response:
value:
[
Expand Down Expand Up @@ -251,6 +470,12 @@ tests:
Endpoints: [1],
GroupName: "Group #4",
},
{
FabricIndex: 2,
GroupId: 0x0105,
Endpoints: [1],
GroupName: "Group #5",
},
]

- label: "KeySet Remove 1"
Expand Down
Loading

0 comments on commit 1286350

Please sign in to comment.