Skip to content
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

PSA Crypto API adoption in Zephyr #43712

Open
ceolin opened this issue Mar 11, 2022 · 37 comments
Open

PSA Crypto API adoption in Zephyr #43712

ceolin opened this issue Mar 11, 2022 · 37 comments
Assignees
Labels
area: Crypto / RNG area: Security Security RFC Request For Comments: want input from the community

Comments

@ceolin
Copy link
Member

ceolin commented Mar 11, 2022

Introduction

Zephyr has currently, at least, three different cryptography available and been used. They are:

The lack of an unified API to cryptography leads to a situation where we have different components using the one that best attend there needs and when an application needs those components we end up having multiple cryptography implementations in a single target. This means resources being waste.

To make things worse, one of these implementations (TinyCrypt) is no longer supported, which put us in a position of maintain it by ourselves or replace it with something else.

PSA Crypto implementation provides a portable interface to cryptographic operations on a wide range of hardware and software. Which means we may have Zephyr's using a single API that can have different implementations depending in build options.
More information about it can be found in https://armmbed.github.io/mbed-crypto/html/overview/intro.html

Problem description

The lack of ONE API may cause waste of resources (multiple implementations needed at same time), make products more vulnerable (multiple implementations increases the code surface and consequently the chance of bugs), and it does help us take full advantage of HW accelerators.

Proposed change

The proposal is to adopt psa crypto API on Zephyr to replace the direct usage of other cryptography implementations.

Detailed RFC

The initial idea is:

  • Select an implementation of the PSA crypto API (recent versions of mbedTLS implement it)
  • Change the code base to use PSA crypto API
  • Enable HW accelerators in the new API.

Concerns and Unresolved Questions

  • The major reason to use TinyCrypt is due constraint resources, will PSA crypto be able to achieve similar requirements.
  • What happens if we end up having two implementations of PSA crypto API in a same target ? e.g TF-M uses mbedTLS implementation and Zephyr's code base uses another one.

Alternatives

@ceolin ceolin added the RFC Request For Comments: want input from the community label Mar 11, 2022
@ceolin ceolin added this to To do in Security features/improvements/documentation via automation Mar 11, 2022
@d3zd3z
Copy link
Collaborator

d3zd3z commented Mar 11, 2022

Given https://www.wolfssl.com/platform-security-architecture-psa-crypto-api-support-wolfssl/, I think we are past the point of really deciding if PSA is the correct API to implement, and just planning on how we are going to get there.

@carlescufi
Copy link
Member

carlescufi commented Mar 14, 2022

Note: In the context of Bluetooth, there have been attempts to:

@joerchan
Copy link
Contributor

Snapshot of the current features of TinyCrypt that we would required replacements for:

zephyr/drivers/crypto/Kconfig:
  28: 	select TINYCRYPT_AES
  29: 	select TINYCRYPT_AES_CBC
  30: 	select TINYCRYPT_AES_CTR
  31: 	select TINYCRYPT_AES_CCM
  32: 	select TINYCRYPT_AES_CMAC

zephyr/subsys/bluetooth/common/Kconfig:
  239: 	select TINYCRYPT_AES

zephyr/subsys/bluetooth/host/Kconfig:
  118: 	select TINYCRYPT_AES
  119: 	select TINYCRYPT_SHA256
  120: 	select TINYCRYPT_SHA256_HMAC
  121: 	select TINYCRYPT_SHA256_HMAC_PRNG
  256: 	select TINYCRYPT_AES
  257: 	select TINYCRYPT_AES_CMAC
  633: 	select TINYCRYPT_ECC_DH

zephyr/subsys/bluetooth/host/Kconfig.gatt:
  94: 	select TINYCRYPT_AES
  95: 	select TINYCRYPT_AES_CMAC

zephyr/subsys/bluetooth/mesh/Kconfig:
  10: 	select TINYCRYPT_AES
  11: 	select TINYCRYPT_AES_CMAC
  34: 	select TINYCRYPT_ECC_DH

zephyr/subsys/jwt/Kconfig:
  25: 	select TINYCRYPT_SHA256
  26: 	select TINYCRYPT_ECC_DSA
  27: 	select TINYCRYPT_CTR_PRNG
  28: 	select TINYCRYPT_AES

zephyr/subsys/mgmt/updatehub/Kconfig:
  21: 	select TINYCRYPT_SHA256

zephyr/subsys/random/Kconfig:
  94: 	select TINYCRYPT_CTR_PRNG if TINYCRYPT
  95: 	select TINYCRYPT_AES if TINYCRYPT

zephyr/subsys/storage/flash_map/Kconfig:
  45: 	select TINYCRYPT_SHA256

@Vge0rge
Copy link
Collaborator

Vge0rge commented Mar 14, 2022

In my opinion moving towards the PSA crypto APIs as a universal solution does make a lot of sense. There are two notes that I would like to mention here.

  1. The PSA APIs do not seem to provide flexible (enough) configuration option support for crypto accelerators yet. To reduce the code size it will be important to enable/disable specific functionalities in the PSA driver level. For example it should be possible for crypto accelerators to enable SHA256 and nothing else. Even though in the current state there are configuration options for this (PSA_WANT_ALG_SHA_256), I could not find any finalized configuration option for accelerators. There are some accelerator configurations (MBEDTLS_PSA_ACCEL_ALG_SHA_256) which does not seem to be used much in the code and they don't have a finalized structure.
  2. The CSRNG support from PSA APIs is lacking at the moment. There is a psa_generate_random function available but the entropy interface as specified in the driver proposal here is not fully implemented. And additionally, configuration options for enabling CTR_DRBG or HMAC_DRBG using purely PSA APIs are lacking.

I still believe that moving to the PSA APIs is the way to go. I am just noting some possible challenges that will need to be handled.

@microbuilder
Copy link
Member

microbuilder commented Mar 14, 2022

@Vge0rge There is an effort to enable support for crypto accelerators in the PSA APIs, which I think is worth highlighting and solves a real problem: https://github.com/ARMmbed/mbedtls/blob/development/docs/proposed/psa-driver-interface.md

EDIT: You link to that proposal in point two, sorry. 🥸

@microbuilder
Copy link
Member

I'd second that moving towards the PSA Crypto API make a lot of sense to me, even if there are clearly some issues to resolve around their use, and valid considerations of code size with the MbedTLS implementation. But resolving those issues will be less work than designing something else from scratch.

@frkv
Copy link
Collaborator

frkv commented Mar 16, 2022

We are very happy with an intention to gravitate to PSA Crypto APIs!

The latest (released) version of nRF Connect SDK has an additional file describing Kconfig options for all PSA_WANT_ALG_XXX configurations found in Mbed TLS

https://github.com/nrfconnect/sdk-zephyr/blob/main/modules/mbedtls/Kconfig.psa

We have plans to upstream this, but we were waiting for the final decision on standardization

We have added two new configurations for the (for now) missing feature of configuring CTR_DRBG or HMAC_DRBG PRNG support:

CTR_DRBG and HMAC_DRBG

We have internal logic in our SDK to handle configuring NSPE and SPE in case TF-M is used to make sure that the configurations correctly transition into the TF-M build and is exposed in the secure services.

The logic includes:

  • Converting CONFIG_PSA_WANT_ALG_XXXX into PSA_WANT_ALG_XXXX in generated configuration files (for NSPE and SPE)
  • Passing SPE configurations to TF-M build
  • Adding Nordic specific PSA driver configurations for NSPE or SPE builds (for building and linking)
  • Adding necessary C-files in NSPE or SPE for building PSA Crypto APIs
  • Enabling TF-M secure services based on enabled crypto features

Our intention is to support both with and without TF-M and to make sure that there is only "one unit implementing PSA crypto" in the system.

With regards to out-of-box experience, it wouldn't take much to get the Zephyr-module of Mbed TLS to make use of our proposed Kconfig logic. It is also possible to consider making µECC into a PSA driver to decrease the flash-size (if need be).

HMAC DRBG is not a big item to fix in a downstream adaptation if we make use of our proposed PSA configuration PSA_WANT_ALG_HMAC_DRBG

@ceolin
Copy link
Member Author

ceolin commented Mar 23, 2022

We are very happy with an intention to gravitate to PSA Crypto APIs!

The latest (released) version of nRF Connect SDK has an additional file describing Kconfig options for all PSA_WANT_ALG_XXX configurations found in Mbed TLS

https://github.com/nrfconnect/sdk-zephyr/blob/main/modules/mbedtls/Kconfig.psa

We have plans to upstream this, but we were waiting for the final decision on standardization

We have added two new configurations for the (for now) missing feature of configuring CTR_DRBG or HMAC_DRBG PRNG support:

CTR_DRBG and HMAC_DRBG

We have internal logic in our SDK to handle configuring NSPE and SPE in case TF-M is used to make sure that the configurations correctly transition into the TF-M build and is exposed in the secure services.

The logic includes:

  • Converting CONFIG_PSA_WANT_ALG_XXXX into PSA_WANT_ALG_XXXX in generated configuration files (for NSPE and SPE)
  • Passing SPE configurations to TF-M build
  • Adding Nordic specific PSA driver configurations for NSPE or SPE builds (for building and linking)
  • Adding necessary C-files in NSPE or SPE for building PSA Crypto APIs
  • Enabling TF-M secure services based on enabled crypto features

Our intention is to support both with and without TF-M and to make sure that there is only "one unit implementing PSA crypto" in the system.

Glad to hear it, that is one of my main concerns, we should not make Zephyr dependent of TF-M in order to have cryptography support.

With regards to out-of-box experience, it wouldn't take much to get the Zephyr-module of Mbed TLS to make use of our proposed Kconfig logic. It is also possible to consider making µECC into a PSA driver to decrease the flash-size (if need be).

Nice, we have to find a solution for the TinyCrypt EOL, uECC sounds like the direct alternative for it, but we have to have only one API to not have the same problem in future. We need to understand exactly what is the minimum requirements regarding rom/ram usage.

HMAC DRBG is not a big item to fix in a downstream adaptation if we make use of our proposed PSA configuration PSA_WANT_ALG_HMAC_DRBG

@gregshue
Copy link

From a cursory look it seems PSA Crypto API and Zephyr system interface have a common omission: calls that support a controlled de-initialization of the system. This matters when handing off the hardware from the boot image to the application image. And it affects not only the state of crypto hardware, but also the state of persistent storage, external chips, and even native UI.

To check my assumptions and foster discussion, here are the needs and requirements I have come up with so far:

Stakeholder needs:

  1. As a developer of secure connected devices, I need the device boot image to use cryptographic algorithms to authenticate an image prior to running it, so that I do not run corrupted code.
  2. As a developer of secure connected devices, I need the device boot image to use cryptographic algorithms to attest the source of an image prior to running it, so that I only run authorized code.
  3. As a developer of secure, intermitently connected devices, I need the device boot image to use data in a device-local cryptographic vault (e.g., permanently attached secure element chip) to attest the source of an image prior to running it, so that I only run authorized code.
  4. As a developer of devices, I need the device boot image to indicate boot progress and authentication/attestation status on the device native user interface, so that I can triage an unsucessful boot and/or firmware update.
  5. As a developer of devices, I need the device boot image to use cryptographic algorithms to decode and encode data in off-MCU persistent storage (e.g., external flash, EEPROM).
  6. As a developer of secure devices, I need the device boot image to only expose well-controlled and specified metadata about the boot image and related storage, so that I minimize the device vulnerabilities.
  7. As a developer of devices, I need the device boot image to be extensible (extended without code modification), so that I can reuse the tools, tests, and processes to generate my own security review artifacts for the maximum amount of code.

System Requirements:
A. When the device transitions from the boot image to the application image, the device shall have cleanly completed all updates to persistent storage.
B. When the device transitions from the boot image to the application image, the device shall leave communications with external chips in a quiescent state.
C. When the device transitions from the boot image to the applicaiton image, the device shall have DMAs disabled.
D. When the device transitions from the boot image to the application image, the device shall have interrupts disabled.
E. When the device transitions from the boot image to the application image, the device shall have the caches disabled.

Analysis:
⦁ Algorithmically cleaning up hardware agents activated in the boot image requires a de-initialization of the system (reverse dependency order). The proposed API does not provide a de-initialization call, and so does not support an extensible boot architecture implied by the Zephyr ecosystem. This precludes boot code using MCU-external cryptographic hardware agents.

I expect I am pretty naïve here. What have I missed?

@gregshue
Copy link

Here are several more generally related to APIs:

Stakeholder needs:
8. As a developer of composable resource-constrained embedded software, I need to set the preemption priority of each unit of work so that I get consistent soft-real-time behavior. (In other words, a separate work queue for each preemption priority; cooperative scheduling available at every preemption level or avoidance of cooperative scheduling)
9. As a developer of resource constrained embedded systems that need to support controlled disablement and cancel functionality, I need to use asynchronous API operations so that I can minimize RAM usage (fewer threads/stacks)
10. As a developer of software models of hardware, I need to have a single executable support both hardware and software implementations of a specific functionality so that I can run predictive maintenance models.
11. As a user of the Zephyr ecosystem, I need a single executable to support multiple instances (e.g., simultaneous sessions/contexts) of an internal service, so that I can support asynchronous processing (e.g., receiving and acting on a cancel request)
12. As a potential (re)user of the Zephyr ecosystem with a proprietary 3rd party toolchain, I need the Zephyr ecosystem to be limited to a published, maximum number of significant characters in linker symbols, so that I can determine if Zephyr can be supported by my toolchain for the foreseable future. In the absence of a published maximum number, I need the maximum number of significant characters in the Zephyr ecosystem linker symbols to fit within the limits guaranteed by the C99 standard.
13. As a user of the Zephyr ecosystem and other 3rd party libraries that is not allowed to change either codebase, I need the Zephyr ecosystem to minimize potential conflicts with any other library or external component source tree so that I don't have to replace Zephyr when my feature set grows.
14. As a developer of a family of resource-constrained products that may need the MCU replaced at any time, I need to configure the system to execute interrupt service routines at any configurable thread preemption or interrupt preemption level so that I can minimize the costs of supporting code running on alternative MCUs and/or CPU architectures.
15. As a developer of mechatronic devices built on Zephyr, I need to cancel arbitrary operations so that I can support user-initiated Cancel functionality.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 4, 2022

Let me try to distill this down a bit, since most of these requirements are a bit higher level than what would be affected by the crypto API.

The first point I want to make is that asking this as a question "Should Zephyr Adopt the PSA Crypto API" is kind of a moot point. Mbed TLS has been a component of Zephyr for some time, and the Mbed TLS project is in transition to the PSA crypto API. The question here isn't about a general crypto API, which is already the PSA Crypto API within Zephyr, but about a small number of cryptographic primitives that are used by other functionality built into Zephyr. Right now, these use some other ad-hoc libraries (e.g. tinycrypt) that have to be replaced due to lack of upstream support. We're proposing moving these to use the PSA crypto API, and in some sense, the requirements we need here are in regards to what these subsystems need.

But, back to the above lists from @gregshue:

  1. There is a need for deinitialization of crypto operations. Given that the current implementation is entirely software, there really isn't anything to deinitialize, and as Mbed TLS begins to implement hardware accelerated crypto support, the need to deinitialize can be brought up. This isn't necessarily even part of the crypto API, although it would make sense to definitely include it there.
  2. Related, as far as priority of work, currently for a software solution, work is done in the caller's thread, for basic cryptographic operations. For features such as TLS, this is generally done by networking worker threads. There are likely issues to be addressed here, but not really related to the crypto API itself. As far as work done when hardware acceleration is used, there may indeed need to be prioritization and scheduling done here.
  3. As far as the 31-character identifier limitation: this is pretty hard to enforce within Zephyr, as none of our current toolchains have any way of enforcing this kind of limit. Ideas are welcome, but there really isn't any way to specify this as a requirement that Zephyr meets without a way of checking it. This is worth raising an issue to track, as it has nothing to do with the crypto API (unless you're aware of symbols longer than 31 characters in the PSA API).
  4. As far as symbol conflicts go, this is a general issue, and should be raised as such. As far as I know, the PSA Crypto API does a good job of prefixing its namespace.
  5. Likewise with the remainder of issues. The only issue I see possibly affecting the crypto api would be the ability to cancel an operation. For a software implementation, this would be no different than canceling a running thread. For something with hardware crypto, this is challenging to address in a device-independent way, and until we have a better idea of how hardware addresses these issues, across diverse devices, it is difficult to know what could even be done that is general.

Again, to summarize. There are numerous points raised here. Many of them aren't really related to the use of a crypto API, and should be raised as their own issues.

As far as using the PSA Crypto API within Zephyr, that's already the case. By officially adopting it, we're discussing migrating existing users of other crypto libraries (notably tinycrypt) to use the PSA API, and what requirements there are behind that.

@gregshue
Copy link

gregshue commented Apr 4, 2022

Let me try to distill this down a bit, since most of these requirements are a bit higher level than what would be affected by the crypto API.

The crypto API would be affected by the system requirements, not the other way around.

Given that the current implementation is entirely software, there really isn't anything to deinitialize,

We are evaluating an API to algorithms that could be implemented in hardware and may already be for downstream users, so "current" is irrelevant. For hardware implementations, those that interact with persistent storage, or even those that use generic DMA there may be something to de-initialize, so the API needs to provide the hook.

Related, as far as priority of work, currently for a software solution, work is done in the caller's thread, for basic cryptographic operations. For features such as TLS, this is generally done by networking worker threads.

These need to be documented in the API and applied as constraints to all implementations. Now, what about for non-basic cryptographic operations? What about for the cases not done in by the networking worker threads? The downstream integrator must know this information to figure out if any implementation of this API is even an option.

this is pretty hard to enforce within Zephyr, as none of our current toolchains have any way of enforcing this kind of limit.

Actually, it is pretty easy to enforce. Choose a relatively large number for N (e.g., 255); in a kernel file define two different types of symbols that are that differ only in the last character and the type associated with them (e.g., int8_t, uint8_t). If the linker doesn't support the necessary resolution you will get a build failure. If any symbol file includes a non-mangled symbol of >N characters generate a build failure.

This is worth raising an issue to track, as it has nothing to do with the crypto API

We are gathering requirements related to APIs and related to system so we can evaluate whether or not the PSA crypto API meets them. It happens to meet this requirement, but not others already identified.

As far as symbol conflicts go, this is a general issue, and should be raised as such.

At the last TSC meeting they decided to kick off a new Project in the organization for looking in to Global Namespace Mgmt. It was created 5 days ago.

The only issue I see possibly affecting the crypto api would be the ability to cancel an operation. For a software implementation, this would be no different than canceling a running thread. For something with hardware crypto, this is challenging to address in a device-independent way, and until we have a better idea of how hardware addresses these issues, across diverse devices, it is difficult to know what could even be done that is general.

Zephyr does not support restarting threads, so canceling/killing a running thread is not an option.

In general, the driver must solve the problem. If the problem requires a hardware change to support the capability, then that particular hardware does not meet all the system requirements and has a more limited application space.

Now, what about supporting multiple API instances in the same executable (alternate implementations)?

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 5, 2022

this is pretty hard to enforce within Zephyr, as none of our current toolchains have any way of enforcing this kind of limit.
Actually, it is pretty easy to enforce. Choose a relatively large number for N (e.g., 255); in a kernel file define two different types of symbols that are that differ only in the last character and the type associated with them (e.g., int8_t, uint8_t). If the linker doesn't support the necessary resolution you will get a build failure. If any symbol file includes a non-mangled symbol of >N characters generate a build failure.

This is actually the opposite of what I mean by enforcing. In order for Zephyr to be able to say that it doesn't have any symbols that differ after N characters, we need a tool that can evaluate the entire Zephyr tree and determine if there are any conflicts. I don't believe this tool currently exists (although it wouldn't be difficult to add something to do this based on the output of 'nm' for a given build. You're approach really only would tell you if a given tool doesn't support N distinct characters, which isn't actually something Zephyr needs to know (it might be useful to someone).

I do believe this is a fairly minor issue. It probably should be raised (completely independently of this issue), since it is real. But, I would like to see some evidence of toolchains that have small limits on distinct characters. Practically, I think this is much easier to solve by just fixing the issues that are found (since C99 requires 31 characters, and conflicts past that are likely to be uncommon).

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 5, 2022

These issues aside, we are getting very distracted from the question being raised by this issue, which is whether or not Zephyr should consider the PSA crypto API to replace some of the other APIs that are used within the system. This issue is not about whether Zephyr will use or provide the PSA crypto API. That is already the case. What we are wanting to do here is replace the usage of two existing crypto APIs (both limited to a fairly small scope), and change the in-Zephyr code that uses these APIs to use the PSA API instead. In addition, this will involve deprecating these APIs, and giving an opportunity for any clients that are currently using them to migrate to the PSA API.

In this light, I'd like to ask if we can try to keep this issue focused on this specific question. There are a lot of other concerns raised, and these should be raised in other issues, if necessary. Notably, whether we provide the PSA crypto API is not a question here, as this API is already available in Zephyr. This issue should focus on code, both within Zephyr, and external code that makes use of https://github.com/zephyrproject-rtos/zephyr/blob/main/include/crypto/crypto.h, as well as uses the tinycrypt library, as we want to deprecate these interfaces, and migrate this code to use the PSA crypto APIs.

In light of this, the main constraint (brought up in the description of this issue) has to do with the compact implementation of elliptic curve crypto in the tiny crypt library. The Mbed TLS ECC implementation is both larger and slower, and also requires heap allocation, which tinycrypt does not. It is worth investigating whether implementing a PSA wrapper for the ECC operations in uECC (the upstream code used in Tinycrypt) will be sufficient for the needs of current code that makes use of the API.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 5, 2022

I'd also like to know that if anyone is aware of a crypto implementation that uses a different API, it would be good to hear about it. We would have to replace a lot of code to move to something that isn't Mbed TLS (at least that has a different API). I'm aware of Wolf-SSL, which, although would work with Zephyr, the free version is likely to not be license compatible with Zephyr code. Wolf SSL has also implemented the PSA Crypto API. Replacing the entire crypto API with a different one would be a lot of work, and probably of limited benefit, especially as I'm not aware of anything comparable. The Zephyr project is certainly not in a position to implement and maintain its own body of code comparable to Mbed TLS.

@microbuilder
Copy link
Member

I'm a bit confused by the overall conversation, particularly after the last security call.

PSA doesn't cover every possible edge case. Clearly. It's also good to identify the use cases it doesn't give us 100% coverage of and figure out what we can or should do to address those. But does not having every use case covered exclude the choice to migrate to the PSA API, and if PSA is deemed 'insufficient', what's the alternative here? I don't really see anything else that would give us something better. PSA isn't perfect ... but what's the net gain alternative?

This make me think of DeviceTree a bit. Zephyr makes use of DT as a useful abstraction for variations in HW, but the way we do DT today certainly doesn't cover every possible use case. It's imperfect. But that's how engineering works ... you have a goal in mind, and you take the tools that you do have today, you make compromises that at least move in the desired direction, and invest in tooling making things better as you go. We're constantly improving Zephyr's DT implementation and tooling to enable various edge cases as we go. It seems to me the PSA API is similar, and it brings clear value to the project as a common starting point, and we're certainly free to iterate on it, engage with the PSA authors upstream, and adapt it to our needs, and as with DT it brings more benefit to Zephyr than not having anything at all.

I understand the valid concerns being made about things the PSA and Zephyr can't do today. But to me none of that precludes officially adopting PSA as a common API for crypto etc. Those feel like very different issues, and not being able to do X/Y/Z with PSA today doesn't, in my mind, exclude a migration to PSA, anymore than not being able to model X/Y/Z in DT means we should reject DT at a project level either.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 5, 2022

@carlescufi wrote:

Note: In the context of Bluetooth, there have been attempts to:

Unfortunately, we do have to figure something out, because tinycrypt has become unmaintained. However, the ECC code within it isn't originally from tinycrypt, but comes from a project uECC (https://github.com/kmackay/micro-ecc, which although not highly active, does seem to have an active maintainer). It would be possible to replace hash and cipher options with Mbed TLS, and if configured properly, shouldn't be that large of a code-size increase, and either 1. continue to use the uECC code for the ECC operations within Bluetooth, or implement uECC as an ECC backend for PSA.

Whether this solution is viable for Bluetooth is really the main point of this issue and discussion.

@microbuilder
Copy link
Member

@gregshue It seems like some of the concerns you're raising should in fact be individual requirements in the larger security planning of Zephyr, and they should be filed individually and tracked and addressed (via the security project page in Zephyr, etc.) based on current needs, resources and priorities.

This issue specifically is trying to address one such problem: TinyCrypt being unmaintained, and Zephyr likely having to move away from it, and what we do at an API level to harmonize our general approach to crypto to resolve this issue today and in the future.

PSA is the one viable API solution I'm aware of myself, imperfect as it may be, meaning that we need to figure out how to address the performance issues of MbedTLS as one implementation of the PSA API with the requirements of the BLE and other security-depedent subsystems. That can mean several things:

  • Adding a new, more performant implementation of the PSA APIs based on uECC, etc.
  • Improving MbedTLS performance
  • Someone stepping up to maintain TinyCrypt
  • ???

Relying on an unmaintained crypto library doesn't seem like a valid long term solution here, though, and PSA gives us a neutral, somewhat mature and vetted translation layer that allows us to solve this problem (with additional engineering effort), and adds flexibility as other crypto issues bubble to the surface in the future. It doesn't solve every problem, or consider every use case, but we can address those requirements case by case unless a better intial proposition is put on the table. The problem with TinyCrypt isn't going away, so we need to propose something.

I'd encourage you to raise individual issues with the larger security planning/goals of the project (controlled de-initialization, etc.), tracking those goals here: https://github.com/zephyrproject-rtos/zephyr/projects/24

But I'm not persuaded those concerns preclude adopting PSA as a first large steps towards making crypto more maintainable longer term for the Zephyr project simply because PSA won't solve every problem out of the box. Of course it won't, but as a community we can still work towards addressing needs case by case, and again ... what's a better alternative if PSA is insufficient, as were early attempts at adopting DT in Zephyr?

@frkv
Copy link
Collaborator

frkv commented Apr 5, 2022

The statement that the PSA Crypto APIs doesn't do deinitialization is only partially true. I want to split my answer into two parts:

  1. Transient states
  2. Global states

I'm going to use the standardized terms in PSA for crypto drivers. Crypto-functionality executed on an external chip is interfaced through an opaque PSA driver. The "normal" PSA driver is called a transparent PSA driver and it gets its name from the fact that memory of keys is accessible from the CPU.

Transient states

There are in fact standard features in Mbed TLS to do deinitialization of context information for individual crypto operations. These are used for error-scenarios and for aborted or finalized operations. A similar functionality is extended to PSA Crypto APIs run in environments with TrustZone features is used (e.g. in TF-M). In this case the functionality is also coupled with bookkeeping of a finite number of "accessible slots" to do cryptographic operations inside the secure image. Anything that fails or is finalized must be deinitialized by the system to make room for the next operation.

The same functionality also exists for cryptography executed using oqaue drivers. It is completely up to the implementer of an opaque driver type to do bookeeping at failure or finalization or an operation to set the external chip in a quiescent state (including optionally handling DMA, interrupts etc.)

Global states

The Mbed TLS implementation keeps some global information after psa_crypto_init has been called, mainly keeping track of the RNG state. There exists a function to deinitialize this global state, but it may be a bit broad for the use-case as it clears all data associated with the PSA layer, including the whole key store. (volatile key store, not persistent key store)

I would suggest proposing an addition to the PSA Crypto API spec for deinitialization. We can propose psa_crypto_deinit, a function that clears out the RNG state and any additional held states in a user function (e.g. in HW accelerators or external security chips)

When it comes to PSA driver layer (transparent and opaque) the documentation in Mbed TLS project states that the drivers can have an init-function (if need be). We would need to propose an addition to do PSA driver deinitialization to handle clearing out global states.

With a few additions, then there will be no problem to do cleanups of DMA, interrupts and caches and reaching quiescent state of any external chips. PSA Crypto APIs (and drivers) may be a bit simplistic on global state management right now, but it wouldn't take a lot to rectify this....

@frkv
Copy link
Collaborator

frkv commented Apr 5, 2022

There are multiple ways persistent storage work in the PSA crypto APIs so a bit of explanation is needed to ensure a full understanding of the options (and the impacts).

Simple usecase: Keys stored in regular flash

The PSA Crypto APIs have been developed with the Arm PSA (e.g. via TF-M) as the backdrop. There is a distinguishing between crypto operations and storage operations inside the Arm PSA specification. The key storage for crypto operations is placed in a module called the internal trusted storage or ITS for short. This exists as a separated module in TF-M implementation, living outside of the PSA Crypto module.

Every API that handles persistent key storage and retrieval in PSA Crypto APIs in Mbed TLS will sooner or later call an ITS API under the hood.

On devices that can use TF-M: Implement a flash driver for your device and everything is provided out-of-the-box
On devices that can't use TF-M: If you are using Mbed TLS as the defacto implementation of PSA APIs, then you would need to enable native PSA ITS support and implement 4 APIs to give the ability to store/retrieve data from regular FLASH. Our company has done this using the Settings subsystem in Zephyr as a "backend" for PSA ITS (pure C)

Efforts on our part: The decision will be whether or not to use Mbed TLS implementation for key handling (with addition of native ITS support) or to build this functionality from scratch

Complex usecase: Keys stored in external security chip

The PSA Crypto implementation in Mbed TLS describes the term opaque PSA drivers. Opaque PSA drivers enables key usage that happens outside of the chip, meaning the CPU can only refer to the key material but never actually hold it.

The opaque PSA drivers is supported in such a way that you can use pre-provisioned keys or import keys on your own (if the external security chip supports it). The way to distinguish between these keys and volatile key or keys stored in regular flash (e.g. using PSA ITS) is the identifier for the key.

Efforts on our part: Limited. We just need to support opaque PSA drivers to ensure that any customer that has a specialized way of storing keys off-chip still get the features they need

@gregshue
Copy link

gregshue commented Apr 5, 2022

@d3zd3z wrote:

This issue is not about whether Zephyr will use or provide the PSA crypto API. That is already the case. What we are wanting to do here is replace the usage of two existing crypto APIs (both limited to a fairly small scope), and change the in-Zephyr code that uses these APIs to use the PSA API instead.

In that case this issue is poorly named and described. It sounds like you simply want to deprecate the existing non-PSA crypto APIs. There is no need to "adopt" or evaluate anything, the listing of a proprietary api as an alternative is really out-of-scope, and the problem of not being able to "take full advantage of HW accelerators" is irrelevant to this issue. Maintainers would automatically migrate to the existing non-deprecated API.

@microbuilder wrote:

But that's how engineering works ... you have a goal in mind, and you take the tools that you do have today, you make compromises that at least move in the desired direction, and invest in tooling making things better as you go.

Safety systems, and now secure IoT systems (due to regulations), need to be engineered to meet requirements imposed on the system by the customers (end users)/stakeholders (Linux Foundation, etc.). We get to compromise on design decisions, but compromises on requirements must be negotiated with the stakeholders.

@frkv wrote:

With a few additions, then there will be no problem to do cleanups of DMA, interrupts and caches and reaching quiescent state of any external chips. PSA Crypto APIs (and drivers) may be a bit simplistic on global state management right now, but it wouldn't take a lot to rectify this....

I completely agree that it wouldn't take a lot to rectify this. I just was raising awareness that it currently doesn't solve the stated problem and we have needs that they don't seem to be aware of or concerned with.

@gregshue
Copy link

gregshue commented Apr 5, 2022

@microbuilder Do you see de-initialization as an enhancement request (to SYS_INIT()), as a new feature, or as an RFC/proposal?

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 5, 2022

In that case this issue is poorly named and described. It sounds like you simply want to deprecate the existing non-PSA crypto APIs. There is no need to "adopt" or evaluate anything, the listing of a proprietary api as an alternative is really out-of-scope, and the problem of not being able to "take full advantage of HW accelerators" is irrelevant to this issue. Maintainers would automatically migrate to the existing non-deprecated API.

Indeed, most of this probably is out of scope for this issue. But, that doesn't mean the discussion is unwanted or unwarranted. Bringing the issues up is how we learn about them.

Keep in mind, also, that bringing up requirements, even creating issues to track them, doesn't necessarily mean they are going to be addressed. Zephyr is a collaborative effort. For the most part, there isn't anybody sitting around waiting to work on requested features, and for the most part, features will have to be implemented by those that need them.

The advantage we have, is that a lot of these issues, esp things like deinitialization, are likely to be shared needs, and the implementation effort can also be shared.

@microbuilder
Copy link
Member

microbuilder commented Apr 5, 2022

@microbuilder Do you see de-initialization as an enhancement request (to SYS_INIT()), as a new feature, or as an RFC/proposal?

Ideally, an RFC/proposal in my opinion stating clearly how you think this could or should work.

This brings up an interesting detail for the PSA API, though. PSA is written in isolation from the implementation(s) itself. As you alluded to in an earlier comment, it defines requirements, codifying those in an API and reference documentation, and those are then implemented in something like TF-M (as a reference PSA implementation). The requirement comes first.

In order to influence the PSA API, we need to define new requirements for consideration in the published standards, and make a specific API suggestions to them. I'm not 100% sure what the ideal workflow for that is. We can do that in Zephyr as an RFC/proposal, and once consensus is reached that it solves our needs, escalate that for review to the PSA authors. But we probably do need to determine how this works to have it officially considered for adoption into the PSA, or if we're OK with Zephyr-specific additions on top of the published standards.

@gregshue
Copy link

gregshue commented Apr 5, 2022

I'll create the initial RFC for clean handoff of HW state from boot to app image. This will provide one user need for general de-initialization, leading to the PSA proposal.

@gregshue
Copy link

gregshue commented Apr 5, 2022

#44564 Security: Clean disable of boot image and hardware before handoff to app image has been created.

@gilles-peskine-arm
Copy link

Hi! I'm an Mbed TLS developer and architect of the PSA crypto API. I'm happy that you're considering our work and always open to feedback. I'll address some of the points raised here. Please don't hesitate to ask further questions and let me know if I've missed something (this is a long thread!).

Resource usage

The major reason to use TinyCrypt is due constraint resources, will PSA crypto be able to achieve similar requirements.

The PSA API is pretty abstract because it caters for many different scenarios. The more of this flexibility an implementation of this API supports, the more RAM and code size it's likely to require. Additionally, an implementation that's structured as the PSA API on top of another API will have some overhead due to the API translation layer.

It would certainly be possible to build a PSA API layer on top of (e.g.) tinycrypt. You'd get the performance of tinycrypt with some RAM and code overhead. You could reduce this overhead by forking tinycrypt and changing data representations and dispatch, but then you'd have to become maintainers of a crypto implementation. The PSA API is not just a crypto API, but simultaneously also a key store API: keys are always accessed via handles. (The reason for this design choice is to allow applications to transparently access opaque keys, e.g. keys in a secure element/enclave/world/service/...) This is likely to incur a small inescapable overhead compared to an implementation where keys are accessed as pointers to the key material.

Mbed TLS has quite a bit of flexibility (support for acceleration drivers, support for opaque keys, support for multiple curves, ...) and security (defense against local timing side channels) which have a cost, mainly in code size. In addition, it's structured as PSA over the legacy mbedtls_ API, so there's overhead that we haven't optimized yet. But even an ideally optimized Mbed TLS can't beat a specialized implementation for a single curve on a specific platform.

To keep the code size down with Mbed TLS, please make sure to use link-time optimization. This has a significant impact when optimizing for code size (but negligible when optimizing for performance).

Driver support in Mbed TLS

As of Mbed TLS 3.1, drivers are supported for most algorithms, but you have to write your own psa_crypto_driver_wrappers.c and psa/crypto_driver_contexts_*.h. The generation of these files from a JSON description is ongoing work. Make sure to enable MBEDTLS_PSA_CRYPTO_CONFIG in addition to MBEDTLS_PSA_CRYPTO_DRIVERS to disable the software implementation. Unfortunately, this is not yet fully supported for X.509/TLS (once again, it's ongoing work).

You can follow our major work items on the Mbed TLS epic board. If you'd like to influence our schedule, let us know what your priorities are, but please be aware that our schedule is already pretty full for 2022 and beyond. Pull requests are welcome, but our main bottleneck is review — if you'd like Mbed TLS to progress faster, the best way you can help us is by onboarding for the long haul as a reviewer.

The PSA APIs do not seem to provide flexible (enough) configuration option support for crypto accelerators yet. To reduce the code size it will be important to enable/disable specific functionalities in the PSA driver level. For example it should be possible for crypto accelerators to enable SHA256 and nothing else. Even though in the current state there are configuration options for this (PSA_WANT_ALG_SHA_256), I could not find any finalized configuration option for accelerators. There are some accelerator configurations (MBEDTLS_PSA_ACCEL_ALG_SHA_256) which does not seem to be used much in the code and they don't have a finalized structure.

#define MBEDTLS_PSA_CRYPTO_CONFIG
#define MBEDTLS_PSA_CRYPTO_DRIVERS
#define PSA_WANT_ALG_SHA_256
#define MBEDTLS_PSA_ACCEL_ALG_SHA_256
  • edit library/psa_crypto_driver_wrappers.c to add your dispatch code and include/psa/crypto_driver_contexts_primitives.h to add your operation representation.

The CSRNG support from PSA APIs is lacking at the moment. There is a psa_generate_random function available but the entropy interface as specified in the driver proposal here is not fully implemented.

The only thing we support for now is to completely replace the PSA RNG subsystem with MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG. We aim to fully implement the PSA API in the future.

And additionally, configuration options for enabling CTR_DRBG or HMAC_DRBG using purely PSA APIs are lacking.

This is also on our backlog.

Deinitialization and handover

From a cursory look it seems PSA Crypto API and Zephyr system interface have a common omission: calls that support a controlled de-initialization of the system.

I would suggest proposing an addition to the PSA Crypto API spec for deinitialization. We can propose psa_crypto_deinit, a function that clears out the RNG state and any additional held states in a user function (e.g. in HW accelerators or external security chips)

We've considered having deinitialization in the API, but currently it's left out. Should init(); init(); deinit(); guarantee deinitialization when deinit() returns, or should it take as many deinit() calls as init() calls? (In practice the init() calls would come from separate libraries that aren't aware of each other.) I think the best answer heavily depends on the system architecture, which is why it was left out even as an optional feature (some platforms just won't support deinit).

Mbed TLS has mbedtls_psa_crypto_free() which deinitializes the whole PSA subsystem immediately (moslty intended for use in tests).

We do plan to add partial initialization as an optional, but standardized feature (e.g. initialize the key store without initializing the RNG).

I think it would make sense for bootloader handover to use a Zephyr-specific partial deinitialization feature.

Asynchronous operations

As a developer of resource constrained embedded systems that need to support controlled disablement and cancel functionality, I need to use asynchronous API operations so that I can minimize RAM usage (fewer threads/stacks)

We have no plans for an asynchronous crypto API in PSA. The PSA firmware framework API is entirely synchronous. The PSA crypto API is independent of the firmware framework API, but it's designed to be compatible. An asynchronous API is very difficult to implement and is very sensitive to the precise way communication works, so I don't think Arm is likely to design an asynchronous crypto API.

For highly constrained systems that need to perform long interruptible operations without relying on multiple threads, Mbed TLS has a feature we call restartable ECC which allows ECC operations to be broken down into short-duration blocks that the application loops over. We plan to standardize this for at least signature and key agreement in the PSA API.

As a developer of mechatronic devices built on Zephyr, I need to cancel arbitrary operations so that I can support user-initiated Cancel functionality.

We are unlikely to ever support cancellation as such inside Mbed TLS functions. However, you can implement this on top of restartable operations, or (with a bit less RAM overhead) we could add cancellation points as a build-time alternative to restartable operations.

Multiple implementations

As a developer of software models of hardware, I need to have a single executable support both hardware and software implementations of a specific functionality so that I can run predictive maintenance models.

The PSA API allows fallback from accelerator drivers: you can specify at build time that a driver requires a fallback, and then if the driver returns PSA_ERROR_NOT_SUPPORTED the software implementation (or the next driver) will take over. While this was primarily intended for partial hardware support (e.g. hardware that only supports certain key sizes), a driver could choose to request fallback for other reasons.

Discussion venues

These issues aside, we are getting very distracted from the question being raised by this issue, which is whether or not Zephyr should consider the PSA crypto API to replace some of the other APIs that are used within the system.

Sorry...

PSA doesn't cover every possible edge case. Clearly. It's also good to identify the use cases it doesn't give us 100% coverage of and figure out what we can or should do to address those. But does not having every use case covered exclude the choice to migrate to the PSA API, and if PSA is deemed 'insufficient', what's the alternative here? I don't really see anything else that would give us something better. PSA isn't perfect ... but what's the net gain alternative?

In order to influence the PSA API, we need to define new requirements for consideration in the published standards, and make a specific API suggestions to them. I'm not 100% sure what the ideal workflow for that is.

If there are things you think we should add to PSA crypto API, please let us know. The best place would be the psa-crypto mailing list. If you'd like to contact Arm confidentially about the PSA crypto API, you can email us at mbed-crypto@arm.com. We'll also take into accounts things requested via Mbed TLS issues (which are of course the best place to request for something to be implemented).

We can do that in Zephyr as an RFC/proposal, and once consensus is reached that it solves our needs, escalate that for review to the PSA authors. But we probably do need to determine how this works to have it officially considered for adoption into the PSA, or if we're OK with Zephyr-specific additions on top of the published standards.

Although we do require evidence of feasibility before finalizing an API, it's perfectly fine (and, I think, better) to start the discussion early, to avoid committing resources into something that we know won't be acceptable for some reason.

@gregshue
Copy link

gregshue commented Apr 5, 2022

@gilles-peskine-arm Thank you for the answers. Here are my thoughts:

Deinitialization and handover

I think the best answer heavily depends on the system architecture, which is why it was left out even as an optional feature (some platforms just won't support deinit).

I understand the struggle, but leaving it out entirely is the least desirable option for those of us that are reusing (not tweaking) the API or code.

I think you identified the most practical use cases: require a deinit for every init, executing the functionality on the last call; execute deinit on the first call. This begs for a build configuration. The default configuration probably doesn't matter since somebody won't be happy either way.

I struggle trying to recommend how to handle excessive deinit calls. It seems like it indicates a poorly-controlled API usage, but it should be an idempotent operation.

I think it would make sense for bootloader handover to use a Zephyr-specific partial deinitialization feature.

For Zephyr-based systems, I don't expect there to be more than one init and one deinit call. The MCUboot image executes the multi-threaded RTOS kernel and can contain whatever other drivers/subsystems/persistent storage and UI is needed by the product developer. So our "Zephyr-specific partial deinitialization" needs to be composable, extensible, and in reverse-dependency order. Hence, the need for deinit (essentially a static object destructor).

I think we ought to request support for deinit - after we align on our use case(s). :-)

Asynchronous operations

I expect we can build a sufficient Cancel solution around restartable ECC. Thank you for pointing this option out.

Multiple implementations

The fallback mechanism covers the practical use cases that come to mind. At some point we just have to use multiple builds and inject data for verification. Thank you for pointing this option out.

@ruuddw
Copy link
Member

ruuddw commented Apr 8, 2022

As discussed and agreed in last Security WG, here's my list of some requirements that come to mind when selecting/defining a Crypto API for Zephyr. The discussion here seems to mix few things, let's separate those:

  1. Zephyr currently doesn't have a 'complete' crypto api that abstracts from various implementations and allows for portable driver/application code.
  2. Tinycrypt is end of life and needs an alternative. Note, Tinycrypt is used both as implementation and as API in BT.
    We could solve the tinycrypt issue without standardizing on a preferred Zephyr crypto API (e.g. use uECC).

Common API related requirements

  • support hardware, software, and hybrid HW/SW solutions
    • HW offload / asynchronous operations
    • DMA support
    • multiple instances/sessions/contexts
    • secure keys, not (plaintext) accessible to software
    • HW capabilities can be queried and system optimized for these (SW fallbacks, number of contexts/keys supported, etc.)
  • allow for security hardened implementations (side-channel protections, fault-injection)
  • support for all mainstream crypto, but also SM2/3/4, Edwards/Curve 25519, Poly/Chacha, Siphash, ...
  • extensible
    • additional, new crypto algorithms can be added (e.g. PQC)
  • random number generation included? Or stick with Zephyr current solution for that?
  • footprint/performance
    • scalable solution, only include the crypto you need
    • API should allow for very small implementations, on par with current tinycrypt
    • match interfaces of existing (open source) crypto implementations and hardware accelerators/drivers, so no extensive 'glue' or 'shim' code is required

Alternative APIs that could be considered:
- PKCS#11, OpenSSL, WolfCrypt, (and many more ...)
- Zephyr device driver style API in stead of crypto lib API (like current crypto.h, but extend with PK crypto)
- no preferred Zephyr API, simply support multiple/different ones and accept that applications are not portable

@ceolin
Copy link
Member Author

ceolin commented Apr 9, 2022

Alternative APIs that could be considered: - PKCS#11, OpenSSL, WolfCrypt, (and many more ...) - Zephyr device driver style API in stead of crypto lib API (like current crypto.h, but extend with PK crypto) - no preferred Zephyr API, simply support multiple/different ones and accept that applications are not portable

Not having a preferred Zephyr API is not problem IMHO, the problem is having multiple API / implementations being used internally. It doesn't make a sense for me having to have TinyCrypt and mbedTLS in the same binary just because different modules decided to use different APIs / implementation.

@ceolin ceolin self-assigned this Apr 9, 2022
@dleach02
Copy link
Member

coming late to this. Does PSA crypto APIs have the ability to switch between HW accelerated crypto and SW only crypto? I'm aware of a use case where that would be needed.

@dleach02
Copy link
Member

coming late to this. Does PSA crypto APIs have the ability to switch between HW accelerated crypto and SW only crypto? I'm aware of a use case where that would be needed.

As I read through above I see the this which implies PSA API meets this need:

The PSA API allows fallback from accelerator drivers: you can specify at build time that a driver requires a fallback, and then if the driver returns PSA_ERROR_NOT_SUPPORTED the software implementation (or the next driver) will take over. While this was primarily intended for partial hardware support (e.g. hardware that only supports certain key sizes), a driver could choose to request fallback for other reasons.

@ceolin
Copy link
Member Author

ceolin commented Apr 11, 2022

coming late to this. Does PSA crypto APIs have the ability to switch between HW accelerated crypto and SW only crypto? I'm aware of a use case where that would be needed.

yes, it is possible AFAIK. You can have multiple drivers for the same operation.

@dpkristensen
Copy link

Seems like there's a lot of the security folks in here, so I wanted to ask about this in relation to #56154

I think having a better API would help facilitate the transition to newer algorithms like this; thoughts?

@carlescufi carlescufi changed the title PSA Crypto API adoption on Zephyr PSA Crypto API adoption in Zephyr Jun 29, 2023
tomi-font added a commit to tomi-font/zephyr that referenced this issue Apr 25, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@nordicjm
Copy link
Collaborator

nordicjm commented Apr 26, 2024

This is the first I have heard about this (for tinycrypt to be removed) from this pr: #71947

I absolutely object to this, tinycrypt has an important use for MCUmgr which is for generating sha256 hashes of files and sending them to clients, the proposed PR removes tinycrypt and only allows using mbedtls which has a 10.6KiB bump in flash requirements, this is a flat out no and I will never ack that PR. So some compromise has to be made for the many devices in zephyr that have lower amounts of flash, I don't really see C code generating sha256 hashes as needing to be maintained - it's a simple enough feature, it's worked for years and it works fine now so I do not see the merit in forcing the removal without any sort of usable replacement. It generates a hash for a file, I really don't care less about PSA crypto for that, it doesn't interest me in the slightest, it needs to be a small code size, that's what is important (if someone wants to use PSA crypto for it instead, at their choice then that's fine, but the minimal size option should still exist and be the default). And not withstanding you would be silently breaking many devices of users and prevent them from evening being able to use zephyr 3.7, I can count 4 zephyr-based devices I developed in my home that would just completely break from this change due to insufficient flash (2xstm32f301, 1xnrf52832, 1xnrf52810)

@dpkristensen
Copy link

@nordicjm PSA is a relatively new thing; they have an API which looks like it's meant to serve as a HAL. PSA as an API surface could be hooked into TC just as well as other libraries like Oberon, but it would have to be a first-class API provided by Zephyr. Right now, PSA only exists as a backend to mbed TLS, and probably only because mbed TLS started using it. The down-side to such a solution is that if TC doesn't support some operation, it will just generate a linker error when the function isn't implemented.

I had proposed another solution for a flexible API surface as well on #56154 which is not based on PSA.

@ceolin
Copy link
Member Author

ceolin commented Apr 26, 2024

This is the first I have heard about this (for tinycrypt to be removed) from this pr: #71947

That's is not fair, this discussion is happening for a long time, not only in this issue but also in security working group and
it was presented in TSC (long time ago) as a goal.

I absolutely object to this, tinycrypt has an important use for MCUmgr which is for generating sha256 hashes of files and sending them to clients, the proposed PR removes tinycrypt and only allows using mbedtls which has a 10.6KiB bump in flash requirements, this is a flat out no and I will never ack that PR. So some compromise has to be made for

I commented in #71947 (comment)
These additional 10k is bringing much more than just sha256 (md5, sha512, sha1, dtls ....) . Disabling these things the difference is ~500 bytes (and consumes less ram)

the many devices in zephyr that have lower amounts of flash, I don't really see C code generating sha256 hashes as needing to be maintained - it's a simple enough feature, it's worked for years and it works fine now so I do not see the merit in forcing the removal without any sort of usable replacement. It generates a hash for a file, I really don't care less about PSA crypto for that, it doesn't interest me in the slightest, it needs to be a small code size, that's what is important (if someone wants to use PSA crypto for it instead, at their choice then that's fine, but the minimal size option should still exist and be the default). And not withstanding you would be silently breaking many devices of users and prevent them from evening being able to use zephyr 3.7, I can count 4 zephyr-based devices I developed in my home that would just completely break from this change due to insufficient flash (2xstm32f301, 1xnrf52832, 1xnrf52810)

We are not removing TinyCrypt for the sake of remove. TinyCrypt is no longer maintained, which means no security fixes. i understand that this is working but it we have a serious dependability issue in continue supporting this library. .

tomi-font added a commit to tomi-font/zephyr that referenced this issue Apr 30, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
tomi-font added a commit to tomi-font/zephyr that referenced this issue May 2, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
tomi-font added a commit to tomi-font/zephyr that referenced this issue May 2, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
tomi-font added a commit to tomi-font/zephyr that referenced this issue May 2, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
tomi-font added a commit to tomi-font/zephyr that referenced this issue May 3, 2024
As part of ongoing work to move away from TinyCrypt and towards PSA
(zephyrproject-rtos#43712), make fs_mgmt use either PSA (when available) or MbedTLS
(as a fallback) for SHA-256.

For now this is guarded by CONFIG_BUILD_WITH_TFM.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Crypto / RNG area: Security Security RFC Request For Comments: want input from the community
Projects
Status: No status
Status: Todo
Development

No branches or pull requests