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

mbedtls: add a new Kconfig file for PSA_WANT logic #78531

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

valeriosetti
Copy link
Collaborator

The already existing Kconfig.psa maps Mbed TLS's PSA_WANT_xxx symbols to Kconfigs that can be used in Zephyr to select which PSA crypto API feature should be enabled in the build. In order to ease maintainability, Kconfig.psa is automatically generated so it should not be edited manually to add logic between Kconfigs symbols defined there. As a consequence a new Kconfig file is introduced in this PR, named Kconfig.psa.logic, to address this limitation. This new Kconfig file does not add new public symbols (only hidden ones, if needed) and it simply adds logic between PSA_WANT ones.

This commit also updates the pyhton script create_psa_files.py in order to adapt it to the changes done in Kconfig.psa.

@valeriosetti valeriosetti force-pushed the psa-depedency-logic branch 3 times, most recently from ade8e9e to eb033cc Compare September 18, 2024 08:27
@tomi-font tomi-font removed their request for review September 18, 2024 08:35
modules/mbedtls/create_psa_files.py Outdated Show resolved Hide resolved
modules/mbedtls/Kconfig.psa Outdated Show resolved Hide resolved
modules/mbedtls/create_psa_files.py Show resolved Hide resolved
modules/mbedtls/Kconfig.psa.logic Outdated Show resolved Hide resolved
modules/mbedtls/Kconfig.psa.logic Outdated Show resolved Hide resolved
EC-JPAKE.

# All the following logical constraints are taken from the PSA API documentation:
# https://arm-software.github.io/psa-api/crypto/1.1/index.html
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which section exactly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It could've been good to keep this reference, but just to make it more precise if possible.

modules/mbedtls/Kconfig.psa.logic Outdated Show resolved Hide resolved
modules/mbedtls/Kconfig.psa.logic Outdated Show resolved Hide resolved
modules/mbedtls/Kconfig.psa.logic Outdated Show resolved Hide resolved

# Mbed TLS does some assumption on asymmetric keys' build symbols
# (see modules/crypto/mbedtls/include/psa/crypto_adjust_config_key_pair_types.h):
# - PSA_WANT_KEY_TYPE_xxx_KEY_PAIR_BASIC is automatically set whenever one of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# - PSA_WANT_KEY_TYPE_xxx_KEY_PAIR_BASIC is automatically set whenever one of
# - that PSA_WANT_KEY_TYPE_xxx_KEY_PAIR_BASIC is set whenever one of

Copy link
Collaborator

Choose a reason for hiding this comment

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

automatically is still there but could be removed IMO.

The already existing "Kconfig.psa" maps Mbed TLS's PSA_WANT_xxx
symbols to Kconfigs that can be used in Zephyr to select which
PSA crypto API feature should be enabled in the build. In order
to ease maintainability, "Kconfig.psa" is automatically generated
so it should not be edited manually to add logic between
Kconfigs symbols defined there. As a consequence a new Kconfig file
is introduced in this commit, named "Kconfig.psa.logic", to
address this limitation. This new Kconfig file does not add new
public symbols (only hidden ones, if needed) and it simply adds
logic between PSA_WANT ones.

This commit also renames "Kconfig.psa" as "Kconfig.psa.auto" to
put it at the same "naming level" as the newly created file and,
at the same time, emphasize that it is an automatically generated
file.

Signed-off-by: Valerio Setti <vsetti@baylibre.com>
# Copyright (c) 2024 BayLibre SAS
# SPDX-License-Identifier: Apache-2.0

# This file extends Kconfig.psa (which is automatically generated) by adding
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# This file extends Kconfig.psa (which is automatically generated) by adding
# This file extends Kconfig.psa.auto (which is automatically generated) by adding

# SPDX-License-Identifier: Apache-2.0

# This file extends Kconfig.psa (which is automatically generated) by adding
# some logic between PSA_WANT symbols.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# some logic between PSA_WANT symbols.
# some logic around PSA_WANT symbols.

EC-JPAKE.

# All the following logical constraints are taken from the PSA API documentation:
# https://arm-software.github.io/psa-api/crypto/1.1/index.html
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could've been good to keep this reference, but just to make it more precise if possible.

Copy link
Collaborator

@tomi-font tomi-font left a comment

Choose a reason for hiding this comment

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

Almost there!

Comment on lines +66 to +67
# Dependencies between KDF (key derivation function) algorithms and low-level
# crypto ones.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you are making such a comment, then it would make sense to properly cover all the sections. Because I think that this section stops at PSA_WANT_ALG_TLS12_PRF?
(Or just keep the old comment, IMO it was good, with a reference to the PSA Crypto spec.)

config PSA_WANT_ALG_PBKDF2_HMAC
depends on PSA_WANT_KEY_TYPE_HMAC
depends on PSA_WANT_ALG_HMAC
depends on PSA_CAN_SOME_HASH
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove all the depends on PSA_WANT_KEY_TYPE_* in here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants