Add invalid padding_len check in get_pkcs_padding

When trying to decrypt data with an invalid key, we found that `mbedtls`
returned `0x6200` (`-25088`), which means "_CIPHER - Input data contains
invalid padding and is rejected_" from `mbedtls_cipher_finish`, but it also
set the output len as `18446744073709551516`.

In case we detect an error with padding, we leave the output len zero'ed
and return `MBEDTLS_ERR_CIPHER_INVALID_PADDING`.

Here's a reference for the way `openssl` checks the padding length:
  - 1848c561ec/crypto/evp/evp_enc.c (L1023)
  - b554eef43b

So add a check ensuring output is set to the least-harmful value in the
error cases.

With the robustness fix:
`PASSED (125 suites, 26644 tests run)`

Without the robustness fix:
`FAILED (125 suites, 26644 tests run)`

Signed-off-by: Andre Goddard Rosa <andre.goddard@gmail.com>
Signed-off-by: Andre Goddard Rosa <agoddardrosa@roku.com>
This commit is contained in:
Andre Goddard Rosa 2024-05-01 11:47:12 -05:00
parent f1415b65d0
commit 3711734a0a
2 changed files with 11 additions and 0 deletions

View File

@ -849,6 +849,9 @@ static int get_pkcs_padding(unsigned char *input, size_t input_len,
}
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);

View File

@ -549,6 +549,10 @@ void enc_fail(int cipher_id, int pad_mode, int key_len, int length_val,
/* encode length number of bytes from inbuf */
TEST_ASSERT(0 == mbedtls_cipher_update(&ctx, inbuf, length, encbuf, &outlen));
TEST_ASSERT(ret == mbedtls_cipher_finish(&ctx, encbuf + outlen, &outlen));
if (0 != ret) {
/* Check output parameter is set to the least-harmful value on error */
TEST_ASSERT(0 == outlen);
}
/* done */
exit:
@ -826,6 +830,10 @@ void decrypt_test_vec(int cipher_id, int pad_mode, data_t *key,
total_len += outlen;
TEST_ASSERT(finish_result == mbedtls_cipher_finish(&ctx, output + outlen,
&outlen));
if (0 != finish_result) {
/* Check output parameter is set to the least-harmful value on error */
TEST_ASSERT(0 == outlen);
}
total_len += outlen;
#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C)
int tag_expected = (ctx.cipher_info->mode == MBEDTLS_MODE_GCM ||