From 3b92283b7f83fb8ccad14ce1407d423c0ab249ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Fri, 1 Jul 2022 05:08:22 +0200 Subject: [PATCH] [door-lock] Validate OperatingMode attribute (#20046) Signed-off-by: Damian Krolik --- .../door-lock-server/door-lock-server.cpp | 34 +++++--- .../door-lock-server/door-lock-server.h | 4 +- src/app/tests/suites/DL_LockUnlock.yaml | 18 ++++ .../chip-tool/zap-generated/test/Commands.h | 38 +++++++- .../zap-generated/test/Commands.h | 87 ++++++++++++++++++- 5 files changed, 161 insertions(+), 20 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 d2a00bab7ea9b9..9b2482adf6e1cd 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -3094,6 +3094,14 @@ void DoorLockServer::clearHolidaySchedule(chip::app::CommandHandler * commandObj emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); } +bool DoorLockServer::RemoteOperationEnabled(chip::EndpointId endpointId) const +{ + DlOperatingMode mode; + + return GetAttribute(endpointId, Attributes::OperatingMode::Id, Attributes::OperatingMode::Get, mode) && + mode != DlOperatingMode::kPrivacy && mode != DlOperatingMode::kNoRemoteLockUnlock; +} + bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, DlLockOperationType opType, RemoteLockOpHandler opHandler, const Optional & pinCode) @@ -3107,19 +3115,21 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma uint16_t pinUserIdx = 0; uint16_t pinCredIdx = 0; bool credentialsOk = false; - bool operationOk = false; + bool success = false; + + VerifyOrExit(RemoteOperationEnabled(endpoint), reason = DlOperationError::kUnspecified); // appclusters.pdf 5.3.4.1: // When the PINCode field is provided an invalid PIN will count towards the WrongCodeEntryLimit and the - // UserCodeTemporaryDisableTime will be triggered if the WrongCodeEntryLimit is exceeded. The lock SHALL ignore any attempts to - // lock/unlock the door until the UserCodeTemporaryDisableTime expires. + // UserCodeTemporaryDisableTime will be triggered if the WrongCodeEntryLimit is exceeded. The lock SHALL ignore any attempts + // to lock/unlock the door until the UserCodeTemporaryDisableTime expires. // TODO: check whether UserCodeTemporaryDisableTime expired or not. if (pinCode.HasValue()) { // appclusters.pdf 5.3.4.1: - // If the PINCode field is provided, the door lock SHALL verify PINCode before granting access regardless of the value of - // RequirePINForRemoteOperation attribute. + // If the PINCode field is provided, the door lock SHALL verify PINCode before granting access regardless of the value + // of RequirePINForRemoteOperation attribute. VerifyOrExit(SupportsPIN(endpoint) && SupportsUSR(endpoint), emberAfDoorLockClusterPrintln( "PIN code is supplied while USR/PIN features are disabled. Exiting [endpoint=%d, lock_op=%d]", endpoint, @@ -3140,9 +3150,9 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma bool requirePin = false; // appclusters.pdf 5.3.4.1: - // If the RequirePINForRemoteOperation attribute is True then PINCode field SHALL be provided and the door lock SHALL NOT - // grant access if it is not provided. This attribute exists when COTA and PIN features are both enabled. Otherwise we - // assume PIN to be OK. + // If the RequirePINForRemoteOperation attribute is True then PINCode field SHALL be provided and the door lock SHALL + // NOT grant access if it is not provided. This attribute exists when COTA and PIN features are both enabled. Otherwise + // we assume PIN to be OK. if (SupportsCredentialsOTA(endpoint) && SupportsPIN(endpoint)) { auto status = Attributes::RequirePINforRemoteOperation::Get(endpoint, &requirePin); @@ -3161,15 +3171,13 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma }); // credentials check succeeded, try to lock/unlock door - operationOk = opHandler(endpoint, pinCode, reason); - VerifyOrExit(operationOk, /* reason is set by the above call */); + success = opHandler(endpoint, pinCode, reason); + VerifyOrExit(success, /* reason is set by the above call */); // door locked, set cluster attribute VerifyOrDie(SetLockState(endpoint, newLockState, DlOperationSource::kRemote)); exit: - bool success = credentialsOk && operationOk; - // Send command response emberAfSendImmediateDefaultResponse(success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE); @@ -3256,7 +3264,7 @@ void DoorLockServer::SendEvent(chip::EndpointId endpointId, T & event) template bool DoorLockServer::GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId, - EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) + EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) const { EmberAfStatus status = getFn(endpointId, &value); bool success = (EMBER_ZCL_STATUS_SUCCESS == status); diff --git a/src/app/clusters/door-lock-server/door-lock-server.h b/src/app/clusters/door-lock-server/door-lock-server.h index efe8a0ace57ae3..ac0cce099e6a56 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.h +++ b/src/app/clusters/door-lock-server/door-lock-server.h @@ -313,6 +313,8 @@ class DoorLockServer void clearHolidaySchedule(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, uint8_t holidayIndex); + bool RemoteOperationEnabled(chip::EndpointId endpointId) const; + /** * @brief Common handler for LockDoor, UnlockDoor, UnlockWithTimeout commands * @@ -378,7 +380,7 @@ class DoorLockServer */ template bool GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId, - EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value); + EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) const; /** * @brief Set generic attribute value diff --git a/src/app/tests/suites/DL_LockUnlock.yaml b/src/app/tests/suites/DL_LockUnlock.yaml index cacad862b725b4..08ff372ee6b45e 100644 --- a/src/app/tests/suites/DL_LockUnlock.yaml +++ b/src/app/tests/suites/DL_LockUnlock.yaml @@ -134,7 +134,25 @@ tests: response: value: 1 + - label: "Set OperatingMode to NoRemoteLockUnlock" + command: "writeAttribute" + attribute: "OperatingMode" + arguments: + value: 3 + + - label: "Try to unlock the door when OperatingMode is NoRemoteLockUnlock" + command: "LockDoor" + timedInteractionTimeoutMs: 10000 + response: + error: FAILURE + # Clean-up + - label: "Set OperatingMode to Normal" + command: "writeAttribute" + attribute: "OperatingMode" + arguments: + value: 0 + - label: "Clean the created credential" command: "ClearCredential" timedInteractionTimeoutMs: 10000 diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 3506efe95327e7..c4bf101e02c23e 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -56578,7 +56578,7 @@ class DL_UsersAndCredentialsSuite : public TestCommand class DL_LockUnlockSuite : public TestCommand { public: - DL_LockUnlockSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("DL_LockUnlock", 15, credsIssuerConfig) + DL_LockUnlockSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("DL_LockUnlock", 18, credsIssuerConfig) { AddArgument("nodeId", 0, UINT64_MAX, &mNodeId); AddArgument("cluster", &mCluster); @@ -56704,6 +56704,15 @@ class DL_LockUnlockSuite : public TestCommand case 14: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 15: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_FAILURE)); + break; + case 16: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 17: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; default: LogErrorOnFailure(ContinueOnChipMainThread(CHIP_ERROR_INVALID_ARGUMENT)); } @@ -56837,7 +56846,32 @@ class DL_LockUnlockSuite : public TestCommand chip::NullOptional); } case 14: { - LogStep(14, "Clean the created credential"); + LogStep(14, "Set OperatingMode to NoRemoteLockUnlock"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::DlOperatingMode value; + value = static_cast(3); + return WriteAttribute(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Attributes::OperatingMode::Id, value, + chip::NullOptional, chip::NullOptional); + } + case 15: { + LogStep(15, "Try to unlock the door when OperatingMode is NoRemoteLockUnlock"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::Commands::LockDoor::Type value; + return SendCommand(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Commands::LockDoor::Id, value, + chip::Optional(10000), chip::NullOptional + + ); + } + case 16: { + LogStep(16, "Set OperatingMode to Normal"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::DlOperatingMode value; + value = static_cast(0); + return WriteAttribute(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Attributes::OperatingMode::Id, value, + chip::NullOptional, chip::NullOptional); + } + case 17: { + LogStep(17, "Clean the created credential"); ListFreer listFreer; chip::app::Clusters::DoorLock::Commands::ClearCredential::Type value; value.credential.SetNonNull(); diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 17f5842c402c73..d882221b974457 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -93256,8 +93256,20 @@ class DL_LockUnlock : public TestCommandBridge { err = TestVerifyThatLockStateAttributeValueIsSetToLocked_13(); break; case 14: - ChipLogProgress(chipTool, " ***** Test Step 14 : Clean the created credential\n"); - err = TestCleanTheCreatedCredential_14(); + ChipLogProgress(chipTool, " ***** Test Step 14 : Set OperatingMode to NoRemoteLockUnlock\n"); + err = TestSetOperatingModeToNoRemoteLockUnlock_14(); + break; + case 15: + ChipLogProgress(chipTool, " ***** Test Step 15 : Try to unlock the door when OperatingMode is NoRemoteLockUnlock\n"); + err = TestTryToUnlockTheDoorWhenOperatingModeIsNoRemoteLockUnlock_15(); + break; + case 16: + ChipLogProgress(chipTool, " ***** Test Step 16 : Set OperatingMode to Normal\n"); + err = TestSetOperatingModeToNormal_16(); + break; + case 17: + ChipLogProgress(chipTool, " ***** Test Step 17 : Clean the created credential\n"); + err = TestCleanTheCreatedCredential_17(); break; } @@ -93315,6 +93327,15 @@ class DL_LockUnlock : public TestCommandBridge { case 14: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 15: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_FAILURE)); + break; + case 16: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 17: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; } // Go on to the next test. @@ -93328,7 +93349,7 @@ class DL_LockUnlock : public TestCommandBridge { private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 15; + const uint16_t mTestCount = 18; chip::Optional mNodeId; chip::Optional mCluster; @@ -93642,7 +93663,65 @@ class DL_LockUnlock : public TestCommandBridge { return CHIP_NO_ERROR; } - CHIP_ERROR TestCleanTheCreatedCredential_14() + CHIP_ERROR TestSetOperatingModeToNoRemoteLockUnlock_14() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + id operatingModeArgument; + operatingModeArgument = [NSNumber numberWithUnsignedChar:3U]; + [cluster writeAttributeOperatingModeWithValue:operatingModeArgument + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Set OperatingMode to NoRemoteLockUnlock Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); + + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestTryToUnlockTheDoorWhenOperatingModeIsNoRemoteLockUnlock_15() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + __auto_type * params = [[CHIPDoorLockClusterLockDoorParams alloc] init]; + [cluster lockDoorWithParams:params + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Try to unlock the door when OperatingMode is NoRemoteLockUnlock Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, EMBER_ZCL_STATUS_FAILURE)); + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestSetOperatingModeToNormal_16() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + id operatingModeArgument; + operatingModeArgument = [NSNumber numberWithUnsignedChar:0U]; + [cluster writeAttributeOperatingModeWithValue:operatingModeArgument + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Set OperatingMode to Normal Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); + + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestCleanTheCreatedCredential_17() { CHIPDevice * device = GetDevice("alpha"); CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue];