is it really mandatory to gate this with MBEDTLS_USE_PSA_CRYPTO as well? I am thinking about a use case where the client is just interested in doing Crypto, without really the need of doing TLS, so they won't care much about setting (or not setting) MBEDTLS_USE_PSA_CRYPTO. I guess it's more of a question for @gilles-peskine-arm though

If it were only for crypto operations then I would agree with you, but for what concerns PK/TLS (and perhaps also MD/cipher) they might size buffers internally depending on USE_PSA or not. In other words, in a case where !USE_PSA, if legacy symbols are not set from the PSA ones, TLS/PK might under-estimate the size of some buffers.
This is just my guess though. I'm sure @gilles-peskine-arm or @mpg can provide a better answer :)

I agree with Valerio's assessment. Note that in 4.0 USE_PSA will go away, and when that happens things will get simpler, but in the meantime (and in 3.6) I think we still need it to gate this.

Understood, thanks. It makes sense.

Upon reflection, the goal of config_adjust_legacy_from_psa.h is to ensure that if PSA wants a crypto feature then it's provided somehow, either through a PSA driver or through the a built-in implementation. This is independent of USE_PSA_CRYPTO. When PSA_CRYPTO_CLIENT && !PSA_CRYPTO_C, all PSA crypto feature come from the server, so config_adjust_legacy_from_psa.h is useless, regardless of what's using it.

they might size buffers internally depending on USE_PSA or not

That would also break driver-only configurations. So this is only a concern for mechanisms where driver-only configurations are not supported. The only mechanism for which the limitations affect basic key management (as opposed to more advanced capabilities) is RSA. The pk/x509/ssl code gates its RSA support on whether MBEDTLS_RSA_C is enabled, not on PSA_WANT_*RSA*, whether or not MBEDTLS_USE_PSA_CRYPTO is enabled. So when PSA crypto is client-server, on the client, you won't be able to use the PSA RSA if MBEDTLS_RSA_C is disabled, but there should be no undersized buffers.

This does mean that there's an incompatible change here: if you had PSA_WANT_ALG_RSA_xxx and PSA_WANT_KEY_PAIR_RSA_xxx and MBEDTLS_PSA_CRYPTO_CLIENT and !MBEDTLS_PSA_CRYPTO_C and !MBEDTLS_USE_PSA_CRYPTO in 3.6.0, then MBEDTLS_RSA_C was automatically enabled and RSA worked (through the local built-in implementation). (With the same configuration but USE_PSA_CRYPTO enabled, RSA would not work in pk/x509/TLS since the code would call PSA functions.) We can make this change, even in an LTS, because CLIENT && !C is documented as experimental, it's a weird configuration in the first place, and there's an easy workaround of explicitly requesting MBEDTLS_RSA_C. But we should document it in a changelog entry.

Therefore, I think (but I'd like @mpg's opinion on this as well) the best course of action is:

• Don't check MBEDTLS_USE_PSA_CRYPTO here.
• Explain the consequences of this check in a comment.
• Add a changelog entry.

I think (but I'd like @mpg's opinion on this as well) the best course of action is

@mpg do you agree with the plan than @gilles-peskine-arm proposed? :)

Hi @mpg / @valeriosetti / @gilles-peskine-arm , just a little nudge to keep the review alive.

I'm convinced by Gilles's reasonning.

I'm not sure I like the way this is written. How about:

#if defined(MBEDTLS_PSA_CRYPTO_C)
/* If we are implementing PSA crypto ourselves, then we want to enable the required built-ins.
 * Otherwise, PSA features will be provided by the server. */
#include "mbedtls/config_adjust_legacy_from_psa.h"
#endif

The comment can be tuned, but my points are:

• I don't think we need to check for CLIENT here as I don't think we're ever reading config_psa.h with CLIENT disabled.
• Then I prefer "if condition then" to "if !condition else".

I'm sorry, I just don't understand this sentence.

Taking the perspective of a user: No support will be provided for something? So we're fixing the lack of support for something? But what is it? What do client-only builds have to do with drivers — don't drivers live on the server side?

Knowing what this is about: I still don't see how drivers are involved. What we're fixing is auto-enabling some built-in algorithms. So many people (amongst people who use client-only builds) benefit from a code size improvement (a non-functional improvement which alone would normally not justify a changelog entry), but some applications may break if they were accidentally relying on the auto-enablement, which is why this does need a changelog entry.

Minor: “pure PSA client build” is hard to parse: is it a (pure PSA) (client build)? a pure (PSA client build)? a (pure (PSA client)) build? What does this mean anyway: MBEDTLS_PSA_CRYPTO_CLIENT and nothing else?

I suggest calling it a “PSA client-only build”. (Or should this be hyphenated as “PSA-client-only build”?)

Question: since such builds are not officially supported yet, isn't there a risk that this changelog entry gives the (wrong) impression that they are officially supported already?

I think that's ok: we document MBEDTLS_PSA_CRYPTO_CLIENT as experimental, and we don't say much about what it does.

I still find “no built-in support will be provided” to be unclear and potentially misleading. You can build with local legacy crypto: the change is that you can now also build without. How about this?

In a PSA-client-only build (i.e. MBEDTLS_PSA_CRYPTO_CLIENT && !MBEDTLS_PSA_CRYPTO_C), do not automatically enable local crypto when the corresponding PSA mechanism is enabled, since the server provides the crypto. Fixes #9126.