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

Add support for client to override device attestation procedure failure while using default device commissioner #16681

Closed
carricdsilva-apple opened this issue Mar 25, 2022 · 0 comments · Fixed by #17028
Assignees

Comments

@carricdsilva-apple
Copy link
Contributor

Problem

  • Current behavior: While using the default device commissioner and the default device attestation verifier if a device attestation error occurs. The default device attestation verifier informs the commissioner via OnAttestationInformationVerification callback. However the default device commissioner acts on this error and fails commissioning. The client does not have an opportunity to override attestation failure are proceed with commissioning.

Proposed Solution

Add a mechanism to the default device commissioner to inform the client of device attestation procedure failure. Based on clients response the device commissioner should fail or proceed with commissioning.

Also need an Objective-C mechanism to bubble up the attestation failure to the Objective-C client of CHIPDeviceController. The Objective-C client of CHIPDeviceController would respond with user's decision.

anush-apple added a commit to anush-apple/connectedhomeip that referenced this issue Apr 4, 2022
…t/user input

Fixes project-chip#16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.
woody-apple pushed a commit to anush-apple/connectedhomeip that referenced this issue Apr 5, 2022
…t/user input

Fixes project-chip#16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.
anush-apple added a commit to anush-apple/connectedhomeip that referenced this issue Apr 6, 2022
…t/user input

Fixes project-chip#16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.
bzbarsky-apple added a commit that referenced this issue Apr 7, 2022
…t/user (#17028)

* Added mechanism to override device attestation failure based on client/user input

Fixes #16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Fixed linker error for chip-tool-darwin

* Restyled by gn

* Changed CHIPDeviceAttestationDelegate delegate call to match Obj-C API conventions

* Restyled by clang-format

* Switched to passing DeviceProxy* in DA delegate methods

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* Restyled by whitespace

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this issue Apr 14, 2022
…t/user (project-chip#17028)

* Added mechanism to override device attestation failure based on client/user input

Fixes project-chip#16681

Change overview

Added delegates that can be optionally set by the client of the SDK.
When the device attestation delegate is set and when a DA failure is
encountered during commissioning, the delegate is invoked letting the
client decide on the behavior to either ignore the error and continue
commissioning or fail the commissioning. The arm failsafe timer can also
be adjusted by the client to handle any delays due to user input.

Testing

Flashed an M5 with the changes. Used the iOS CHIPTool with the changes
and verified no regression in current commissioning process when the
delegate is either setup or not.
Modified the kTestPaaRoots array in DefaultDeviceAttestationVerifier.cpp
to be empty causing DA failure. Verified the dialog is presented to the
user and that commissioning succeeds with error is ignored and fails
when the user declines to proceed.

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Fixed linker error for chip-tool-darwin

* Restyled by gn

* Changed CHIPDeviceAttestationDelegate delegate call to match Obj-C API conventions

* Restyled by clang-format

* Switched to passing DeviceProxy* in DA delegate methods

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* Restyled by whitespace

* Restyled by clang-format

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants