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.

Sign up for GitHub

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

Jump to bottom

Stop using a fixed salt when opening commissioning windows. #16645

Merged
merged 3 commits into from
Mar 28, 2022
Merged

Stop using a fixed salt when opening commissioning windows. #16645

merged 3 commits into from
Mar 28, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

Fixes #10586

Problem

We're using a hardcoded salt value, which is not the right thing.

Change overview

Add a way for consumers to pass in a salt (if they want to reuse a single onboarding payload for multiple devices) or have it be randomly generated.

Testing

Verified that chip-tool pairing open-commissioning-window works and I can then commission into a different fabric using the resulting onboarding payload.

@github-actions
Copy link

github-actions bot commented Mar 24, 2022 •
edited
Loading

PR #16645: Size comparison from 710b9d8 to 348c0e1

Increases (2 builds for linux)
platform target config section 710b9d8 348c0e1 change % change
linux chip-tool debug (read only) 10214669 10215261 592 0.0
.text 8911333 8911925 592 0.0
chip-tool-ipv6only arm64 (read only) 9833460 9833876 416 0.0
.text 8286020 8286452 432 0.0
Decreases (1 build for linux)
platform target config section 710b9d8 348c0e1 change % change
linux chip-tool-ipv6only arm64 .rodata 493964 493948 -16 -0.0
Full report (25 builds for cyw30739, efr32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 710b9d8 348c0e1 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 603634 603634 0 0.0
.app_xip_area 510804 510804 0 0.0
.bss 75576 75576 0 0.0
.data 604 604 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 561422 561422 0 0.0
.app_xip_area 470120 470120 0 0.0
.bss 74080 74080 0 0.0
.data 568 568 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 573522 573522 0 0.0
.app_xip_area 472572 472572 0 0.0
.bss 83408 83408 0 0.0
.data 508 508 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 923536 923536 0 0.0
(read/write) 128680 128680 0 0.0
.bss 126680 126680 0 0.0
.data 1996 1996 0 0.0
.text 923528 923528 0 0.0
BRD4161A+rpc (read only) 952344 952344 0 0.0
(read/write) 144632 144632 0 0.0
.bss 142456 142456 0 0.0
.data 2176 2176 0 0.0
.text 952336 952336 0 0.0
window-app BRD4161A (read only) 853340 853340 0 0.0
(read/write) 126648 126648 0 0.0
.bss 124776 124776 0 0.0
.data 1872 1872 0 0.0
.text 853332 853332 0 0.0
k32w light k32w061+release (read/write) 701776 701776 0 0.0
.bss 77568 77568 0 0.0
.data 1876 1876 0 0.0
.text 616532 616532 0 0.0
lock k32w061+release (read/write) 701228 701228 0 0.0
.bss 77544 77544 0 0.0
.data 1916 1916 0 0.0
.text 615968 615968 0 0.0
linux all-clusters-app debug (read only) 2484001 2484001 0 0.0
(read/write) 143344 143344 0 0.0
.bss 57408 57408 0 0.0
.data 1280 1280 0 0.0
.data.rel.ro 78920 78920 0 0.0
.dynamic 592 592 0 0.0
.got 4176 4176 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 212101 212101 0 0.0
.text 2108178 2108178 0 0.0
bridge-app debug+rpc (read only) 1756261 1756261 0 0.0
(read/write) 89552 89552 0 0.0
.bss 44552 44552 0 0.0
.data 1984 1984 0 0.0
.data.rel.ro 37936 37936 0 0.0
.dynamic 592 592 0 0.0
.got 3920 3920 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 144140 144140 0 0.0
.text 1496885 1496885 0 0.0
chip-tool debug (read only) 10214669 10215261 592 0.0
(read/write) 355720 355720 0 0.0
.bss 22496 22496 0 0.0
.data 1072 1072 0 0.0
.data.rel.ro 326056 326056 0 0.0
.dynamic 608 608 0 0.0
.got 4816 4816 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 518805 518805 0 0.0
.text 8911333 8911925 592 0.0
chip-tool-ipv6only arm64 (read only) 9833460 9833876 416 0.0
(read/write) 473825 473825 0 0.0
.bss 40769 40769 0 0.0
.data 1144 1144 0 0.0
.data.rel.ro 372688 372688 0 0.0
.dynamic 560 560 0 0.0
.got 55424 55424 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 493964 493948 -16 -0.0
.text 8286020 8286452 432 0.0
door-lock-app debug (read only) 2008225 2008225 0 0.0
(read/write) 116896 116896 0 0.0
.bss 47680 47680 0 0.0
.data 992 992 0 0.0
.data.rel.ro 62840 62840 0 0.0
.dynamic 592 592 0 0.0
.got 4120 4120 0 0.0
.init 27 27 0 0.0
.init_array 664 664 0 0.0
.rodata 180860 180860 0 0.0
.text 1677682 1677682 0 0.0
lighting-app debug+rpc (read only) 2182169 2182169 0 0.0
(read/write) 123888 123888 0 0.0
.bss 48960 48960 0 0.0
.data 1472 1472 0 0.0
.data.rel.ro 67928 67928 0 0.0
.dynamic 608 608 0 0.0
.got 4168 4168 0 0.0
.init 27 27 0 0.0
.init_array 744 744 0 0.0
.rodata 174876 174876 0 0.0
.text 1848226 1848226 0 0.0
ota-provider-app debug (read only) 1949241 1949241 0 0.0
(read/write) 112624 112624 0 0.0
.bss 47552 47552 0 0.0
.data 1256 1256 0 0.0
.data.rel.ro 58184 58184 0 0.0
.dynamic 608 608 0 0.0
.got 4376 4376 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 166571 166571 0 0.0
.text 1630930 1630930 0 0.0
ota-requestor-app debug (read only) 1976017 1976017 0 0.0
(read/write) 115880 115880 0 0.0
.bss 48544 48544 0 0.0
.data 1480 1480 0 0.0
.data.rel.ro 60408 60408 0 0.0
.dynamic 592 592 0 0.0
.got 4184 4184 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 162532 162532 0 0.0
.text 1660834 1660834 0 0.0
shell debug (read only) 2432809 2432809 0 0.0
(read/write) 147400 147400 0 0.0
.bss 67336 67336 0 0.0
.data 800 800 0 0.0
.data.rel.ro 73568 73568 0 0.0
.dynamic 592 592 0 0.0
.got 4152 4152 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 209586 209586 0 0.0
.text 2066354 2066354 0 0.0
thermostat-no-ble arm64 (read only) 2267020 2267020 0 0.0
(read/write) 148385 148385 0 0.0
.bss 62833 62833 0 0.0
.data 1040 1040 0 0.0
.data.rel.ro 77000 77000 0 0.0
.dynamic 560 560 0 0.0
.got 4504 4504 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 139692 139692 0 0.0
.text 1903920 1903920 0 0.0
tv-app debug (read only) 2681625 2681625 0 0.0
(read/write) 247712 247712 0 0.0
.bss 164352 164352 0 0.0
.data 3136 3136 0 0.0
.data.rel.ro 74160 74160 0 0.0
.dynamic 592 592 0 0.0
.got 4552 4552 0 0.0
.init 27 27 0 0.0
.init_array 888 888 0 0.0
.rodata 205931 205931 0 0.0
.text 2300802 2300802 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2353868 2353868 0 0.0
.bss 184572 184572 0 0.0
.data 5760 5760 0 0.0
.text 1316468 1316468 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1140095 1140095 0 0.0
bss 142512 142512 0 0.0
rodata 141872 141872 0 0.0
text 780868 780868 0 0.0
p6 all-clusters-app default (read/write) 2495088 2495088 0 0.0
.bss 117992 117992 0 0.0
.data 2640 2640 0 0.0
.text 1453352 1453352 0 0.0
light-app default (read/write) 2397464 2397464 0 0.0
.bss 111464 111464 0 0.0
.data 2496 2496 0 0.0
.text 1355728 1355728 0 0.0
lock-app default (read/write) 2360992 2360992 0 0.0
.bss 111208 111208 0 0.0
.data 2456 2456 0 0.0
.text 1319256 1319256 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 896686 896686 0 0.0
bss 87356 87356 0 0.0
noinit 37160 37160 0 0.0
text 634102 634102 0 0.0

@Damian-Nordic Damian-Nordic merged commit 467c696 into project-chip:master Mar 28, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-window-opener-salt branch March 28, 2022 12:25
rochaferraz pushed a commit to rochaferraz/connectedhomeip that referenced this pull request Mar 31, 2022
@bzbarsky-apple @Damian-Nordic @rochaferraz
Stop using a fixed salt when opening commissioning windows. (project-…
d94e712 
…chip#16645)

* Stop using a fixed salt when opening commissioning windows.

Fixes project-chip#10586

* Address review comment.

* Apply suggestions from code review to fix bug in salt size checking.

Co-authored-by: Damian Królik <[email protected]>

Co-authored-by: Damian Królik <[email protected]>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@bzbarsky-apple @Damian-Nordic @andrei-menzopol
Stop using a fixed salt when opening commissioning windows. (project-…
cbff04c 
…chip#16645)

* Stop using a fixed salt when opening commissioning windows.

Fixes project-chip#10586

* Address review comment.

* Apply suggestions from code review to fix bug in salt size checking.

Co-authored-by: Damian Królik <[email protected]>

Co-authored-by: Damian Królik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@anush-apple anush-apple Awaiting requested review from anush-apple

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@kghost kghost Awaiting requested review from kghost

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@lzgrablic02 lzgrablic02 Awaiting requested review from lzgrablic02

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tecimovic tecimovic Awaiting requested review from tecimovic

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

@woody-apple woody-apple Awaiting requested review from woody-apple

Assignees
No one assigned
Labels
controller darwin examples review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Salt used for commissioning window should be randomly generated
4 participants
@bzbarsky-apple @tcarmelveilleux @woody-apple @Damian-Nordic