From 652ea21737fcc6b2fdcc0be24e2462c17f16d67c Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 20 Jan 2025 17:44:00 +0000 Subject: [PATCH] Fix timing side-channel in PKCS7 padding Previously, we were checking if the last padding byte was in the range 1-16 and returning early if not. This was to prevent an integer overflow in the output length. Instead, do the checks in constant-time and conditionally set the output length based on whether the padding is correct or not, preventing both the side-channel and the integer overflow. Signed-off-by: David Horstmann --- library/cipher.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index 5e14e1eab6..9be53b7427 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -851,10 +851,6 @@ MBEDTLS_STATIC_TESTABLE int get_pkcs_padding(unsigned char *input, } padding_len = input[input_len - 1]; - if (padding_len == 0 || padding_len > input_len) { - return MBEDTLS_ERR_CIPHER_INVALID_PADDING; - } - *data_len = input_len - padding_len; mbedtls_ct_condition_t bad = mbedtls_ct_uint_gt(padding_len, input_len); bad = mbedtls_ct_bool_or(bad, mbedtls_ct_uint_eq(padding_len, 0)); @@ -868,6 +864,9 @@ MBEDTLS_STATIC_TESTABLE int get_pkcs_padding(unsigned char *input, bad = mbedtls_ct_bool_or(bad, mbedtls_ct_bool_and(in_padding, different)); } + /* If the padding is invalid, set the output length to 0 */ + *data_len = mbedtls_ct_if(bad, 0, input_len - padding_len); + return mbedtls_ct_error_if_else_0(bad, MBEDTLS_ERR_CIPHER_INVALID_PADDING); } #endif /* MBEDTLS_CIPHER_PADDING_PKCS7 */