component_test_full_no_bignum doesn't actually disable bignum

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.

https://www.trustedfirmware.org/projects/mbed-tls/

Other

5.56k stars 2.61k forks source link

component_test_full_no_bignum doesn't actually disable bignum #9190

Open gilles-peskine-arm opened 6 months ago

gilles-peskine-arm commented 6 months ago

As of https://github.com/Mbed-TLS/mbedtls/pull/9172, component_test_full_no_bignum actually enables all crypto because it has MBEDTLS_PSA_CRYPTO_CONFIG enabled. For example, PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY is enabled by default, so MBEDTLS_RSA_C and MBEDTLS_BIGNUM_C are auto-enabled to implement that.

The goal of this task is:

Fix component_test_full_no_bignum to do what it actually claims. For 3.6, it's enough to disable MBEDTLS_PSA_CRYPTO_CONFIG. For development leading to 4.0, we need to disable all asymmetric crypto through both MBEDTLS_xxx and PSA_WANT_xxx.
In component_test_full_no_bignum, check that mbedtls_bignum_xxx functions aren't getting included in the build.
Review other similar components.

Alternative goal: remove component_test_full_no_bignum because it's redundant with test-ref-configs on configs/config-symmetric-only.h?

ronald-cron-arm commented 6 months ago

I've fixed that in #9185.

mpg commented 5 months ago

Alternative goal: remove component_test_full_no_bignum because it's redundant with test-ref-configs on configs/config-symmetric-only.h?

It is especially redundant with component_test_psa_crypto_config_accel_ecc_no_bignum(). Specifically, component_test_full_no_bignum was added as part of preparation work for that component, so now that we have component_test_psa_crypto_config_accel_ecc_no_bignum() I don't think component_test_full_no_bignum really serves a purpose any more.

ronald-cron-arm commented 5 months ago

Alternative goal: remove component_test_full_no_bignum because it's redundant with test-ref-configs on configs/config-symmetric-only.h?

It is especially redundant with component_test_psa_crypto_config_accel_ecc_no_bignum(). Specifically, component_test_full_no_bignum was added as part of preparation work for that component, so now that we have component_test_psa_crypto_config_accel_ecc_no_bignum() I don't think component_test_full_no_bignum really serves a purpose any more.

Thanks Manuel. I am removing it in #9185.

mpg commented 4 months ago

Looks like this was fixed by #9185 - I'm not seeing full_no_bignum in development any more. So I'm closing this issue. Please shout if I missed something.

gilles-peskine-arm commented 4 months ago

I'm reopening because this is moot in development, but still a bug in 3.6.