From bba5d7c439a35a4aa4244a1b69750e1c14dc9bed Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 27 Jul 2025 18:10:01 +0200 Subject: [PATCH 1/9] Add constant-time AES-CBC encrypt and decrypt tests through PSA The main goal is to validate that unpadding is constant-time, including error reporting. Use a separate test function, not annotations in the existing function, so that the functional tests can run on any platform, and we know from test outcomes where we have run the constant-time tests. The tests can only be actually constant-time if AES is constant time, since AES computations are part of what is checked. Thus this requires hardware-accelerated AES. We can't run our AESNI (or AESCE?) code under Msan (it doesn't detect when memory is written from assembly code), so these tests can only be run with Valgrind. Signed-off-by: Gilles Peskine --- .../test_suite_psa_crypto_constant_time.data | 39 +++++ ...st_suite_psa_crypto_constant_time.function | 161 ++++++++++++++++++ 2 files changed, 200 insertions(+) create mode 100644 tests/suites/test_suite_psa_crypto_constant_time.data create mode 100644 tests/suites/test_suite_psa_crypto_constant_time.function diff --git a/tests/suites/test_suite_psa_crypto_constant_time.data b/tests/suites/test_suite_psa_crypto_constant_time.data new file mode 100644 index 0000000000..9b4c64e6c9 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_constant_time.data @@ -0,0 +1,39 @@ +CT encrypt CHACHA20 +depends_on:PSA_WANT_ALG_STREAM_CIPHER:PSA_WANT_KEY_TYPE_CHACHA20 +ct_cipher_encrypt:PSA_ALG_STREAM_CIPHER:PSA_KEY_TYPE_CHACHA20:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f":"000000000000004a00000000":"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000":"af051e40bba0354981329a806a140eafd258a22a6dcb4bb9f6569cb3efe2deaf837bd87ca20b5ba12081a306af0eb35c41a239d20dfc74c81771560d9c9c1e4b224f51f3401bd9e12fde276fb8631ded8c131f823d2c06e27e4fcaec9ef3cf788a3b0aa372600a92b57974cded2b9334794cba40c63e34cdea212c4cf07d41b769a6749f3f630f4122cafe28ec4dc47e26d4346d70b98c73f3e9c53ac40c5945398b6eda1a832c89c167eacd901d7e2bf363" + +CT encrypt AES-CTR +depends_on:PSA_WANT_ALG_CTR:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_encrypt:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"dd3b5e5319b7591daab1e1a92687feb2":"396ee84fb75fdbb5c2b13c7fe5a654aa" + +CT encrypt AES-CBC-nopad +depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_encrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"49e4e66c89a86b67758df89db9ad6955":"396ee84fb75fdbb5c2b13c7fe5a654aa" + +CT encrypt AES-CBC-PKCS7 +depends_on:PSA_WANT_ALG_CBC_PKCS7:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_encrypt:PSA_ALG_CBC_PKCS7:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee22e409f96e93d7e117393172a":"a076ec9dfbe47d52afc357336f20743bca7e8a15dc3c776436314293031cd4f3" + +CT decrypt CHACHA20 +depends_on:PSA_WANT_ALG_STREAM_CIPHER:PSA_WANT_KEY_TYPE_CHACHA20 +ct_cipher_decrypt:PSA_ALG_STREAM_CIPHER:PSA_KEY_TYPE_CHACHA20:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f":"000000000000004a00000000":"af051e40bba0354981329a806a140eafd258a22a6dcb4bb9f6569cb3efe2deaf837bd87ca20b5ba12081a306af0eb35c41a239d20dfc74c81771560d9c9c1e4b224f51f3401bd9e12fde276fb8631ded8c131f823d2c06e27e4fcaec9ef3cf788a3b0aa372600a92b57974cded2b9334794cba40c63e34cdea212c4cf07d41b769a6749f3f630f4122cafe28ec4dc47e26d4346d70b98c73f3e9c53ac40c5945398b6eda1a832c89c167eacd901d7e2bf363":"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000":0 + +CT decrypt AES-CTR +depends_on:PSA_WANT_ALG_CTR:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_decrypt:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"396ee84fb75fdbb5c2b13c7fe5a654aa":"dd3b5e5319b7591daab1e1a92687feb2":0 + +CT decrypt AES-CBC-nopad +depends_on:PSA_WANT_ALG_CBC_NO_PADDING:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_decrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"396ee84fb75fdbb5c2b13c7fe5a654aa":"49e4e66c89a86b67758df89db9ad6955":0 + +CT decrypt AES-CBC-PKCS7 good +depends_on:PSA_WANT_ALG_CBC_PKCS7:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_decrypt:PSA_ALG_CBC_PKCS7:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"a076ec9dfbe47d52afc357336f20743bca7e8a15dc3c776436314293031cd4f3":"6bc1bee22e409f96e93d7e117393172a":0 + +CT decrypt AES-CBC-PKCS7 invalid padding @0 +depends_on:PSA_WANT_ALG_CBC_PKCS7:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_decrypt:PSA_ALG_CBC_PKCS7:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"a076ec9dfbe47d52afc357336f20743bf42ddf64c420325affb343d5d5f5d5dc":"6bc1bee22e409f96e93d7e117393172a":1 + +CT decrypt AES-CBC-PKCS7 invalid padding @16 +depends_on:PSA_WANT_ALG_CBC_PKCS7:PSA_WANT_KEY_TYPE_AES:HAVE_CONSTANT_TIME_AES +ct_cipher_decrypt:PSA_ALG_CBC_PKCS7:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"a076ec9dfbe47d52afc357336f20743ba3d6a86d0a9d172eeb1b754512d04416":"6bc1bee22e409f96e93d7e117393172a":1 diff --git a/tests/suites/test_suite_psa_crypto_constant_time.function b/tests/suites/test_suite_psa_crypto_constant_time.function new file mode 100644 index 0000000000..fb8368ad31 --- /dev/null +++ b/tests/suites/test_suite_psa_crypto_constant_time.function @@ -0,0 +1,161 @@ +/* BEGIN_HEADER */ +/* Positive test cases for PSA crypto APIs that assert constant-time + * (more accurately constant-flow) behavior. */ + +#include +#include + +/* Our software AES implementation is not constant-time. For constant-time + * testing involving AES, require a hardware-assisted AES that is + * constant-time. + * + * We assume that if the hardware-assisted version is available in the build, + * it will be available at runtime. The AES tests will fail if run on a + * processor without AESNI/AESCE. + */ +#include "aesce.h" +#include "aesni.h" +#if defined(MBEDTLS_AESCE_HAVE_CODE) || defined(MBEDTLS_AESNI_HAVE_CODE) +#define HAVE_CONSTANT_TIME_AES +#endif + +/* END_HEADER */ + +/* BEGIN_DEPENDENCIES + * depends_on:MBEDTLS_PSA_CRYPTO_C + * END_DEPENDENCIES + */ + +/* BEGIN_CASE */ +/* Known answer test for cipher multipart encryption. + * There is no known answer test for one-shot encryption because that + * uses a random IV. */ +void ct_cipher_encrypt(int alg_arg, + int key_type_arg, const data_t *key_data, + const data_t *iv, + const data_t *plaintext, + const data_t *expected_ciphertext) +{ + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; + unsigned char *output = NULL; + size_t output_size = PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE(plaintext->len); + size_t update_length = SIZE_MAX; + size_t finish_length = SIZE_MAX; + + PSA_INIT(); + TEST_CALLOC(output, output_size); + TEST_CF_SECRET(key_data->x, key_data->len); + TEST_CF_SECRET(plaintext->x, plaintext->len); + //TEST_ASSERT(key_data->x[0] != 42); // uncomment to trip constant-flow test + + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_ENCRYPT); + psa_set_key_algorithm(&attributes, alg); + psa_set_key_type(&attributes, key_type); + PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); + + PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_cipher_set_iv(&operation, iv->x, iv->len)); + PSA_ASSERT(psa_cipher_update(&operation, + plaintext->x, plaintext->len, + output, output_size, &update_length)); + TEST_LE_U(update_length, output_size); + PSA_ASSERT(psa_cipher_finish(&operation, + output + update_length, + output_size - update_length, + &finish_length)); + + TEST_CF_PUBLIC(output, output_size); + TEST_MEMORY_COMPARE(expected_ciphertext->x, expected_ciphertext->len, + output, update_length + finish_length); + +exit: + mbedtls_free(output); + psa_cipher_abort(&operation); + psa_destroy_key(key); + PSA_DONE(); +} +/* END_CASE */ + +/* BEGIN_CASE */ +/* Known answer for cipher decryption (one-shot and multipart). + * Supports good cases and invalid padding cases. */ +void ct_cipher_decrypt(int alg_arg, + int key_type_arg, const data_t *key_data, + const data_t *iv, + const data_t *ciphertext, + const data_t *expected_plaintext, + int expect_invalid_padding) +{ + psa_key_type_t key_type = key_type_arg; + psa_algorithm_t alg = alg_arg; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; + unsigned char *input = NULL; + unsigned char *output = NULL; + size_t output_size = PSA_CIPHER_DECRYPT_OUTPUT_MAX_SIZE(ciphertext->len); + size_t update_length = SIZE_MAX; + size_t finish_length = SIZE_MAX; + size_t output_length = SIZE_MAX; + psa_status_t status; + + PSA_INIT(); + TEST_CALLOC(output, output_size); + TEST_CF_SECRET(key_data->x, key_data->len); + TEST_CF_SECRET(ciphertext->x, ciphertext->len); + //TEST_ASSERT(key_data->x[0] != 42); // uncomment to trip constant-flow test + + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DECRYPT); + psa_set_key_algorithm(&attributes, alg); + psa_set_key_type(&attributes, key_type); + PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); + + PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_cipher_set_iv(&operation, iv->x, iv->len)); + PSA_ASSERT(psa_cipher_update(&operation, + ciphertext->x, ciphertext->len, + output, output_size, &update_length)); + TEST_LE_U(update_length, output_size); + status = psa_cipher_finish(&operation, + output + update_length, + output_size - update_length, + &finish_length); + TEST_CF_PUBLIC(output, output_size); + + if (expect_invalid_padding) { + TEST_EQUAL(status, PSA_ERROR_INVALID_PADDING); + } else { + TEST_EQUAL(status, PSA_SUCCESS); + TEST_MEMORY_COMPARE(expected_plaintext->x, expected_plaintext->len, + output, update_length + finish_length); + } + + memset(output, 0, output_size); + TEST_CALLOC(input, iv->len + ciphertext->len); + memcpy(input, iv->x, iv->len); + memcpy(input + iv->len, ciphertext->x, ciphertext->len); + status = psa_cipher_decrypt(key, alg, + input, iv->len + ciphertext->len, + output, output_size, &output_length); + TEST_CF_PUBLIC(output, output_size); + + if (expect_invalid_padding) { + TEST_EQUAL(status, PSA_ERROR_INVALID_PADDING); + } else { + TEST_EQUAL(status, PSA_SUCCESS); + TEST_MEMORY_COMPARE(expected_plaintext->x, expected_plaintext->len, + output, output_length); + } + +exit: + mbedtls_free(input); + mbedtls_free(output); + psa_cipher_abort(&operation); + psa_destroy_key(key); + PSA_DONE(); +} +/* END_CASE */ From b6b1a8299b666fb827093de24c54e5788e8075dc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 Aug 2025 20:28:34 +0200 Subject: [PATCH 2/9] Factor API calls into auxiliary functions Factor some common code for one-shot or multipart encryption/decryption into auxiliary functions. No behavior change. Signed-off-by: Gilles Peskine --- ...st_suite_psa_crypto_constant_time.function | 163 +++++++++++------- 1 file changed, 102 insertions(+), 61 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_constant_time.function b/tests/suites/test_suite_psa_crypto_constant_time.function index fb8368ad31..c212ec7250 100644 --- a/tests/suites/test_suite_psa_crypto_constant_time.function +++ b/tests/suites/test_suite_psa_crypto_constant_time.function @@ -19,6 +19,83 @@ #define HAVE_CONSTANT_TIME_AES #endif +static int ct_cipher_multipart(psa_cipher_operation_t *operation, + const data_t *iv, + const data_t *input, + size_t output_size, + const data_t *expected_output, + psa_status_t expected_finish_status) +{ + unsigned char *output = NULL; + size_t update_length = SIZE_MAX; + size_t finish_length = SIZE_MAX; + psa_status_t status; + int ok = 0; + + TEST_CALLOC(output, output_size); + + PSA_ASSERT(psa_cipher_set_iv(operation, iv->x, iv->len)); + status = psa_cipher_update(operation, + input->x, input->len, + output, output_size, &update_length); + if (expected_finish_status == PSA_ERROR_BUFFER_TOO_SMALL && + status == PSA_ERROR_BUFFER_TOO_SMALL) { + /* The output buffer is already too small for update. That's ok. */ + ok = 1; + goto exit; + } else { + PSA_ASSERT(status); + } + TEST_LE_U(update_length, output_size); + TEST_EQUAL(psa_cipher_finish(operation, + output + update_length, + output_size - update_length, + &finish_length), + expected_finish_status); + + TEST_CF_PUBLIC(output, output_size); + if (expected_finish_status == PSA_SUCCESS) { + TEST_MEMORY_COMPARE(expected_output->x, expected_output->len, + output, update_length + finish_length); + } + ok = 1; + +exit: + mbedtls_free(output); + psa_cipher_abort(operation); + return ok; +} + +static int ct_cipher_decrypt_oneshot(mbedtls_svc_key_id_t key, + psa_algorithm_t alg, + const data_t *input, + size_t output_size, + const data_t *expected_output, + psa_status_t expected_status) +{ + unsigned char *output = NULL; + size_t output_length = SIZE_MAX; + int ok = 0; + + TEST_CALLOC(output, output_size); + + TEST_EQUAL(psa_cipher_decrypt(key, alg, + input->x, input->len, + output, output_size, &output_length), + expected_status); + + TEST_CF_PUBLIC(output, output_size); + if (expected_status == PSA_SUCCESS) { + TEST_MEMORY_COMPARE(expected_output->x, expected_output->len, + output, output_length); + } + ok = 1; + +exit: + mbedtls_free(output); + return ok; +} + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -41,13 +118,8 @@ void ct_cipher_encrypt(int alg_arg, psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; - unsigned char *output = NULL; - size_t output_size = PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE(plaintext->len); - size_t update_length = SIZE_MAX; - size_t finish_length = SIZE_MAX; PSA_INIT(); - TEST_CALLOC(output, output_size); TEST_CF_SECRET(key_data->x, key_data->len); TEST_CF_SECRET(plaintext->x, plaintext->len); //TEST_ASSERT(key_data->x[0] != 42); // uncomment to trip constant-flow test @@ -58,22 +130,14 @@ void ct_cipher_encrypt(int alg_arg, PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); - PSA_ASSERT(psa_cipher_set_iv(&operation, iv->x, iv->len)); - PSA_ASSERT(psa_cipher_update(&operation, - plaintext->x, plaintext->len, - output, output_size, &update_length)); - TEST_LE_U(update_length, output_size); - PSA_ASSERT(psa_cipher_finish(&operation, - output + update_length, - output_size - update_length, - &finish_length)); - - TEST_CF_PUBLIC(output, output_size); - TEST_MEMORY_COMPARE(expected_ciphertext->x, expected_ciphertext->len, - output, update_length + finish_length); + if (!ct_cipher_multipart(&operation, iv, plaintext, + PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE(plaintext->len), + expected_ciphertext, + PSA_SUCCESS)) { + goto exit; + } exit: - mbedtls_free(output); psa_cipher_abort(&operation); psa_destroy_key(key); PSA_DONE(); @@ -92,19 +156,16 @@ void ct_cipher_decrypt(int alg_arg, { psa_key_type_t key_type = key_type_arg; psa_algorithm_t alg = alg_arg; + size_t sufficient_output_size = + PSA_CIPHER_DECRYPT_OUTPUT_SIZE(key_type, alg, ciphertext->len); + psa_status_t expected_status = + expect_invalid_padding ? PSA_ERROR_INVALID_PADDING : PSA_SUCCESS; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; - unsigned char *input = NULL; - unsigned char *output = NULL; - size_t output_size = PSA_CIPHER_DECRYPT_OUTPUT_MAX_SIZE(ciphertext->len); - size_t update_length = SIZE_MAX; - size_t finish_length = SIZE_MAX; - size_t output_length = SIZE_MAX; - psa_status_t status; + data_t input = { NULL, iv->len + ciphertext->len }; PSA_INIT(); - TEST_CALLOC(output, output_size); TEST_CF_SECRET(key_data->x, key_data->len); TEST_CF_SECRET(ciphertext->x, ciphertext->len); //TEST_ASSERT(key_data->x[0] != 42); // uncomment to trip constant-flow test @@ -115,45 +176,25 @@ void ct_cipher_decrypt(int alg_arg, PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); - PSA_ASSERT(psa_cipher_set_iv(&operation, iv->x, iv->len)); - PSA_ASSERT(psa_cipher_update(&operation, - ciphertext->x, ciphertext->len, - output, output_size, &update_length)); - TEST_LE_U(update_length, output_size); - status = psa_cipher_finish(&operation, - output + update_length, - output_size - update_length, - &finish_length); - TEST_CF_PUBLIC(output, output_size); - - if (expect_invalid_padding) { - TEST_EQUAL(status, PSA_ERROR_INVALID_PADDING); - } else { - TEST_EQUAL(status, PSA_SUCCESS); - TEST_MEMORY_COMPARE(expected_plaintext->x, expected_plaintext->len, - output, update_length + finish_length); + if (!ct_cipher_multipart(&operation, iv, ciphertext, + PSA_CIPHER_DECRYPT_OUTPUT_MAX_SIZE(ciphertext->len), + expected_plaintext, + expected_status)) { + goto exit; } - memset(output, 0, output_size); - TEST_CALLOC(input, iv->len + ciphertext->len); - memcpy(input, iv->x, iv->len); - memcpy(input + iv->len, ciphertext->x, ciphertext->len); - status = psa_cipher_decrypt(key, alg, - input, iv->len + ciphertext->len, - output, output_size, &output_length); - TEST_CF_PUBLIC(output, output_size); - - if (expect_invalid_padding) { - TEST_EQUAL(status, PSA_ERROR_INVALID_PADDING); - } else { - TEST_EQUAL(status, PSA_SUCCESS); - TEST_MEMORY_COMPARE(expected_plaintext->x, expected_plaintext->len, - output, output_length); + TEST_CALLOC(input.x, input.len); + memcpy(input.x, iv->x, iv->len); + memcpy(input.x + iv->len, ciphertext->x, ciphertext->len); + if (!ct_cipher_decrypt_oneshot(key, alg, &input, + sufficient_output_size, + expected_plaintext, + expected_status)) { + goto exit; } exit: - mbedtls_free(input); - mbedtls_free(output); + mbedtls_free(input.x); psa_cipher_abort(&operation); psa_destroy_key(key); PSA_DONE(); From d3e182e7dac9619b3efd7fa3de3452a160e7d971 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 Aug 2025 21:25:23 +0200 Subject: [PATCH 3/9] Add BUFFER_TOO_SMALL testing Signed-off-by: Gilles Peskine --- ...st_suite_psa_crypto_constant_time.function | 120 +++++++++++++++++- 1 file changed, 114 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_constant_time.function b/tests/suites/test_suite_psa_crypto_constant_time.function index c212ec7250..740b8d7d18 100644 --- a/tests/suites/test_suite_psa_crypto_constant_time.function +++ b/tests/suites/test_suite_psa_crypto_constant_time.function @@ -115,6 +115,8 @@ void ct_cipher_encrypt(int alg_arg, { psa_key_type_t key_type = key_type_arg; psa_algorithm_t alg = alg_arg; + size_t sufficient_output_size = + PSA_CIPHER_ENCRYPT_OUTPUT_SIZE(key_type, alg, plaintext->len); psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; @@ -129,9 +131,51 @@ void ct_cipher_encrypt(int alg_arg, psa_set_key_type(&attributes, key_type); PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); + /* Output buffer too small for the actual output */ + mbedtls_test_set_step(1); PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); if (!ct_cipher_multipart(&operation, iv, plaintext, - PSA_CIPHER_ENCRYPT_OUTPUT_MAX_SIZE(plaintext->len), + expected_ciphertext->len - 1, + expected_ciphertext, + PSA_ERROR_BUFFER_TOO_SMALL)) { + goto exit; + } + + if (expected_ciphertext->len < sufficient_output_size) { + /* For a buffer of intermediate size (between the actual output length + * and the guaranteed sufficient size), either PSA_SUCCESS or + * PSA_ERROR_BUFFER_TOO_SMALL is acceptable. Require what the our + * built-in implementation currently does. */ + psa_status_t intermediate_size_status = PSA_SUCCESS; + + /* Output buffer size just large enough for the actual output + * but less than the guaranteed sufficient size */ + mbedtls_test_set_step(2); + PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, plaintext, + expected_ciphertext->len, + expected_ciphertext, + intermediate_size_status)) { + goto exit; + } + + /* Output buffer size large enough for the actual output + * but one less than the guaranteed sufficient size */ + mbedtls_test_set_step(3); + PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, plaintext, + sufficient_output_size - 1, + expected_ciphertext, + intermediate_size_status)) { + goto exit; + } + } + + /* Guaranteed sufficient output buffer size */ + mbedtls_test_set_step(4); + PSA_ASSERT(psa_cipher_encrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, plaintext, + sufficient_output_size, expected_ciphertext, PSA_SUCCESS)) { goto exit; @@ -170,22 +214,86 @@ void ct_cipher_decrypt(int alg_arg, TEST_CF_SECRET(ciphertext->x, ciphertext->len); //TEST_ASSERT(key_data->x[0] != 42); // uncomment to trip constant-flow test + TEST_CALLOC(input.x, input.len); + memcpy(input.x, iv->x, iv->len); + memcpy(input.x + iv->len, ciphertext->x, ciphertext->len); + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_DECRYPT); psa_set_key_algorithm(&attributes, alg); psa_set_key_type(&attributes, key_type); PSA_ASSERT(psa_import_key(&attributes, key_data->x, key_data->len, &key)); + /* Output buffer too small for the actual output */ + mbedtls_test_set_step(1); PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); if (!ct_cipher_multipart(&operation, iv, ciphertext, - PSA_CIPHER_DECRYPT_OUTPUT_MAX_SIZE(ciphertext->len), + expected_plaintext->len - 1, + expected_plaintext, + PSA_ERROR_BUFFER_TOO_SMALL)) { + goto exit; + } + if (!ct_cipher_decrypt_oneshot(key, alg, &input, + expected_plaintext->len - 1, + expected_plaintext, + PSA_ERROR_BUFFER_TOO_SMALL)) { + goto exit; + } + + if (expected_plaintext->len < sufficient_output_size) { + /* For a buffer of intermediate size (between the actual output length + * and the guaranteed sufficient size), either PSA_SUCCESS (or + * PSA_ERROR_INVALID_PADDING if the padding is invalid) or + * PSA_ERROR_BUFFER_TOO_SMALL is acceptable. Require what the our + * built-in implementation currently does. */ + psa_status_t intermediate_size_status = expected_status; + if (alg == PSA_ALG_CBC_PKCS7) { + intermediate_size_status = PSA_ERROR_BUFFER_TOO_SMALL; + } + + /* Output buffer size just large enough for the actual output + * but less than the guaranteed sufficient size */ + mbedtls_test_set_step(2); + PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, ciphertext, + expected_plaintext->len, + expected_plaintext, + intermediate_size_status)) { + goto exit; + } + if (!ct_cipher_decrypt_oneshot(key, alg, &input, + expected_plaintext->len - 1, + expected_plaintext, + intermediate_size_status)) { + goto exit; + } + + /* Output buffer size large enough for the actual output + * but one less than the guaranteed sufficient size */ + mbedtls_test_set_step(3); + PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, ciphertext, + sufficient_output_size - 1, + expected_plaintext, + intermediate_size_status)) { + goto exit; + } + if (!ct_cipher_decrypt_oneshot(key, alg, &input, + sufficient_output_size - 1, + expected_plaintext, + intermediate_size_status)) { + goto exit; + } + } + + /* Guaranteed sufficient output buffer size */ + mbedtls_test_set_step(4); + PSA_ASSERT(psa_cipher_decrypt_setup(&operation, key, alg)); + if (!ct_cipher_multipart(&operation, iv, ciphertext, + sufficient_output_size, expected_plaintext, expected_status)) { goto exit; } - - TEST_CALLOC(input.x, input.len); - memcpy(input.x, iv->x, iv->len); - memcpy(input.x + iv->len, ciphertext->x, ciphertext->len); if (!ct_cipher_decrypt_oneshot(key, alg, &input, sufficient_output_size, expected_plaintext, From d179dc80a5b13189c79fe4531eacb28698a7a0e9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 27 Jul 2025 18:57:04 +0200 Subject: [PATCH 4/9] Use mbedtls_psa_cipher_finish() in PSA Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index efc5813ff0..4443d73ba4 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -552,6 +552,7 @@ psa_status_t mbedtls_psa_cipher_finish( { psa_status_t status = PSA_ERROR_GENERIC_ERROR; uint8_t temp_output_buffer[MBEDTLS_MAX_BLOCK_LENGTH]; + size_t invalid_padding = 0; if (operation->ctx.cipher.unprocessed_len != 0) { if (operation->alg == PSA_ALG_ECB_NO_PADDING || @@ -562,9 +563,10 @@ psa_status_t mbedtls_psa_cipher_finish( } status = mbedtls_to_psa_error( - mbedtls_cipher_finish(&operation->ctx.cipher, - temp_output_buffer, - output_length)); + mbedtls_cipher_finish_padded(&operation->ctx.cipher, + temp_output_buffer, + output_length, + &invalid_padding)); if (status != PSA_SUCCESS) { goto exit; } @@ -581,6 +583,9 @@ exit: mbedtls_platform_zeroize(temp_output_buffer, sizeof(temp_output_buffer)); + if (status == PSA_SUCCESS && invalid_padding) { + status = PSA_ERROR_INVALID_PADDING; + } return status; } From e74b42832e4af11606ef8aae2c9404b4acaa2c6d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 27 Jul 2025 21:29:40 +0200 Subject: [PATCH 5/9] Return PSA_ERROR_INVALID_PADDING in constant time Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 4443d73ba4..6d0378bd7e 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -13,6 +13,7 @@ #include "psa_crypto_cipher.h" #include "psa_crypto_core.h" #include "psa_crypto_random_impl.h" +#include "constant_time_internal.h" #include "mbedtls/cipher.h" #include "mbedtls/error.h" @@ -583,8 +584,9 @@ exit: mbedtls_platform_zeroize(temp_output_buffer, sizeof(temp_output_buffer)); - if (status == PSA_SUCCESS && invalid_padding) { - status = PSA_ERROR_INVALID_PADDING; + if (status == PSA_SUCCESS) { + status = mbedtls_ct_size_if_else_0(invalid_padding, + PSA_ERROR_INVALID_PADDING); } return status; } From 3b380daedbce9fae3e7ed7e84f18e97876e7e6f3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 Aug 2025 21:59:07 +0200 Subject: [PATCH 6/9] psa_cipher_finish: treat status and output length as sensitive In `psa_cipher_finish()` and in the corresponding function in our built-in implementation `mbedtls_psa_cipher_finish()`, treat `status` and `*output_length` as sensitive variables whose value must not leak through a timing side channel. This is important when doing decryption with unpadding, where leaking the validity or amount of padding can enable a padding oracle attack. With this change, `psa_cipher_finish()` should be constant-time if the underlying legacy function (including the cipher implementation) is. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 26 +++++++++++++++++++++----- library/psa_crypto_cipher.c | 36 ++++++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9c28609d7e..51ec72fa19 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -73,6 +73,8 @@ #include "mbedtls/psa_util.h" #include "mbedtls/threading.h" +#include "constant_time_internal.h" + #if defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF_EXTRACT) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF_EXPAND) @@ -4692,13 +4694,27 @@ psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, output_length); exit: - if (status == PSA_SUCCESS) { - status = psa_cipher_abort(operation); - } else { - *output_length = 0; - (void) psa_cipher_abort(operation); + /* C99 doesn't allow a declaration to follow a label */; + psa_status_t abort_status = psa_cipher_abort(operation); + /* Normally abort shouldn't fail unless the operation is in a bad + * state, in which case we'd expect finish to fail with the same error. + * So it doesn't matter much which call's error code we pick when both + * fail. However, in unauthenticated decryption specifically, the + * distinction between PSA_SUCCESS and PSA_ERROR_INVALID_PADDING is + * security-sensitive (risk of a padding oracle attack), so here we + * must not have a code path that depends on the value of status. */ + if (abort_status != PSA_SUCCESS) { + status = abort_status; } + /* Set *output_length to 0 if status != PSA_SUCCESS, without + * leaking the value of status through a timing side channel + * (status == PSA_ERROR_INVALID_PADDING is sensitive when doing + * unpadded decryption, due to the risk of padding oracle attack). */ + mbedtls_ct_condition_t success = + mbedtls_ct_bool_not(mbedtls_ct_bool(status)); + *output_length = mbedtls_ct_size_if_else_0(success, *output_length); + LOCAL_OUTPUT_FREE(output_external, output); return status; diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 6d0378bd7e..022bdfa49f 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -552,9 +552,21 @@ psa_status_t mbedtls_psa_cipher_finish( uint8_t *output, size_t output_size, size_t *output_length) { psa_status_t status = PSA_ERROR_GENERIC_ERROR; - uint8_t temp_output_buffer[MBEDTLS_MAX_BLOCK_LENGTH]; size_t invalid_padding = 0; + uint8_t temp_output_buffer[MBEDTLS_MAX_BLOCK_LENGTH] = { 0 }; + if (output_size > sizeof(temp_output_buffer)) { + output_size = sizeof(temp_output_buffer); + } + /* We will copy output_size bytes from temp_output_buffer to the + * output buffer. We can't use *output_length to determine how + * much to copy because we must not leak that value through timing + * when doing decryption with unpadding. But the underlying function + * is not guaranteed to write beyond *output_length. To ensure we don't + * leak the former content of the stack to the caller, wipe that + * former content. */ + memset(temp_output_buffer, 0, output_size); + if (operation->ctx.cipher.unprocessed_len != 0) { if (operation->alg == PSA_ALG_ECB_NO_PADDING || operation->alg == PSA_ALG_CBC_NO_PADDING) { @@ -572,22 +584,26 @@ psa_status_t mbedtls_psa_cipher_finish( goto exit; } - if (*output_length == 0) { + if (output_size == 0) { ; /* Nothing to copy. Note that output may be NULL in this case. */ - } else if (output_size >= *output_length) { - memcpy(output, temp_output_buffer, *output_length); } else { - status = PSA_ERROR_BUFFER_TOO_SMALL; + /* Do not use the value of *output_length to determine how much + * to copy. When decrypting a padded cipher, the output length is + * sensitive, and leaking it could allow a padding oracle attack. */ + memcpy(output, temp_output_buffer, output_size); } + status = mbedtls_ct_error_if_else_0(invalid_padding, + PSA_ERROR_INVALID_PADDING); + mbedtls_ct_condition_t buffer_too_small = + mbedtls_ct_uint_lt(output_size, *output_length); + status = mbedtls_ct_error_if(buffer_too_small, + PSA_ERROR_BUFFER_TOO_SMALL, + status); + exit: mbedtls_platform_zeroize(temp_output_buffer, sizeof(temp_output_buffer)); - - if (status == PSA_SUCCESS) { - status = mbedtls_ct_size_if_else_0(invalid_padding, - PSA_ERROR_INVALID_PADDING); - } return status; } From 04dfd704325a6dbc2a13eb7f418eaca9ae9ca549 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 Aug 2025 22:27:26 +0200 Subject: [PATCH 7/9] psa_cipher_decrypt: treat status and output length as sensitive In `psa_cipher_decrypt()` and in the corresponding function in our built-in implementation `mbedtls_psa_cipher_decrypt()`, treat `status` and `*output_length` as sensitive variables whose value must not leak through a timing side channel. This is important when doing decryption with unpadding, where leaking the validity or amount of padding can enable a padding oracle attack. With this change, `psa_cipher_decrypt()` should be constant-time if the underlying legacy function (including the cipher implementation) is. Signed-off-by: Gilles Peskine --- library/psa_crypto.c | 12 ++++++++---- library/psa_crypto_cipher.c | 18 +++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 51ec72fa19..5f2cad42b7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4857,13 +4857,17 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, exit: unlock_status = psa_unregister_read_under_mutex(slot); - if (status == PSA_SUCCESS) { + if (unlock_status != PSA_SUCCESS) { status = unlock_status; } - if (status != PSA_SUCCESS) { - *output_length = 0; - } + /* Set *output_length to 0 if status != PSA_SUCCESS, without + * leaking the value of status through a timing side channel + * (status == PSA_ERROR_INVALID_PADDING is sensitive when doing + * unpadded decryption, due to the risk of padding oracle attack). */ + mbedtls_ct_condition_t success = + mbedtls_ct_bool_not(mbedtls_ct_bool(status)); + *output_length = mbedtls_ct_size_if_else_0(success, *output_length); LOCAL_INPUT_FREE(input_external, input); LOCAL_OUTPUT_FREE(output_external, output); diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 022bdfa49f..83069c7852 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -724,17 +724,21 @@ psa_status_t mbedtls_psa_cipher_decrypt( &operation, mbedtls_buffer_offset(output, accumulated_length), output_size - accumulated_length, &olength); - if (status != PSA_SUCCESS) { - goto exit; - } *output_length = accumulated_length + olength; exit: - if (status == PSA_SUCCESS) { - status = mbedtls_psa_cipher_abort(&operation); - } else { - mbedtls_psa_cipher_abort(&operation); + /* C99 doesn't allow a declaration to follow a label */; + psa_status_t abort_status = mbedtls_psa_cipher_abort(&operation); + /* Normally abort shouldn't fail unless the operation is in a bad + * state, in which case we'd expect finish to fail with the same error. + * So it doesn't matter much which call's error code we pick when both + * fail. However, in unauthenticated decryption specifically, the + * distinction between PSA_SUCCESS and PSA_ERROR_INVALID_PADDING is + * security-sensitive (risk of a padding oracle attack), so here we + * must not have a code path that depends on the value of status. */ + if (abort_status != PSA_SUCCESS) { + status = abort_status; } return status; From 2d666646bace4de2fce0f8dd9f95df560dd55f73 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 7 Aug 2025 23:07:31 +0200 Subject: [PATCH 8/9] Changelog entry for PSA CBC-PKCS7 padding oracle fix Signed-off-by: Gilles Peskine --- ChangeLog.d/pkcs7-padding-error-leak.txt | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ChangeLog.d/pkcs7-padding-error-leak.txt diff --git a/ChangeLog.d/pkcs7-padding-error-leak.txt b/ChangeLog.d/pkcs7-padding-error-leak.txt new file mode 100644 index 0000000000..5d204d5bef --- /dev/null +++ b/ChangeLog.d/pkcs7-padding-error-leak.txt @@ -0,0 +1,5 @@ +Security + * Fix a timing side channel in CBC-PKCS7 decryption that could + allow an attacker who can submit chosen ciphertexts to recover + some plaintexts through a timing-based padding oracle attack. + Credits to Beat Heeb from Oberon microsystems AG. CVE-TODO From cc908ad04c388b50b81fa3b3a8b509cf62797fcf Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 25 Aug 2025 17:01:34 +0200 Subject: [PATCH 9/9] Remove redundant memset on freshly initialized buffer Signed-off-by: Gilles Peskine --- library/psa_crypto_cipher.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 83069c7852..7f691c1d95 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -554,10 +554,6 @@ psa_status_t mbedtls_psa_cipher_finish( psa_status_t status = PSA_ERROR_GENERIC_ERROR; size_t invalid_padding = 0; - uint8_t temp_output_buffer[MBEDTLS_MAX_BLOCK_LENGTH] = { 0 }; - if (output_size > sizeof(temp_output_buffer)) { - output_size = sizeof(temp_output_buffer); - } /* We will copy output_size bytes from temp_output_buffer to the * output buffer. We can't use *output_length to determine how * much to copy because we must not leak that value through timing @@ -565,7 +561,10 @@ psa_status_t mbedtls_psa_cipher_finish( * is not guaranteed to write beyond *output_length. To ensure we don't * leak the former content of the stack to the caller, wipe that * former content. */ - memset(temp_output_buffer, 0, output_size); + uint8_t temp_output_buffer[MBEDTLS_MAX_BLOCK_LENGTH] = { 0 }; + if (output_size > sizeof(temp_output_buffer)) { + output_size = sizeof(temp_output_buffer); + } if (operation->ctx.cipher.unprocessed_len != 0) { if (operation->alg == PSA_ALG_ECB_NO_PADDING ||