From 6958355a51582875c851586924662c1c7801261c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 12:46:46 +0200 Subject: [PATCH 1/6] Use PSA Crypto more often in pk_verify_ext() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/Mbed-TLS/mbedtls/issues/5277 - strategy 1. Signed-off-by: Manuel Pégourié-Gonnard --- library/pk.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/library/pk.c b/library/pk.c index 8dc19ef99b..1ae041db84 100644 --- a/library/pk.c +++ b/library/pk.c @@ -482,9 +482,7 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, pss_opts = (const mbedtls_pk_rsassa_pss_options *) options; #if defined(MBEDTLS_USE_PSA_CRYPTO) - if( pss_opts->mgf1_hash_id == md_alg && - ( (size_t) pss_opts->expected_salt_len == hash_len || - pss_opts->expected_salt_len == MBEDTLS_RSA_SALT_LEN_ANY ) ) + if( pss_opts->mgf1_hash_id == md_alg ) { /* see RSA_PUB_DER_MAX_BYTES in pkwrite.c */ unsigned char buf[ 38 + 2 * MBEDTLS_MPI_MAX_SIZE ]; @@ -497,10 +495,7 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, psa_algorithm_t psa_md_alg = mbedtls_hash_info_psa_from_md( md_alg ); mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - psa_algorithm_t psa_sig_alg = - ( pss_opts->expected_salt_len == MBEDTLS_RSA_SALT_LEN_ANY ? - PSA_ALG_RSA_PSS_ANY_SALT(psa_md_alg) : - PSA_ALG_RSA_PSS(psa_md_alg) ); + psa_algorithm_t psa_sig_alg = PSA_ALG_RSA_PSS_ANY_SALT( psa_md_alg ); p = buf + sizeof( buf ); key_len = mbedtls_pk_write_pubkey( &p, buf, ctx ); From 4dacf58d6d12e0a1a0c5f2849bb905d9fae32893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 12:50:00 +0200 Subject: [PATCH 2/6] Take advantage of now-public macro in pk.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Used to be private, hence the duplication, but that's been fixed in the meantime, I guess we just missed this occurrence. Signed-off-by: Manuel Pégourié-Gonnard --- library/pk.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/pk.c b/library/pk.c index 1ae041db84..a73fa56f50 100644 --- a/library/pk.c +++ b/library/pk.c @@ -484,8 +484,7 @@ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, #if defined(MBEDTLS_USE_PSA_CRYPTO) if( pss_opts->mgf1_hash_id == md_alg ) { - /* see RSA_PUB_DER_MAX_BYTES in pkwrite.c */ - unsigned char buf[ 38 + 2 * MBEDTLS_MPI_MAX_SIZE ]; + unsigned char buf[MBEDTLS_PK_RSA_PUB_DER_MAX_BYTES]; unsigned char *p; int key_len; size_t signature_length; From 3b1a7069354e177478d3bbbb82ee1d9922ea34f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 28 Jun 2022 12:47:44 +0200 Subject: [PATCH 3/6] Disable 'wrong salt len' test with USE_PSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We've decided not to check it, see https://github.com/Mbed-TLS/mbedtls/issues/5277 Also add a test that we accept the certificate with USE_PSA. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_x509parse.data | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index bd03016cc6..ee204f8229 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -831,10 +831,14 @@ X509 CRT verification #67 (Valid, RSASSA-PSS, all defaults) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA x509_verify:"data_files/server9-defaults.crt":"data_files/test-ca.crt":"data_files/crl-rsa-pss-sha1.pem":"NULL":0:0:"compat":"NULL" -X509 CRT verification #68 (RSASSA-PSS, wrong salt_len) -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA +X509 CRT verification #68 (RSASSA-PSS, wrong salt_len, !USE_PSA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA:!MBEDTLS_USE_PSA_CRYPTO x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" +X509 CRT verification #68 (RSASSA-PSS, wrong salt_len, USE_PSA) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO +x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"compat":"NULL" + X509 CRT verification #69 (RSASSA-PSS, wrong mgf_hash) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA x509_verify:"data_files/server9-bad-mgfhash.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" From a6e0291c5118231926959db9a14c2830e45c8e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 21 Dec 2022 09:59:33 +0100 Subject: [PATCH 4/6] Update documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- docs/use-psa-crypto.md | 8 +++----- include/mbedtls/pk.h | 4 +++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/use-psa-crypto.md b/docs/use-psa-crypto.md index 194d96fb4e..fc5317af89 100644 --- a/docs/use-psa-crypto.md +++ b/docs/use-psa-crypto.md @@ -95,8 +95,6 @@ Other than the above exceptions, all crypto operations are based on PSA when Current exceptions: -- Verification of RSA-PSS signatures with a salt length that is different from - the hash length. - Restartable operations when `MBEDTLS_ECP_RESTARTABLE` is also enabled (see the documentation of that option). @@ -107,11 +105,11 @@ Other than the above exception, all crypto operations are based on PSA when Current exceptions: -- Verification of RSA-PSS signatures with a salt length that is different from - the hash length, or with an MGF hash that's different from the message hash. +- Verification of RSA-PSS signatures with an MGF hash that's different from + the message hash. - Restartable operations when `MBEDTLS_ECP_RESTARTABLE` is also enabled (see the documentation of that option). -Other than the above exception, all crypto operations are based on PSA when +Other than the above exceptions, all crypto operations are based on PSA when `MBEDTLS_USE_PSA_CRYPTO` is enabled. diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index db0bfacab3..386ec42025 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -496,7 +496,9 @@ int mbedtls_pk_verify_restartable( mbedtls_pk_context *ctx, * * \note If type is MBEDTLS_PK_RSASSA_PSS, then options must point * to a mbedtls_pk_rsassa_pss_options structure, - * otherwise it must be NULL. + * otherwise it must be NULL. Note that if + * #MBEDTLS_USE_PSA_CRYPTO is defined, the salt length is not + * verified as PSA_ALG_RSA_PSS_ANY_SALT is used. */ int mbedtls_pk_verify_ext( mbedtls_pk_type_t type, const void *options, mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg, From 6ea0a8d88349f1d132ebb0477c0010b70f5949b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 29 Dec 2022 10:07:08 +0100 Subject: [PATCH 5/6] Disable 'wrong salt len' PK test with USE_PSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_pk.data | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index bd5d31ec41..d796f6ffa0 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -456,10 +456,14 @@ Verify ext RSA #4 (PKCS1 v2.1, salt_len = max, OK) depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA pk_rsa_verify_ext_test_vec:"c0719e9a8d5d838d861dc6f675c899d2b309a3a65bb9fe6b11e5afcbf9a2c0b1":MBEDTLS_MD_SHA256:1024:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":"010001":"0d2bdb0456a3d651d5bd48a4204493898f72cf1aaddd71387cc058bc3f4c235ea6be4010fd61b28e1fbb275462b53775c04be9022d38b6a2e0387dddba86a3f8554d2858044a59fddbd594753fc056fe33c8daddb85dc70d164690b1182209ff84824e0be10e35c379f2f378bf176a9f7cb94d95e44d90276a298c8810f741c9":MBEDTLS_PK_RSASSA_PSS:MBEDTLS_MD_SHA256:94:128:0 -Verify ext RSA #5 (PKCS1 v2.1, wrong salt_len) -depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA +Verify ext RSA #5a (PKCS1 v2.1, wrong salt_len) !USE_PSA +depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA:!MBEDTLS_USE_PSA_CRYPTO pk_rsa_verify_ext_test_vec:"c0719e9a8d5d838d861dc6f675c899d2b309a3a65bb9fe6b11e5afcbf9a2c0b1":MBEDTLS_MD_SHA256:1024:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":"010001":"0d2bdb0456a3d651d5bd48a4204493898f72cf1aaddd71387cc058bc3f4c235ea6be4010fd61b28e1fbb275462b53775c04be9022d38b6a2e0387dddba86a3f8554d2858044a59fddbd594753fc056fe33c8daddb85dc70d164690b1182209ff84824e0be10e35c379f2f378bf176a9f7cb94d95e44d90276a298c8810f741c9":MBEDTLS_PK_RSASSA_PSS:MBEDTLS_MD_SHA256:32:128:MBEDTLS_ERR_RSA_INVALID_PADDING +Verify ext RSA #5b (PKCS1 v2.1, wrong salt_len) USE_PSA +depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA:MBEDTLS_USE_PSA_CRYPTO +pk_rsa_verify_ext_test_vec:"c0719e9a8d5d838d861dc6f675c899d2b309a3a65bb9fe6b11e5afcbf9a2c0b1":MBEDTLS_MD_SHA256:1024:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":"010001":"0d2bdb0456a3d651d5bd48a4204493898f72cf1aaddd71387cc058bc3f4c235ea6be4010fd61b28e1fbb275462b53775c04be9022d38b6a2e0387dddba86a3f8554d2858044a59fddbd594753fc056fe33c8daddb85dc70d164690b1182209ff84824e0be10e35c379f2f378bf176a9f7cb94d95e44d90276a298c8810f741c9":MBEDTLS_PK_RSASSA_PSS:MBEDTLS_MD_SHA256:32:128:0 + Verify ext RSA #6 (PKCS1 v2.1, MGF1 alg != MSG hash alg) depends_on:MBEDTLS_PKCS1_V21:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA pk_rsa_verify_ext_test_vec:"c0719e9a8d5d838d861dc6f675c899d2b309a3a65bb9fe6b11e5afcbf9a2c0b1":MBEDTLS_MD_NONE:1024:"00dd118a9f99bab068ca2aea3b6a6d5997ed4ec954e40deecea07da01eaae80ec2bb1340db8a128e891324a5c5f5fad8f590d7c8cacbc5fe931dafda1223735279461abaa0572b761631b3a8afe7389b088b63993a0a25ee45d21858bab9931aedd4589a631b37fcf714089f856549f359326dd1e0e86dde52ed66b4a90bda4095":"010001":"0d2bdb0456a3d651d5bd48a4204493898f72cf1aaddd71387cc058bc3f4c235ea6be4010fd61b28e1fbb275462b53775c04be9022d38b6a2e0387dddba86a3f8554d2858044a59fddbd594753fc056fe33c8daddb85dc70d164690b1182209ff84824e0be10e35c379f2f378bf176a9f7cb94d95e44d90276a298c8810f741c9":MBEDTLS_PK_RSASSA_PSS:MBEDTLS_MD_SHA256:MBEDTLS_RSA_SALT_LEN_ANY:128:0 From 4511ca063afa82ee0023c0594e72da2d6f3c3725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 30 Dec 2022 10:13:41 +0100 Subject: [PATCH 6/6] Use PSS-signed CRL for PSS tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise, in builds without PKSC1_V15, tests that are supposed to accept the certificate will fail, because once the cert is OK they will move on to checking the CRL and will choke on its non-PSS signature. Tests that are supposed to reject the cert due to an invalid signature from the CA will not check the CRL because they don't recognize the CA as valid, so they have no reason to check the CA's CRL. This was hiding the problem until the recent commit that added a test where the cert is supposed to be accepted. Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_x509parse.data | 4 ++-- tests/suites/test_suite_x509parse.function | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index ee204f8229..914dffa867 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -833,11 +833,11 @@ x509_verify:"data_files/server9-defaults.crt":"data_files/test-ca.crt":"data_fil X509 CRT verification #68 (RSASSA-PSS, wrong salt_len, !USE_PSA) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA:!MBEDTLS_USE_PSA_CRYPTO -x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" +x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl-rsa-pss-sha1.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED:"compat":"NULL" X509 CRT verification #68 (RSASSA-PSS, wrong salt_len, USE_PSA) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_USE_PSA_CRYPTO -x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl.pem":"NULL":0:0:"compat":"NULL" +x509_verify:"data_files/server9-bad-saltlen.crt":"data_files/test-ca.crt":"data_files/crl-rsa-pss-sha1.pem":"NULL":0:0:"compat":"NULL" X509 CRT verification #69 (RSASSA-PSS, wrong mgf_hash) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_X509_RSASSA_PSS_SUPPORT:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_HAS_ALG_SHA_1_VIA_MD_OR_PSA_BASED_ON_USE_PSA diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index dc36b81665..388d45e452 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -665,8 +665,8 @@ void x509_verify( char *crt_file, char *ca_file, char *crl_file, res = mbedtls_x509_crt_verify_with_profile( &crt, &ca, &crl, profile, cn_name, &flags, f_vrfy, NULL ); - TEST_ASSERT( res == ( result ) ); - TEST_ASSERT( flags == (uint32_t)( flags_result ) ); + TEST_EQUAL( res, result ); + TEST_EQUAL( flags, (uint32_t) flags_result ); #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) /* CRLs aren't supported with CA callbacks, so skip the CA callback