See https://github.com/Mbed-TLS/mbedtls/blob/development/docs/architecture/psa-migration/psa-limitations.md#rsa-pss-parameters for context.

See https://github.com/Mbed-TLS/mbedtls/blob/development/docs/architecture/psa-migration/psa-limitations.md#rsa-pss-parameters for context.

If I understood correctly this document explains the difference between legacy RSA and PSA for what concerns RSA-PSS signature. However this issue is about MBEDTLS_X509_RSASSA_PSS_SUPPORT and to me this means that it only concerns with X.509.
On a first read I found this section which looked related to what I should do in this issue:

Both the existing mbedtls_ API and the PSA API support only MGF1 as the generation function (and only 0xBC as the trailer field), but there are discrepancies in handling the salt length and which of the various hash algorithms can differ from each other.

Reading that document further though I found this paragraph:

Note: since X.509 parsing ensures that message hash = encoding hash, and mbedtls_pk_verify_ext() uses encoding hash = mgf1 hash, it looks like all three hash algorithms must be equal, which would be good news as it would match a limitation of the PSA API.

So albeit legacy and PSA support different options, it seems that X.509 already checks and enforces the "PSA way" (i.e. the stricter in terms of requirements). It seems to me that the only missing part that should be checked is the salt length. Am I wrong?

Good question, I agree it's not very clear from the issue description.

it seems that X.509 already checks and enforces the "PSA way"

Not entirely. Part of the enforcement is done on the X.509 side (message hash = encoding hash) but the other (encoding hash = mgf1 hash) is done on the PK side. We'll like everything to be enforce on the X.509 side.

Looking at the code, it actually goes a bit deeper than that: currently the X.509 code uses mbedtls_pk_rsassa_pss_options which is going away in 4.0/1.0. So we should remove that reliance. Unfortunately so far PK insists on getting passed non-NULL options for PSS.

I suggest the following course of action, trying to avoid inter-dependent crypto/x509 PRs:

1. tf-psa-crypto: change pk_verify_ext() so that it always accepts NULL options. When the type argument is PSS, options are simply ignored and PSA is always used (that is, here we remove both the #if and the if checks, and remove the corresponding else branch - and a few lines above obviously remove the options == NULL check). (Crucially we still accept non-NULL options, so that the X.509 code keeps working unchanged, we just ignored them.)
2. mbedtls: change the X.509 code to entirely remove signature options: that is, remove the sig_opts field from the CRT and CSR structs, as well the the argument from mbedtls_x509_get_sig_alg(). Now mbedtls_x509_get_rsassa_pss_params() has only one output parameter: *md_alg, but it does check that what's currently mgf_md would be equal to it (and ignores salt_len beyond just parsing it to ensure things are properly formatted).

Note that ensuring md_alg == mgf_md will be slightly subtle since the second is optional, defaulting to SHA-1. So we need to remember not to skip the check if optional parameters are omitted.

Note on this issue's scope: the title and description make it look like it's about X.509 but the labels say component-crypto. Given this existing inconsistency, I'm not feeling bad about suggesting a plan that involves changes on both sides :)

Note that ensuring md_alg == mgf_md will be slightly subtle since the second is optional, defaulting to SHA-1.

A SHA-1 default has been obsolete for a while. Maybe we should stop accepting certificates where the PSS parameters are omitted? Or would that be problematic even today, maybe for some root CA?

tf-psa-crypto: change pk_verify_ext() so that it always accepts NULL options. When the type argument is PSS, options are simply ignored and PSA is always used (that is, here we remove both the #if and the if checks, and remove the corresponding else branch - and a few lines above obviously remove the options == NULL check). (Crucially we still accept non-NULL options, so that the X.509 code keeps working unchanged, we just ignored them.)

@mpg thanks for the workflow, but I have a question. Having the 2 PRs independent from each other it means that in the tf-psa-crypto one I cannot remove struct mbedtls_pk_rsassa_pss_options, which after this issue would become unused. IMO we might need a follow-up issue (+PR) which removes things like this that would then become unused. Am I wrong?

Yes, that sounds correct. I think it's one of this situations where we can either have only 2 PRs but then they're inter-dependent, or we can avoid the inter-dependency but then need 3 PRs. Considering inter-dependent PRs are a pain, I think it's worth trying the 3-PR way here.