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

Added mechanism to override device attestation failure based on client/user #17028

Conversation

anush-apple
Copy link
Contributor

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.

@CLAassistant
Copy link

CLAassistant commented Apr 4, 2022

CLA assistant check
All committers have signed the CLA.

@woody-apple
Copy link
Contributor

/rebase

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

PR #17028: Size comparison from f69dfe8 to 84b2e31

Increases (19 builds for cc13x2_26x2, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section f69dfe8 84b2e31 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 670511 670735 224 0.0
.bss 81752 81760 8 0.0
.rodata 79975 80039 64 0.1
.text 590056 590216 160 0.0
lock-mtd LP_CC2652R7 (read only) 619639 619863 224 0.0
(read/write) 154500 154508 8 0.0
.bss 77480 77488 8 0.0
.rodata 79863 79927 64 0.1
.text 539288 539448 160 0.0
pump-app LP_CC2652R7 (read only) 689983 690199 216 0.0
.bss 82152 82168 16 0.0
.rodata 82063 82119 56 0.1
.text 607436 607596 160 0.0
pump-controller-app LP_CC2652R7 (read only) 672207 672439 232 0.0
.bss 81888 81896 8 0.0
.rodata 78399 78455 56 0.1
.text 593324 593500 176 0.0
efr32 lighting-app BRD4161A (read only) 919164 919420 256 0.0
(read/write) 129784 129792 8 0.0
.bss 127792 127800 8 0.0
.text 919156 919412 256 0.0
BRD4161A+rpc (read only) 947060 947316 256 0.0
(read/write) 145732 145740 8 0.0
.bss 143560 143568 8 0.0
.text 947052 947308 256 0.0
window-app BRD4161A (read only) 854380 854636 256 0.0
(read/write) 127808 127816 8 0.0
.bss 125936 125944 8 0.0
.text 854372 854628 256 0.0
esp32 all-clusters-app c3devkit (read only) 988094 988250 156 0.0
(read/write) 1460842 1460922 80 0.0
.dram0.bss 62944 62952 8 0.0
.flash.rodata 198224 198288 64 0.0
.flash.text 988094 988250 156 0.0
m5stack (read only) 1040455 1040599 144 0.0
(read/write) 461840 461912 72 0.0
.dram0.bss 68472 68480 8 0.0
.flash.rodata 227168 227232 64 0.0
.flash.text 1035071 1035215 144 0.0
k32w light k32w061+release (read/write) 710388 710620 232 0.0
.bss 77952 77968 16 0.0
.text 624732 624948 216 0.0
lock k32w061+release (read/write) 709784 710024 240 0.0
.bss 77952 77960 8 0.0
.text 624088 624320 232 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 10083572 10085044 1472 0.0
(read/write) 486977 487009 32 0.0
.data.rel.ro 384712 384728 16 0.0
.got 57008 57024 16 0.0
.rodata 508404 508740 336 0.1
.text 8484324 8485364 1040 0.0
thermostat-no-ble arm64 (read only) 2324428 2325020 592 0.0
(read/write) 149441 149521 80 0.1
.bss 62945 62977 32 0.1
.data.rel.ro 77704 77760 56 0.1
.rodata 143308 143340 32 0.0
.text 1955408 1955840 432 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2362700 2362956 256 0.0
.bss 185036 185052 16 0.0
.text 1325300 1325556 256 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1151231 1151471 240 0.0
bss 143060 143072 12 0.0
rodata 143592 143656 64 0.0
text 789708 789880 172 0.0
p6 all-clusters-app default (read/write) 2507528 2507784 256 0.0
.bss 118472 118480 8 0.0
.text 1465792 1466048 256 0.0
light-app default (read/write) 2408704 2408960 256 0.0
.bss 111928 111944 16 0.0
.text 1366968 1367224 256 0.0
lock-app default (read/write) 2372352 2372608 256 0.0
.bss 111672 111688 16 0.0
.text 1330616 1330872 256 0.0
telink lighting-app tlsr9518adk80d (read/write) 794808 795080 272 0.0
bss 70288 70300 12 0.0
text 564354 564544 190 0.0
Decreases (3 builds for cc13x2_26x2)
platform target config section f69dfe8 84b2e31 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 181200 180984 -216 -0.1
pump-app LP_CC2652R7 (read/write) 162896 162696 -200 -0.1
pump-controller-app LP_CC2652R7 (read/write) 180408 180184 -224 -0.1
Full report (19 builds for cc13x2_26x2, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section f69dfe8 84b2e31 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 670511 670735 224 0.0
(read/write) 181200 180984 -216 -0.1
.bss 81752 81760 8 0.0
.data 3164 3164 0 0.0
.rodata 79975 80039 64 0.1
.text 590056 590216 160 0.0
lock-mtd LP_CC2652R7 (read only) 619639 619863 224 0.0
(read/write) 154500 154508 8 0.0
.bss 77480 77488 8 0.0
.data 3164 3164 0 0.0
.rodata 79863 79927 64 0.1
.text 539288 539448 160 0.0
pump-app LP_CC2652R7 (read only) 689983 690199 216 0.0
(read/write) 162896 162696 -200 -0.1
.bss 82152 82168 16 0.0
.data 3196 3196 0 0.0
.rodata 82063 82119 56 0.1
.text 607436 607596 160 0.0
pump-controller-app LP_CC2652R7 (read only) 672207 672439 232 0.0
(read/write) 180408 180184 -224 -0.1
.bss 81888 81896 8 0.0
.data 3160 3160 0 0.0
.rodata 78399 78455 56 0.1
.text 593324 593500 176 0.0
efr32 lighting-app BRD4161A (read only) 919164 919420 256 0.0
(read/write) 129784 129792 8 0.0
.bss 127792 127800 8 0.0
.data 1992 1992 0 0.0
.text 919156 919412 256 0.0
BRD4161A+rpc (read only) 947060 947316 256 0.0
(read/write) 145732 145740 8 0.0
.bss 143560 143568 8 0.0
.data 2172 2172 0 0.0
.text 947052 947308 256 0.0
window-app BRD4161A (read only) 854380 854636 256 0.0
(read/write) 127808 127816 8 0.0
.bss 125936 125944 8 0.0
.data 1872 1872 0 0.0
.text 854372 854628 256 0.0
esp32 all-clusters-app c3devkit (read only) 988094 988250 156 0.0
(read/write) 1460842 1460922 80 0.0
.dram0.bss 62944 62952 8 0.0
.dram0.data 14196 14196 0 0.0
.flash.rodata 198224 198288 64 0.0
.flash.text 988094 988250 156 0.0
.iram0.text 62572 62572 0 0.0
m5stack (read only) 1040455 1040599 144 0.0
(read/write) 461840 461912 72 0.0
.dram0.bss 68472 68480 8 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 227168 227232 64 0.0
.flash.text 1035071 1035215 144 0.0
.iram0.text 123415 123415 0 0.0
k32w light k32w061+release (read/write) 710388 710620 232 0.0
.bss 77952 77968 16 0.0
.data 1904 1904 0 0.0
.text 624732 624948 216 0.0
lock k32w061+release (read/write) 709784 710024 240 0.0
.bss 77952 77960 8 0.0
.data 1944 1944 0 0.0
.text 624088 624320 232 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 10083572 10085044 1472 0.0
(read/write) 486977 487009 32 0.0
.bss 40337 40337 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 384712 384728 16 0.0
.dynamic 560 560 0 0.0
.got 57008 57024 16 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 508404 508740 336 0.1
.text 8484324 8485364 1040 0.0
thermostat-no-ble arm64 (read only) 2324428 2325020 592 0.0
(read/write) 149441 149521 80 0.1
.bss 62945 62977 32 0.1
.data 1136 1136 0 0.0
.data.rel.ro 77704 77760 56 0.1
.dynamic 560 560 0 0.0
.got 4632 4632 0 0.0
.init 24 24 0 0.0
.init_array 368 368 0 0.0
.rodata 143308 143340 32 0.0
.text 1955408 1955840 432 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2362700 2362956 256 0.0
.bss 185036 185052 16 0.0
.data 5784 5784 0 0.0
.text 1325300 1325556 256 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1151231 1151471 240 0.0
bss 143060 143072 12 0.0
rodata 143592 143656 64 0.0
text 789708 789880 172 0.0
p6 all-clusters-app default (read/write) 2507528 2507784 256 0.0
.bss 118472 118480 8 0.0
.data 2672 2672 0 0.0
.text 1465792 1466048 256 0.0
light-app default (read/write) 2408704 2408960 256 0.0
.bss 111928 111944 16 0.0
.data 2528 2528 0 0.0
.text 1366968 1367224 256 0.0
lock-app default (read/write) 2372352 2372608 256 0.0
.bss 111672 111688 16 0.0
.data 2488 2488 0 0.0
.text 1330616 1330872 256 0.0
telink lighting-app tlsr9518adk80d (read/write) 794808 795080 272 0.0
bss 70288 70300 12 0.0
noinit 40416 40416 0 0.0
text 564354 564544 190 0.0

src/controller/CHIPDeviceController.cpp Show resolved Hide resolved
src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved
src/darwin/Framework/CHIP/BUILD.gn Show resolved Hide resolved
anush-apple and others added 13 commits April 6, 2022 11:21
…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 anush-apple force-pushed the issue-fix/16681-DA-fail-overrides branch from 624cc28 to 3296ffa Compare April 6, 2022 18:36
@github-actions
Copy link

github-actions bot commented Apr 6, 2022

PR #17028: Size comparison from 6da0db8 to 3296ffa

Increases above 0.2%:

platform target config section 6da0db8 3296ffa change % change
linux tv-app debug .rodata 212523 213259 736 0.3
Increases (3 builds for linux)
platform target config section 6da0db8 3296ffa change % change
linux chip-tool debug (read only) 10675813 10678037 2224 0.0
(read/write) 371640 371672 32 0.0
.data.rel.ro 341584 341600 16 0.0
.rodata 538005 538773 768 0.1
.text 9309013 9310421 1408 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 10278220 10279612 1392 0.0
(read/write) 492033 492065 32 0.0
.data.rel.ro 388704 388720 16 0.0
.got 57360 57376 16 0.0
.rodata 512396 512732 336 0.1
.text 8663044 8664004 960 0.0
tv-app debug (read only) 2787361 2789553 2192 0.1
.data.rel.ro 75360 75376 16 0.0
.rodata 212523 213259 736 0.3
.text 2395362 2396770 1408 0.1
Full report (28 builds for cc13x2_26x2, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 6da0db8 3296ffa change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 674307 674307 0 0.0
(read/write) 177204 177204 0 0.0
.bss 81552 81552 0 0.0
.data 3168 3168 0 0.0
.rodata 80579 80579 0 0.0
.text 593248 593248 0 0.0
lock-mtd LP_CC2652R7 (read only) 623443 623443 0 0.0
(read/write) 154304 154304 0 0.0
.bss 77280 77280 0 0.0
.data 3168 3168 0 0.0
.rodata 80467 80467 0 0.0
.text 542488 542488 0 0.0
pump-app LP_CC2652R7 (read only) 691607 691607 0 0.0
(read/write) 161064 161064 0 0.0
.bss 81944 81944 0 0.0
.data 3200 3200 0 0.0
.rodata 82511 82511 0 0.0
.text 608612 608612 0 0.0
pump-controller-app LP_CC2652R7 (read only) 675979 675979 0 0.0
(read/write) 176436 176436 0 0.0
.bss 81688 81688 0 0.0
.data 3164 3164 0 0.0
.rodata 79011 79011 0 0.0
.text 596484 596484 0 0.0
efr32 lighting-app BRD4161A (read only) 904532 904532 0 0.0
(read/write) 133144 133144 0 0.0
.bss 131152 131152 0 0.0
.data 1992 1992 0 0.0
.text 904524 904524 0 0.0
BRD4161A+rpc (read only) 932420 932420 0 0.0
(read/write) 149092 149092 0 0.0
.bss 146920 146920 0 0.0
.data 2172 2172 0 0.0
.text 932412 932412 0 0.0
window-app BRD4161A (read only) 841868 841868 0 0.0
(read/write) 131204 131204 0 0.0
.bss 129328 129328 0 0.0
.data 1876 1876 0 0.0
.text 841860 841860 0 0.0
esp32 all-clusters-app c3devkit (read only) 991668 991668 0 0.0
(read/write) 1461530 1461530 0 0.0
.dram0.bss 63000 63000 0 0.0
.dram0.data 14260 14260 0 0.0
.flash.rodata 198784 198784 0 0.0
.flash.text 991668 991668 0 0.0
.iram0.text 62572 62572 0 0.0
m5stack (read only) 1044367 1044367 0 0.0
(read/write) 462584 462584 0 0.0
.dram0.bss 68528 68528 0 0.0
.dram0.data 34064 34064 0 0.0
.flash.rodata 227848 227848 0 0.0
.flash.text 1038983 1038983 0 0.0
.iram0.text 123415 123415 0 0.0
k32w light k32w061+release (read/write) 684828 684828 0 0.0
.bss 78240 78240 0 0.0
.data 1988 1988 0 0.0
.text 598800 598800 0 0.0
lock k32w061+release (read/write) 689668 689668 0 0.0
.bss 78816 78816 0 0.0
.data 1948 1948 0 0.0
.text 603104 603104 0 0.0
linux all-clusters-app debug (read only) 2617705 2617705 0 0.0
(read/write) 145384 145384 0 0.0
.bss 57856 57856 0 0.0
.data 1408 1408 0 0.0
.data.rel.ro 80232 80232 0 0.0
.dynamic 592 592 0 0.0
.got 4312 4312 0 0.0
.init 27 27 0 0.0
.init_array 960 960 0 0.0
.rodata 222149 222149 0 0.0
.text 2226866 2226866 0 0.0
bridge-app debug+rpc (read only) 1826845 1826845 0 0.0
(read/write) 90824 90824 0 0.0
.bss 44792 44792 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 38904 38904 0 0.0
.dynamic 592 592 0 0.0
.got 3928 3928 0 0.0
.init 27 27 0 0.0
.init_array 552 552 0 0.0
.rodata 148409 148409 0 0.0
.text 1561157 1561157 0 0.0
chip-tool debug (read only) 10675813 10678037 2224 0.0
(read/write) 371640 371672 32 0.0
.bss 22784 22784 0 0.0
.data 1040 1040 0 0.0
.data.rel.ro 341584 341600 16 0.0
.dynamic 624 624 0 0.0
.got 4928 4928 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 538005 538773 768 0.1
.text 9309013 9310421 1408 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 10278220 10279612 1392 0.0
(read/write) 492033 492065 32 0.0
.bss 41041 41041 0 0.0
.data 1120 1120 0 0.0
.data.rel.ro 388704 388720 16 0.0
.dynamic 560 560 0 0.0
.got 57360 57376 16 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 512396 512732 336 0.1
.text 8663044 8664004 960 0.0
door-lock-app debug (read only) 2098057 2098057 0 0.0
(read/write) 118864 118864 0 0.0
.bss 48128 48128 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 64024 64024 0 0.0
.dynamic 592 592 0 0.0
.got 4256 4256 0 0.0
.init 27 27 0 0.0
.init_array 680 680 0 0.0
.rodata 186441 186441 0 0.0
.text 1757154 1757154 0 0.0
lighting-app debug+rpc (read only) 2277625 2277625 0 0.0
(read/write) 125920 125920 0 0.0
.bss 49472 49472 0 0.0
.data 1600 1600 0 0.0
.data.rel.ro 69160 69160 0 0.0
.dynamic 608 608 0 0.0
.got 4304 4304 0 0.0
.init 27 27 0 0.0
.init_array 760 760 0 0.0
.rodata 180553 180553 0 0.0
.text 1933122 1933122 0 0.0
ota-provider-app debug (read only) 2036033 2036033 0 0.0
(read/write) 114432 114432 0 0.0
.bss 47968 47968 0 0.0
.data 1384 1384 0 0.0
.data.rel.ro 59352 59352 0 0.0
.dynamic 608 608 0 0.0
.got 4456 4456 0 0.0
.init 27 27 0 0.0
.init_array 632 632 0 0.0
.rodata 172291 172291 0 0.0
.text 1708370 1708370 0 0.0
ota-requestor-app debug (read only) 2065617 2065617 0 0.0
(read/write) 117720 117720 0 0.0
.bss 48928 48928 0 0.0
.data 1608 1608 0 0.0
.data.rel.ro 61592 61592 0 0.0
.dynamic 592 592 0 0.0
.got 4296 4296 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 168820 168820 0 0.0
.text 1739650 1739650 0 0.0
shell debug (read only) 2513177 2513177 0 0.0
(read/write) 148912 148912 0 0.0
.bss 67560 67560 0 0.0
.data 848 848 0 0.0
.data.rel.ro 74776 74776 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 213874 213874 0 0.0
.text 2140098 2140098 0 0.0
thermostat-no-ble arm64 (read only) 2344188 2344188 0 0.0
(read/write) 150257 150257 0 0.0
.bss 63169 63169 0 0.0
.data 1136 1136 0 0.0
.data.rel.ro 78184 78184 0 0.0
.dynamic 560 560 0 0.0
.got 4752 4752 0 0.0
.init 24 24 0 0.0
.init_array 368 368 0 0.0
.rodata 144364 144364 0 0.0
.text 1972672 1972672 0 0.0
tv-app debug (read only) 2787361 2789553 2192 0.1
(read/write) 250488 250488 0 0.0
.bss 165576 165576 0 0.0
.data 3360 3360 0 0.0
.data.rel.ro 75360 75376 16 0.0
.dynamic 592 592 0 0.0
.got 4672 4672 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 212523 213259 736 0.3
.text 2395362 2396770 1408 0.1
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2366580 2366580 0 0.0
.bss 185092 185092 0 0.0
.data 5792 5792 0 0.0
.text 1329180 1329180 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1152967 1152967 0 0.0
bss 135604 135604 0 0.0
rodata 145632 145632 0 0.0
text 793156 793156 0 0.0
p6 all-clusters-app default (read/write) 2511992 2511992 0 0.0
.bss 118528 118528 0 0.0
.data 2672 2672 0 0.0
.text 1470256 1470256 0 0.0
light-app default (read/write) 2413168 2413168 0 0.0
.bss 111992 111992 0 0.0
.data 2528 2528 0 0.0
.text 1371432 1371432 0 0.0
lock-app default (read/write) 2376848 2376848 0 0.0
.bss 111736 111736 0 0.0
.data 2488 2488 0 0.0
.text 1335112 1335112 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 798836 798836 0 0.0
bss 70096 70096 0 0.0
noinit 40416 40416 0 0.0
text 567918 567918 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit b06bd8c into project-chip:master Apr 7, 2022
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request 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
6 participants