Update the reference configs to use MBEDTLS_PSA_CRYPTO_CONFIG
#9057
Conversation
Ryan-Everett-arm
added
needs-review
Ryan-Everett-arm
force-pushed
the
crypto-config-test_ref_configs
branch
from
4be3dfd
to
d7f32a9
Compare
Ryan-Everett-arm
added
size-s
|
FYI all_u16-test_m32_o2 error is a timeout rather than a test failure |
|
I am happy to review this PR but won't have time today, and am out Mon and Tues next week, which is not ideal timing for Ryan's rotation. If this is not fully reviewed by Wednesday morning I will pick it up and do it Weds. |
gilles-peskine-arm
removed
needs-ci
There was a problem hiding this comment.
This is on the right track, but incomplete in several ways, notably because some features are still enabled only because we used to need them and not because we still want them.
I noticed a few preexisting small issues, which should be fixed in 3.6 with a small backport.
For validation we should check that we're still testing what we intended to test, and I found a preexisting problem that we're currently not testing what we intended to test. We'll probably need to put this pull request on hold until that is fixed, or at least until we understand the cause and how to fix it.
gilles-peskine-arm
added
needs-work
needs-preceding-pr
|
I have addressed the uncontroversial issues with this PR. The test coverage comments and HMAC comments may need some more discussion before a change can be made. |
ronald-cron-arm
requested review from
tom-daubney-arm
and removed request for
ronald-cron-arm
ronald-cron-arm
force-pushed
the
crypto-config-test_ref_configs
branch
from
93a7c66
to
33897b9
Compare
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
ronald-cron-arm
force-pushed
the
crypto-config-test_ref_configs
branch
from
33897b9
to
1f95ede
Compare
There was a problem hiding this comment.
The SSL tests that pass (grep 'configs/config' | grep -v ';test_suite_' | grep PASS) look sensible to me.
The configuration adjustment is unfortunately not quite right. Other than that and a minor thing in the Thread configuration, this looks good to me.
| * \brief Adjust PSA configuration by resolving some dependencies. | ||
| * | ||
| * See docs/proposed/psa-conditional-inclusion-c.md. | ||
| * If a cryptographic mechanism A depends on a cryptographic mechanism B and |
There was a problem hiding this comment.
This is actually not quite right when drivers are involved. If our implementation of A uses B, then MBEDTLS_PSA_BUILTIN_A should automatically enable PSA_WANT_B. (Not MBEDTLS_PSA_BUILTIN_B, so that we can provide A on top of a driver for B if there is a driver for B but not A.) But there could be a driver for A that doesn't do B.
Please clarify the documentation and fix the implementation. This may also require tweaking the order of the adjustments.
| */ | ||
|
|
||
| #ifndef PSA_CRYPTO_ADJUST_CONFIG_DEPENDENCIES_H | ||
| #define PSA_CRYPTO_ADJUST_CONFIG_DEPENDENCIES_H |
There was a problem hiding this comment.
Note: this will need to be reconciled with #9061, which changes the boilerplate that goes into all adjust-config headers.
gilles-peskine-arm
added
needs-work
and removed
needs-preceding-pr
The Mbed TLS implementations of ALG_TLS12_PRF, ALG_TLS12_PSK_TO_MS, ALG_HKDF, ALG_HKDF_EXTRACT, ALG_HKDF_EXPAND and ALG_PBKDF2 rely on HMAC operations through the driver interface. Thus if one of these algorithms is enabled and not accelerated, we need ALG_HMAC to be enabled (PSA_WANT_ALG_HMAC and PSA_WANT_KEY_TYPE_HMAC defined). As HMAC operations occur through the driver interface, HMAC operations can be accelerated even if the caller algorithm is not. Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
ronald-cron-arm
moved this from In Development
to In Review
in Roadmap Board for Mbed TLS
|
@gilles-peskine-arm I've addressed your last comments, please have another look. Thanks. |
ronald-cron-arm
added
the
needs-backports
tom-daubney-arm
added
approved
f5473a0
…configs-3.6 [Backport 3.6] Partial backport of #9057
Description
Update the reference configs to use the new PSA symbols and have
MBEDTLS_PSA_CRYPTO_CONFIGturned on. These configs are tested bycomponent_test_ref_configs, which runs them with PSA disabled/enabled.This doesn't modify
config-no-entropy.h, it is my understanding that PSA requires entropy so this config does not work with this change.The new config files were created by replacing legacy symbols with equivalent PSA symbols (the equivalences can be derived from
config_adjust_legacy_from_psa.handconfig_adjust_psa_from_legacy.h). The crypto config files are referenced in the same styleconfig-tfmuses. Defined and inferred symbols can be checked via./build/programs/test/query_compile_time_config -lProgresses #8153.
Dependency: #9063
PR checklist
Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")