Remove MBEDTLS_SHA3_C config option

[gabor-mezei-arm]

Description

Remove MBEDTLS_SHA3_C config option and replace it's references with PSA_WANT macros.

Depends on: Mbed-TLS/mbedtls#10201

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line.
If the provided content is part of the present PR remove the # symbol.

• changelog provided
• framework PR not required: not related
• mbedtls development PR provided Remove MBEDTLS_SHA3_C config option mbedtls#10145
• mbedtls 3.6 PR not required because: 4.0/1.0 changes
• tests provided

[felixc-arm]

LGTM 👍

[mpg]

@bjwtaylor Requesting a review from you since you're already reviewing the companion Mbed TLS PR.

[mpg]

@gabor-mezei-arm check-names seems unhappy

[gabor-mezei-arm]

The deleted macro occurrence is located in the error.c which generation is handled in the mbedtls repo. So it is fixed there.

[gilles-peskine-arm]

Merging a crypto PR that breaks the CI can be done, but it's a major hassle. Since the failure here is just in check_names, it shouldn't be too hard to make a backward-compatible transition. For example, add a temporary definition of MBEDTLS_SHA3_C, which can be removed in a future PR made after the mbedtls consumer has been merged.

[mpg]

Merging a crypto PR that breaks the CI can be done, but it's a major hassle.

100% agreed, let's try avoiding that.

Since the failure here is just in check_names, it shouldn't be too hard to make a backward-compatible transition. For example, add a temporary definition of MBEDTLS_SHA3_C, which can be removed in a future PR made after the mbedtls consumer has been merged.

IMO when we're removing something on the crypto side, the logical order is to remove users on the mbedtls side, then on the crypto side. From a quick look, it seems to me this should be doable here: first merge Mbed-TLS/mbedtls#10145 (without the submodule update), then merge this one, and be done with it.

I think this is a bit faster than what you were suggesting - which IIUC was (1) this PR with a dummy define, (2) 10145, (3) removing the dummy define, so 3 PRs total.

[mpg]

Looking pretty good to me, except for one change I don't understand. I'm sure you had a reason for doing it, but it's not apparent to me, and also the change doesn't look logical to me. So I'm holding my approval until this is clarified (and quite possibly if the change is correct and I'm just missing why, I'll request a comment explaining it, so we don't stumble again on it later).

[mpg]

LGTM, thanks - just need to fix the code style to make the CI fully happy.

[gabor-mezei-arm]

I'm inclined to just add 4 temporary all.sh components in components-configuration.sh that each build with full minus PSA_WANT_ALG_SHA3_xxx for the 4 possible values of xxx. Then this PR should be mergeable on its own, then we can merge the mbedtls PR then the depends.py PR then remove the temporary components and other temporary hacks.

Adding a new component is not working because in the CI the analyze_outcome runs separately for the TLS and PSA repo tests. So test results from PSA repo will not take into account when analyzing the TLS repo and it will fail.

[gilles-peskine-arm]

I think we need to merge the new test component in the mbedtls repository first.

(And until we disentangle configuration coverage testing somehow, the mbedtls repository needs to do complete testing of crypto configurations, even if they don't impact X.509 or TLS.)

[gabor-mezei-arm]

I think we need to merge the new test component in the mbedtls repository first.

Done in Mbed-TLS/mbedtls#10201

[felixc-arm]

LGTM now that the CI is green 🙂

[mpg]

LGTM, thanks!