sys/psa_crypto: Fix macro for public key max size and SE example #19995

Einhornhool · 2023-10-19T12:35:41Z

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

benpicco · 2023-10-19T12:49:46Z

examples/psa_crypto/example_ecdsa_p256.c

@@ -99,6 +102,10 @@ psa_status_t example_ecdsa_p256(void)
        return status;
    }

+#ifdef SECURE_ELEMENT
+    return psa_verify_hash(pubkey_id, ECC_ALG, hash, sizeof(hash), signature, sig_length);


Would be great to have a comment here how this differs from psa_verify_message()

I added a comment on why it's necessary to call psa_verify_hash.

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 is, but since this is supposed to be the Secure Element example, I would like to use the available SE functions.

I'm just surprised because psa_verify_message() has no reference to a secure element, so I would expect it would work either way.

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 select psa_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.

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.

I can do that, but I'd rather submit a separate PR for it, since it would be a new feature =)

benpicco · 2023-10-19T14:42:27Z

bors merge

bors · 2023-10-19T14:42:31Z

🕐 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.

riot-ci · 2023-10-19T14:44:43Z

Murdock results

✔️ PASSED

b2e1c69 examples/psa_crypto: Update example to work with SEs

Success	Failures	Total	Runtime
7937	0	7937	14m:07s

Artifacts

Documentation preview

bors · 2023-10-19T14:45:41Z

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.

bors · 2023-10-20T00:39:58Z

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.
For more help, visit the forum.

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 <benjamin.valentin@ml-pa.com> Co-authored-by: Lena Boeckmann <lena.boeckmann@haw-hamburg.de>

sys/psa_crypto: Fix conditions for public key max export size

1304202

Einhornhool added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Oct 19, 2023

github-actions bot added Area: sys Area: System Area: examples Area: Example Applications labels Oct 19, 2023

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Oct 19, 2023

benpicco reviewed Oct 19, 2023

View reviewed changes

benpicco requested a review from jue89 October 19, 2023 12:50

examples/psa_crypto: Update example to work with SEs

b2e1c69

Einhornhool force-pushed the pr/psa-crypto-fix-se-example branch from 616cbb3 to b2e1c69 Compare October 19, 2023 13:31

benpicco approved these changes Oct 19, 2023

View reviewed changes

bors bot merged commit 03d3874 into RIOT-OS:master Oct 20, 2023
31 checks passed

MrKevinWeiss mentioned this pull request Nov 2, 2023

sys/psa_crypto: Fix macro for public key max size and SE example [backport 2023.10] #20039

Merged

MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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 #19995

Einhornhool commented Oct 19, 2023 •

edited

Loading

benpicco Oct 19, 2023

Einhornhool Oct 19, 2023

benpicco Oct 19, 2023

Einhornhool Oct 19, 2023 •

edited

Loading

benpicco Oct 19, 2023

Einhornhool Oct 19, 2023 •

edited

Loading

mguetschow Oct 19, 2023

Einhornhool Oct 19, 2023

benpicco commented Oct 19, 2023

bors bot commented Oct 19, 2023

riot-ci commented Oct 19, 2023

bors bot commented Oct 19, 2023

bors bot commented Oct 20, 2023

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 #19995

Conversation

Einhornhool commented Oct 19, 2023 • edited Loading

Contribution description

1. Wrong public key size when using secure elements, introduced by #19954

2. EdDSA and ECDSA examples fail when using a secure element because of unsopported changes introduced by #19954

Testing procedure

benpicco Oct 19, 2023

Choose a reason for hiding this comment

Einhornhool Oct 19, 2023

Choose a reason for hiding this comment

benpicco Oct 19, 2023

Choose a reason for hiding this comment

Einhornhool Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

benpicco Oct 19, 2023

Choose a reason for hiding this comment

Einhornhool Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

mguetschow Oct 19, 2023

Choose a reason for hiding this comment

Einhornhool Oct 19, 2023

Choose a reason for hiding this comment

benpicco commented Oct 19, 2023

bors bot commented Oct 19, 2023

riot-ci commented Oct 19, 2023

Murdock results

Artifacts

bors bot commented Oct 19, 2023

bors bot commented Oct 20, 2023

Einhornhool commented Oct 19, 2023 •

edited

Loading

Einhornhool Oct 19, 2023 •

edited

Loading

Einhornhool Oct 19, 2023 •

edited

Loading