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

Backport 2.28: prepare for dynamically sized key store #9256

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jun 13, 2024

Partial backport of #9403.

  • Improve MBEDTLS_STATIC_ASSERT
  • Fix mbedtls_psa_register_se_key is unusable with volatile keys #9253.
  • Documentation and test improvements.
  • I'm not sure about 7dea096: fixing configurations where MBEDTLS_PSA_KEY_SLOT_COUNT > 4096 and MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled. That seems unlikely, and nobody complained in the three years since we introduced MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS in Mbed TLS 2.27.0. It's a less unlikely configuration in 3.6, which is more PSA (especially due to TLS 1.3), which is why I wanted to fix it even if the change is a bit disruptive for an LTS (but still not breaking any documented behavior). At the moment, the 2.28 backport doesn't have this fix.
  • Skip everything related to CTR_DRBG using PSA AES, which is impossible in 2.28.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

@gilles-peskine-arm gilles-peskine-arm added bug component-psa PSA keystore/dispatch layer (storage, drivers, …) priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Jun 13, 2024
@gilles-peskine-arm gilles-peskine-arm changed the title Backport 2.28: dynamically sized key store Backport 2.28: prepare for dynamically sized key store Aug 6, 2024
@gilles-peskine-arm gilles-peskine-arm added needs-ci Needs to pass CI tests priority-very-high Highest priority - prioritise this over other review work and removed needs-work priority-high High priority - will be reviewed soon labels Aug 6, 2024
At the top level, the macro would have had to be used without a following
semicolon (except with permissive compilers that accept spurious semicolons
outside of a function), which is confusing to humans and indenters. Fix
that.

Signed-off-by: Gilles Peskine <[email protected]>
mbedtls_psa_register_se_key() is not usable with volatile keys, since there
is no way to return the implementation-chosen key identifier which would be
needed to use the key. Document this limitation. Reject an attempt to create
such an unusable key. Fixes Mbed-TLS#9253.

Signed-off-by: Gilles Peskine <[email protected]>
Restricting the built-in key range would be an API break since applications
can hard-code a built-in key value and expect that it won't clash with
anything else. Make it harder to accidentally break the API.

Signed-off-by: Gilles Peskine <[email protected]>
Ensure that a key ID can't be in range for more than one of volatile keys,
persistent (i.e. user-chosen) keys or built-in keys.

Signed-off-by: Gilles Peskine <[email protected]>
The description was misleading: setting the option doesn't “restrict” the
number of slots, that restriction exists anyway. Setting the option merely
determines the value of the limit.

Signed-off-by: Gilles Peskine <[email protected]>
Split the "many transient keys" test function in two: one that expects to
successfully create many keys, and one that expects to fill the key store.
This will make things easier when we add a dynamic key store where filling
the key store is not practical unless artificially limited.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm force-pushed the psa-keystore-dynamic-backport-2.28 branch from af6f624 to 3a51fdc Compare August 7, 2024 09:18
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-ci Needs to pass CI tests labels Aug 7, 2024
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@gilles-peskine-arm gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Aug 9, 2024
@gilles-peskine-arm gilles-peskine-arm added this pull request to the merge queue Aug 9, 2024
Merged via the queue into Mbed-TLS:mbedtls-2.28 with commit 36548ee Aug 9, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-psa PSA keystore/dispatch layer (storage, drivers, …) priority-very-high Highest priority - prioritise this over other review work size-xs Estimated task size: extra small (a few hours at most)
Development

Successfully merging this pull request may close these issues.

3 participants