ssl: accept TLS 1.2 rsa_pss_rsae signature algorithms

[Maokaman1]

Description

Fixes #10668

Fix a TLS 1.2 client regression introduced by the signature-algorithm validation added in commit 9f19fe1.

The new validation correctly rejects signature algorithms that are invalid for TLS 1.2 or were not offered by the client, but it also rejects valid rsa_pss_rsae_* SignatureScheme values even when RSA-PSS support is compiled in. As a result, handshakes with TLS 1.2 servers using RSA-PSS signatures can fail with MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER (-0x6600).

This PR updates the TLS 1.2 support check to accept rsa_pss_rsae_sha256/384/512 when the corresponding RSA-PSS and hash support is available, and adds regression tests for these values.

PR checklist

• changelog provided
• development PR provided here
• TF-PSA-Crypto PR not required because: no changes
• framework PR not required
• 4.1 PR provided Backport 4.1: ssl: accept TLS 1.2 rsa_pss_rsae signature algorithms #10704
• 3.6 PR provided Backport 3.6: ssl: accept TLS 1.2 rsa_pss_rsae signature algorithms #10674
• tests provided

[gilles-peskine-arm]

Thank you very much for providing a fix! We'll review it ASAP since it's a regression that causes a major interoperability problem.

[gilles-peskine-arm]

Thank you very much for contributing this!

The fix looks plausible to me, but I'm not sure it's complete. We really should have a non-regression test in ssl-opt.sh. I'm not sure how to write it.

[gilles-peskine-arm]

LGTM, with perhaps a question of whether we're doing enough testing in ssl-opt.sh. I'll defer to Ronald on this as he's much more familiar than I am with sigalgs-related testing.

[ronald-cron-arm]

Sorry for the delay, please find my comments below.

[ronald-cron-arm]

Thanks for addressing my comments. LGTM.

[gilles-peskine-arm]

LGTM

Can you please update the 3.6 backport, and also make a backport for the new LTS branch mbedtls-4.1?

[Maokaman1]

Do you think the patch below is warranted for this PR?

@@ -2069,13 +2073,13 @@ start_processing:
         }
 #endif

-#if defined(MBEDTLS_X509_RSASSA_PSS_SUPPORT)
+#if defined(MBEDTLS_X509_RSASSA_PSS_SUPPORT) || defined(PSA_WANT_ALG_RSA_PSS)
         if (pk_alg == MBEDTLS_PK_SIGALG_RSA_PSS) {
             ret = mbedtls_pk_verify_ext((mbedtls_pk_sigalg_t) pk_alg, peer_pk,
                                         md_alg, hash, hashlen,
                                         p, sig_len);
         } else
-#endif /* MBEDTLS_X509_RSASSA_PSS_SUPPORT */
+#endif /* MBEDTLS_X509_RSASSA_PSS_SUPPORT || PSA_WANT_ALG_RSA_PSS */
         ret = mbedtls_pk_verify_restartable(peer_pk,
                                             md_alg, hash, hashlen, p, sig_len, rs_ctx);

@@ -2090,7 +2094,14 @@ start_processing:
                     MBEDTLS_SSL_ALERT_LEVEL_FATAL,
                     MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR);
             }
-            MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_pk_verify_restartable", ret);
+#if defined(MBEDTLS_X509_RSASSA_PSS_SUPPORT) || defined(PSA_WANT_ALG_RSA_PSS)
+            if (pk_alg == MBEDTLS_PK_SIGALG_RSA_PSS) {
+                MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_pk_verify_ext", ret);
+            } else
+#endif /* MBEDTLS_X509_RSASSA_PSS_SUPPORT || PSA_WANT_ALG_RSA_PSS */
+            {
+                MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_pk_verify_restartable", ret);
+            }
 #if defined(MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED)
             if (ret == MBEDTLS_ERR_ECP_IN_PROGRESS) {
                 ret = MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS;

[gilles-peskine-arm]

Regarding MBEDTLS_X509_RSASSA_PSS_SUPPORT, we are aware that it's misused in places in the TLS code. But I would strongly prefer to keep it a separate concern. This patch is already a subtle balance between interoperability and security, so I don't want to add more complexity.

[gilles-peskine-arm]

Approving the merge.

For future reference, please don't add merge commits to a pull request unless there's a merge conflict.

[gilles-peskine-arm]

LGTM

[ronald-cron-arm]

LGTM