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

Ensure that all Fabric-scoped commands respond with UNSUPPORTED_ACCESS if there is no accessing fabric #20811

Closed
bzbarsky-apple opened this issue Jul 15, 2022 · 4 comments · Fixed by #21022 or #21072
Assignees
Labels

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

We have some fabric-scoped commands (e.g. UpdateFabricLabel) that are not following the spec, which says:

if the command in the path is fabric-scoped and there is no accessing fabric, a CommandStatusIB SHALL be generated with the UNSUPPORTED_ACCESS Status Code.

and are returning some other responses instead.

Some of the test plans may in fact be expecting those other responses, hence the cert blocker bit....

Proposed Solution

Audit and fix. The relevant set of commands looks like the following (git grep run in the spec repo):

% git grep "[| ] *F *[ |]" | grep "=>" | sed 's/^[^:]*:\|[^|]*\| *//' | sed 's/ *\|.*//'
AddGroup
ViewGroup
GetGroupMembership
RemoveGroup
RemoveAllGroups
AddGroupIfIdentifying
<<ref_cmd_KeySetWrite>>
<<ref_cmd_KeySetRead>>
<<ref_cmd_KeySetRemove>>
<<ref_cmd_KeySetReadAllIndices>>
CommissioningComplete
UpdateNOC
UpdateFabricLabel
@bzbarsky-apple bzbarsky-apple added V1.0 spec Mismatch between spec and implementation request cert blocker labels Jul 15, 2022
@bzbarsky-apple
Copy link
Contributor Author

Oh, and ideally this would be enforced in IM, but we don't have a way to annotate commands as fabric-scoped in XML right now. Maybe we should just add one and do this in IM, the same way we handle timed invoke.

@tcarmelveilleux tcarmelveilleux self-assigned this Jul 15, 2022
@tcarmelveilleux
Copy link
Contributor

Example of current failure:

# Example of opening a PASE session
#
# Shell 1
$ ./chip-all-clusters-app
# Shell 2
$ ./chip-tool interactive-start
# Once in interactive mode
pairing code-paseonly 0x12344321 34970112332
# Once over PASE
operationalcredentials update-fabric-label HelloWorld 0x12344321 0
# Example Result
NOCResponse: {
  statusCode: 8
  debugText: Current fabric not found
}

@tcarmelveilleux
Copy link
Contributor

Assigning to @bzbarsky-apple to do the code-gen bits for the table once ZAP has the support (project-chip/zap#609)

@woody-apple woody-apple added v1.1 and removed V1.0 spec Mismatch between spec and implementation request cert blocker labels Jul 19, 2022
@woody-apple
Copy link
Contributor

Cert Blocker Review: Moving this 1.1, given the current SDK is conformant with the spec, and we can live with the hardcoded version for now.

bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Jul 20, 2022
andy31415 pushed a commit that referenced this issue Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment