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

Ensure that a PASE session doesn't outlive the fail-safe (and vice versa). #19636

Merged
merged 1 commit into from
Jun 15, 2022
Merged

Ensure that a PASE session doesn't outlive the fail-safe (and vice versa). #19636

merged 1 commit into from
Jun 15, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple commented Jun 15, 2022 •
edited
Loading

Per spec, when a fail-safe expires step 1 is:

Terminate any open PASE secure session by clearing any associated Secure
Session Context at the Server.

We were not doing that.

Similarly, per spec if a PASE session is terminated by CloseSession, it should
expire the pending fail-safe, if any. We weren't doing that either.

Problem

See above.

Change overview

Expire the fail-safe when the PASE session is closed, and close the PASE session when the fail-safe expires.

Testing

  1. Compile and run all-clusters-app on Mac.
  2. Compile chip-tool.
  3. chip-tool interactive start
  4. Issue the following commands: 
    pairing code-paseonly 17 749701123365521327694
generalcommissioning arm-fail-safe 0 0 17 0
onoff toggle 17 1

Before these changes, the toggle command was received by the server and the response was received by chip-tool.

After these changes, the toggle command gets dropped with:

[1655317670249] [9204:12201281] CHIP: [IN] Data received on an unknown session (LSID=48053). Dropping it!

Testing the other direction will happen once I fix #16839 and can test CloseSession.

@bzbarsky-apple
Ensure that a PASE session doesn't outlive the fail-safe (and vice ve…
25db7de 
…rsa).

Per spec, when a fail-safe expires step 1 is:

> Terminate any open PASE secure session by clearing any associated Secure
> Session Context at the Server.

We were not doing that.

Similarly, per spec if a PASE session is terminated by CloseSession, it should
expire the pending fail-safe, if any.  We weren't doing that either.
@boring-cyborg boring-cyborg bot added the app label Jun 15, 2022
@github-actions
Copy link

github-actions bot commented Jun 15, 2022 •
edited
Loading

PR #19636: Size comparison from a8b3a07 to 25db7de

Increases (29 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section a8b3a07 25db7de change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 657803 657975 172 0.0
.bss 73708 73732 24 0.0
.rodata 86883 86919 36 0.0
.text 570608 570744 136 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 646303 646483 180 0.0
(read/write) 157260 157284 24 0.0
.bss 72988 73012 24 0.0
.rodata 90159 90195 36 0.0
.text 555824 555968 144 0.0
lock-ftd LP_CC2652R7 (read only) 688639 688827 188 0.0
.bss 70700 70724 24 0.0
.rodata 98911 98947 36 0.0
.text 589244 589396 152 0.0
lock-mtd LP_CC2652R7 (read only) 638047 638235 188 0.0
(read/write) 143832 143856 24 0.0
.bss 66436 66460 24 0.0
.rodata 98791 98827 36 0.0
.text 538764 538916 152 0.0
pump-app LP_CC2652R7 (read only) 669863 670043 180 0.0
.bss 70820 70844 24 0.0
.rodata 87007 87043 36 0.0
.text 582372 582516 144 0.0
pump-controller-app LP_CC2652R7 (read only) 660611 660783 172 0.0
.bss 70948 70972 24 0.0
.rodata 84931 84967 36 0.0
.text 575196 575332 136 0.0
shell LP_CC2652R7 (read only) 687130 687302 172 0.0
.bss 76004 76028 24 0.0
.rodata 109410 109446 36 0.0
.text 577408 577544 136 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 611154 611334 180 0.0
.app_xip_area 467844 468000 156 0.0
.bss 86232 86256 24 0.0
lock cyw930739m2evb_01 (read/write) 608238 608426 188 0.0
.app_xip_area 464784 464948 164 0.0
.bss 86408 86432 24 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 614250 614430 180 0.0
.app_xip_area 471860 472016 156 0.0
.bss 85400 85424 24 0.0
efr32 lighting-app BRD4161A (read only) 920708 921236 528 0.1
(read/write) 132396 132420 24 0.0
.bss 130280 130304 24 0.0
.text 920700 921228 528 0.1
BRD4161A+rpc (read only) 956364 956916 552 0.1
(read/write) 149272 149296 24 0.0
.bss 146952 146976 24 0.0
.text 956356 956908 552 0.1
BRD4161A+rs911x (read only) 795824 796368 544 0.1
(read/write) 128676 128700 24 0.0
.bss 126548 126572 24 0.0
.text 795816 796360 544 0.1
lock-app BRD4161A+wf200 (read only) 963720 963900 180 0.0
(read/write) 129012 129036 24 0.0
.bss 126924 126948 24 0.0
.text 963712 963892 180 0.0
window-app BRD4161A (read only) 905580 906124 544 0.1
(read/write) 132460 132484 24 0.0
.bss 130352 130376 24 0.0
.text 905572 906116 544 0.1
esp32 all-clusters-app c3devkit (read only) 1010748 1010944 196 0.0
(read/write) 1482626 1482682 56 0.0
.dram0.bss 69344 69368 24 0.0
.flash.rodata 213200 213224 24 0.0
.flash.text 1010748 1010944 196 0.0
m5stack (read only) 1065027 1065207 180 0.0
(read/write) 484744 484808 64 0.0
.dram0.bss 74864 74888 24 0.0
.flash.rodata 243708 243748 40 0.0
.flash.text 1059643 1059823 180 0.0
k32w light k32w061+release (read/write) 657352 657556 204 0.0
.bss 68708 68732 24 0.0
.text 580816 580996 180 0.0
lock k32w061+release (read/write) 719008 719196 188 0.0
.bss 69140 69164 24 0.0
.text 642068 642232 164 0.0
linux thermostat-no-ble arm64 (read only) 2561996 2562620 624 0.0
(read/write) 191729 191889 160 0.1
.bss 99393 99457 64 0.1
.data.rel.ro 82560 82656 96 0.1
.text 2160704 2161136 432 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read/write) 2435720 2435888 168 0.0
.bss 208148 208172 24 0.0
.text 1398364 1398532 168 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1196803 1197095 292 0.0
bss 141554 141578 24 0.0
rodata 155420 155456 36 0.0
text 820968 821096 128 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1144835 1145143 308 0.0
bss 140794 140818 24 0.0
rodata 131948 131984 36 0.0
text 793284 793416 132 0.0
p6 all-clusters-app default (read/write) 2550112 2550656 544 0.0
.bss 143344 143368 24 0.0
.text 1508376 1508920 544 0.0
all-clusters-minimal-app default (read/write) 2496056 2496600 544 0.0
.bss 142608 142632 24 0.0
.text 1454320 1454864 544 0.0
light-app default (read/write) 2426936 2427480 544 0.0
.bss 134688 134712 24 0.0
.text 1385200 1385744 544 0.0
lock-app default (read/write) 2447352 2447880 528 0.0
.bss 134512 134536 24 0.0
.text 1405616 1406144 528 0.0
telink light-switch-app tlsr9518adk80d (read/write) 786908 787136 228 0.0
bss 69836 69860 24 0.0
text 557804 557972 168 0.0
lighting-app tlsr9518adk80d (read/write) 806888 807116 228 0.0
bss 70088 70112 24 0.0
text 574496 574664 168 0.0
Decreases (5 builds for cc13x2_26x2)
platform target config section a8b3a07 25db7de change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read/write) 193012 192864 -148 -0.1
lock-ftd LP_CC2652R7 (read/write) 152280 152116 -164 -0.1
pump-app LP_CC2652R7 (read/write) 171944 171788 -156 -0.1
pump-controller-app LP_CC2652R7 (read/write) 181324 181176 -148 -0.1
shell LP_CC2652R7 (read/write) 159180 159032 -148 -0.1
Full report (30 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section a8b3a07 25db7de change % change
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 657803 657975 172 0.0
(read/write) 193012 192864 -148 -0.1
.bss 73708 73732 24 0.0
.data 3380 3380 0 0.0
.rodata 86883 86919 36 0.0
.text 570608 570744 136 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 646303 646483 180 0.0
(read/write) 157260 157284 24 0.0
.bss 72988 73012 24 0.0
.data 3356 3356 0 0.0
.rodata 90159 90195 36 0.0
.text 555824 555968 144 0.0
lock-ftd LP_CC2652R7 (read only) 688639 688827 188 0.0
(read/write) 152280 152116 -164 -0.1
.bss 70700 70724 24 0.0
.data 3280 3280 0 0.0
.rodata 98911 98947 36 0.0
.text 589244 589396 152 0.0
lock-mtd LP_CC2652R7 (read only) 638047 638235 188 0.0
(read/write) 143832 143856 24 0.0
.bss 66436 66460 24 0.0
.data 3280 3280 0 0.0
.rodata 98791 98827 36 0.0
.text 538764 538916 152 0.0
pump-app LP_CC2652R7 (read only) 669863 670043 180 0.0
(read/write) 171944 171788 -156 -0.1
.bss 70820 70844 24 0.0
.data 3316 3316 0 0.0
.rodata 87007 87043 36 0.0
.text 582372 582516 144 0.0
pump-controller-app LP_CC2652R7 (read only) 660611 660783 172 0.0
(read/write) 181324 181176 -148 -0.1
.bss 70948 70972 24 0.0
.data 3276 3276 0 0.0
.rodata 84931 84967 36 0.0
.text 575196 575332 136 0.0
shell LP_CC2652R7 (read only) 687130 687302 172 0.0
(read/write) 159180 159032 -148 -0.1
.bss 76004 76028 24 0.0
.data 3384 3384 0 0.0
.rodata 109410 109446 36 0.0
.text 577408 577544 136 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 611154 611334 180 0.0
.app_xip_area 467844 468000 156 0.0
.bss 86232 86256 24 0.0
.data 764 764 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 608238 608426 188 0.0
.app_xip_area 464784 464948 164 0.0
.bss 86408 86432 24 0.0
.data 732 732 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 614250 614430 180 0.0
.app_xip_area 471860 472016 156 0.0
.bss 85400 85424 24 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 920708 921236 528 0.1
(read/write) 132396 132420 24 0.0
.bss 130280 130304 24 0.0
.data 2116 2116 0 0.0
.text 920700 921228 528 0.1
BRD4161A+rpc (read only) 956364 956916 552 0.1
(read/write) 149272 149296 24 0.0
.bss 146952 146976 24 0.0
.data 2320 2320 0 0.0
.text 956356 956908 552 0.1
BRD4161A+rs911x (read only) 795824 796368 544 0.1
(read/write) 128676 128700 24 0.0
.bss 126548 126572 24 0.0
.data 2124 2124 0 0.0
.text 795816 796360 544 0.1
lock-app BRD4161A+wf200 (read only) 963720 963900 180 0.0
(read/write) 129012 129036 24 0.0
.bss 126924 126948 24 0.0
.data 2088 2088 0 0.0
.text 963712 963892 180 0.0
window-app BRD4161A (read only) 905580 906124 544 0.1
(read/write) 132460 132484 24 0.0
.bss 130352 130376 24 0.0
.data 2108 2108 0 0.0
.text 905572 906116 544 0.1
esp32 all-clusters-app c3devkit (read only) 1010748 1010944 196 0.0
(read/write) 1482626 1482682 56 0.0
.dram0.bss 69344 69368 24 0.0
.dram0.data 14656 14656 0 0.0
.flash.rodata 213200 213224 24 0.0
.flash.text 1010748 1010944 196 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1065027 1065207 180 0.0
(read/write) 484744 484808 64 0.0
.dram0.bss 74864 74888 24 0.0
.dram0.data 34176 34176 0 0.0
.flash.rodata 243708 243748 40 0.0
.flash.text 1059643 1059823 180 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 657352 657556 204 0.0
.bss 68708 68732 24 0.0
.data 2028 2028 0 0.0
.text 580816 580996 180 0.0
lock k32w061+release (read/write) 719008 719196 188 0.0
.bss 69140 69164 24 0.0
.data 2000 2000 0 0.0
.text 642068 642232 164 0.0
linux chip-tool-no-interactive-ipv6only arm64 (read only) 9690572 9690572 0 0.0
(read/write) 646001 646001 0 0.0
.bss 42625 42625 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 585264 585264 0 0.0
.dynamic 528 528 0 0.0
.got 13152 13152 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 466396 466396 0 0.0
.text 7754036 7754036 0 0.0
thermostat-no-ble arm64 (read only) 2561996 2562620 624 0.0
(read/write) 191729 191889 160 0.1
.bss 99393 99457 64 0.1
.data 1688 1688 0 0.0
.data.rel.ro 82560 82656 96 0.1
.dynamic 528 528 0 0.0
.got 5064 5064 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 163004 163004 0 0.0
.text 2160704 2161136 432 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2435720 2435888 168 0.0
.bss 208148 208172 24 0.0
.data 5864 5864 0 0.0
.text 1398364 1398532 168 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1196803 1197095 292 0.0
bss 141554 141578 24 0.0
rodata 155420 155456 36 0.0
text 820968 821096 128 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1144835 1145143 308 0.0
bss 140794 140818 24 0.0
rodata 131948 131984 36 0.0
text 793284 793416 132 0.0
p6 all-clusters-app default (read/write) 2550112 2550656 544 0.0
.bss 143344 143368 24 0.0
.data 2800 2800 0 0.0
.text 1508376 1508920 544 0.0
all-clusters-minimal-app default (read/write) 2496056 2496600 544 0.0
.bss 142608 142632 24 0.0
.data 2776 2776 0 0.0
.text 1454320 1454864 544 0.0
light-app default (read/write) 2426936 2427480 544 0.0
.bss 134688 134712 24 0.0
.data 2624 2624 0 0.0
.text 1385200 1385744 544 0.0
lock-app default (read/write) 2447352 2447880 528 0.0
.bss 134512 134536 24 0.0
.data 2600 2600 0 0.0
.text 1405616 1406144 528 0.0
telink light-switch-app tlsr9518adk80d (read/write) 786908 787136 228 0.0
bss 69836 69860 24 0.0
noinit 40416 40416 0 0.0
text 557804 557972 168 0.0
lighting-app tlsr9518adk80d (read/write) 806888 807116 228 0.0
bss 70088 70112 24 0.0
noinit 40416 40416 0 0.0
text 574496 574664 168 0.0

@tcarmelveilleux tcarmelveilleux merged commit 600f1b9 into project-chip:master Jun 15, 2022
@bzbarsky-apple bzbarsky-apple deleted the close-PASE-when-failsafe-expires branch June 15, 2022 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@krypton36 krypton36 krypton36 approved these changes

@mrjerryjohns mrjerryjohns mrjerryjohns approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

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

@arkq arkq Awaiting requested review from arkq

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

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

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

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

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

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

@gjc13 gjc13 Awaiting requested review from gjc13

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

@hawk248 hawk248 Awaiting requested review from hawk248

@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

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@rgoliver rgoliver Awaiting requested review from rgoliver

@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

@tehampson tehampson Awaiting requested review from tehampson

@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

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

Successfully merging this pull request may close these issues.

Need to implement handling of CloseSession status messages
5 participants
@bzbarsky-apple @tcarmelveilleux @mrjerryjohns @krypton36 @woody-apple