From fe47e9ed89f546010235736df5cdfb4991e5210a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 4 Jun 2026 19:38:27 +0200 Subject: [PATCH] Define and use mbedtls_rsa_decrypt_decompose_ret() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use a more structured way of isolating and translating sensitive error codes from RSA PKCS#1v1.5 decryption. Signed-off-by: Gilles Peskine Signed-off-by: Manuel Pégourié-Gonnard --- library/pk_wrap.c | 28 ++++++--------- library/psa_crypto_rsa.c | 32 +++++++---------- library/rsa.c | 26 ++++++++++++++ library/rsa_internal.h | 30 ++++++++++++++++ tests/suites/test_suite_pkcs1_v15.data | 12 +++++++ tests/suites/test_suite_pkcs1_v15.function | 41 ++++++++++++++++++++++ 6 files changed, 133 insertions(+), 36 deletions(-) diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 876523e5c2..45c274aa94 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -331,24 +331,18 @@ static int rsa_decrypt_wrap(mbedtls_pk_context *pk, NULL, 0, output, osize, olen); - /* We want to convert status into an MBEDTLS code without allowing an - * attacker to distinguish between 0, invalid padding, or buffer too - * small. (It's OK if the attacker distinguishes between one of the - * above three and other status such as out of memory.) Since - * PSA_PK_RSA_TO_MBEDTLS_ERR() is leaky, hide the difference from it. */ - mbedtls_ct_condition_t bad_padding = mbedtls_ct_uint_eq( - (mbedtls_ct_uint_t) status, - (mbedtls_ct_uint_t) PSA_ERROR_INVALID_PADDING); - mbedtls_ct_condition_t large_msg = mbedtls_ct_uint_eq( - (mbedtls_ct_uint_t) status, - (mbedtls_ct_uint_t) PSA_ERROR_BUFFER_TOO_SMALL); - status = mbedtls_ct_error_if(bad_padding, 0, status); - status = mbedtls_ct_error_if(large_msg, 0, status); + /* Translate error codes from PSA to legacy + * Success vs INVALID_PADDING vs BUFFER_TOO_SMALL is sensitive + * (padding oracle attack), so we take care to translate that + * part in constant time. + */ + int problem; + status = mbedtls_rsa_decrypt_decompose_ret( + PSA_ERROR_INVALID_PADDING, MBEDTLS_ERR_RSA_INVALID_PADDING, + PSA_ERROR_BUFFER_TOO_SMALL, MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, + status, &problem); ret = PSA_PK_RSA_TO_MBEDTLS_ERR(status); - ret = mbedtls_ct_error_if(bad_padding, - MBEDTLS_ERR_RSA_INVALID_PADDING, ret); - ret = mbedtls_ct_error_if(large_msg, - MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, ret); + ret |= problem; cleanup: mbedtls_zeroize_and_free(buf, buf_size); diff --git a/library/psa_crypto_rsa.c b/library/psa_crypto_rsa.c index 322ede0701..85a9f88d22 100644 --- a/library/psa_crypto_rsa.c +++ b/library/psa_crypto_rsa.c @@ -653,28 +653,22 @@ psa_status_t mbedtls_psa_asymmetric_decrypt(const psa_key_attributes_t *attribut if (alg == PSA_ALG_RSA_PKCS1V15_CRYPT) { #if defined(MBEDTLS_PSA_BUILTIN_ALG_RSA_PKCS1V15_CRYPT) - int ret = mbedtls_rsa_rsaes_pkcs1_v15_decrypt( + int combined_ret = mbedtls_rsa_rsaes_pkcs1_v15_decrypt( rsa, mbedtls_psa_get_random, MBEDTLS_PSA_RANDOM_STATE, output_length, input, output, output_size); - /* We want to convert ret into a psa status without allowing an - * attacker to distinguish between 0, invalid padding, or buffer too - * small. (It's OK if the attacker distinguishes between one of the - * above three and other status such as out of memory.) Since - * mbedtls_to_psa_error() is leaky, hide the difference from it. */ - mbedtls_ct_condition_t bad_padding = mbedtls_ct_uint_eq( - (mbedtls_ct_uint_t) ret, - (mbedtls_ct_uint_t) MBEDTLS_ERR_RSA_INVALID_PADDING); - mbedtls_ct_condition_t large_msg = mbedtls_ct_uint_eq( - (mbedtls_ct_uint_t) ret, - (mbedtls_ct_uint_t) MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE); - ret = mbedtls_ct_error_if(bad_padding, 0, ret); - ret = mbedtls_ct_error_if(large_msg, 0, ret); - status = mbedtls_to_psa_error(ret); - status = mbedtls_ct_error_if(bad_padding, - PSA_ERROR_INVALID_PADDING, status); - status = mbedtls_ct_error_if(large_msg, - PSA_ERROR_BUFFER_TOO_SMALL, status); + /* Translate error codes from legacy to PSA. + * Success vs INVALID_PADDING vs OUTPUT_TOO_LARGE is sensitive + * (padding oracle attack), so we take care to translate that + * part in constant time. + */ + int problem; + int public_ret = mbedtls_rsa_decrypt_decompose_ret( + MBEDTLS_ERR_RSA_INVALID_PADDING, PSA_ERROR_INVALID_PADDING, + MBEDTLS_ERR_RSA_OUTPUT_TOO_LARGE, PSA_ERROR_BUFFER_TOO_SMALL, + combined_ret, &problem); + status = mbedtls_to_psa_error(public_ret); + status |= problem; #else status = PSA_ERROR_NOT_SUPPORTED; #endif /* MBEDTLS_PSA_BUILTIN_ALG_RSA_PKCS1V15_CRYPT */ diff --git a/library/rsa.c b/library/rsa.c index 94db8282c4..e2b8fdd26b 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -542,6 +542,32 @@ MBEDTLS_STATIC_TESTABLE int mbedtls_ct_rsaes_pkcs1_v15_unpadding( #endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) +int mbedtls_rsa_decrypt_decompose_ret( + int invalid_padding_in, int invalid_padding_out, + int output_too_large_in, int output_too_large_out, + int combined_ret, + int *problem) +{ + *problem = 0; + int ret = combined_ret; + + const mbedtls_ct_condition_t invalid_padding_cond = + mbedtls_ct_uint_eq((mbedtls_ct_uint_t) ret, + (mbedtls_ct_uint_t) invalid_padding_in); + *problem = mbedtls_ct_error_if(invalid_padding_cond, invalid_padding_out, *problem); + ret = mbedtls_ct_error_if(invalid_padding_cond, 0, ret); + + const mbedtls_ct_condition_t output_too_large_cond = + mbedtls_ct_uint_eq((mbedtls_ct_uint_t) ret, + (mbedtls_ct_uint_t) output_too_large_in); + *problem = mbedtls_ct_error_if(output_too_large_cond, output_too_large_out, *problem); + ret = mbedtls_ct_error_if(output_too_large_cond, 0, ret); + + return ret; +} +#endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C */ + #if !defined(MBEDTLS_RSA_ALT) int mbedtls_rsa_import(mbedtls_rsa_context *ctx, diff --git a/library/rsa_internal.h b/library/rsa_internal.h index 3b6186c7e8..9fe1eef811 100644 --- a/library/rsa_internal.h +++ b/library/rsa_internal.h @@ -155,4 +155,34 @@ MBEDTLS_STATIC_TESTABLE int mbedtls_ct_rsaes_pkcs1_v15_unpadding( #endif /* MBEDTLS_PKCS1_V15 && MBEDTLS_RSA_C && ! MBEDTLS_RSA_ALT */ #endif /* MBEDTLS_TEST_HOOKS */ +#if defined(MBEDTLS_PKCS1_V15) && defined(MBEDTLS_RSA_C) +/** Decompose sensitive return values out of a return code, in constant time. + * + * \param invalid_padding_in The value of \p combined_ret that indicates + * invalid padding. + * \param invalid_padding_out The value to set \p problem to in case of + * invalid padding. + * \param output_too_large_in The value of \p combined_ret that indicates + * an insufficient output buffer size. + * \param output_too_large_out The value to set \p problem to in case of + * an insufficient output buffer size. + * \param combined_ret The value to decompose. + * \param[out] problem On output: + * - \p invalid_padding_out, + * if \p combined_ret = \p invalid_padding_in; + * - \p output_too_large_out, + * if \p combined_ret = \p output_too_large_in; + * - otherwise \c 0. + * + * \return - \c 0 if \p combined_ret = \p invalid_padding_in + * or \p combined_ret = \p output_too_large_in; + * - otherwise \c combined_ret. + */ +int mbedtls_rsa_decrypt_decompose_ret( + int invalid_padding_in, int invalid_padding_out, + int output_too_large_in, int output_too_large_out, + int combined_ret, + int *problem); +#endif + #endif /* rsa_internal.h */ diff --git a/tests/suites/test_suite_pkcs1_v15.data b/tests/suites/test_suite_pkcs1_v15.data index 44e5a0124d..49f4598898 100644 --- a/tests/suites/test_suite_pkcs1_v15.data +++ b/tests/suites/test_suite_pkcs1_v15.data @@ -52,6 +52,18 @@ RSASSA-V15 Verification Test Vector Int depends_on:MBEDTLS_MD_CAN_SHA1 pkcs1_rsassa_v15_verify:1024:"a2ba40ee07e3b2bd2f02ce227f36a195024486e49c19cb41bbbdfbba98b22b0e577c2eeaffa20d883a76e65e394c69d4b3c05a1e8fadda27edb2a42bc000fe888b9b32c22d15add0cd76b3e7936e19955b220dd17d4ea904b1ec102b2e4de7751222aa99151024c7cb41cc5ea21d00eeb41f7c800834d2c6e06bce3bce7ea9a5":"010001":MBEDTLS_MD_SHA1:MBEDTLS_MD_SHA1:"37b66ae0445843353d47ecb0b4fd14c110e62d6a":"e3b5d5d002c1bce50c2b65ef88a188d83bce7e61":"2154f928615e5101fcdeb57bc08fc2f35c3d5996403861ae3efb1d0712f8bb05cc21f7f5f11f62e5b6ea9f0f2b62180e5cbe7ba535032d6ac8068fff7f362f73d2c3bf5eca6062a1723d7cfd5abb6dcf7e405f2dc560ffe6fc37d38bee4dc9e24fe2bece3e3b4a3f032701d3f0947b42930083dd4ad241b3309b514595482d42":0 +RSA sensitive ret: 0 +rsa_decrypt_decompose_ret:0:0:0 + +RSA sensitive ret: sensitive 1 +rsa_decrypt_decompose_ret:-1:0:-11 + +RSA sensitive ret: sensitive 2 +rsa_decrypt_decompose_ret:-2:0:-12 + +RSA sensitive ret: public +rsa_decrypt_decompose_ret:-3:-3:0 + RSAES-V15 decoding: good, payload=max, tight output buffer pkcs1_v15_decode:"0002505152535455565700":117:117:0 diff --git a/tests/suites/test_suite_pkcs1_v15.function b/tests/suites/test_suite_pkcs1_v15.function index e7c1ce22fd..0478b75820 100644 --- a/tests/suites/test_suite_pkcs1_v15.function +++ b/tests/suites/test_suite_pkcs1_v15.function @@ -157,6 +157,47 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void rsa_decrypt_decompose_ret(int input, + int expected_ret, int expected_problem) +{ + const int sensitive_in_1 = -1; + const int sensitive_in_2 = -2; + const int problem_1 = -11; + const int problem_2 = -12; + + int combined_ret = input; + TEST_CF_SECRET(&combined_ret, sizeof(combined_ret)); + int actual_problem = 42; + TEST_CF_SECRET(&actual_problem, sizeof(actual_problem)); + + int actual_ret = mbedtls_rsa_decrypt_decompose_ret(sensitive_in_1, problem_1, + sensitive_in_2, problem_2, + input, + &actual_problem); + + TEST_EQUAL(actual_problem, expected_problem); + TEST_EQUAL(actual_ret, expected_ret); + + /* Check invariant when there's no translation, only separation */ + actual_ret = mbedtls_rsa_decrypt_decompose_ret(sensitive_in_1, sensitive_in_1, + sensitive_in_2, sensitive_in_2, + input, + &actual_problem); + TEST_EQUAL(actual_ret, expected_ret); + if (expected_problem == problem_1) { + TEST_EQUAL(actual_problem, sensitive_in_1); + } else if (expected_problem == problem_2) { + TEST_EQUAL(actual_problem, sensitive_in_2); + } else { + TEST_EQUAL(actual_problem, 0); + } + TEST_CF_PUBLIC(&actual_problem, sizeof(actual_problem)); + TEST_CF_PUBLIC(&combined_ret, sizeof(combined_ret)); + TEST_EQUAL(actual_problem | actual_ret, combined_ret); +} +/* END_CASE */ + /* BEGIN_CASE */ void pkcs1_v15_decode(data_t *input, int expected_plaintext_length_arg,