From a28e622e76a71435edb318d8e27c232cf2cae831 Mon Sep 17 00:00:00 2001 From: Jon Rhees Date: Fri, 28 Jun 2024 11:29:51 -0600 Subject: [PATCH 1/2] Add fabric restrictions to SetWeekDaySchedule and SetYearDaySchedule commands to allow schedules to be added/modified only by the fabric that created the corresponding user. Update YAML schedule test to verify behavior Accompanies DRLK spec change: https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/10005 --- .../door-lock-server/door-lock-server.cpp | 68 +++++++- src/app/tests/suites/DL_Schedules.yaml | 150 ++++++++++++++++++ 2 files changed, 216 insertions(+), 2 deletions(-) diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp index 528cbd6eb0971d..a6dde6b8ff265f 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -995,7 +995,17 @@ void DoorLockServer::setWeekDayScheduleCommandHandler(chip::app::CommandHandler return; } - if (!userExists(endpointId, userIndex)) + EmberAfPluginDoorLockUserInfo user; + if (!emberAfPluginDoorLockGetUser(endpointId, userIndex, user)) + { + ChipLogError(Zcl, + "[SetWeekDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", + endpointId, userIndex); + commandObj->AddStatus(commandPath, Status::Failure); + return; + } + + if (UserStatusEnum::kAvailable == user.userStatus) { ChipLogProgress(Zcl, "[SetWeekDaySchedule] Unable to add schedule - user does not exist " @@ -1005,6 +1015,17 @@ void DoorLockServer::setWeekDayScheduleCommandHandler(chip::app::CommandHandler return; } + // Return INVALID_COMMAND if accessing FabricIndex does not match CreatorFabricIndex of User + if (user.createdBy != fabricIdx) + { + ChipLogProgress(Zcl, + "[SetWeekDaySchedule] Unable to add schedule to user created by different fabric " + "[endpointId=%d,userIndex=%d,creatorIdx=%d,fabricIdx=%d]", + endpointId, userIndex, user.createdBy, fabricIdx); + commandObj->AddStatus(commandPath, Status::InvalidCommand); + return; + } + uint8_t rawDaysMask = daysMask.Raw(); // Check that bits are within range @@ -1056,6 +1077,17 @@ void DoorLockServer::setWeekDayScheduleCommandHandler(chip::app::CommandHandler "[endpointId=%d,weekDayIndex=%d,userIndex=%d,daysMask=%d,startTime=\"%d:%d\",endTime=\"%d:%d\"]", endpointId, weekDayIndex, userIndex, daysMask.Raw(), startHour, startMinute, endHour, endMinute); + // Update LastModifiedFabricIndex of user + if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, + user.userUniqueId, user.userStatus, user.userType, user.credentialRule, + user.credentials.data(), user.credentials.size())) + { + ChipLogError( + Zcl, + "[SetWeekDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", + endpointId, fabricIdx, userIndex); + } + sendRemoteLockUserChange(endpointId, LockDataTypeEnum::kWeekDaySchedule, DataOperationTypeEnum::kAdd, sourceNodeId, fabricIdx, userIndex, static_cast(weekDayIndex)); @@ -1220,7 +1252,17 @@ void DoorLockServer::setYearDayScheduleCommandHandler(chip::app::CommandHandler return; } - if (!userExists(endpointId, userIndex)) + EmberAfPluginDoorLockUserInfo user; + if (!emberAfPluginDoorLockGetUser(endpointId, userIndex, user)) + { + ChipLogError(Zcl, + "[SetYearDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", + endpointId, userIndex); + commandObj->AddStatus(commandPath, Status::Failure); + return; + } + + if (UserStatusEnum::kAvailable == user.userStatus) { ChipLogProgress(Zcl, "[SetYearDaySchedule] Unable to add schedule - user does not exist " @@ -1230,6 +1272,17 @@ void DoorLockServer::setYearDayScheduleCommandHandler(chip::app::CommandHandler return; } + // Return INVALID_COMMAND if accessing FabricIndex does not match CreatorFabricIndex of User + if (user.createdBy != fabricIdx) + { + ChipLogProgress(Zcl, + "[SetYearDaySchedule] Unable to add schedule to user created by different fabric " + "[endpointId=%d,userIndex=%d,creatorIdx=%d,fabricIdx=%d]", + endpointId, userIndex, user.createdBy, fabricIdx); + commandObj->AddStatus(commandPath, Status::InvalidCommand); + return; + } + if (localEndTime <= localStartTime) { ChipLogProgress(Zcl, @@ -1257,6 +1310,17 @@ void DoorLockServer::setYearDayScheduleCommandHandler(chip::app::CommandHandler "[endpointId=%d,yearDayIndex=%d,userIndex=%d,localStartTime=%" PRIu32 ",endTime=%" PRIu32 "]", endpointId, yearDayIndex, userIndex, localStartTime, localEndTime); + // Update LastModifiedFabricIndex of user + if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, + user.userUniqueId, user.userStatus, user.userType, user.credentialRule, + user.credentials.data(), user.credentials.size())) + { + ChipLogError( + Zcl, + "[SetYearDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", + endpointId, fabricIdx, userIndex); + } + sendRemoteLockUserChange(endpointId, LockDataTypeEnum::kYearDaySchedule, DataOperationTypeEnum::kAdd, sourceNodeId, fabricIdx, userIndex, static_cast(yearDayIndex)); diff --git a/src/app/tests/suites/DL_Schedules.yaml b/src/app/tests/suites/DL_Schedules.yaml index f0c32449c0f3e4..0fb81dacb29416 100644 --- a/src/app/tests/suites/DL_Schedules.yaml +++ b/src/app/tests/suites/DL_Schedules.yaml @@ -18,6 +18,18 @@ config: nodeId: 0x12344321 cluster: "Door Lock" endpoint: 1 + payload: + type: char_string + defaultValue: "MT:-24J0AFN00KA0648G00" + discriminator: + type: int16u + defaultValue: 3840 + waitAfterCommissioning: + type: int16u + defaultValue: 1000 + PakeVerifier: + type: octet_string + defaultValue: "hex:b96170aae803346884724fe9a3b287c30330c2a660375d17bb205a8cf1aecb350457f8ab79ee253ab6a8e46bb09e543ae422736de501e3db37d441fe344920d09548e4c18240630c4ff4913c53513839b7c07fcc0627a1b8573a149fcd1fa466cf" tests: - label: "Wait for the commissioned device to be retrieved" @@ -86,6 +98,57 @@ tests: saveAs: NumberOfHolidaySchedulesSupportedValue value: 10 + # + # Commission to second fabric to facilitate testing schedule/fabric restrictions + # + - label: "Open Commissioning Window from alpha" + endpoint: 0 + cluster: "Administrator Commissioning" + command: "OpenCommissioningWindow" + timedInteractionTimeoutMs: 10000 + arguments: + values: + - name: "CommissioningTimeout" + value: 180 + - name: "PAKEPasscodeVerifier" + value: PakeVerifier + - name: "Discriminator" + value: discriminator + - name: "Iterations" + value: 1000 + - name: "Salt" + value: "SPAKE2P Key Salt" + + - label: "Waiting after opening commissioning window" + cluster: "DelayCommands" + command: "WaitForMs" + arguments: + values: + - name: "ms" + value: waitAfterCommissioning + + - label: "Commission from TH2" + identity: "beta" + endpoint: 0 + cluster: "CommissionerCommands" + command: "PairWithCode" + arguments: + values: + - name: "nodeId" + value: nodeId + - name: "payload" + value: payload + + - label: "Wait for the commissioned device to be retrieved for TH2" + endpoint: 0 + identity: beta + cluster: "DelayCommands" + command: "WaitForCommissionee" + arguments: + values: + - name: "nodeId" + value: nodeId + # # Excercise SetWeekDay schedules with invalid parameters # @@ -885,6 +948,93 @@ tests: - name: "LocalEndTime" value: 12345689 + # Verify that schedule modification fails from different fabric than created user + - label: "Attempt to set Week Day schedule from different fabric" + command: "SetWeekDaySchedule" + identity: "beta" + arguments: + values: + - name: "WeekDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + - name: "DaysMask" + value: 0x01 + - name: "StartHour" + value: 14 + - name: "StartMinute" + value: 15 + - name: "EndHour" + value: 19 + - name: "EndMinute" + value: 00 + response: + error: INVALID_COMMAND + + - label: "Verify schedule was not modified" + command: "GetWeekDaySchedule" + arguments: + values: + - name: "WeekDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + response: + values: + - name: "WeekDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + - name: "Status" + value: 0x0 + - name: "DaysMask" + value: 0x01 + - name: "StartHour" + value: 15 + - name: "StartMinute" + value: 16 + - name: "EndHour" + value: 18 + - name: "EndMinute" + value: 00 + + - label: "Attempt to set Year Day schedule from different fabric" + command: "SetYearDaySchedule" + identity: "beta" + arguments: + values: + - name: "YearDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + - name: "LocalStartTime" + value: 12356 + - name: "LocalEndTime" + value: 12345765 + response: + error: INVALID_COMMAND + + - label: "Verify schedule was not modified" + command: "GetYearDaySchedule" + arguments: + values: + - name: "YearDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + response: + values: + - name: "YearDayIndex" + value: 1 + - name: "UserIndex" + value: 1 + - name: "Status" + value: 0x0 + - name: "LocalStartTime" + value: 12345 + - name: "LocalEndTime" + value: 12345689 + # # Excercise ClearWeekDay schedules with invalid parameters # From 6b0d364b0543b10da76f40bcfe3f5ede92343c37 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 28 Jun 2024 17:33:14 +0000 Subject: [PATCH 2/2] Restyled by clang-format --- .../door-lock-server/door-lock-server.cpp | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp index a6dde6b8ff265f..4fa7c81ccb5b9c 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -998,9 +998,8 @@ void DoorLockServer::setWeekDayScheduleCommandHandler(chip::app::CommandHandler EmberAfPluginDoorLockUserInfo user; if (!emberAfPluginDoorLockGetUser(endpointId, userIndex, user)) { - ChipLogError(Zcl, - "[SetWeekDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", - endpointId, userIndex); + ChipLogError(Zcl, "[SetWeekDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", endpointId, + userIndex); commandObj->AddStatus(commandPath, Status::Failure); return; } @@ -1078,13 +1077,11 @@ void DoorLockServer::setWeekDayScheduleCommandHandler(chip::app::CommandHandler endpointId, weekDayIndex, userIndex, daysMask.Raw(), startHour, startMinute, endHour, endMinute); // Update LastModifiedFabricIndex of user - if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, - user.userUniqueId, user.userStatus, user.userType, user.credentialRule, - user.credentials.data(), user.credentials.size())) + if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, user.userUniqueId, + user.userStatus, user.userType, user.credentialRule, user.credentials.data(), + user.credentials.size())) { - ChipLogError( - Zcl, - "[SetWeekDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", + ChipLogError(Zcl, "[SetWeekDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", endpointId, fabricIdx, userIndex); } @@ -1255,9 +1252,8 @@ void DoorLockServer::setYearDayScheduleCommandHandler(chip::app::CommandHandler EmberAfPluginDoorLockUserInfo user; if (!emberAfPluginDoorLockGetUser(endpointId, userIndex, user)) { - ChipLogError(Zcl, - "[SetYearDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", - endpointId, userIndex); + ChipLogError(Zcl, "[SetYearDaySchedule] Unable to get the user - internal error [endpointId=%d,userIndex=%d]", endpointId, + userIndex); commandObj->AddStatus(commandPath, Status::Failure); return; } @@ -1311,13 +1307,11 @@ void DoorLockServer::setYearDayScheduleCommandHandler(chip::app::CommandHandler endpointId, yearDayIndex, userIndex, localStartTime, localEndTime); // Update LastModifiedFabricIndex of user - if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, - user.userUniqueId, user.userStatus, user.userType, user.credentialRule, - user.credentials.data(), user.credentials.size())) + if (!emberAfPluginDoorLockSetUser(endpointId, userIndex, user.createdBy, fabricIdx, user.userName, user.userUniqueId, + user.userStatus, user.userType, user.credentialRule, user.credentials.data(), + user.credentials.size())) { - ChipLogError( - Zcl, - "[SetYearDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", + ChipLogError(Zcl, "[SetYearDaySchedule] Unable to update user - internal error [endpointId=%d,fabricIndex=%d,userIndex=%d]", endpointId, fabricIdx, userIndex); }