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

Added additional AttestationVerificationResult values #12655

Merged
merged 2 commits into from
Dec 9, 2021
Merged

Added additional AttestationVerificationResult values #12655

merged 2 commits into from
Dec 9, 2021

Conversation

vijs
Copy link
Collaborator

@vijs vijs commented Dec 7, 2021

Problem

Current VerifyAttestationInformation does simplified processing that will return AttestationVerificationResult::kDacSignatureInvalid for any failure of the certificate chain validation, event if not related to DAC signature. This is because there is only a bool result used and we do not expose the internal outcome of the certificate chain validation that would expose the true error, such as validity period expiration, malformed certs, missing extensions.

Reference: Issue #11918

Change overview

  • Added new entries to AttestationVerificationResult enum.
  • Added new method that maps CertificateChainValidationResult to AttestationVerificationResult (DAC Verifier Example).
  • Added new enum (CertificateChainValidationResult) to show detailed error codes during Certificate Chain Validation.
  • Updated ValidateCertificateChain method w/ new parameter - CertificateChainValidationResult enum.
  • Made ICA mandatory in ValidateCertificateChain method.
  • Updated both OpenSSL/mbedTLS methods to give a more detailed failure result using new parameter.
  • Removed test case that didn't use ICA certificate.
  • Updated test case to use new CertificateChainValidationResult parameter.

Testing

  • Matter Unit Tests

@boring-cyborg boring-cyborg bot added the crypto label Dec 7, 2021
@github-actions
Copy link

github-actions bot commented Dec 7, 2021 •
edited
Loading

PR #12655: Size comparison from 21392ef to a4d8e10

Increases (2 builds for linux)
platform target config section 21392ef a4d8e10 change % change
linux chip-tool debug (read only) 6661157 6661749 592 0.0
.text 5918213 5918805 592 0.0
tv-app debug (read only) 2049985 2050577 592 0.0
.text 1719714 1720306 592 0.0
Full report (39 builds for efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 21392ef a4d8e10 change % change
efr32 lighting-app BRD4161A (read only) 796540 796540 0 0.0
(read/write) 120740 120740 0 0.0
.bss 118912 118912 0 0.0
.data 1828 1828 0 0.0
.text 796532 796532 0 0.0
BRD4161A+rpc (read only) 824508 824508 0 0.0
(read/write) 139048 139048 0 0.0
.bss 137112 137112 0 0.0
.data 1936 1936 0 0.0
.text 824500 824500 0 0.0
lock-app BRD4161A (read only) 771224 771224 0 0.0
(read/write) 118680 118680 0 0.0
.bss 116888 116888 0 0.0
.data 1788 1788 0 0.0
.text 771216 771216 0 0.0
window-app BRD4161A (read only) 773496 773496 0 0.0
(read/write) 118880 118880 0 0.0
.bss 117088 117088 0 0.0
.data 1792 1792 0 0.0
.text 773488 773488 0 0.0
esp32 all-clusters-app c3devkit (read only) 856134 856134 0 0.0
(read/write) 1298882 1298882 0 0.0
.dram0.bss 61424 61424 0 0.0
.dram0.data 14132 14132 0 0.0
.flash.rodata 170288 170288 0 0.0
.flash.text 856134 856134 0 0.0
.iram0.text 62076 62076 0 0.0
m5stack (read only) 959387 959387 0 0.0
(read/write) 443208 443208 0 0.0
.dram0.bss 68776 68776 0 0.0
.dram0.data 34056 34056 0 0.0
.flash.rodata 208588 208588 0 0.0
.flash.text 954003 954003 0 0.0
.iram0.text 123451 123451 0 0.0
k32w lighting-app k32w061+se05x+release (read/write) 731592 731592 0 0.0
.bss 79408 79408 0 0.0
.data 1860 1860 0 0.0
.text 644524 644524 0 0.0
lock-app k32w061+debug (read/write) 622308 622308 0 0.0
.bss 70072 70072 0 0.0
.data 1828 1828 0 0.0
.text 544608 544608 0 0.0
shell k32w061+debug (read/write) 688216 688216 0 0.0
.bss 81720 81720 0 0.0
.data 1800 1800 0 0.0
.text 598896 598896 0 0.0
linux all-clusters-app debug (read only) 1902129 1902129 0 0.0
(read/write) 128712 128712 0 0.0
.bss 54064 54064 0 0.0
.data 1216 1216 0 0.0
.data.rel.ro 67968 67968 0 0.0
.dynamic 592 592 0 0.0
.got 4120 4120 0 0.0
.init 27 27 0 0.0
.init_array 720 720 0 0.0
.rodata 154933 154933 0 0.0
.text 1601666 1601666 0 0.0
bridge-app debug+rpc (read only) 1450789 1450789 0 0.0
(read/write) 74904 74904 0 0.0
.bss 36464 36464 0 0.0
.data 1728 1728 0 0.0
.data.rel.ro 31632 31632 0 0.0
.dynamic 592 592 0 0.0
.got 3992 3992 0 0.0
.init 27 27 0 0.0
.init_array 480 480 0 0.0
.rodata 122692 122692 0 0.0
.text 1223029 1223029 0 0.0
chip-tool debug (read only) 6661157 6661749 592 0.0
(read/write) 201512 201512 0 0.0
.bss 34728 34728 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 160072 160072 0 0.0
.dynamic 592 592 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 568 568 0 0.0
.rodata 334449 334449 0 0.0
.text 5918213 5918805 592 0.0
lighting-app debug+rpc (read only) 1736569 1736569 0 0.0
(read/write) 107968 107968 0 0.0
.bss 42160 42160 0 0.0
.data 1280 1280 0 0.0
.data.rel.ro 59136 59136 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 143281 143281 0 0.0
.text 1450450 1450450 0 0.0
ota-provider-app debug (read only) 1408089 1408089 0 0.0
(read/write) 73104 73104 0 0.0
.bss 39040 39040 0 0.0
.data 928 928 0 0.0
.data.rel.ro 27944 27944 0 0.0
.dynamic 592 592 0 0.0
.got 4056 4056 0 0.0
.init 27 27 0 0.0
.init_array 520 520 0 0.0
.rodata 124078 124078 0 0.0
.text 1179154 1179154 0 0.0
ota-requestor-app debug (read only) 1512865 1512865 0 0.0
(read/write) 78120 78120 0 0.0
.bss 42208 42208 0 0.0
.data 992 992 0 0.0
.data.rel.ro 29704 29704 0 0.0
.dynamic 592 592 0 0.0
.got 4064 4064 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 136823 136823 0 0.0
.text 1267282 1267282 0 0.0
shell debug (read only) 823577 823577 0 0.0
(read/write) 60616 60616 0 0.0
.bss 16936 16936 0 0.0
.data 256 256 0 0.0
.data.rel.ro 38936 38936 0 0.0
.dynamic 592 592 0 0.0
.got 3520 3520 0 0.0
.init 27 27 0 0.0
.init_array 344 344 0 0.0
.rodata 84754 84754 0 0.0
.text 631986 631986 0 0.0
tv-app debug (read only) 2049985 2050577 592 0.0
(read/write) 320288 320288 0 0.0
.bss 247480 247480 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 64208 64208 0 0.0
.dynamic 592 592 0 0.0
.got 4456 4456 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 175436 175436 0 0.0
.text 1719714 1720306 592 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2316272 2316272 0 0.0
.bss 184596 184596 0 0.0
.data 5240 5240 0 0.0
.heap 846608 846608 0 0.0
.text 1278848 1278848 0 0.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2299968 2299968 0 0.0
.bss 173400 173400 0 0.0
.data 5496 5496 0 0.0
.heap 857552 857552 0 0.0
.text 1262568 1262568 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2273072 2273072 0 0.0
.bss 172440 172440 0 0.0
.data 5496 5496 0 0.0
.heap 858512 858512 0 0.0
.text 1235672 1235672 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1140008 1140008 0 0.0
.bss 11756 11756 0 0.0
.data 4376 4376 0 0.0
.heap 1020312 1020312 0 0.0
.text 103392 103392 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2047472 2047472 0 0.0
.bss 156732 156732 0 0.0
.data 4872 4872 0 0.0
.heap 874840 874840 0 0.0
.text 1010072 1010072 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 894699 894699 0 0.0
bss 113852 113852 0 0.0
rodata 99676 99676 0 0.0
text 605624 605624 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 858075 858075 0 0.0
bss 110200 110200 0 0.0
rodata 91036 91036 0 0.0
text 580620 580620 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 820598 820598 0 0.0
bss 115224 115224 0 0.0
rodata 94936 94936 0 0.0
text 535952 535952 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 867283 867283 0 0.0
bss 111112 111112 0 0.0
rodata 95796 95796 0 0.0
text 584996 584996 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 793434 793434 0 0.0
bss 112524 112524 0 0.0
rodata 91084 91084 0 0.0
text 515416 515416 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 497463 497463 0 0.0
bss 51820 51820 0 0.0
rodata 45852 45852 0 0.0
text 339492 339492 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 872019 872019 0 0.0
bss 111024 111024 0 0.0
rodata 97148 97148 0 0.0
text 588396 588396 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 865243 865243 0 0.0
bss 110904 110904 0 0.0
rodata 95284 95284 0 0.0
text 583608 583608 0 0.0
shell nrf52840dk_nrf52840 (read/write) 779939 779939 0 0.0
bss 109696 109696 0 0.0
rodata 73792 73792 0 0.0
text 521948 521948 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 694966 694966 0 0.0
bss 110680 110680 0 0.0
rodata 68432 68432 0 0.0
text 442548 442548 0 0.0
p6 all-clusters-app default (read/write) 2352208 2352208 0 0.0
.bss 110868 110868 0 0.0
.data 2520 2520 0 0.0
.heap 919952 919952 0 0.0
.text 1310472 1310472 0 0.0
light-app default (read/write) 2283256 2283256 0 0.0
.bss 98632 98632 0 0.0
.data 2336 2336 0 0.0
.heap 932376 932376 0 0.0
.text 1241520 1241520 0 0.0
lock-app default (read/write) 2259424 2259424 0 0.0
.bss 97512 97512 0 0.0
.data 2296 2296 0 0.0
.heap 933536 933536 0 0.0
.text 1217688 1217688 0 0.0
qpg lighting-app qpg6100+debug (read only) 513300 513300 0 0.0
(read/write) 122332 122332 0 0.0
.bss 80360 80360 0 0.0
.data 964 964 0 0.0
.text 507980 507980 0 0.0
lock-app qpg6100+debug (read only) 487604 487604 0 0.0
(read/write) 122336 122336 0 0.0
.bss 79496 79496 0 0.0
.data 920 920 0 0.0
.text 482284 482284 0 0.0
persistent-storage-app qpg6100+debug (read only) 108224 108224 0 0.0
(read/write) 122332 122332 0 0.0
.bss 36696 36696 0 0.0
.data 292 292 0 0.0
.text 102904 102904 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 798086 798086 0 0.0
bss 80428 80428 0 0.0
noinit 37160 37160 0 0.0
text 557696 557696 0 0.0

@vijs
Initialized result to error at the beginning of ValidateCertificateCh…
45c4216 
…ain and added failure scenarios to the unit tests.
@vijs vijs merged commit b6297f3 into project-chip:master Dec 9, 2021
@vijs vijs deleted the feature/detailed_da_report branch December 9, 2021 17:56
@vijs vijs mentioned this pull request Dec 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@woody-apple woody-apple woody-apple approved these changes

@hawk248 hawk248 hawk248 approved these changes

@andy31415 andy31415 andy31415 approved these changes

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

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

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

@cecille cecille Awaiting requested review from cecille

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

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

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@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

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@LuDuda LuDuda Awaiting requested review from LuDuda

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

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@pan-apple pan-apple Awaiting requested review from pan-apple

@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

@turon turon Awaiting requested review from turon

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

@wbschiller wbschiller Awaiting requested review from wbschiller

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

Assignees
No one assigned
Labels
crypto review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vijs @tcarmelveilleux @andy31415 @hawk248 @woody-apple