Update the reference configs to use `MBEDTLS_PSA_CRYPTO_CONFIG` #9057

Ryan-Everett-arm · 2024-04-25T16:36:34Z

Description

Update the reference configs to use the new PSA symbols and have MBEDTLS_PSA_CRYPTO_CONFIG turned on. These configs are tested by component_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.h and config_adjust_psa_from_legacy.h). The crypto config files are referenced in the same style config-tfm uses. Defined and inferred symbols can be checked via ./build/programs/test/query_compile_time_config -l

Progresses #8153.

Dependency: #9063

PR checklist

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

changelog not required
3.6 backport for a few small preexisting issues: [Backport 3.6] Partial backport of #9057 #9160
2.28 backport not required - 4.0 work
tests check that each configuration still seems to do what it's intended to do, e.g. that it's executing the right test cases

tom-daubney-arm · 2024-04-26T13:30:28Z

FYI all_u16-test_m32_o2 error is a timeout rather than a test failure

tom-daubney-arm · 2024-04-26T13:31:38Z

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

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.

configs/config-symmetric-only.h

configs/crypto-config-ccm-psk-tls1_2.h

configs/config-ccm-psk-dtls1_2.h

configs/config-suite-b.h

configs/crypto-config-thread.h

configs/config-ccm-psk-dtls1_2.h

Ryan-Everett-arm · 2024-04-30T16:24:25Z

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.

gilles-peskine-arm

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.

configs/crypto-config-thread.h

tests/scripts/test-ref-configs.pl

gilles-peskine-arm · 2024-05-16T14:07:25Z

include/psa/crypto_adjust_config_dependencies.h

+ * \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


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.

gilles-peskine-arm · 2024-05-16T14:10:02Z

include/psa/crypto_adjust_config_dependencies.h

+ */
+
+#ifndef PSA_CRYPTO_ADJUST_CONFIG_DEPENDENCIES_H
+#define PSA_CRYPTO_ADJUST_CONFIG_DEPENDENCIES_H


Note: this will need to be reconciled with #9061, which changes the boilerplate that goes into all adjust-config headers.

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>

ronald-cron-arm · 2024-05-21T06:29:23Z

@gilles-peskine-arm I've addressed your last comments, please have another look. Thanks.

gilles-peskine-arm

LGTM

tom-daubney-arm

LGTM - thanks

…configs-3.6 [Backport 3.6] Partial backport of #9057

Ryan-Everett-arm added needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review labels Apr 25, 2024

Ryan-Everett-arm self-assigned this Apr 25, 2024

Ryan-Everett-arm force-pushed the crypto-config-test_ref_configs branch from 4be3dfd to d7f32a9 Compare April 26, 2024 09:46

Ryan-Everett-arm added size-s Estimated task size: small (~2d) priority-high High priority - will be reviewed soon labels Apr 26, 2024

ronald-cron-arm self-requested a review April 26, 2024 11:43

gilles-peskine-arm self-requested a review April 26, 2024 13:51

gilles-peskine-arm removed needs-ci Needs to pass CI tests needs-reviewer This PR needs someone to pick it up for review labels Apr 26, 2024

gilles-peskine-arm requested changes Apr 26, 2024

View reviewed changes

gilles-peskine-arm added needs-work needs-preceding-pr Requires another PR to be merged first and removed needs-review Every commit must be reviewed by at least two team members, labels Apr 26, 2024

ronald-cron-arm mentioned this pull request Apr 30, 2024

[DI] Move files from tests/scripts to mbedtls-framework Mbed-TLS/mbedtls-framework#12

Closed

ronald-cron-arm assigned ronald-cron-arm and unassigned Ryan-Everett-arm May 14, 2024

ronald-cron-arm requested review from tom-daubney-arm and removed request for ronald-cron-arm May 14, 2024 09:10

ronald-cron-arm added the needs-ci Needs to pass CI tests label May 15, 2024

ronald-cron-arm force-pushed the crypto-config-test_ref_configs branch from 93a7c66 to 33897b9 Compare May 15, 2024 17:25

ronald-cron-arm removed needs-work needs-ci Needs to pass CI tests labels May 16, 2024

ronald-cron-arm force-pushed the crypto-config-test_ref_configs branch from 33897b9 to 1f95ede Compare May 16, 2024 06:12

gilles-peskine-arm requested changes May 16, 2024

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-preceding-pr Requires another PR to be merged first labels May 16, 2024

gilles-peskine-arm mentioned this pull request May 17, 2024

ssl-opt.sh, compat.sh: Error out if not executing any tests #9149

Merged

4 tasks

ronald-cron-arm added 5 commits May 17, 2024 14:33

Fix PBKDF2_AES_CMAC_PRF_128 dependencies

c4c8bdf

Signed-off-by: Ronald Cron <ronald.cron@arm.com>

Fix crypto_adjust_config_dependencies.h documentation

b48c870

Signed-off-by: Ronald Cron <ronald.cron@arm.com>

Adjust crypto-config-thread.h

ca6b1e9

Signed-off-by: Ronald Cron <ronald.cron@arm.com>

Improve test-ref-configs.pl

b30cd3b

Signed-off-by: Ronald Cron <ronald.cron@arm.com>

ronald-cron-arm removed the needs-work label May 17, 2024

gilles-peskine-arm mentioned this pull request May 21, 2024

Document check-config.h and *adjust*.h as internal headers #9061

Merged

4 tasks

gilles-peskine-arm approved these changes May 21, 2024

View reviewed changes

ronald-cron-arm added the needs-backports Backports are missing or are pending review and approval. label May 22, 2024

ronald-cron-arm mentioned this pull request May 22, 2024

[Backport 3.6] Partial backport of #9057 #9160

Merged

4 tasks

tom-daubney-arm approved these changes May 22, 2024

View reviewed changes

tom-daubney-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 May 22, 2024

ronald-cron-arm added this pull request to the merge queue May 23, 2024

Merged via the queue into Mbed-TLS:development with commit f5473a0 May 23, 2024
5 of 6 checks passed

ronald-cron-arm mentioned this pull request May 23, 2024

Study: PSA_CRYPTO_CONFIG always on #8153

Closed

4 tasks

github-merge-queue bot pushed a commit that referenced this pull request May 23, 2024

Merge pull request #9160 from ronald-cron-arm/crypto-config-test_ref_…

baf6775

…configs-3.6 [Backport 3.6] Partial backport of #9057

gilles-peskine-arm mentioned this pull request May 30, 2024

Backport 2.28: ssl-opt.sh, compat.sh: Error out if not executing any tests #9204

Merged

3 tasks

mpg mentioned this pull request Oct 3, 2024

[DI] Move scripts used in the CI to the framework Mbed-TLS/mbedtls-framework#52

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the reference configs to use `MBEDTLS_PSA_CRYPTO_CONFIG` #9057

Update the reference configs to use `MBEDTLS_PSA_CRYPTO_CONFIG` #9057

Ryan-Everett-arm commented Apr 25, 2024 •

edited by ronald-cron-arm

Loading

tom-daubney-arm commented Apr 26, 2024

tom-daubney-arm commented Apr 26, 2024

gilles-peskine-arm left a comment

Ryan-Everett-arm commented Apr 30, 2024

gilles-peskine-arm left a comment

gilles-peskine-arm May 16, 2024

gilles-peskine-arm May 16, 2024

ronald-cron-arm commented May 21, 2024

gilles-peskine-arm left a comment

tom-daubney-arm left a comment

Update the reference configs to use MBEDTLS_PSA_CRYPTO_CONFIG #9057

Update the reference configs to use MBEDTLS_PSA_CRYPTO_CONFIG #9057

Conversation

Ryan-Everett-arm commented Apr 25, 2024 • edited by ronald-cron-arm Loading

Description

PR checklist

tom-daubney-arm commented Apr 26, 2024

tom-daubney-arm commented Apr 26, 2024

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Ryan-Everett-arm commented Apr 30, 2024

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm May 16, 2024

Choose a reason for hiding this comment

gilles-peskine-arm May 16, 2024

Choose a reason for hiding this comment

ronald-cron-arm commented May 21, 2024

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

tom-daubney-arm left a comment

Choose a reason for hiding this comment

Update the reference configs to use `MBEDTLS_PSA_CRYPTO_CONFIG` #9057

Update the reference configs to use `MBEDTLS_PSA_CRYPTO_CONFIG` #9057

Ryan-Everett-arm commented Apr 25, 2024 •

edited by ronald-cron-arm

Loading