Make PK tests work with only some curves accelerated

[mpg]

Currently (since #8075), all.sh components test_psa_crypto_config_accel_ecc_non_weierstrass_curves and test_psa_crypto_config_accel_ecc_weierstrass_curves have to skip the test suites for PK (and above), as their would be failures in test_suite_pk, test_suite_pkparse and test_suite_pkwrite.

@gilles-peskine-arm writes:

If some curves are accelerated but not others, and MBEDTLS_USE_PSA_CRYPTO is enabled, we don't define MBEDTLS_PK_USE_PSA_EC_DATA (makes sense: that's only for a pure-PSA world, not for a mixed world). Then the code in test_suite_pk uses the legacy code path to generate an EC key. That's a bug in the test code: nowadays it should base the choice of code path on where the support is for the specific curve, or it could dispatch to PSA whenever PSA is available

and later:

The test needs to generate an EC key on a curve which may or may not be accelerated. If the curve is not accelerated then MBEDTLS_ECP_C is enabled and the mbedtls_ecp_gen_keypair code path is taken, which is fine. If all curves are accelerated then MBEDTLS_PK_USE_PSA_EC_DATA is enabled, and all is fine. But if this curve is accelerated while others are not, we have a problem. Since MBEDTLS_ECP_C is present (for other curves), MBEDTLS_PK_USE_PSA_EC_DATA is disabled. But mbedtls_ecp_gen_keypair and even mbedtls_ecp_group_load won't work for the accelerated curve.

[mpg]

Note: after taking a look, I think there are at least two distinct issues:

1. The test function pk_genkey() does not work on curves that are not built-in when ECP_C is present. This looks like a problem in our tests only.
2. The library functions for parsing private and public keys don't work on curves that are not built-in when ECP_C is present. This is a problem in the library.

Actually I think there's a deep problem here: when ECP_C is present, the key is stored as an mbedtls_ecp_keypair structure (that's publicly visible via mbedtls_pk_ec()) and that structure includes an mbedtls_ecp_group structure representing the curve - but that structure can only do that if the curve is built-in.

So, I think our hands are tide here until 4.0: when ECP_C is present, users can access the mbedtls_ecp_keypair structure which is supposed to be valid and usable with any legacy function that uses such a structure, and that's only possible if the mbedtls_ecp_group sub-structure is valid, which it can only be if the curve is built-in.

As a result, I think as soon as ECP_C is present and PK_C is enable, we need all the curves to be built-in, and it's going to remain that way until 4.0.

@gilles-peskine-arm any thoughts?

[gilles-peskine-arm]

Both are the same issue: pk only knows how to cope with ECC keys when either all ECC keys are PSA, or the ECC key is built in. It doesn't know how to cope with an ECC key on a PSA-only curve when some curves are built in.

The fact that it appears only in tests for key generation is because mbedtls_pk_gen_key is missing as a library function. Generally, we have API gaps that make pk difficult to use, and here we're moving from difficult to impossible.

our hands are tide here until 4.0: when ECP_C is present, users can access the mbedtls_ecp_keypair structure

I'm not sure. mbedtls_ecp_keypair only has private fields, so while we need the structure to be present, we don't actually need it to contain much data. I think we can support “external” curves, which have no group structure, just a group ID, when the corresponding MBEDTLS_ECP_DP_xxx_ENABLED is disabled.

I don't know if it's worth the time investment to get this into 3.6. On the one hand, having a mix of accelerated and non-accelerated curves is useful in practice. On the other hand, I'd rather defer all these improvements until we've moved to a PSA-only world.

[mpg]

I agree that both are the same underlying issue, and the only reason the first one is "test only" is because we have a gap in the library.

I also agree that we should consider carefully how much time is worth spending on this for 3.6, as opposed to leaving for later when our world is PSA only.

I'll let this rest for a few days before I come back to it, perhaps run a few experiments, and try to get a feel for how hard or easy this would be to fix in 3.6 - I think the answer to "do we want to do it" depends on "how much effort would this be".

[mpg]

Looking again at this a few month later, my impression is that it would be better to postpone this until after 4.0, when mbedtls_pk_ec() has been removed and we have more freedom to resolve this properly.

Currently, I think we'd end up just auto-enabling all curves as built-in as soon as PK and ECP_C are enabled, which would be of limited value.