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.

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

[BUG] GroupKeyManagement ignores fabricFiltered request #23322

Closed
bluebin14 opened this issue Oct 24, 2022 · 6 comments · Fixed by #25319
Closed

[BUG] GroupKeyManagement ignores fabricFiltered request #23322

bluebin14 opened this issue Oct 24, 2022 · 6 comments · Fixed by #25319
Assignees
Labels
bug Something isn't working group messaging Interaction Model Work spec Mismatch between spec and implementation

Comments

@bluebin14
Copy link
Contributor

bluebin14 commented Oct 24, 2022

Reproduction steps

1. Set up a server with 2 fabrics with some groupKeyManagement settings
2. Read GroupKeyMap and GroupTable fabric scoped list with --fabric-filtered 0
3. Resulting list only has entries from current fabric. It should have placeholder entries for the other fabric as well.

Bug prevalence

always

GitHub hash of the SDK that was being used

current

Platform

android, darwin, nrf

Platform Version(s)

No response

Anything else?

No response

@tcarmelveilleux tcarmelveilleux added bug Something isn't working spec Mismatch between spec and implementation Interaction Model Work labels Oct 24, 2022
@bluebin14
Copy link
Contributor Author

bluebin14 commented Oct 24, 2022

./chip-all-clusters-app --passcode 12312123

./chip-tool pairing onnetwork 1 12312123

./chip-tool administratorcommissioning open-basic-commissioning-window 180 1 0 --timedInteractionTimeoutMs 10000

./chip-tool pairing onnetwork 2 12312123 --commissioner-name beta

./chip-tool groupkeymanagement key-set-write '{"groupKeySetID": 42, "groupKeySecurityPolicy": 0, "epochKey0": "d0d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime0": 2220000,"epochKey1": "d1d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime1": 2220001,"epochKey2": "d2d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime2": 2220002 }' 1 0

./chip-tool groupkeymanagement write group-key-map '[{"groupId": 1, "groupKeySetID": 42}]' 1 0

./chip-tool groupkeymanagement key-set-write '{"groupKeySetID": 43, "groupKeySecurityPolicy": 0, "epochKey0": "d0d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime0": 2220000,"epochKey1": "d1d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime1": 2220001,"epochKey2": "d2d1d2d3d4d5d6d7d8d9dadbdcdddedf", "epochStartTime2": 2220002 }' 2 0  --commissioner-name beta

./chip-tool groupkeymanagement write group-key-map '[{"groupId": 2, "groupKeySetID": 43}]' 2 0 --commissioner-name beta

./chip-tool groupkeymanagement read group-key-map 1 0 --fabric-filtered 0

./chip-tool groupkeymanagement read group-key-map 2 0 --fabric-filtered 0 --commissioner-name beta

@bluebin14
Copy link
Contributor Author

bluebin14 commented Oct 24, 2022

$ ./chip-tool groupkeymanagement read group-key-map 1 0 --fabric-filtered 0
...
[1666624422597] [22068:5160187] CHIP: [DMG] }
[1666624422600] [22068:5160187] CHIP: [TOO] Endpoint: 0 Cluster: 0x0000_003F Attribute 0x0000_0000 DataVersion: 1881152597
[1666624422601] [22068:5160187] CHIP: [TOO]   GroupKeyMap: 1 entries
[1666624422601] [22068:5160187] CHIP: [TOO]     [1]: {
[1666624422601] [22068:5160187] CHIP: [TOO]       GroupId: 1
[1666624422601] [22068:5160187] CHIP: [TOO]       GroupKeySetID: 42
[1666624422601] [22068:5160187] CHIP: [TOO]       FabricIndex: 1
[1666624422601] [22068:5160187] CHIP: [TOO]      }
...
$ ./chip-tool groupkeymanagement read group-key-map 2 0 --fabric-filtered 0 --commissioner-name beta
...
[1666624458115] [22094:5160689] CHIP: [TOO] Endpoint: 0 Cluster: 0x0000_003F Attribute 0x0000_0000 DataVersion: 1881152597
[1666624458116] [22094:5160689] CHIP: [TOO]   GroupKeyMap: 1 entries
[1666624458116] [22094:5160689] CHIP: [TOO]     [1]: {
[1666624458116] [22094:5160689] CHIP: [TOO]       GroupId: 2
[1666624458116] [22094:5160689] CHIP: [TOO]       GroupKeySetID: 43
[1666624458116] [22094:5160689] CHIP: [TOO]       FabricIndex: 2
[1666624458116] [22094:5160689] CHIP: [TOO]      }

@bzbarsky-apple
Copy link
Contributor

GroupKeyManagementAttributeAccess::ReadGroupKeyMap does:

        auto fabric_index = aEncoder.AccessingFabricIndex();
....
            auto iter = provider->IterateGroupKeys(fabric_index);

as in, it iterates only group keys for the accessing fabric index, instead of iterating all group keys and then deciding what to do with them.

Similarly, GroupKeyManagementAttributeAccess::ReadGroupTable does:

        auto fabric_index = aEncoder.AccessingFabricIndex();
....
            auto iter = provider->IterateGroupInfo(fabric_index);

@bzbarsky-apple
Copy link
Contributor

And the group data provider API does not seem to have a way to iterate all group info or group keys across all fabric indices (but providing the fabric index in the iteration results, so the filtering can happen at the right level)....

@bzbarsky-apple bzbarsky-apple removed their assignment Oct 24, 2022
@tcarmelveilleux
Copy link
Contributor

We can emulate it by iterating over all fabrics and then doing the ReadXXX, if we know it's a non-fabric-filtered read

@bzbarsky-apple
Copy link
Contributor

That information is not public on AttributeValueEncoder, to keep people from trying to do weird things and shooting themselves in the foot. We could expose it, I guess, or just do the iteration unconditionally and AttributeValueEncoder will ignore things as needed.

@nivi-apple nivi-apple self-assigned this Feb 6, 2023
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Feb 24, 2023
We were only returning the entries for the accessing fabric, not for
all fabrics.

Fixes project-chip#23322
jmartinez-silabs pushed a commit that referenced this issue Mar 7, 2023
…5319)

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

Fixes #23322
kkasperczyk-no pushed a commit to kkasperczyk-no/sdk-connectedhomeip that referenced this issue Mar 15, 2023
…5319)

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

Fixes project-chip/connectedhomeip#23322
lecndav pushed a commit to lecndav/connectedhomeip that referenced this issue Mar 22, 2023
…oject-chip#25319)

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

Fixes project-chip#23322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working group messaging Interaction Model Work spec Mismatch between spec and implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants