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

Bugfix: Dont read non-existing attributes for lock/unlock commands #19373

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 12 additions & 6 deletions src/app/clusters/door-lock-server/door-lock-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3143,13 +3143,19 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma
}
else
{
// 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.
bool requirePin = false;
VerifyOrExit(GetAttribute(endpoint, Attributes::RequirePINforRemoteOperation::Id,
Attributes::RequirePINforRemoteOperation::Get, requirePin),
/* credentialsOk is false here */);

// 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 (SupportsCredentialsOTA(endpoint) && SupportsPIN(endpoint))
{
auto status = Attributes::RequirePINforRemoteOperation::Get(endpoint, &requirePin);
VerifyOrExit(
EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE == status || EMBER_ZCL_STATUS_SUCCESS == status,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if COTA and PIN are both enabled, shouldn't we fail out if the then-required attribute does not exist?

Also, the attribute is allowed to be present even if those features are disabled, I believe. @CamWms can you confirm?

@Morozov-5F

ChipLogError(Zcl, "Failed to read Require PIN For Remote Operation attribute, status=0x%x", to_underlying(status)));
}
credentialsOk = !requirePin;
}

Expand Down
5 changes: 5 additions & 0 deletions src/app/clusters/door-lock-server/door-lock-server.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ class DoorLockServer

inline bool SupportsSchedules(chip::EndpointId endpointId) { return HasFeature(endpointId, DoorLockFeature::kAccessSchedules); }

inline bool SupportsCredentialsOTA(chip::EndpointId endpointId)
{
return HasFeature(endpointId, DoorLockFeature::kCredentialsOTA);
}

inline bool SupportsUSR(chip::EndpointId endpointId)
{
// appclusters, 5.2.2: USR feature has conformance [PIN | RID | FGP | FACE]
Expand Down
20 changes: 20 additions & 0 deletions src/app/tests/suites/DL_LockUnlock.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@ tests:
- name: "nodeId"
value: nodeId

- label: "Try to unlock the door without PIN"
command: "UnlockDoor"
timedInteractionTimeoutMs: 10000

- label: "Verify that lock state attribute value is set to Unlocked"
command: "readAttribute"
attribute: "LockState"
response:
value: 2

- label: "Try to unlock the door without a PIN"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This label is wrong...

command: "LockDoor"
timedInteractionTimeoutMs: 10000

- label: "Verify that lock state attribute value is set to Locked"
command: "readAttribute"
attribute: "LockState"
response:
value: 1

- label: "Create new PIN credential and lock/unlock user"
command: "SetCredential"
timedInteractionTimeoutMs: 10000
Expand Down
110 changes: 81 additions & 29 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading