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

component_test_full_no_bignum doesn't actually disable bignum #9190

Open
gilles-peskine-arm opened this issue May 27, 2024 · 5 comments
Open
Assignees
Labels
bug component-test Test framework and CI scripts size-xs Estimated task size: extra small (a few hours at most)

Comments

@gilles-peskine-arm
Copy link
Contributor

As of #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
Copy link
Contributor

I've fixed that in #9185.

@mpg
Copy link
Contributor

mpg commented Jun 4, 2024

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
Copy link
Contributor

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
Copy link
Contributor

mpg commented Jul 22, 2024

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.

@mpg mpg closed this as completed Jul 22, 2024
@gilles-peskine-arm
Copy link
Contributor Author

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

@gilles-peskine-arm gilles-peskine-arm added size-xs Estimated task size: extra small (a few hours at most) and removed size-s Estimated task size: small (~2d) labels Jul 22, 2024
@gilles-peskine-arm gilles-peskine-arm self-assigned this Jul 22, 2024
@minosgalanakis minosgalanakis moved this to Test cases not executed in Mbed TLS Epics Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-test Test framework and CI scripts size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: Test cases not executed
Development

No branches or pull requests

3 participants