sys/psa_crypto: Fix macro for public key max size and SE example#19995
sys/psa_crypto: Fix macro for public key max size and SE example#19995bors[bot] merged 2 commits intoRIOT-OS:masterfrom
Conversation
Einhornhool
added
the
Type: bug
github-actions
bot
added
Area: sys
benpicco
added
CI: ready for build
| } | ||
|
|
||
| #ifdef SECURE_ELEMENT | ||
| return psa_verify_hash(pubkey_id, ECC_ALG, hash, sizeof(hash), signature, sig_length); |
There was a problem hiding this comment.
Would be great to have a comment here how this differs from psa_verify_message()
There was a problem hiding this comment.
I added a comment on why it's necessary to call psa_verify_hash.
There was a problem hiding this comment.
Ah so the example really does two different things depending on whether SECURE_ELEMENT is set.
Is there no software fallback for message verification?
There was a problem hiding this comment.
There is, but since this is supposed to be the Secure Element example, I would like to use the available SE functions.
There was a problem hiding this comment.
I'm just surprised because psa_verify_message() has no reference to a secure element, so I would expect it would work either way.
There was a problem hiding this comment.
Aaah, I didn't understand your question correctly.
In this case there are two issues:
- In this build there is no fallback. Secure Element modules are selected by different symbols than regular PSA Crypto modules. This configuration only selects
psa_secure_element_ateccx08a_ecc_p256. To have a fallback in this case, I would have to explicitly selectpsa_asymmetric_ecc_p256r1.
This could happen automatically, but I deliberately chose to build SEs without fallbacks, since they are also meant to be a memory efficient alternative to software implementations, especially on platforms that don't have enough memory for them. Also, if keys are stored on the SE, a software fallback couldn't do anything anyways. - In this case the key for verification is stored on the secure element, which means that a software backend couldn't access it. I could change the example to store the public key in RAM and make it usable for a different backend. But then it's not a "secure element example" anymore, but a "secure element signature with software verification example". It's not a big problem, but a different use case, I guess.
There was a problem hiding this comment.
I think a simple fallback for secure elements or other configurations where signing/verifying messages is not directly supported could be the following: internally calculate the hash using psa_hash_compute() (on the secure element, or in software if not supported) and then call psa_verify_hash() on the hash.
That's what I did for ed25519 on nRF52840dk iirc.
There was a problem hiding this comment.
I can do that, but I'd rather submit a separate PR for it, since it would be a new feature =)
Einhornhool
force-pushed
the
pr/psa-crypto-fix-se-example
branch
from
616cbb3 to
b2e1c69
Compare
|
bors merge |
|
🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
|
Stopped waiting for PR status (GitHub check) without running due to duplicate requests to run. You may check Bors to see that this PR is included in a batch by one of the other requests. |
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
20037: nib/_nib-6ln: bail out early if address is no longer assigned [backport 2023.10] r=benpicco a=MrKevinWeiss # Backport of #19999 20038: nanocoap: prevent integer underflow in coap_opt_put_uri_pathquery() [backport 2023.10] r=benpicco a=MrKevinWeiss # Backport of #19994 20039: sys/psa_crypto: Fix macro for public key max size and SE example [backport 2023.10] r=benpicco a=MrKevinWeiss # Backport of #19995 ### Contribution description #### 1. Wrong public key size when using secure elements, introduced by #19954 Fixed conditions for key size macros in `crypto_sizes.h`. #### 2. EdDSA and ECDSA examples fail when using a secure element because of unsopported changes introduced by #19954 Updated `example/psa_crypto` to use only supported functions for secure elements. ### Testing procedure Build `example/psa_crypto` for secure elements and run application Output on master: ``` 2023-10-19 14:33:24,372 # main(): This is RIOT! (Version: 2019.07-devel-22378-gb6772) 2023-10-19 14:33:24,372 # HMAC SHA256 took 56393 us 2023-10-19 14:33:24,372 # Cipher AES 128 took 68826 us 2023-10-19 14:33:24,372 # *** RIOT kernel panic: 2023-10-19 14:33:24,373 # HARD FAULT HANDLER 2023-10-19 14:33:24,373 # 2023-10-19 14:33:24,373 # *** rebooting... ``` Output with fixes: ``` 2023-10-19 13:35:24,715 # main(): This is RIOT! (Version: 2019.07-devel-22384-g8ef66-dev/psa-crypto-fixes) 2023-10-19 13:35:24,715 # HMAC SHA256 took 56374 us 2023-10-19 13:35:24,715 # Cipher AES 128 took 68805 us 2023-10-19 13:35:24,715 # ECDSA took 281164 us 2023-10-19 13:35:24,715 # All Done ``` Co-authored-by: Benjamin Valentin <[email protected]> Co-authored-by: Lena Boeckmann <[email protected]>
5 participants
Contribution description
1. Wrong public key size when using secure elements, introduced by #19954
Fixed conditions for key size macros in
crypto_sizes.h.2. EdDSA and ECDSA examples fail when using a secure element because of unsopported changes introduced by #19954
Updated
example/psa_cryptoto use only supported functions for secure elements.Testing procedure
Build
example/psa_cryptofor secure elements and run applicationOutput on master:
Output with fixes: