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

Group key management cluster XML looks nothing like the spec #13176

Closed
bzbarsky-apple opened this issue Dec 21, 2021 · 1 comment · Fixed by #13625
Closed

Group key management cluster XML looks nothing like the spec #13176

bzbarsky-apple opened this issue Dec 21, 2021 · 1 comment · Fixed by #13625
Assignees
Labels
security spec Mismatch between spec and implementation V1.0

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

The XML bits added in #12479 look nothing like the spec. The naming is off. The integer types used are wrong in a bunch of places (e.g. hardcoding sized ints for things like fabric indices, group ids, etc, some of them with the wrong sizes). Times are not using the epoch-us type, which is what the spec has. Nullable fields are not marked nullable. Attributes that are supposed to be marked writable are not marked writable.

Proposed Solution

Go through and make the XML match the spec.

@bzbarsky-apple bzbarsky-apple added the spec Mismatch between spec and implementation label Dec 21, 2021
@bzbarsky-apple
Copy link
Contributor Author

Oh, and because it does not handle fabric index properly it's not getting the security checks around messing with other fabrics' data. So this is a critical security bug too.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Dec 21, 2021
It's not needed; all that code already exists in Structs::GroupKey.

The XML change is to fix a compile issue due to the type being wrong,
but pretty much all the XML is wrong there.
project-chip#13176 tracks
addressing that.
andy31415 added a commit that referenced this issue Jan 4, 2022
* Remove GroupKeyCodec.

It's not needed; all that code already exists in Structs::GroupKey.

The XML change is to fix a compile issue due to the type being wrong,
but pretty much all the XML is wrong there.
#13176 tracks
addressing that.

* Remove GroupKeyCodec again - apparently the conflict file was not zap generated

Co-authored-by: Andrei Litvin <[email protected]>
step0035 pushed a commit to hank820/connectedhomeip that referenced this issue Feb 8, 2022
* Remove GroupKeyCodec.

It's not needed; all that code already exists in Structs::GroupKey.

The XML change is to fix a compile issue due to the type being wrong,
but pretty much all the XML is wrong there.
project-chip#13176 tracks
addressing that.

* Remove GroupKeyCodec again - apparently the conflict file was not zap generated

Co-authored-by: Andrei Litvin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security spec Mismatch between spec and implementation V1.0
Projects
None yet
5 participants