sys: PSA Crypto API implementation #18547

Einhornhool · 2022-09-02T12:13:54Z

Contribution description

This adds an implementation of the ARM PSA Crypto API specification to RIOT.

It is a cryptographic API that supports software and hardware backends as well as the use of multiple secure elements, which can be configured with Kconfig.
It integrates indirect, identifier based key management to support persistent storage of key material in local memory and devices with protected key storage.

A description of the implementation design and an evaluation of the processing time and memory overhead in RIOT has been published here: Usable Security for an IoT OS: Integrating the Zoo of Embedded Crypto Components Below a Common API

Implementation status

So far this implementation supports the following operations:

Volatile key storage
AES in CBC mode
Hashes (MD5, SHA1, SHA224, SHA256)
HMAC SHA256
ECDSA with NIST P192 and P256 curves

The following backends are supported so far:

RIOT Cipher Module
RIOT Hash Module
Micro ECC library package
Cryptocell 310 hardware accelerator on the Nordic NRF52840dk
Microchip ATECC608A secure element

Other operations and backends as well as persistent key storage can and will be implemented by me and anyone who wants to contribute in the future.

Testing procedure

So far there is a show case application in examples/psa_crypto to demonstrate the usage and configuration of different backends of the API (refer to the application README for more information).

benpicco · 2022-09-02T13:15:55Z

Nice work!

But a single 20kSLOC commit is a bit hard to handle - can you please split this up into more manageable chunks? Ideally even multiple PRs since there seem to be several things in there that could be merged independently.

Einhornhool · 2022-09-02T14:08:53Z

Thank you!
I know it's big and I can definitely split it up for reviewing.
If it's okay, I would like to keep it this way until after the summit next week (there probably is going to be a discussion about PSA Crypto in RIOT), so people who are interested can take a look at how everything is supposed to work together.

benpicco · 2022-09-02T14:24:36Z

You can of course do this in whatever time scale suits you best, but splitting this into multiple commits would be a good first step independent of whether the PR should be split or not.

Einhornhool · 2022-09-02T14:28:17Z

Alright, I will do that

maribu

Lots of style nitpicks. I didn't look at all and event failed to grasp the high level picture yet, as it is really hard to read code on a mobile phone :( I will provide better feadback one I am back at a keyboard.

boards/common/nrf52/include/cfg_i2c_default.h

examples/atecc608a_configure_and_lock/main.c

examples/psa_crypto/aes_cbc.c

pkg/driver_cryptocell/psa_cc/hashes_common.c

pkg/driver_cryptocell/psa_cc/hashes_sha1.c

MrKevinWeiss

Maybe it would be nice to add a simple automatic test to the CI can test it as well?

diff --git a/examples/psa_crypto/tests/01-run.py b/examples/psa_crypto/tests/01-run.py
new file mode 100755
index 0000000000..25257b8ca8
--- /dev/null
+++ b/examples/psa_crypto/tests/01-run.py
@@ -0,0 +1,13 @@
+#!/usr/bin/env python3
+
+import sys
+from testrunner import run
+
+
+def testfunc(child):
+    child.expect_exact('All Done')
+    print("[TEST PASSED]")
+
+
+if __name__ == "__main__":
+    sys.exit(run(testfunc))

aabadie · 2022-09-05T08:46:10Z

pkg/driver_cryptocell/Makefile

@@ -0,0 +1,14 @@
+PKG_NAME=driver_cryptocell
+PKG_URL=https://github.com/Einhornhool/RIOT-cryptocell-driver.git


This package points to an archive (.a) only, with headers repository. That's quite limited: only nrf52840 cpu is supported. Is there a possibility and a plan to provide an open-source implementation ?

Also note that the package is too much limited IMHO: it can only be built for nrf52840dk board but I think any nrf52840 CPU based board should work with it.

Unfortunately this driver is closed source and there don't seem to be any plans to make it open source.
I got this from the Nordic SDK, where they only provide the archives and header files. There is a Git Repositiory (nrfxlib), which also provides archive files, but only headers with SHA 256 and ECC P256 support.

The limitation is my mistake, I did not realize that the nrf52840 also had an accelerator, I will fix that.

boards/nrf52840dk/Kconfig

boards/common/nrf52/include/cfg_i2c_default.h

kfessel

wow this is a great PR
since you already said you want to split it, I just had a first glance and found some small nit picks.

kfessel · 2022-09-07T13:47:16Z

boards/common/nrf52/include/cfg_i2c_default.h

@@ -21,6 +21,7 @@
 #define CFG_I2C_DEFAULT_H

 #include "periph_cpu.h"
+#include "kernel_defines.h"


are the "kernel_defines.h" needed in this header? (cant see anything being added that requires them)

resolved in 18582

No resolved here... Please remove

boards/common/nrf52/include/cfg_i2c_default.h

pkg/driver_cryptocell/include/psa_error.h

pkg/driver_cryptocell/psa_cc/ecc_p192.c

pkg/driver_cryptocell/psa_cc/ecc_p256.c

Einhornhool · 2023-08-31T11:43:09Z

I think I'm done.

MrKevinWeiss · 2023-08-31T15:15:30Z

bors try

MrKevinWeiss · 2023-08-31T15:15:40Z

just to see everything :)

bors · 2023-08-31T15:53:19Z

try

Build failed:

Murdock

miri64 · 2023-08-31T16:33:27Z

Just one ATmega being too small for this :-)

MrKevinWeiss

If everything works I would give it my ACK. It is just a matter of @Teufelchen1 to finish checking the changes!

MrKevinWeiss · 2023-09-04T07:59:25Z

If everyone is happy then.

MrKevinWeiss · 2023-09-04T07:59:34Z

bors merge

bors · 2023-09-04T07:59:39Z

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

bors · 2023-09-04T11:53:30Z

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.

Ollrogge · 2023-09-04T14:10:49Z

Congrats @Einhornhool !!

miri64 · 2023-09-04T14:17:11Z

🎉

19954: sys/psa_crypto: Ed25519 (EdDSA) support r=benpicco a=mguetschow ### Contribution description - implement [`psa_sign_message()`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.psa_sign_message) and [`psa_verify_message()`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.psa_verify_message) for the two already supported [`PSA_ALG_ECDSA`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.PSA_ALG_ECDSA) algorithms, together with the CryptoCell and `micro-ecc` backends (*not* for the SE backend) - add support for [`PSA_ALG_PURE_EDDSA`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.PSA_ALG_PURE_EDDSA), together with the CryptoCell hardware and `c25519` software backend (*not* for the SE backend) - wipe private key data from stack for both ECDSA and EdDSA algorithms using `explicit_bzero()` (opinions from experienced Riot maintainers about usage of `goto` to avoid duplicating that function call before every `return`?) ### Testing procedure - `examples/psa_crypto` has been updated to include EdDSA - successfully tested configurations: - `nrf52840dk` with cryptocell (hardware) and `c25519` (software) backend - `native` with software backend ### Issues/PRs references Thanks `@Einhornhool` for the PSA Crypto framework implementation #18547 which is great to work with! 19966: sys/event: add event_is_queued() r=benpicco a=fabian18 Co-authored-by: Mikolai Gütschow <mikolai.guetschow@tu-dresden.de> Co-authored-by: Fabian Hüßler <fabian.huessler@ml-pa.com>

19954: sys/psa_crypto: Ed25519 (EdDSA) support r=miri64 a=mguetschow ### Contribution description - implement [`psa_sign_message()`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.psa_sign_message) and [`psa_verify_message()`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.psa_verify_message) for the two already supported [`PSA_ALG_ECDSA`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.PSA_ALG_ECDSA) algorithms, together with the CryptoCell and `micro-ecc` backends (*not* for the SE backend) - add support for [`PSA_ALG_PURE_EDDSA`](https://armmbed.github.io/mbed-crypto/html/api/ops/sign.html#c.PSA_ALG_PURE_EDDSA), together with the CryptoCell hardware and `c25519` software backend (*not* for the SE backend) - wipe private key data from stack for both ECDSA and EdDSA algorithms using `explicit_bzero()` (opinions from experienced Riot maintainers about usage of `goto` to avoid duplicating that function call before every `return`?) ### Testing procedure - `examples/psa_crypto` has been updated to include EdDSA - successfully tested configurations: - `nrf52840dk` with cryptocell (hardware) and `c25519` (software) backend - `native` with software backend ### Issues/PRs references Thanks `@Einhornhool` for the PSA Crypto framework implementation #18547 which is great to work with! Co-authored-by: Mikolai Gütschow <mikolai.guetschow@tu-dresden.de>

Einhornhool requested review from leandrolanzieri, jia200x, MrKevinWeiss, PeterKietzmann, aabadie and haukepetersen as code owners September 2, 2022 12:13

github-actions bot added Area: boards Area: Board ports Area: build system Area: Build system Area: doc Area: Documentation Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: sys Area: System labels Sep 2, 2022

benpicco requested review from maribu and kfessel September 2, 2022 13:07

Einhornhool force-pushed the pr/psa-crypto-implementation branch from 24395e3 to 03407f8 Compare September 2, 2022 14:58

maribu reviewed Sep 5, 2022

View reviewed changes

MrKevinWeiss reviewed Sep 5, 2022

View reviewed changes

aabadie reviewed Sep 5, 2022

View reviewed changes

benpicco reviewed Sep 6, 2022

View reviewed changes

boards/nrf52840dk/Kconfig Outdated Show resolved Hide resolved

boards/common/nrf52/include/cfg_i2c_default.h Outdated Show resolved Hide resolved

github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Sep 7, 2022

kfessel reviewed Sep 7, 2022

View reviewed changes

benpicco requested a review from jue89 September 8, 2022 10:41

This was referenced Sep 12, 2022

sys: Add PSA Crypto module #18578

Closed

sys/crypto: Add PSA Crypto wrappers #18579

Closed

Einhornhool force-pushed the pr/psa-crypto-implementation branch from 37315cf to a3f7d59 Compare August 31, 2023 11:40

Einhornhool added 6 commits August 31, 2023 14:38

sys: Add PSA Crypto Module

d4c73d6

pkg: Add cryptocell driver and nrf52 HW features

c01d689

pkg/micro-ecc: Add PSA Crypto Wrappers

a6f3b59

pkg/cryptoauthlib: Add PSA Wrappers

40902cd

sys/crypto: Add PSA Crypto Wrappers

b63ca79

sys/hashes: Add PSA Crypto Wrappers

2584298

Einhornhool force-pushed the pr/psa-crypto-implementation branch from a3f7d59 to 17da706 Compare August 31, 2023 12:38

bors bot added a commit that referenced this pull request Aug 31, 2023

Try #18547:

43df5d2

MrKevinWeiss approved these changes Sep 4, 2023

View reviewed changes

Teufelchen1 approved these changes Sep 4, 2023

View reviewed changes

Einhornhool added 2 commits September 4, 2023 09:58

examples: Add PSA Crypto example

c71441d

dist/tools/codespell: Add ignored words

a0ccbce

Einhornhool force-pushed the pr/psa-crypto-implementation branch from 17da706 to a0ccbce Compare September 4, 2023 07:59

bors bot merged commit 9be022a into RIOT-OS:master Sep 4, 2023
25 checks passed

mguetschow mentioned this pull request Sep 28, 2023

sys/psa_crypto: Ed25519 (EdDSA) support #19954

Merged

MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sys: PSA Crypto API implementation #18547

sys: PSA Crypto API implementation #18547

Einhornhool commented Sep 2, 2022 •

edited

benpicco commented Sep 2, 2022

Einhornhool commented Sep 2, 2022

benpicco commented Sep 2, 2022

Einhornhool commented Sep 2, 2022

maribu left a comment

MrKevinWeiss left a comment

aabadie Sep 5, 2022

Einhornhool Sep 6, 2022 •

edited

kfessel left a comment

kfessel Sep 7, 2022

kfessel Sep 12, 2022

MrKevinWeiss Jan 24, 2023

Einhornhool commented Aug 31, 2023

MrKevinWeiss commented Aug 31, 2023

MrKevinWeiss commented Aug 31, 2023

bors bot commented Aug 31, 2023

miri64 commented Aug 31, 2023

MrKevinWeiss left a comment

MrKevinWeiss commented Sep 4, 2023

MrKevinWeiss commented Sep 4, 2023

bors bot commented Sep 4, 2023

bors bot commented Sep 4, 2023

Ollrogge commented Sep 4, 2023 •

edited

miri64 commented Sep 4, 2023

		@@ -0,0 +1,14 @@
		PKG_NAME=driver_cryptocell
		PKG_URL=https://github.com/Einhornhool/RIOT-cryptocell-driver.git

sys: PSA Crypto API implementation #18547

sys: PSA Crypto API implementation #18547

Conversation

Einhornhool commented Sep 2, 2022 • edited

Contribution description

Implementation status

Testing procedure

benpicco commented Sep 2, 2022

Einhornhool commented Sep 2, 2022

benpicco commented Sep 2, 2022

Einhornhool commented Sep 2, 2022

maribu left a comment

Choose a reason for hiding this comment

MrKevinWeiss left a comment

Choose a reason for hiding this comment

aabadie Sep 5, 2022

Choose a reason for hiding this comment

Einhornhool Sep 6, 2022 • edited

Choose a reason for hiding this comment

kfessel left a comment

Choose a reason for hiding this comment

kfessel Sep 7, 2022

Choose a reason for hiding this comment

kfessel Sep 12, 2022

Choose a reason for hiding this comment

MrKevinWeiss Jan 24, 2023

Choose a reason for hiding this comment

Einhornhool commented Aug 31, 2023

MrKevinWeiss commented Aug 31, 2023

MrKevinWeiss commented Aug 31, 2023

bors bot commented Aug 31, 2023

try

miri64 commented Aug 31, 2023

MrKevinWeiss left a comment

Choose a reason for hiding this comment

MrKevinWeiss commented Sep 4, 2023

MrKevinWeiss commented Sep 4, 2023

bors bot commented Sep 4, 2023

bors bot commented Sep 4, 2023

Ollrogge commented Sep 4, 2023 • edited

miri64 commented Sep 4, 2023

Einhornhool commented Sep 2, 2022 •

edited

Einhornhool Sep 6, 2022 •

edited

Ollrogge commented Sep 4, 2023 •

edited