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

Example lock apps do not implement segmenting of credential database by credential type #21656

Closed
bzbarsky-apple opened this issue Aug 5, 2022 · 3 comments · Fixed by #24532
Assignees
Labels
app-clusters Application cluster work examples V1.X

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

In the door lock spec, credential indices are scoped to a particular credential type. So (RFID, index 1) and (PIN, index 1) are two different credential slots.

In the Linux lock app there is a single mLockCredentials array (verified by testing and code inspection). In the EFR32 and P6 and CYW30739 lock apps code inspection suggests the same thing is happening.

Concretely, if I run the following against the linux lock-app:

  1. chip-tool doorlock set-credential 0 '{ "credentialType": 1, "credentialIndex": 1 }' "123456" null null null 17 1 --timedInteractionTimeoutMs 1000
  2. chip-tool doorlock get-credential-status '{ "credentialType": 2, "credentialIndex": 1 }' 17 1
  3. chip-tool doorlock set-credential 0 '{ "credentialType": 2, "credentialIndex": 1 }' "1234567890" null null null 17 1 --timedInteractionTimeoutMs 1000

Then:

  • In step 2 I get a FAILURE response instead of the expected success response saying there is no such credential.
  • In step 3 I get an OCCUPIED status response instead of the credential being added, as it should be.

Proposed Solution

The credential indices need to be scoped to the credential type. A flat store can still be used, if the scoping is implemented in some way on top of it, or per-type stores.

@bzbarsky-apple bzbarsky-apple added examples spec Mismatch between spec and implementation app-clusters Application cluster work V1.0 labels Aug 5, 2022
@woody-apple
Copy link
Contributor

Spec Issue Review: Given there are no test cases for this, we should mark multiple credential types as provisional for v1.0 test plans.

@Dan-Matosian
Copy link

I think the simplest path forward for 1.0 is to just mark certain credential types as provisional. This would limit locks in v1.0 to only pin codes.

PR: https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/5537

@woody-apple
Copy link
Contributor

SDK Review: Given that this is no longer required for v1.0. Moving this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app-clusters Application cluster work examples V1.X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants