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.

Sign up for GitHub

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

Enforce constraints on UserStatus in SetUser/SetCredential commands. #21657

Merged
merged 2 commits into from
Aug 9, 2022
Merged

Enforce constraints on UserStatus in SetUser/SetCredential commands. #21657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0dd55f
Enforce constraints on UserStatus in SetUser/SetCredential commands.
bzbarsky-apple Aug 4, 2022
b3e21dc
Fix incorrect userStatus in precondition setup in cert YAMLs.
bzbarsky-apple Aug 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/app/clusters/door-lock-server/door-lock-server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,16 @@ bool DoorLockServer::SendLockAlarmEvent(chip::EndpointId endpointId, DlAlarmCode
return true;
}

namespace {
// Check whether this is valid UserStatus for a SetUser or SetCredential
// command.
bool IsValidUserStatusForSet(const Nullable<DlUserStatus> & userStatus)
{
return userStatus.IsNull() || (userStatus.Value() == DlUserStatus::kOccupiedEnabled) ||
(userStatus.Value() == DlUserStatus::kOccupiedDisabled);
}
} // anonymous namespace

void DoorLockServer::setUserCommandHandler(chip::app::CommandHandler * commandObj,
const chip::app::ConcreteCommandPath & commandPath,
const chip::app::Clusters::DoorLock::Commands::SetUser::DecodableType & commandData)
Expand Down Expand Up @@ -362,8 +372,7 @@ void DoorLockServer::setUserCommandHandler(chip::app::CommandHandler * commandOb
return;
}

if (!userStatus.IsNull() &&
(userStatus.Value() < DlUserStatus::kAvailable || userStatus.Value() > DlUserStatus::kOccupiedDisabled))
if (!IsValidUserStatusForSet(userStatus))
{
emberAfDoorLockClusterPrintln(
"[SetUser] Unable to set the user: user status is out of range [endpointId=%d,userIndex=%d,userStatus=%u]",
Expand Down Expand Up @@ -677,9 +686,7 @@ void DoorLockServer::setCredentialCommandHandler(
return;
}

// OPTIMIZE: We can unify the checks for enum validity here and in set user command handler
if (!userStatus.IsNull() &&
(userStatus.Value() < DlUserStatus::kAvailable || userStatus.Value() > DlUserStatus::kOccupiedDisabled))
if (!IsValidUserStatusForSet(userStatus))
{
emberAfDoorLockClusterPrintln("[SetCredential] Unable to set the credential: user status is out of range "
"[endpointId=%d,credentialIndex=%d,userStatus=%u]",
Expand Down
277 changes: 270 additions & 7 deletions src/app/tests/suites/DL_UsersAndCredentials.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,157 @@ tests:
- name: "nextUserIndex"
value: null

- label: "Try to add a user with userStatus 0"
command: "SetUser"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "operationType"
value: 0
- name: "userIndex"
value: 3
- name: "userName"
value: "test_user3"
- name: "userUniqueId"
value: 0xBABA
- name: "userStatus"
value: 0
- name: "userType"
value: null
- name: "credentialRule"
value: null
response:
error: INVALID_COMMAND

- label: "Make sure the user did not get created"
command: "GetUser"
arguments:
values:
- name: "userIndex"
value: 3
response:
values:
- name: "userIndex"
value: 3
- name: "userName"
value: null
- name: "userUniqueId"
value: null
- name: "userStatus"
value: null
- name: "userType"
value: null
- name: "credentialRule"
value: null
- name: "credentials"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null
- name: "nextUserIndex"
value: null

- label: "Try to add a user with userStatus 2"
command: "SetUser"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "operationType"
value: 0
- name: "userIndex"
value: 3
- name: "userName"
value: "test_user3"
- name: "userUniqueId"
value: 0xBABA
- name: "userStatus"
value: 2
- name: "userType"
value: null
- name: "credentialRule"
value: null
response:
error: INVALID_COMMAND

- label: "Make sure the user did not get created"
command: "GetUser"
arguments:
values:
- name: "userIndex"
value: 3
response:
values:
- name: "userIndex"
value: 3
- name: "userName"
value: null
- name: "userUniqueId"
value: null
- name: "userStatus"
value: null
- name: "userType"
value: null
- name: "credentialRule"
value: null
- name: "credentials"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null
- name: "nextUserIndex"
value: null

- label: "Try to add a user with userStatus 3"
command: "SetUser"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "operationType"
value: 0
- name: "userIndex"
value: 3
- name: "userName"
value: "test_user3"
- name: "userUniqueId"
value: 0xBABA
- name: "userStatus"
value: 3
- name: "userType"
value: null
- name: "credentialRule"
value: null

- label: "Read the new third user back and verify its fields"
command: "GetUser"
arguments:
values:
- name: "userIndex"
value: 3
response:
values:
- name: "userIndex"
value: 3
- name: "userName"
value: "test_user3"
- name: "userUniqueId"
value: 0xBABA
- name: "userStatus"
value: 3
- name: "userType"
value: 0
- name: "credentialRule"
value: 0
- name: "credentials"
value: null
- name: "creatorFabricIndex"
value: 1
- name: "lastModifiedFabricIndex"
value: 1
- name: "nextUserIndex"
value: null

- label: "Create user in the last slot"
command: "SetUser"
timedInteractionTimeoutMs: 10000
Expand Down Expand Up @@ -793,7 +944,7 @@ tests:
- name: "nextCredentialIndex"
value: null

- label: "Reading PIN credential with index 0 fails"
- label: "Reading PIN credential with index 0 returns no credential"
command: "GetCredentialStatus"
arguments:
values:
Expand All @@ -812,7 +963,8 @@ tests:
- name: "nextCredentialIndex"
value: null

- label: "Reading PIN credential with out-of-bounds index fails"
- label:
"Reading PIN credential with out-of-bounds index returns no credential"
command: "GetCredentialStatus"
arguments:
values:
Expand All @@ -823,7 +975,73 @@ tests:
CredentialIndex: NumberOfPINUsersSupported + 1,
}
response:
error: INVALID_COMMAND
values:
- name: "credentialExists"
value: false
- name: "userIndex"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null
- name: "nextCredentialIndex"
value: null

- label:
"Verify that a user with UserStatus = 0 cannot be added via
SetCredential"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "operationType"
value: 0
- name: "credential"
value: { CredentialType: 1, CredentialIndex: 1 }
- name: "credentialData"
value: "000000"
- name: "userIndex"
value: null
- name: "userStatus"
value: 0
- name: "userType"
value: null
response:
values:
- name: "status"
value: 0x85 # INVALID_COMMAND
- name: "userIndex"
value: null
- name: "nextCredentialIndex"
value: 2

- label:
"Verify that a user with UserStatus = 2 cannot be added via
SetCredential"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
arguments:
values:
- name: "operationType"
value: 0
- name: "credential"
value: { CredentialType: 1, CredentialIndex: 1 }
- name: "credentialData"
value: "000000"
- name: "userIndex"
value: null
- name: "userStatus"
value: 2
- name: "userType"
value: null
response:
values:
- name: "status"
value: 0x85 # INVALID_COMMAND
- name: "userIndex"
value: null
- name: "nextCredentialIndex"
value: 2

- label: "Create new PIN credential and user"
command: "SetCredential"
Expand Down Expand Up @@ -962,16 +1180,51 @@ tests:
saveAs: NumberOfRFIDUsersSupported
value: 10

- label: "Reading RFID credential with index 0 fails"
# Disabled because due to https://github.com/project-chip/connectedhomeip/issues/21656: that causes the nextCredentialIndex
# here to have the wrong value.
- label: "Reading RFID credential with index 0 returns no credential"
disabled: true
command: "GetCredentialStatus"
arguments:
values:
- name: "credential"
value: { CredentialType: 2, CredentialIndex: 0 }
response:
error: INVALID_COMMAND
values:
- name: "credentialExists"
value: false
- name: "userIndex"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null
- name: "nextCredentialIndex"
value: null

- label: "Reading RFID credential with out-of-bounds index fails"
# 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"
command: "GetCredentialStatus"
arguments:
values:
- name: "credential"
value: { CredentialType: 2, CredentialIndex: 0 }
response:
values:
- name: "credentialExists"
value: false
- name: "userIndex"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null

- label:
"Reading RFID credential with out-of-bounds index returns no
credential"
command: "GetCredentialStatus"
arguments:
values:
Expand All @@ -982,7 +1235,17 @@ tests:
CredentialIndex: NumberOfRFIDUsersSupported + 1,
}
response:
error: INVALID_COMMAND
values:
- name: "credentialExists"
value: false
- name: "userIndex"
value: null
- name: "creatorFabricIndex"
value: null
- name: "lastModifiedFabricIndex"
value: null
- name: "nextCredentialIndex"
value: null

- label: "Check that RFID credential does not exist"
command: "GetCredentialStatus"
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/suites/certification/Test_TC_DRLK_2_2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ tests:
- name: "userIndex"
value: 1
- name: "userStatus"
value: 0
value: null
- name: "userType"
value: 0
value: null
response:
values:
- name: "status"
Expand Down
4 changes: 2 additions & 2 deletions src/app/tests/suites/certification/Test_TC_DRLK_2_3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ tests:
- name: "userIndex"
value: 1
- name: "userStatus"
value: 0
value: null
- name: "userType"
value: 0
value: null
response:
values:
- name: "status"
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ function getTests() {
];

const DoorLock = [
//"DL_UsersAndCredentials", TODO: This test is not aligned with spec
"DL_UsersAndCredentials",
"DL_LockUnlock",
"DL_Schedules",
"Test_TC_DRLK_2_2",
Expand Down
Loading