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

Refactor message encryption and pairing code #4401

Merged
merged 8 commits into from
Jan 20, 2021

Conversation

pan-apple
Copy link
Contributor

@pan-apple pan-apple commented Jan 16, 2021

Problem

The packet encryption/decryption code is duplicated in SecureSessionMgr and RendezvousSession classes. The code can use some refactoring.

Summary of Changes

  • Extract message encryption/decryption to its own functions.
    These can be reused for pairing, and CASE based session setup.
  • Cleanup pairing and rendezvous code to use the new functions.
  • Use SecureSessionMgr to send encrypted messages generated by Rendezvous process.

- Extract message encryption/decryption to its own functions.
  These can be reused for pairing, and CASE based session setup
- Cleanup pairing and rendezvous code to use the new functions.
@github-actions
Copy link

Size increase report for "esp32-example-build" from 6243e6a

File Section File VM
chip-all-clusters-app.elf .flash.rodata -376 -376
chip-all-clusters-app.elf .flash.text -508 -508
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,25100
.debug_line,0,3068
.debug_abbrev,0,2637
.debug_str,0,440
[Unmapped],0,376
.debug_ranges,0,208
.strtab,0,84
.xt.prop._ZN4chip9Transport19PeerConnectionStateaSEOS1_,0,80
.debug_aranges,0,16
.debug_frame,0,-8
.symtab,0,-32
.xt.lit._ZNK4chip8OptionalINS_9Transport11PeerAddressEE5ValueEv,0,-40
.xt.prop._ZN4chip9Transport15PeerConnectionsILj16ELNS_4Time6SourceE0EE23FindPeerConnectionStateENS_8OptionalIyEEtPNS0_19PeerConnectionStateE,0,-40
.xt.prop._ZNK4chip20RendezvousParameters12IsControllerEv,0,-40
.shstrtab,0,-76
.xt.prop._ZN4chip8OptionalIyEaSERKS1_,0,-76
.debug_loc,0,-341
.flash.rodata,-376,-376
.flash.text,-508,-508


@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 6243e6a

File Section File VM
chip-lighting.elf rodata -352 -352
chip-lighting.elf text -384 -384
chip-lock.elf rodata -352 -352
chip-lock.elf text -384 -384
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lighting.elf and ./pull_artifact/chip-lighting.elf:

sections,vmsize,filesize
.debug_info,0,25758
.debug_line,0,2884
.debug_abbrev,0,2562
.debug_str,0,456
.strtab,0,134
.debug_ranges,0,96
.debug_aranges,0,16
.symtab,0,16
.shstrtab,0,-2
.debug_loc,0,-92
rodata,-352,-352
text,-384,-384

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,25758
.debug_line,0,2884
.debug_abbrev,0,2562
.debug_str,0,456
.strtab,0,134
.debug_ranges,0,96
.debug_aranges,0,16
.symtab,0,16
.shstrtab,0,-2
.debug_loc,0,-92
rodata,-352,-352
text,-384,-384


@pan-apple pan-apple requested a review from kpschoedel January 19, 2021 21:30
@pan-apple
Copy link
Contributor Author

@saurabhst, @jelderton, @BroderickCarlin, any feedback?

src/transport/SecureMessageCodec.cpp Outdated Show resolved Hide resolved
@jelderton jelderton merged commit fad6d05 into project-chip:master Jan 20, 2021
@jelderton
Copy link

Sorry, I may have merged too soon. I just saw that you asked for a couple more people to take a look.

@pan-apple pan-apple deleted the refactor-pairing branch January 20, 2021 20:27
@pan-apple
Copy link
Contributor Author

Sorry, I may have merged too soon. I just saw that you asked for a couple more people to take a look.

@jelderton, that's totally fine. Only one of the requested reviews was needed.

Damian-Nordic added a commit to Damian-Nordic/connectedhomeip that referenced this pull request Feb 8, 2021
Since project-chip#4401 has been merged, the test pairing/secret is
added to the session manager only if rendezvous is bypassed.
However, we would like to keep the possibility to test
nRF Connect examples with and without the full rendezvous
procedure without the need to recompile a project with
different settings.

Add a setting CHIP_DEVICE_CONFIG_USE_TEST_PAIRING which
allows to initialize the session manager with the test
secret even if rendezvous bypass mode is not used.
Damian-Nordic added a commit to Damian-Nordic/connectedhomeip that referenced this pull request Feb 8, 2021
Since project-chip#4401 has been merged, the test pairing/secret is
added to the session manager only if rendezvous is bypassed.
However, we would like to keep the possibility to test
nRF Connect examples with and without the full rendezvous
procedure without the need to recompile a project with
different settings.

Add a setting CHIP_DEVICE_CONFIG_USE_TEST_PAIRING which
allows to initialize the session manager with the test
secret even if rendezvous bypass mode is not used.
woody-apple pushed a commit that referenced this pull request Feb 10, 2021
Since #4401 has been merged, the test pairing/secret is
added to the session manager only if rendezvous is bypassed.
However, we would like to keep the possibility to test
nRF Connect examples with and without the full rendezvous
procedure without the need to recompile a project with
different settings.

Add a setting CHIP_DEVICE_CONFIG_USE_TEST_PAIRING which
allows to initialize the session manager with the test
secret even if rendezvous bypass mode is not used.
LuDuda pushed a commit to LuDuda/connectedhomeip that referenced this pull request Feb 17, 2021
Since project-chip#4401 has been merged, the test pairing/secret is
added to the session manager only if rendezvous is bypassed.
However, we would like to keep the possibility to test
nRF Connect examples with and without the full rendezvous
procedure without the need to recompile a project with
different settings.

Add a setting CHIP_DEVICE_CONFIG_USE_TEST_PAIRING which
allows to initialize the session manager with the test
secret even if rendezvous bypass mode is not used.

Signed-off-by: Damian Krolik <[email protected]>
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.

5 participants