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

Fix Signature R and S Elements Size Checks in ECDSA_sign_hash(). #18629

Conversation

emargolis
Copy link
Contributor

Problem

The implementation was erroneously using || instead of &&.
The OpenSSL implementation updated the verify that "size <= kP256_FE_Length" instead of "==".

In the OpenSSL implementation the current check was never performed because the first
condition was always true "(r != nullptr)".

Change overview

fixed

Testing

existing tests

The implementation was erroneously using || instead of &&.
The OpenSSL implementation updated the verify that "size <= kP256_FE_Length" instead of "==".

In the OpenSSL implementation the current check was never performed because the first
condition was always true "(r != nullptr)".
@github-actions
Copy link

github-actions bot commented May 19, 2022

PR #18629: Size comparison from 1165152 to cef10e4

Increases (10 builds for linux)
platform target config section 1165152 cef10e4 change % change
linux all-clusters-app debug (read only) 2751049 2751273 224 0.0
.text 2335986 2336210 224 0.0
bridge-app debug+rpc (read only) 2033457 2033681 224 0.0
.text 1707330 1707554 224 0.0
chip-tool debug (read only) 9303893 9304117 224 0.0
.text 7496533 7496757 224 0.0
lighting-app debug+rpc (read only) 2327937 2328161 224 0.0
.text 1973570 1973794 224 0.0
lock-app debug (read only) 2241401 2241625 224 0.0
.text 1882818 1883042 224 0.0
ota-provider-app debug (read only) 2063393 2063617 224 0.0
.text 1725586 1725810 224 0.0
ota-requestor-app debug (read only) 2092313 2092537 224 0.0
.text 1757010 1757234 224 0.0
shell debug (read only) 2566809 2567017 208 0.0
.text 2184146 2184354 208 0.0
tv-app debug (read only) 2853257 2853465 208 0.0
.text 2450130 2450338 208 0.0
tv-casting-app debug (read only) 5416393 5416601 208 0.0
.text 4717330 4717538 208 0.0
Decreases (4 builds for esp32, telink)
platform target config section 1165152 cef10e4 change % change
esp32 all-clusters-app c3devkit (read only) 1000788 1000784 -4 -0.0
.flash.text 1000788 1000784 -4 -0.0
m5stack (read only) 1055991 1055979 -12 -0.0
.flash.text 1050607 1050595 -12 -0.0
telink light-switch-app tlsr9518adk80d (read/write) 782436 782428 -8 -0.0
text 553338 553334 -4 -0.0
lighting-app tlsr9518adk80d text 570104 570100 -4 -0.0
Full report (37 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 1165152 cef10e4 change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 667083 667083 0 0.0
(read/write) 183596 183596 0 0.0
.bss 73660 73660 0 0.0
.data 3404 3404 0 0.0
.rodata 100051 100051 0 0.0
.text 566804 566804 0 0.0
lock-ftd LP_CC2652R7 (read only) 677079 677079 0 0.0
(read/write) 165576 165576 0 0.0
.bss 72692 72692 0 0.0
.data 3236 3236 0 0.0
.rodata 94831 94831 0 0.0
.text 581764 581764 0 0.0
lock-mtd LP_CC2652R7 (read only) 625903 625903 0 0.0
(read/write) 145524 145524 0 0.0
.bss 68428 68428 0 0.0
.data 3236 3236 0 0.0
.rodata 94719 94719 0 0.0
.text 530692 530692 0 0.0
pump-app LP_CC2652R7 (read only) 659307 659307 0 0.0
(read/write) 184628 184628 0 0.0
.bss 72948 72948 0 0.0
.data 3268 3268 0 0.0
.rodata 79347 79347 0 0.0
.text 579480 579480 0 0.0
pump-controller-app LP_CC2652R7 (read only) 653819 653819 0 0.0
(read/write) 189924 189924 0 0.0
.bss 73012 73012 0 0.0
.data 3232 3232 0 0.0
.rodata 83459 83459 0 0.0
.text 569880 569880 0 0.0
shell LP_CC2652R7 (read only) 660150 660150 0 0.0
(read/write) 186088 186088 0 0.0
.bss 76020 76020 0 0.0
.data 3408 3408 0 0.0
.rodata 97006 97006 0 0.0
.text 562920 562920 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 625094 625094 0 0.0
.app_xip_area 528708 528708 0 0.0
.bss 79028 79028 0 0.0
.data 708 708 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 628034 628034 0 0.0
.app_xip_area 533104 533104 0 0.0
.bss 77604 77604 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570858 570858 0 0.0
.app_xip_area 466212 466212 0 0.0
.bss 87024 87024 0 0.0
.data 584 584 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 917500 917500 0 0.0
(read/write) 133244 133244 0 0.0
.bss 131184 131184 0 0.0
.data 2060 2060 0 0.0
.text 917492 917492 0 0.0
BRD4161A+rpc (read only) 951672 951672 0 0.0
(read/write) 149928 149928 0 0.0
.bss 147664 147664 0 0.0
.data 2264 2264 0 0.0
.text 951664 951664 0 0.0
BRD4161A+rs911x (read only) 791012 791012 0 0.0
(read/write) 129512 129512 0 0.0
.bss 127444 127444 0 0.0
.data 2068 2068 0 0.0
.text 791004 791004 0 0.0
lock-app BRD4161A+wf200 (read only) 947056 947056 0 0.0
(read/write) 123996 123996 0 0.0
.bss 121972 121972 0 0.0
.data 2024 2024 0 0.0
.text 947048 947048 0 0.0
window-app BRD4161A (read only) 897612 897612 0 0.0
(read/write) 133304 133304 0 0.0
.bss 131256 131256 0 0.0
.data 2048 2048 0 0.0
.text 897604 897604 0 0.0
esp32 all-clusters-app c3devkit (read only) 1000788 1000784 -4 -0.0
(read/write) 1477874 1477874 0 0.0
.dram0.bss 68216 68216 0 0.0
.dram0.data 14624 14624 0 0.0
.flash.rodata 209600 209600 0 0.0
.flash.text 1000788 1000784 -4 -0.0
.iram0.text 62954 62954 0 0.0
m5stack (read only) 1055991 1055979 -12 -0.0
(read/write) 479868 479868 0 0.0
.dram0.bss 73736 73736 0 0.0
.dram0.data 34200 34200 0 0.0
.flash.rodata 239936 239936 0 0.0
.flash.text 1050607 1050595 -12 -0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 682572 682572 0 0.0
.bss 80224 80224 0 0.0
.data 2016 2016 0 0.0
.text 598628 598628 0 0.0
lock k32w061+release (read/write) 729132 729132 0 0.0
.bss 80656 80656 0 0.0
.data 1976 1976 0 0.0
.text 644796 644796 0 0.0
linux all-clusters-app debug (read only) 2751049 2751273 224 0.0
(read/write) 175936 175936 0 0.0
.bss 85088 85088 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 82616 82616 0 0.0
.dynamic 608 608 0 0.0
.got 4496 4496 0 0.0
.init 27 27 0 0.0
.init_array 1016 1016 0 0.0
.rodata 240797 240797 0 0.0
.text 2335986 2336210 224 0.0
bridge-app debug+rpc (read only) 2033457 2033681 224 0.0
(read/write) 147736 147736 0 0.0
.bss 72864 72864 0 0.0
.data 3936 3936 0 0.0
.data.rel.ro 65352 65352 0 0.0
.dynamic 592 592 0 0.0
.got 4272 4272 0 0.0
.init 27 27 0 0.0
.init_array 688 688 0 0.0
.rodata 169129 169129 0 0.0
.text 1707330 1707554 224 0.0
chip-tool debug (read only) 9303893 9304117 224 0.0
(read/write) 579128 579128 0 0.0
.bss 23936 23936 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 547752 547752 0 0.0
.dynamic 624 624 0 0.0
.got 5000 5000 0 0.0
.init 27 27 0 0.0
.init_array 656 656 0 0.0
.rodata 480573 480573 0 0.0
.text 7496533 7496757 224 0.0
chip-tool-no-interactive-ipv6only arm64 (read only) 9075876 9075876 0 0.0
(read/write) 645249 645249 0 0.0
.bss 42225 42225 0 0.0
.data 1192 1192 0 0.0
.data.rel.ro 583000 583000 0 0.0
.dynamic 560 560 0 0.0
.got 14984 14984 0 0.0
.init 24 24 0 0.0
.init_array 184 184 0 0.0
.rodata 445508 445508 0 0.0
.text 7168516 7168516 0 0.0
lighting-app debug+rpc (read only) 2327937 2328161 224 0.0
(read/write) 153152 153152 0 0.0
.bss 74656 74656 0 0.0
.data 2048 2048 0 0.0
.data.rel.ro 70696 70696 0 0.0
.dynamic 608 608 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 792 792 0 0.0
.rodata 188137 188137 0 0.0
.text 1973570 1973794 224 0.0
lock-app debug (read only) 2241401 2241625 224 0.0
(read/write) 147864 147864 0 0.0
.bss 73344 73344 0 0.0
.data 1568 1568 0 0.0
.data.rel.ro 67256 67256 0 0.0
.dynamic 592 592 0 0.0
.got 4336 4336 0 0.0
.init 27 27 0 0.0
.init_array 752 752 0 0.0
.rodata 198745 198745 0 0.0
.text 1882818 1883042 224 0.0
ota-provider-app debug (read only) 2063393 2063617 224 0.0
(read/write) 140944 140944 0 0.0
.bss 72800 72800 0 0.0
.data 1768 1768 0 0.0
.data.rel.ro 60568 60568 0 0.0
.dynamic 608 608 0 0.0
.got 4504 4504 0 0.0
.init 27 27 0 0.0
.init_array 648 648 0 0.0
.rodata 179360 179360 0 0.0
.text 1725586 1725810 224 0.0
ota-requestor-app debug (read only) 2092313 2092537 224 0.0
(read/write) 143752 143752 0 0.0
.bss 73472 73472 0 0.0
.data 1992 1992 0 0.0
.data.rel.ro 62632 62632 0 0.0
.dynamic 592 592 0 0.0
.got 4344 4344 0 0.0
.init 27 27 0 0.0
.init_array 672 672 0 0.0
.rodata 175392 175392 0 0.0
.text 1757010 1757234 224 0.0
shell debug (read only) 2566809 2567017 208 0.0
(read/write) 199800 199800 0 0.0
.bss 115784 115784 0 0.0
.data 1376 1376 0 0.0
.data.rel.ro 76880 76880 0 0.0
.dynamic 608 608 0 0.0
.got 4192 4192 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 221778 221778 0 0.0
.text 2184146 2184354 208 0.0
thermostat-no-ble arm64 (read only) 2353756 2353756 0 0.0
(read/write) 176305 176305 0 0.0
.bss 87617 87617 0 0.0
.data 1520 1520 0 0.0
.data.rel.ro 79360 79360 0 0.0
.dynamic 560 560 0 0.0
.got 4768 4768 0 0.0
.init 24 24 0 0.0
.init_array 376 376 0 0.0
.rodata 147356 147356 0 0.0
.text 1977536 1977536 0 0.0
tv-app debug (read only) 2853257 2853465 208 0.0
(read/write) 278784 278784 0 0.0
.bss 190776 190776 0 0.0
.data 4672 4672 0 0.0
.data.rel.ro 77064 77064 0 0.0
.dynamic 592 592 0 0.0
.got 4720 4720 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 221073 221073 0 0.0
.text 2450130 2450338 208 0.0
tv-casting-app debug (read only) 5416393 5416601 208 0.0
(read/write) 225024 225024 0 0.0
.bss 78360 78360 0 0.0
.data 2368 2368 0 0.0
.data.rel.ro 138064 138064 0 0.0
.dynamic 608 608 0 0.0
.got 4728 4728 0 0.0
.init 27 27 0 0.0
.init_array 864 864 0 0.0
.rodata 339713 339713 0 0.0
.text 4717330 4717538 208 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2420320 2420320 0 0.0
.bss 202668 202668 0 0.0
.data 5872 5872 0 0.0
.text 1382964 1382964 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1180659 1180659 0 0.0
bss 138388 138388 0 0.0
rodata 152780 152780 0 0.0
text 810636 810636 0 0.0
p6 all-clusters-app default (read/write) 2538600 2538600 0 0.0
.bss 136184 136184 0 0.0
.data 2808 2808 0 0.0
.text 1496864 1496864 0 0.0
light-app default (read/write) 2425048 2425048 0 0.0
.bss 129488 129488 0 0.0
.data 2608 2608 0 0.0
.text 1383312 1383312 0 0.0
lock-app default (read/write) 2435752 2435752 0 0.0
.bss 129304 129304 0 0.0
.data 2568 2568 0 0.0
.text 1394016 1394016 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 782436 782428 -8 -0.0
bss 70616 70616 0 0.0
noinit 40416 40416 0 0.0
text 553338 553334 -4 -0.0
lighting-app tlsr9518adk80d (read/write) 802508 802508 0 0.0
bss 70872 70872 0 0.0
noinit 40416 40416 0 0.0
text 570104 570100 -4 -0.0

@bzbarsky-apple bzbarsky-apple merged commit a1e7fa3 into project-chip:master May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants