-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Allow platforms to bring their own crypto backend, and implement PSA Crypto and opaque keys for [EFR32] #21415
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
Merged
jmartinez-silabs
merged 27 commits into
project-chip:master
from
stevew817:platform_crypto_implementation
Aug 4, 2022
Merged
Allow platforms to bring their own crypto backend, and implement PSA Crypto and opaque keys for [EFR32] #21415
jmartinez-silabs
merged 27 commits into
project-chip:master
from
stevew817:platform_crypto_implementation
Aug 4, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This saves quite some codespace by allowing to strip dead code dangling from OpenThread's use of mbedtls_pk_parse_key (which, when PKCS5 is enabled, will always include code for parsing password-protected PEM files, but no such files are ever used in the context of Matter)
* Implemented persistence of operational key map and runtime resizing on key map should the setting change and be OTA'ed over. * Changed namespacing to put EFR32 classes under the internal layer * Added 'ConfigValueExists' overload to the config manager which returns the size of an object if it exists.
Moving the override of chip_crypto in case it isn't set into crypto's BUILD.gn instead of crypto.gni means that the toplevel BUILD.gn no longer can see which crypto instance it ends up building with. This was only used to determine whether or not to build chip-cert tool by default. This commit clarifies that by duplicating the exact logic that was backing this into the tools.gni file under a more descriptive variable name. The actual logic probably needs cleaning up, but that would be outside the scope of this PR.
Instead of viewing crypto as a monolithic library, encode it more granular to better allow platform crypto implementations. It now consists of its public headers (which the crypto backends depend on), a static library containing the abstractly-implemented functions, and a set of source sets for each of the crypto backends provided by the main tree.
The EFR32 crypto backend (brought in by setting chip_crypto to platform) now correctly advertises a dependency on the matter crypto PAL. It only needs the intermediate layer of the crypto PAL, not a backend, which is taken care of due to chip_crypto being set to platform.
Use GenerateCertificateSigningRequest instead of replicating the mbedTLS implementation of CSR generation. Since there were no other parts of the app depending on mbedTLS CSR writing functionality, this saved 3.5kB of code space for the light sample app on EFR32 BRD4161A.
707da42
to
bad8469
Compare
PR #21415: Size comparison from a42dffb to bad8469 Increases above 0.2%:
Increases (19 builds for cc13x2_26x2, cyw30739, efr32, esp32, linux, mbed)
Decreases (27 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, nrfconnect, p6, telink)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
jmartinez-silabs
approved these changes
Aug 4, 2022
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
…Crypto and opaque keys for [EFR32] (project-chip#21415) * Remove unused member from struct * Add a way to use a platform-defined crypto backend * Moving the checks and defaulting to the BUILD.gn file, to allow both platform and commandline overriding of the chip_crypto setting * Add an option 'platform' which skips adding dependencies and compiling 'CHIPCryptoPALxxx.cpp' into the library. Whoever/whatever is setting chip_crypto to platform is responsible for maintaining their own dependencies and keeping their implementation up to date with CHIPCryptoPAL.h * [EFR32] Add a PSA Crypto backend for EFR32 The EFR32 SDK has an SDK-supplied implementation of PSA which is hardware backed. * [EFR32] Speed and size improvements by going straight for the driver layer Considering all keys pass in plaintext through this abstraction, this doesn't really matter all that much for the time being. * [EFR32] Finetune PSA Crypto configuration Remove blanket 'BUILTIN_ALG' defines since they will cause PSA Crypto to not be able to compile out these algorithm implementations in software, which again causes pretty hefty dependencies on mbedTLS * [EFR32] Now that we have HMAC through PSA, we don't need PKCS5 anymore This saves quite some codespace by allowing to strip dead code dangling from OpenThread's use of mbedtls_pk_parse_key (which, when PKCS5 is enabled, will always include code for parsing password-protected PEM files, but no such files are ever used in the context of Matter) * [EFR32] Add implementation of opaque keys and opaque operational key store * [EFR32] Implement persistence for operational keys and cleanup * Implemented persistence of operational key map and runtime resizing on key map should the setting change and be OTA'ed over. * Changed namespacing to put EFR32 classes under the internal layer * Added 'ConfigValueExists' overload to the config manager which returns the size of an object if it exists. * [EFR32] Set correct SHA context size on devices with HW acceleration * [EFR32] fix incorrect usage of VerifyOrExit * Add empty definition of TestAddEntropySources for platform * [EFR32] Fix circular dependency * [EFR32] Fix dependencies for Wifi build * Fix default crypto build Moving the override of chip_crypto in case it isn't set into crypto's BUILD.gn instead of crypto.gni means that the toplevel BUILD.gn no longer can see which crypto instance it ends up building with. This was only used to determine whether or not to build chip-cert tool by default. This commit clarifies that by duplicating the exact logic that was backing this into the tools.gni file under a more descriptive variable name. The actual logic probably needs cleaning up, but that would be outside the scope of this PR. * Pull apart the crypto library into headers, intermediate and backend Instead of viewing crypto as a monolithic library, encode it more granular to better allow platform crypto implementations. It now consists of its public headers (which the crypto backends depend on), a static library containing the abstractly-implemented functions, and a set of source sets for each of the crypto backends provided by the main tree. * [EFR32] Fix dependency encoding of crypto backend The EFR32 crypto backend (brought in by setting chip_crypto to platform) now correctly advertises a dependency on the matter crypto PAL. It only needs the intermediate layer of the crypto PAL, not a backend, which is taken care of due to chip_crypto being set to platform. * [EFR32] Address readability according to review comments * [EFR32] Use Matter CSR generation instead of mbedTLS Use GenerateCertificateSigningRequest instead of replicating the mbedTLS implementation of CSR generation. Since there were no other parts of the app depending on mbedTLS CSR writing functionality, this saved 3.5kB of code space for the light sample app on EFR32 BRD4161A. * MbedTLS backend depends on Crypto PAL headers regardless of where it lives * Restyled by gn * Fix typo * [EFR32] Allow building EFR32 examples with in-tree crypto backends * [EFR32] Remove dead TinyCrypt code from EFR32 crypto backend * [EFR32] Ensure mbedTLS gets rebuilt when config header is modified * [EFR32] Dynamically size the keymap object for the operational keystore * [EFR32] Some parts need mbedTLS's entropy API still * rebase and restyle Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Junior Martinez <[email protected]>
mkardous-silabs
pushed a commit
to mkardous-silabs/connectedhomeip
that referenced
this pull request
Dec 19, 2022
The silabs_1.0 commit rebased into: fdf1048 Details of these changes can be seen in the initial commits on the feature/matter-slc-integration branch: commit e06dbd63a25bf6c0670481cfa056b545f5312585 (HEAD -> feature/matter-slc-integration, origin/feature/matter-slc-integration) update code owners for gsdk integration commit 14b33a1eb6c09485ee955c20104a594f90df8079 script updates commit 34e65edce927356361a6518aaf04adb5bd9db8b1 Add a script for copying Matter extension files to a give directory commit 4e0b6ea46301de7bf4c3bb0b8fb29a1c3e61658e Fix gsdk_matter.patch, exlcude INCLUDE_xTimerPendFunctionCall This define comes from Matter, exclude the contribution from the FreeRTOS component commit aec69e1a5be0910657388b4edf0c068dea03cfa7 Set OPENTHREAD_CONFIG_LOG_OUTPUT_APP in the lock project file commit b037d18e1752c2496783c5a1129d850c3e1e809e Reenable patching of GSDK now that the patch file is updated commit 66d7d48055a5199b089c8f846765aa1a3738abb3 Update the patch file for the GSDK repo Update the patch file to account for the OpenThread code refactor. The crypto change show no longer be needed because of https://stash.silabs.com/projects/EMBSW/repos/platform_crypto/pull-requests/875/overview commit 8994c0de263d31e2ae51088dd51f82a41f8cac12 Misc fixes to pass compilation commit f596e9d46d5183bb7ea27e6d1a13d578c280fe1f Add SDK extension meta files commit a169862cd14717a2c03bfa7889782ba5d99b1656 Update slc-based lock-app sources to match the example commit 44eaa72b178461c2622f7f4142f3403cc7879fda Misc temporary changes to pass compilation commit 7e13939d82ceb3cfcac5c216c42db173926626e5 Fix rebase error commit d688016aff0d7cb20d2a48426f82dc8b532981ab Add a line lost during rebase commit 1665b4ba3a5a6444cb98c9741677f2c5f4bcaa0b Fix merge errors Picked up changes from project-chip#21415 lost during rebase commit 1518b635ea9d739893ac24598d2700192848cdb6 Temporary fix to get SLC integration to compile. commit 94dd419ca4ac60eed21b5f556211b1b17f9011e1 Update SLCC files after running gen_components.py on the latest code commit 4084163b8dd49f08f9d47552df1e112b0e813651 Add gatt component that is now required by ot_platform_abstraction component (NOT APPLICABLE) commit 34f8a5e831feffeef26ced4a06dbed571f927c97 Update lock example and QR code components to accommodate lcd.c name change commit c5c6d1e959cf059968c7154687de4b4bca1172f6 Initial commit for Matter SLC integration changes from Aksel Mellbeye Changes originaly went into the experimental/slc-integration branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Change overview
chip_crypto
build system variable to take on the value ofplatform
, which allows platforms to provide their own implementation of the functions implemented inCHIPCryptoPAL{backend}.cpp
.Testing
Testing through existing unit tests & the EFR32 sample builds