Validate CSR in separate commissioning step #16913

cecille · 2022-04-01T00:06:11Z

Problem

Validation of the CSR is coupled to NOC generation currently. Separating these steps allows more flexibility for custom CommissioningDelegates. Also adds remaining values required for CSR validation to the GenerateNOCChain command, so the OperationalCertificateIssuer classes can validate the CSR themselves.
Fixes: #11908

Change overview

make Validate CSR a seprate commissioning stage
adds parameters for CSR validation to the GenerateNOCChain command

Testing

Manually commissioned - checked that the ValidateCSR stage was properly entered and returned success.

This will let OperationalCredentialsIssuer impls validate the CSR before generating the NOC chain.

github-actions · 2022-04-01T00:45:26Z

PR #16913: Size comparison from 566cf88 to ec80428

Increases (1 build for linux)

platform	target	config	section	`566cf88`	`ec80428`	change
linux	chip-tool-no-interactive-ipv6only	arm64	(read only)	9972852	9974052	1200
			.rodata	505388	505452	64
			.text	`8405060`	`8406196`	1136

Full report (22 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)

platform	target	config	section	`566cf88`	`ec80428`	change
cc13x2_26x2	lock-ftd	LP_CC2652R7	(read only)	667427	667427	0
			(read/write)	184316	184316	0
			.bss	81784	81784	0
			.data	3132	3132	0
			.rodata	79595	79595	0
			.text	587352	587352	0
	lock-mtd	LP_CC2652R7	(read only)	616555	616555	0
			(read/write)	154500	154500	0
			.bss	77512	77512	0
			.data	3132	3132	0
			.rodata	79475	79475	0
			.text	536592	536592	0
	pump-app	LP_CC2652R7	(read only)	686887	686887	0
			(read/write)	166016	166016	0
			.bss	82176	82176	0
			.data	3164	3164	0
			.rodata	81671	81671	0
			.text	604732	604732	0
	pump-controller-app	LP_CC2652R7	(read only)	669159	669159	0
			(read/write)	183488	183488	0
			.bss	81920	81920	0
			.data	3128	3128	0
			.rodata	78007	78007	0
			.text	590668	590668	0
cyw30739	light	cyw930739m2evb_01	(read/write)	609634	609634	0
			.app_xip_area	516376	516376	0
			.bss	76004	76004	0
			.data	600	600	0
			.rodata	0	0	0
			.text	0	0	0
	lock	cyw930739m2evb_01	(read/write)	567146	567146	0
			.app_xip_area	475424	475424	0
			.bss	74508	74508	0
			.data	564	564	0
			.rodata	0	0	0
			.text	0	0	0
	ota-requestor-no-progress-logging	cyw930739m2evb_01	(read/write)	578926	578926	0
			.app_xip_area	477548	477548	0
			.bss	83836	83836	0
			.data	504	504	0
			.rodata	0	0	0
			.text	112	112	0
efr32	lighting-app	BRD4161A	(read only)	930216	930216	0
			(read/write)	129128	129128	0
			.bss	127136	127136	0
			.data	1992	1992	0
			.text	930208	930208	0
		BRD4161A+rpc	(read only)	959216	959216	0
			(read/write)	145088	145088	0
			.bss	142912	142912	0
			.data	2172	2172	0
			.text	959208	959208	0
	window-app	BRD4161A	(read only)	865512	865512	0
			(read/write)	127136	127136	0
			.bss	125264	125264	0
			.data	1872	1872	0
			.text	865504	865504	0
esp32	all-clusters-app	c3devkit	(read only)	970490	970490	0
			(read/write)	1395138	1395138	0
			.dram0.bss	62456	62456	0
			.dram0.data	14220	14220	0
			.flash.rodata	199544	199544	0
			.flash.text	970490	970490	0
			.iram0.text	62016	62016	0
		m5stack	(read only)	`1026223`	`1026223`	0
			(read/write)	462900	462900	0
			.dram0.bss	67984	67984	0
			.dram0.data	34024	34024	0
			.flash.rodata	229056	229056	0
			.flash.text	1020839	1020839	0
			.iram0.text	123107	123107	0
k32w	light	k32w061+release	(read/write)	707240	707240	0
			.bss	77992	77992	0
			.data	1872	1872	0
			.text	621576	621576	0
	lock	k32w061+release	(read/write)	706488	706488	0
			.bss	77960	77960	0
			.data	1912	1912	0
			.text	620816	620816	0
linux	chip-tool-no-interactive-ipv6only	arm64	(read only)	9972852	9974052	1200
			(read/write)	477329	477329	0
			.bss	40401	40401	0
			.data	1136	1136	0
			.data.rel.ro	375432	375432	0
			.dynamic	560	560	0
			.got	56552	56552	0
			.init	24	24	0
			.init_array	192	192	0
			.rodata	505388	505452	64
			.text	`8405060`	`8406196`	1136
	thermostat-no-ble	arm64	(read only)	2292676	2292676	0
			(read/write)	148657	148657	0
			.bss	62849	62849	0
			.data	1040	1040	0
			.data.rel.ro	77216	77216	0
			.dynamic	560	560	0
			.got	4536	4536	0
			.init	24	24	0
			.init_array	360	360	0
			.rodata	142052	142052	0
			.text	`1926688`	`1926688`	0
mbed	lock-app	CY8CPROTO_062_4343W+release	(read only)	6224	6224	0
			(read/write)	2358540	2358540	0
			.bss	185052	185052	0
			.data	5760	5760	0
			.text	1321140	1321140	0
nrfconnect	all-clusters-app	nrf52840dk_nrf52840	(read/write)	1147827	1147827	0
			bss	143092	143092	0
			rodata	143204	143204	0
			text	786732	786732	0
p6	all-clusters-app	default	(read/write)	`2502872`	`2502872`	0
			.bss	118488	118488	0
			.data	2640	2640	0
			.text	`1461136`	`1461136`	0
	light-app	default	(read/write)	2404360	2404360	0
			.bss	111944	111944	0
			.data	2496	2496	0
			.text	1362624	1362624	0
	lock-app	default	(read/write)	2367968	2367968	0
			.bss	111688	111688	0
			.data	2456	2456	0
			.text	1326232	1326232	0
telink	lighting-app	tlsr9518adk80d	(read/write)	791300	791300	0
			bss	70296	70296	0
			noinit	40416	40416	0
			text	561260	561260	0

src/controller/CHIPDeviceController.cpp

src/controller/ExampleOperationalCredentialsIssuer.cpp

msandstedt

PAA needs to be removed from the noc-chain-generation interface and PAI should be piped through from the commissioner.

Else, looks good. Thanks for cleaning this up.

src/controller/CHIPDeviceController.cpp

src/controller/ExampleOperationalCredentialsIssuer.cpp

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

It's not available.

No one's messing around with these things, may as well.

I accidentally removed it.

I hate myself.

* Vaidate CSR as a separate step * Pass all validataion parameters to GenerateNOCChain This will let OperationalCredentialsIssuer impls validate the CSR before generating the NOC chain. * Restyled by clang-format * Update src/controller/ExampleOperationalCredentialsIssuer.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Remove PAA from GenerateNOCChain command It's not available. * More const is better const No one's messing around with these things, may as well. * Pass PAI to noc chain generation. * Remove stale comment * Bring back namespasing. I accidentally removed it. * Once more. * And again. I hate myself. Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Boris Zbarsky <[email protected]>

cecille added 2 commits March 31, 2022 17:10

Vaidate CSR as a separate step

c1e78d8

Pass all validataion parameters to GenerateNOCChain

5d956b9

This will let OperationalCredentialsIssuer impls validate the CSR before generating the NOC chain.

boring-cyborg bot added controller darwin labels Apr 1, 2022

pullapprove bot requested review from woody-apple, xylophone21 and yufengwangca April 1, 2022 00:06

pullapprove bot added the review - pending label Apr 1, 2022

restyled-io bot mentioned this pull request Apr 1, 2022

Restyle Validate CSR in separate commissioning step #16914

Closed

Restyled by clang-format

ec80428

tcarmelveilleux reviewed Apr 1, 2022

View reviewed changes

src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved

tcarmelveilleux reviewed Apr 1, 2022

View reviewed changes

src/controller/ExampleOperationalCredentialsIssuer.cpp Outdated Show resolved Hide resolved

tcarmelveilleux reviewed Apr 1, 2022

View reviewed changes

src/controller/ExampleOperationalCredentialsIssuer.cpp Outdated Show resolved Hide resolved

tcarmelveilleux approved these changes Apr 1, 2022

View reviewed changes

msandstedt approved these changes Apr 1, 2022

View reviewed changes

pullapprove bot added review - approved and removed review - pending labels Apr 1, 2022

bzbarsky-apple approved these changes Apr 1, 2022

View reviewed changes

src/controller/CHIPDeviceController.cpp Outdated Show resolved Hide resolved

src/controller/ExampleOperationalCredentialsIssuer.cpp Outdated Show resolved Hide resolved

Update src/controller/ExampleOperationalCredentialsIssuer.cpp

c7ddb67

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

woody-apple approved these changes Apr 1, 2022

View reviewed changes

cecille added 4 commits April 1, 2022 12:28

Remove PAA from GenerateNOCChain command

e49684e

It's not available.

More const is better const

90bca14

No one's messing around with these things, may as well.

Pass PAI to noc chain generation.

7027f82

Remove stale comment

c2d84b9

msandstedt approved these changes Apr 1, 2022

View reviewed changes

cecille added 3 commits April 3, 2022 12:03

Bring back namespasing.

58f7d83

I accidentally removed it.

Once more.

7fa7c2f

And again.

d2bdb31

I hate myself.

cecille merged commit a208c3d into project-chip:master Apr 4, 2022

cecille deleted the CSR_check_in_separate_step branch April 4, 2022 12:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate CSR in separate commissioning step #16913

Validate CSR in separate commissioning step #16913

cecille commented Apr 1, 2022

github-actions bot commented Apr 1, 2022 •

edited

Loading

msandstedt left a comment

Validate CSR in separate commissioning step #16913

Validate CSR in separate commissioning step #16913

Conversation

cecille commented Apr 1, 2022

Problem

Change overview

Testing

github-actions bot commented Apr 1, 2022 • edited Loading

msandstedt left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 1, 2022 •

edited

Loading