Replace legacy macros with its `PSA_WANT` counterparts

[gabor-mezei-arm]

Replace these macros:

• MBEDTLS_SSL_HAVE_CBC to PSA_WANT_ALG_CBC_NO_PADDING
• MBEDTLS_SSL_HAVE_CCM to PSA_WANT_ALG_CCM
• MBEDTLS_SSL_HAVE_GCM to PSA_WANT_ALG_GCM
• MBEDTLS_SSL_HAVE_CHACHAPOLY to PSA_WANT_ALG_CHACHA20_POLY1305

The macros should be replaced in all files except mbedtls_config.h, cneck_config.h and config_adjust_*.h.

Ensure that the test are run in the same way.

[gilles-peskine-arm]

Hold on, why would we do this? This seems counterproductive to me. Users who enable a cryptographic algorithm in the crypto configuration don't necessarily want to enable the corresponding cipher suites. That's especially true for CBC, which is still commonly used for data at rest, but deprecated in TLS. But it's also true to a lesser extent with other algorithms. Especially when the crypto library is separate from the TLS library, and it might be configured by different people.

[mpg]

I agree about how things should be, but IMO that's separate from the "replace legacy macros with PSA_WANT" work. The gap in configurability is pre-existing: the MBEDTLS_SSL_HAVE_xxx macros are not part of user configuration, but derived from crypto config (+ USE_PSA).

I'd like us to have a task in the future where we make it possible to have say CBC in the crypto build but no support for CBC ciphersuites in TLS, but I think this will require some design about making it consistent:

• how should the macros be named? (MBEDTLS_SSL_WANT_xxx?)
• should the API cover note just modes but also underlying block cipher?
• should it impact both 1.2 and 1.3 or for 1.3 should we just have a macro for each ciphersuite?
• what about asymmetric crypto (do we want to be able to say "I have FFDH or RSA in my crypto build but I don't want to use them in TLS" - for 1.2 that's mostly but not completely covered by the various KEY_EXCHANGE macros, for 1.3 that's orthogonal)?
• actually what's the goal here: saving code size, reducing the attack surface, other?
• surely we'll want to check consistency of SSL config with crypto config.

I don't think replacing the current MBEDTLS_SSL_HAVE_xxx macros will make that future work any harder: we'll just look for occurrences of PSA_WANT macros in ssl_xxx files and replace them with whatever the new user-configurable macros will be.

So, while I agree with your considerations about the end goal, I think it's quite orthogonal to the present task, and I don't think it would be counter-productive to executed it as stated now.

Wdyt?

[gilles-peskine-arm]

I'd like us to have a task in the future where we make it possible to have say CBC in the crypto build but no support for CBC ciphersuites in TLS, but I think this will require some design about making it consistent:

Same here, and it's on my COULD list for 4.0. But right now the topic is part the implementation, not the interface.

I don't think replacing the current MBEDTLS_SSL_HAVE_xxx macros will make that future work any harder: we'll just look for occurrences of PSA_WANT macros in ssl_xxx files and replace them with whatever the new user-configurable macros will be.

Keeping the existing macro tells us whether we designed the code with SSL-specific considerations or not. The set of supported mechanisms in TLS isn't just about which sequences of crypto functions to call, it's also about things like buffer sizes. Mind you, maybe I'm overestimating the attention to detail in the current code — I've noticed some code assumes that if crypto supports FFDH then so does TLS, and that is not the case for 1.2 when using PSA.

The TLS code currently uses all three of

MBEDTLS_CIPHER_MODE_CBC
MBEDTLS_SSL_HAVE_CBC
MBEDTLS_SSL_SOME_SUITES_USE_CBC

Are they effectively interchangeable? If they are, then true, we might as well forget the history and change them all to PSA_WANT_ALG_CBC.

[mpg]

Keeping the existing macro tells us whether we designed the code with SSL-specific considerations or not.

Good point, though I don't think it applies to the SSL_HAVE macros: they're just USE_PSA-aware, but beyond that there's nothing specific to SSL/TLS (despite the name).

Mind you, maybe I'm overestimating the attention to detail in the current code — I've noticed some code assumes that if crypto supports FFDH then so does TLS, and that is not the case for 1.2 when using PSA.

Actually, for 1.2, FFDH is supported if and only if one of the MBEDTLS_KEY_EXCHANGE_DHE_{RSA,PSK} macros is enabled. So, here we already have TLS-specific config options: the user should be able to have FFDH enabled on the crypto side, without support for it in TLS 1.2. However, we're using DHM_C guards in a number of places, so in a 1.2-only build with FFDH only on the crypto side, we'd still end up having a dhm_ctx field in mbedtls_ssl_handshake_params - that is, part of the FFDH support still compiled in even though it's useless. Which is of course not how it should be.

Before driver-only ECC, similar considerations applied to ECDH(E) but we fixed that by introducing MBEDTLS_KEY_EXCHANGE_SOME_ECDH_OR_ECDHE_1_2_ENABLED.

The FFDH-in-1.2 part is going away in 4.0 anyway (hopefully), so we won't have to clean it up. I'm not sure however if there are other things like that left in the code.

Are they effectively interchangeable?

Excellent question, they're not :)

• MBEDTLS_CIPHER_MODE_CBC is not USE_PSA aware, so it should only be used in places that are guarded with !USE_PSA. (FWIW, I just checked, and that's indeed the case. Phew.) So, it should disappear form our code base when we make USE_PSA mandatory (replaced with PSA_WANT_ALG_CBC_NO_PADDING).
• MBEDTLS_SSL_HAVE_CBC is just a USE_PSA-aware version of the above. It should also disappear in favour of PSA_WANT_ALG_CBC_NO_PADDING, as it just means "CBC mode is available from crypto".
• MBEDTLS_SSL_SOME_SUITES_USE_CBC OTOH embeds TLS-specific considerations, and means "ciphersuites that use CBC mode are available", which currently translates to "CBC mode and one of AES, Aria, Camellia are available from crypto", but in the future should translate to that plus "the user has enabled use of CBC in TLS (1.2)".

So, I agree we should keep MBEDTLS_SSL_SOME_SUITES_USE_CBC - which I don't think was targetted by the issue description anyway. But I still think the SSL_HAVE ones can go, as they're just USE_PSA-aware versions of "if this available from crypto?"

Just to be sure, we should double-check that assertion for each SSL_HAVE macro before replacing it with a PSA_WANT, as without checking I'm not sure this is true for 100% of them - we might have been accidentally inconsistent in creating & naming them.

[mpg]

Just checked because this got me curious. There are only 8 SSL_HAVE macros:

• HAVE_CBC, HAVE_GCM, HAVE_CCM and HAVE_CHACHAPOLY (this issue) all are USE_PSA-aware variants of PSA_WANT macros.
• Same for HAVE_AES, HAVE_ARIA and HAVE_CAMELLIA - issue #9334
• HAVE_AEAD is a shortcut for "GCM, CCM or ChachaPoly" and should stay. (It does contain TLS-specific knowledge: the list of AEAD modes usable in TLS.)

[gilles-peskine-arm]

@mpg Thanks for digging down!

So based on this, ok, I guess we might as well change to PSA_WANT_ALG_CBC_NO_PADDING and same for the other algorithm/type-specific MBEDTLS_SSL_HAVE. I find it a bit weird because we'll eventually go back to not using PSA_WANT_xxx in SSL code. But that will likely not happen until when we start preparing for 5.0 (where the TLS code can work with other implementations of PSA Crypto), and in the meantime the code would be simpler without macros that are synonyms.