Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix incoming command validity checks in Set Credential. (#25264)
Browse files Browse the repository at this point in the history
* When Set Credential happens with OperationType set to Modify and
  UserIndex set to null, that means we are modifying the credential
  for the programming user.  In this case, UserStatus must be null and
  UserType must be ProgrammingUser, but we were not checking that.
* When Set Credential happens with OperationType set to Modify and
  UserIndex not null, UserStatus and UserType must both be null.  We
  were incorrectly allowing UserType to be ProgrammingUser in this case.
* When Set Credential happens with OperationType set to Add and
  UserIndex not null, UserStatus and UserType must both be null.  We
  were not checking for this at all.

Fixes #25259
bzbarsky-apple authored and pull[bot] committed Oct 31, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 9327317 commit 4733612
Showing 4 changed files with 1,118 additions and 504 deletions.
19 changes: 17 additions & 2 deletions src/app/clusters/door-lock-server/door-lock-server.cpp
Original file line number Diff line number Diff line change
@@ -700,6 +700,12 @@ void DoorLockServer::setCredentialCommandHandler(
// if userIndex is NULL then we're changing the programming user PIN
if (userIndex.IsNull())
{
if (!userStatus.IsNull() || userType != UserTypeEnum::kProgrammingUser)
{
emberAfDoorLockClusterPrintln("[SetCredential] Unable to modify programming PIN: invalid argument "
"[endpointId=%d,credentialIndex=%d]",
commandPath.mEndpointId, credentialIndex);
}
status = modifyProgrammingPIN(commandPath.mEndpointId, fabricIdx, sourceNodeId, credentialIndex, credentialType,
existingCredential, credentialData);
sendSetCredentialResponse(commandObj, commandPath, status, 0, nextAvailableCredentialSlot);
@@ -2245,7 +2251,16 @@ DlStatus DoorLockServer::createCredential(chip::EndpointId endpointId, chip::Fab
}
else
{
// appclusters, 5.2.4.40: if user index is NULL, we should try to modify the existing user
// appclusters, 5.2.4.40: if user index is NULL, we should try to modify
// the existing user. In this case userStatus and userType shall both
// be null.
if (!userStatus.IsNull() || !userType.IsNull())
{
emberAfDoorLockClusterPrintln("[SetCredential] Unable to add credential: invalid arguments "
"[endpointId=%d,credentialIndex=%d,credentialType=%u]",
endpointId, credentialIndex, to_underlying(credentialType));
return DlStatus::kInvalidField;
}
status = createNewCredentialAndAddItToUser(endpointId, creatorFabricIdx, userIndex.Value(), credential, credentialData);
}

@@ -2314,7 +2329,7 @@ DlStatus DoorLockServer::modifyCredential(chip::EndpointId endpointId, chip::Fab
{

// appclusters, 5.2.4.40: when modifying a credential, userStatus and userType shall both be NULL.
if (!userStatus.IsNull() || (!userType.IsNull() && UserTypeEnum::kProgrammingUser != userType.Value()))
if (!userStatus.IsNull() || !userType.IsNull())
{
emberAfDoorLockClusterPrintln("[SetCredential] Unable to modify the credential: invalid arguments "
"[endpointId=%d,credentialIndex=%d,credentialType=%u]",
186 changes: 167 additions & 19 deletions src/app/tests/suites/DL_UsersAndCredentials.yaml
Original file line number Diff line number Diff line change
@@ -1199,15 +1199,18 @@ tests:
- name: "NextCredentialIndex"
value: null

# Duplicate of the previous test that does not check the value of nextCredentialIndex
- label:
"Reading RFID credential with index 0 returns no credential duplicate
with bug workaround"
"Reading RFID credential with out-of-bounds index returns no
credential"
command: "GetCredentialStatus"
arguments:
values:
- name: "Credential"
value: { CredentialType: 2, CredentialIndex: 0 }
value:
{
CredentialType: 2,
CredentialIndex: NumberOfRFIDUsersSupported + 1,
}
response:
values:
- name: "CredentialExists"
@@ -1218,19 +1221,15 @@ tests:
value: null
- name: "LastModifiedFabricIndex"
value: null
- name: "NextCredentialIndex"
value: null

- label:
"Reading RFID credential with out-of-bounds index returns no
credential"
- label: "Check that RFID credential does not exist"
command: "GetCredentialStatus"
arguments:
values:
- name: "Credential"
value:
{
CredentialType: 2,
CredentialIndex: NumberOfRFIDUsersSupported + 1,
}
value: { CredentialType: 2, CredentialIndex: 2 }
response:
values:
- name: "CredentialExists"
@@ -1244,24 +1243,89 @@ tests:
- name: "NextCredentialIndex"
value: null

- label: "Check that RFID credential does not exist"
command: "GetCredentialStatus"
- label:
"Create new RFID credential and add it to existing user with non-null
UserStatus should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 0
- name: "Credential"
value: { CredentialType: 2, CredentialIndex: 2 }
value: { CredentialType: 2, CredentialIndex: 1 }
- name: "CredentialData"
value: "rfid_data_123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: 1
- name: "UserType"
value: null
response:
values:
- name: "CredentialExists"
value: false
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "CreatorFabricIndex"
- name: "NextCredentialIndex"
value: 2

- label:
"Create new RFID credential and add it to existing user with non-null
UserType should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 0
- name: "Credential"
value: { CredentialType: 2, CredentialIndex: 1 }
- name: "CredentialData"
value: "rfid_data_123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: null
- name: "LastModifiedFabricIndex"
- name: "UserType"
value: 0
response:
values:
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "NextCredentialIndex"
value: 2

- label:
"Create new RFID credential and add it to existing user with non-null
UserType and UserStatus should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 0
- name: "Credential"
value: { CredentialType: 2, CredentialIndex: 1 }
- name: "CredentialData"
value: "rfid_data_123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: 1
- name: "UserType"
value: 0
response:
values:
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "NextCredentialIndex"
value: 2

- label: "Create new RFID credential and add it to existing user"
command: "SetCredential"
@@ -1632,6 +1696,90 @@ tests:
- name: "NextCredentialIndex"
value: 3

- label:
"Modify credentialData of existing PIN credential with non-null
UserStatus should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 2
- name: "Credential"
value: { CredentialType: 1, CredentialIndex: 1 }
- name: "CredentialData"
value: "123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: 1
- name: "UserType"
value: null
response:
values:
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "NextCredentialIndex"
value: 2

- label:
"Modify credentialData of existing PIN credential with non-null
UserType should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 2
- name: "Credential"
value: { CredentialType: 1, CredentialIndex: 1 }
- name: "CredentialData"
value: "123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: null
- name: "UserType"
value: 0
response:
values:
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "NextCredentialIndex"
value: 2

- label:
"Modify credentialData of existing PIN credential with non-null
UserStatus and UserType should fail"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "OperationType"
value: 2
- name: "Credential"
value: { CredentialType: 1, CredentialIndex: 1 }
- name: "CredentialData"
value: "123456"
- name: "UserIndex"
value: 1
- name: "UserStatus"
value: 1
- name: "UserType"
value: 0
response:
values:
- name: "Status"
value: 0x85 # INVALID_COMMAND
- name: "UserIndex"
value: null
- name: "NextCredentialIndex"
value: 2

- label: "Modify credentialData of existing PIN credential"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
589 changes: 378 additions & 211 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Large diffs are not rendered by default.

828 changes: 556 additions & 272 deletions zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h

Large diffs are not rendered by default.

0 comments on commit 4733612

Please sign in to comment.