Skip to content

Commit a66e9fc

Browse files
bzbarsky-applebzbarsky-apple
authored
Fix non-fabric-filtered reads of Group Key Management attributes. (#25319)
We were only returning the entries for the accessing fabric, not for all fabrics. Fixes #23322
1 parent 2ab8904 commit a66e9fc

File tree

4 files changed

+1713
-266
lines changed

4 files changed

+1713
-266
lines changed

‎src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp

+31-25
Original file line numberDiff line numberDiff line change
@@ -155,25 +155,28 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
155155
}
156156
CHIP_ERROR ReadGroupKeyMap(EndpointId endpoint, AttributeValueEncoder & aEncoder)
157157
{
158-
auto fabric_index = aEncoder.AccessingFabricIndex();
159-
auto provider = GetGroupDataProvider();
158+
auto provider = GetGroupDataProvider();
160159
VerifyOrReturnError(nullptr != provider, CHIP_ERROR_INTERNAL);
161160

162-
CHIP_ERROR err = aEncoder.EncodeList([provider, fabric_index](const auto & encoder) -> CHIP_ERROR {
163-
auto iter = provider->IterateGroupKeys(fabric_index);
164-
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);
165-
166-
GroupDataProvider::GroupKey mapping;
167-
while (iter->Next(mapping))
161+
CHIP_ERROR err = aEncoder.EncodeList([provider](const auto & encoder) -> CHIP_ERROR {
162+
for (auto & fabric : Server::GetInstance().GetFabricTable())
168163
{
169-
GroupKeyManagement::Structs::GroupKeyMapStruct::Type key = {
170-
.groupId = mapping.group_id,
171-
.groupKeySetID = mapping.keyset_id,
172-
.fabricIndex = fabric_index,
173-
};
174-
encoder.Encode(key);
164+
auto fabric_index = fabric.GetFabricIndex();
165+
auto iter = provider->IterateGroupKeys(fabric_index);
166+
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);
167+
168+
GroupDataProvider::GroupKey mapping;
169+
while (iter->Next(mapping))
170+
{
171+
GroupKeyManagement::Structs::GroupKeyMapStruct::Type key = {
172+
.groupId = mapping.group_id,
173+
.groupKeySetID = mapping.keyset_id,
174+
.fabricIndex = fabric_index,
175+
};
176+
encoder.Encode(key);
177+
}
178+
iter->Release();
175179
}
176-
iter->Release();
177180
return CHIP_NO_ERROR;
178181
});
179182
return err;
@@ -240,20 +243,23 @@ class GroupKeyManagementAttributeAccess : public AttributeAccessInterface
240243

241244
CHIP_ERROR ReadGroupTable(EndpointId endpoint, AttributeValueEncoder & aEncoder)
242245
{
243-
auto fabric_index = aEncoder.AccessingFabricIndex();
244-
auto provider = GetGroupDataProvider();
246+
auto provider = GetGroupDataProvider();
245247
VerifyOrReturnError(nullptr != provider, CHIP_ERROR_INTERNAL);
246248

247-
CHIP_ERROR err = aEncoder.EncodeList([provider, fabric_index](const auto & encoder) -> CHIP_ERROR {
248-
auto iter = provider->IterateGroupInfo(fabric_index);
249-
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);
250-
251-
GroupDataProvider::GroupInfo info;
252-
while (iter->Next(info))
249+
CHIP_ERROR err = aEncoder.EncodeList([provider](const auto & encoder) -> CHIP_ERROR {
250+
for (auto & fabric : Server::GetInstance().GetFabricTable())
253251
{
254-
encoder.Encode(GroupTableCodec(provider, fabric_index, info));
252+
auto fabric_index = fabric.GetFabricIndex();
253+
auto iter = provider->IterateGroupInfo(fabric_index);
254+
VerifyOrReturnError(nullptr != iter, CHIP_ERROR_NO_MEMORY);
255+
256+
GroupDataProvider::GroupInfo info;
257+
while (iter->Next(info))
258+
{
259+
encoder.Encode(GroupTableCodec(provider, fabric_index, info));
260+
}
261+
iter->Release();
255262
}
256-
iter->Release();
257263
return CHIP_NO_ERROR;
258264
});
259265
return err;

‎src/app/tests/suites/TestGroupKeyManagementCluster.yaml

+229-4
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,41 @@ config:
1818
nodeId: 0x12344321
1919
cluster: "Group Key Management"
2020
endpoint: 0
21+
payload:
22+
type: char_string
23+
defaultValue: "MT:-24J0AFN00KA0648G00" # This value needs to be generated automatically
2124

2225
tests:
23-
- label: "Wait for the commissioned device to be retrieved"
26+
- label: "Wait for the commissioned device to be retrieved for alpha"
27+
cluster: "DelayCommands"
28+
command: "WaitForCommissionee"
29+
arguments:
30+
values:
31+
- name: "nodeId"
32+
value: nodeId
33+
34+
- label: "Open Commissioning Window from alpha"
35+
cluster: "Administrator Commissioning"
36+
command: "OpenBasicCommissioningWindow"
37+
timedInteractionTimeoutMs: 10000
38+
arguments:
39+
values:
40+
- name: "CommissioningTimeout"
41+
value: 180
42+
43+
- label: "Commission from beta"
44+
identity: "beta"
45+
cluster: "CommissionerCommands"
46+
command: "PairWithCode"
47+
arguments:
48+
values:
49+
- name: "nodeId"
50+
value: nodeId
51+
- name: "payload"
52+
value: payload
53+
54+
- label: "Wait for the commissioned device to be retrieved for beta"
55+
identity: "beta"
2456
cluster: "DelayCommands"
2557
command: "WaitForCommissionee"
2658
arguments:
@@ -76,6 +108,26 @@ tests:
76108
EpochStartTime2: 2110002,
77109
}
78110

111+
- label: "KeySet Write 3"
112+
identity: "beta"
113+
command: "KeySetWrite"
114+
arguments:
115+
values:
116+
- name: "GroupKeySet"
117+
value:
118+
{
119+
GroupKeySetID: 0x01a3,
120+
GroupKeySecurityPolicy: 1,
121+
EpochKey0:
122+
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0a\x0b\x0c\x0d\x0e\x0f",
123+
EpochStartTime0: 2110000,
124+
EpochKey1: "\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f",
125+
EpochStartTime1: 2110001,
126+
EpochKey2:
127+
"\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2a\x2b\x2c\x2d\x2e\x2f",
128+
EpochStartTime2: 2110002,
129+
}
130+
79131
- label: "KeySet Read"
80132
command: "KeySetRead"
81133
arguments:
@@ -129,10 +181,35 @@ tests:
129181
response:
130182
error: FAILURE
131183

132-
- label: "Write Group Keys"
184+
- label: "Write Group Keys on alpha"
185+
command: "writeAttribute"
186+
attribute: "GroupKeyMap"
187+
arguments:
188+
value:
189+
[
190+
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
191+
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
192+
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
193+
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
194+
]
195+
196+
- label: "Write Group Keys on beta"
197+
identity: "beta"
133198
command: "writeAttribute"
134199
attribute: "GroupKeyMap"
135200
arguments:
201+
value: [
202+
# Note: the FabricIndex here does not matter; it's not sent on the wire.
203+
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
204+
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
205+
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
206+
{ FabricIndex: 1, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
207+
]
208+
209+
- label: "Read Group Keys on alpha"
210+
command: "readAttribute"
211+
attribute: "GroupKeyMap"
212+
response:
136213
value:
137214
[
138215
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
@@ -141,16 +218,52 @@ tests:
141218
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
142219
]
143220

144-
- label: "Read Group Keys"
221+
- label: "Read Group Keys on alpha without fabric filtering"
222+
command: "readAttribute"
223+
attribute: "GroupKeyMap"
224+
fabricFiltered: false
225+
response:
226+
value:
227+
[
228+
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
229+
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
230+
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
231+
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
232+
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
233+
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
234+
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
235+
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
236+
]
237+
238+
- label: "Read Group Keys on beta"
239+
identity: "beta"
240+
command: "readAttribute"
241+
attribute: "GroupKeyMap"
242+
response:
243+
value:
244+
[
245+
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
246+
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
247+
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
248+
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
249+
]
250+
251+
- label: "Read Group Keys on beta without fabric filtering"
252+
identity: "beta"
145253
command: "readAttribute"
146254
attribute: "GroupKeyMap"
255+
fabricFiltered: false
147256
response:
148257
value:
149258
[
150259
{ FabricIndex: 1, GroupId: 0x0101, GroupKeySetID: 0x01a1 },
151260
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
152261
{ FabricIndex: 1, GroupId: 0x0103, GroupKeySetID: 0x01a1 },
153262
{ FabricIndex: 1, GroupId: 0x0104, GroupKeySetID: 0x01a2 },
263+
{ FabricIndex: 2, GroupId: 0x0102, GroupKeySetID: 0x01a3 },
264+
{ FabricIndex: 2, GroupId: 0x0103, GroupKeySetID: 0x01a3 },
265+
{ FabricIndex: 2, GroupId: 0x0104, GroupKeySetID: 0x01a3 },
266+
{ FabricIndex: 2, GroupId: 0x0105, GroupKeySetID: 0x01a3 },
154267
]
155268

156269
- label: "Add Group 1"
@@ -221,9 +334,115 @@ tests:
221334
- name: "GroupID"
222335
value: 0x0104
223336

224-
- label: "Read GroupTable"
337+
- label: "Add Group 5"
338+
identity: "beta"
339+
cluster: "Groups"
340+
endpoint: 1
341+
command: "AddGroup"
342+
arguments:
343+
values:
344+
- name: "GroupID"
345+
value: 0x0105
346+
- name: "GroupName"
347+
value: "Group #5"
348+
response:
349+
values:
350+
- name: "Status"
351+
value: 0
352+
- name: "GroupID"
353+
value: 0x0105
354+
355+
- label: "Read GroupTable from alpha"
356+
command: "readAttribute"
357+
attribute: "GroupTable"
358+
response:
359+
value:
360+
[
361+
{
362+
FabricIndex: 1,
363+
GroupId: 0x0101,
364+
Endpoints: [1],
365+
GroupName: "Group #1",
366+
},
367+
{
368+
FabricIndex: 1,
369+
GroupId: 0x0102,
370+
Endpoints: [1],
371+
GroupName: "Group #2",
372+
},
373+
{
374+
FabricIndex: 1,
375+
GroupId: 0x0103,
376+
Endpoints: [1],
377+
GroupName: "Group #3",
378+
},
379+
{
380+
FabricIndex: 1,
381+
GroupId: 0x0104,
382+
Endpoints: [1],
383+
GroupName: "Group #4",
384+
},
385+
]
386+
387+
- label: "Read GroupTable from alpha without fabric filtering"
388+
command: "readAttribute"
389+
attribute: "GroupTable"
390+
fabricFiltered: false
391+
response:
392+
value:
393+
[
394+
{
395+
FabricIndex: 1,
396+
GroupId: 0x0101,
397+
Endpoints: [1],
398+
GroupName: "Group #1",
399+
},
400+
{
401+
FabricIndex: 1,
402+
GroupId: 0x0102,
403+
Endpoints: [1],
404+
GroupName: "Group #2",
405+
},
406+
{
407+
FabricIndex: 1,
408+
GroupId: 0x0103,
409+
Endpoints: [1],
410+
GroupName: "Group #3",
411+
},
412+
{
413+
FabricIndex: 1,
414+
GroupId: 0x0104,
415+
Endpoints: [1],
416+
GroupName: "Group #4",
417+
},
418+
{
419+
FabricIndex: 2,
420+
GroupId: 0x0105,
421+
Endpoints: [1],
422+
GroupName: "Group #5",
423+
},
424+
]
425+
426+
- label: "Read GroupTable from beta"
427+
identity: "beta"
225428
command: "readAttribute"
226429
attribute: "GroupTable"
430+
response:
431+
value:
432+
[
433+
{
434+
FabricIndex: 2,
435+
GroupId: 0x0105,
436+
Endpoints: [1],
437+
GroupName: "Group #5",
438+
},
439+
]
440+
441+
- label: "Read GroupTable from beta without fabric filtering"
442+
identity: "beta"
443+
command: "readAttribute"
444+
attribute: "GroupTable"
445+
fabricFiltered: false
227446
response:
228447
value:
229448
[
@@ -251,6 +470,12 @@ tests:
251470
Endpoints: [1],
252471
GroupName: "Group #4",
253472
},
473+
{
474+
FabricIndex: 2,
475+
GroupId: 0x0105,
476+
Endpoints: [1],
477+
GroupName: "Group #5",
478+
},
254479
]
255480

256481
- label: "KeySet Remove 1"

0 commit comments

Comments
 (0)