Skip to content

bt_crypto: provide a PSA APIs implementation alternative to TinyCrypt #73378

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
Merged
merged 7 commits into from
Jun 14, 2024
Merged

bt_crypto: provide a PSA APIs implementation alternative to TinyCrypt #73378

merged 7 commits into from
Jun 14, 2024

Conversation

@valeriosetti
Copy link
Contributor

@valeriosetti valeriosetti commented May 28, 2024 •
edited
Loading

This PR adds a new kconfig symbol named CONFIG_BT_USE_PSA_API to allow the end user to specifically request PSA API usage over TinyCrypt for crypto operations in bluetooth. This might be useful in platforms/scenarios which already have some PSA API provider (ex: TF-M or Mbed TLS + CRYPTO_C) in order to reduce memory footprint and not duplicate crypto libraries for no reason.

For sake of example here's a memory footprint comparison on some BT test cases on the nrf9160dk/ns platform (which has TFM):

west build -p always -b nrf9160dk/nrf9160/ns -T tests/bluetooth/bt_crypto/bluetooth.bt_crypto
...
Memory region         Used Size  Region Size  %age Used
           FLASH:       44996 B       192 KB     22.89%
             RAM:        8672 B       128 KB      6.62%
        IDT_LIST:          0 GB        32 KB      0.00%

west build -p always -b nrf9160dk/nrf9160/ns -T tests/bluetooth/bt_crypto/bluetooth.bt_crypto.psa
...
Memory region         Used Size  Region Size  %age Used
           FLASH:       44968 B       192 KB     22.87%
             RAM:        8696 B       128 KB      6.63%
        IDT_LIST:          0 GB        32 KB      0.00%

west build -p always -b nrf9160dk/nrf9160/ns -T tests/bluetooth/gatt/bluetooth.gatt
...
Memory region         Used Size  Region Size  %age Used
           FLASH:       81196 B       192 KB     41.30%
             RAM:       13436 B       128 KB     10.25%
        IDT_LIST:          0 GB        32 KB      0.00%

$ west build -p always -b nrf9160dk/nrf9160/ns -T tests/bluetooth/gatt/bluetooth.gatt.psa
...
Memory region         Used Size  Region Size  %age Used
           FLASH:       79132 B       192 KB     40.25%
             RAM:       13156 B       128 KB     10.04%
        IDT_LIST:          0 GB        32 KB      0.00%

west build -p always -b nrf5340dk/nrf5340/cpuapp/ns -T tests/bluetooth/mesh/basic/bluetooth.mesh.gatt
...
Memory region         Used Size  Region Size  %age Used
           FLASH:      179692 B       192 KB     91.40%
             RAM:       35758 B       192 KB     18.19%
        IDT_LIST:          0 GB        32 KB      0.00%

west build -p always -b nrf5340dk/nrf5340/cpuapp/ns -T tests/bluetooth/mesh/basic/bluetooth.mesh.gatt.psa
...
Memory region         Used Size  Region Size  %age Used
           FLASH:      171932 B       192 KB     87.45%
             RAM:       35446 B       192 KB     18.03%
        IDT_LIST:          0 GB        32 KB      0.00%

@zephyrbot
Copy link

zephyrbot commented May 28, 2024 •
edited
Loading

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@Thalley Thalley removed their request for review May 28, 2024 07:17
@zephyrbot zephyrbot added the DNM This PR should not be merged (Do Not Merge) label May 28, 2024
@valeriosetti valeriosetti force-pushed the remove-tc-in-bt branch 3 times, most recently from 5437c0d to 2c4981b Compare May 28, 2024 09:09
tomi-font
tomi-font reviewed May 28, 2024
View reviewed changes
@valeriosetti valeriosetti force-pushed the remove-tc-in-bt branch 2 times, most recently from 4ff6817 to f64efc3 Compare May 29, 2024 04:10
@zephyrbot zephyrbot removed manifest manifest-mbedtls DNM This PR should not be merged (Do Not Merge) labels May 29, 2024
@valeriosetti valeriosetti requested review from frkv and tomi-font May 29, 2024 07:13
@valeriosetti
Copy link
Contributor Author

valeriosetti commented Jun 13, 2024

I added a new commit (7bf45b7) because there are still failures in the CI and BabbleSim due to #73571.

jhedberg
jhedberg previously approved these changes Jun 13, 2024
View reviewed changes
ceolin
ceolin previously approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@ceolin ceolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\0/

@valeriosetti
Copy link
Contributor Author

valeriosetti commented Jun 13, 2024

I added a new commit (7bf45b7) because there are still failures in the CI and BabbleSim due to #73571.

Removing the last commit because it was fixed by 2690cac and now there's a conflict. Therefore I'm also rebasing on main

@valeriosetti valeriosetti dismissed stale reviews from ceolin and jhedberg via 4325d08 June 13, 2024 21:14
@valeriosetti
Copy link
Contributor Author

valeriosetti commented Jun 14, 2024

I think I need a bit of help here: failure on BabbleSim Tests seems to be related to the CI not to the code. Am I wrong?

@valeriosetti valeriosetti requested a review from jori-nordic June 14, 2024 05:59
@theob-pro
Copy link
Contributor

theob-pro commented Jun 14, 2024

I think I need a bit of help here: failure on BabbleSim Tests seems to be related to the CI not to the code. Am I wrong?

Re-triggered the bsim tests, let's see if it solve the issue

@valeriosetti
Copy link
Contributor Author

valeriosetti commented Jun 14, 2024

I think I need a bit of help here: failure on BabbleSim Tests seems to be related to the CI not to the code. Am I wrong?

Re-triggered the bsim tests, let's see if it solve the issue

I already did this early this morning, but it failed in the same point. However let's see, maybe this time is better :)

@theob-pro
Copy link
Contributor

theob-pro commented Jun 14, 2024

I think I need a bit of help here: failure on BabbleSim Tests seems to be related to the CI not to the code. Am I wrong?

Re-triggered the bsim tests, let's see if it solve the issue

I already did this early this morning, but it failed in the same point. However let's see, maybe this time is better :)

Oh my bad, I guess it's a problem on main if the problem persist

@theob-pro theob-pro mentioned this pull request Jun 14, 2024
Closed
@aescolar
Copy link
Member

aescolar commented Jun 14, 2024

I think I need a bit of help here: failure on BabbleSim Tests seems to be related to the CI not to the code. Am I wrong?

Sorry that was my bad, a fix is queued in here #74289

@aescolar
Copy link
Member

aescolar commented Jun 14, 2024

@valeriosetti please rebase. Currently the old workflow file is still being picked and it will fail again.

@valeriosetti
trusted-firmware-m: select PSA_CRYPTO_CLIENT when BUILD_WITH_TFM
ced9d97 
TF-M is a PSA API provider alternative to Mbed TLS one. As
a consequence when CONFIG_BUILD_WITH_TFM is set
CONFIG_PSA_CRYPTO_CLIENT should be set as well.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
bt-crypto: add option to use PSA APIs instead of TinyCrypt
7cb3c23 
This commit adds CONFIG_BT_USE_PSA_API to allow the end
user to prefer PSA APIs over TinyCrypt for crypto operations
in bluetooth. Of course, this is possible only if
a PSA provider is available on the system, i.e.
CONFIG_PSA_CRYPTO_CLIENT is set.

This commit also extends tests/bluetooth/bt_crypto adding
a test case for PSA.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
bt-host: add option to use PSA APIs instead of TinyCrypt
18a63b0 
By enabling CONFIG_BT_USE_PSA_API the user can specify to use
PSA APIs instead of TinyCrypt for crypto operations in bluetooth
host module.

This commit also extends tests/bluetooth/gatt in order to
add a PSA test.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
bluetooth: remove BT_TINYCRYPT_ECC in bluetooth mesh tests
493fd1e 
Some bluetooth test were using BT_TINYCRYPT_ECC without also
setting BT_ECC. This means that BT_TINYCRYPT_ECC gets disabled
as it depends on BT_ECC.
This commit fix this by removing BT_TINYCRYPT_ECC in all test
for bluetooth mesh.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
bt: hci_ecc: add option to use PSA APIs instead of TinyCrypt
57989de 
This commit adds CONFIG_BT_USE_PSA_API to allow the end
user to prefer PSA APIs over TinyCrypt for crypto operations
in bluetooth. Of course, this is possible only if
a PSA provider is available on the system, i.e.
CONFIG_PSA_CRYPTO_CLIENT is set.

This commit also extends
tests/bluetooth/mesh/basic/bluetooth.mesh.gatt adding a specific
case using PSA.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
doc: update migration guide about changes to bt-crypto
bef57d4 
Update migration guide about the support to PSA functions
introduced in bt-crypto.

Signed-off-by: Valerio Setti <[email protected]>
@valeriosetti
bsim: add tests for BT_USE_PSA_API
fabb256 
Add a couple PSA overlay configs for the BT tests in order to evaluate
PSA API support introduced by CONFIG_BT_USE_PSA_API. These test are
performed on nrf52840dk platform.

Signed-off-by: Valerio Setti <[email protected]>
@aescolar aescolar merged commit e9687c7 into zephyrproject-rtos:main Jun 14, 2024
@sjanc sjanc mentioned this pull request Jun 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhedberg jhedberg jhedberg approved these changes

@alxelax alxelax alxelax approved these changes

@tomi-font tomi-font tomi-font left review comments

@asbjornsabo asbjornsabo Awaiting requested review from asbjornsabo

@d3zd3z d3zd3z Awaiting requested review from d3zd3z

@hermabe hermabe Awaiting requested review from hermabe

@ithinuel ithinuel Awaiting requested review from ithinuel

@sjanc sjanc Awaiting requested review from sjanc

@kartben kartben Awaiting requested review from kartben

@nashif nashif Awaiting requested review from nashif

@PavelVPV PavelVPV Awaiting requested review from PavelVPV

@de-nordic de-nordic Awaiting requested review from de-nordic

@aescolar aescolar Awaiting requested review from aescolar

@ceolin ceolin Awaiting requested review from ceolin

@jori-nordic jori-nordic Awaiting requested review from jori-nordic

+2 more reviewers

@frkv frkv frkv approved these changes

@theob-pro theob-pro theob-pro left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@jhedberg jhedberg

Labels

area: Bluetooth Audio area: Bluetooth Controller area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth Mesh area: Bluetooth area: Crypto / RNG area: Samples Samples area: TF-M ARM Trusted Firmware-M (TF-M) platform: nRF BSIM Nordic Semiconductors, nRF BabbleSim Release Notes To be mentioned in the release notes

Projects

Bluetooth LE Audio
Status: Done

Milestone

v3.7.0

Development

Successfully merging this pull request may close these issues.