diff --git a/ChangeLog.d/fix-ccm-finish.txt b/ChangeLog.d/fix-ccm-finish.txt new file mode 100644 index 0000000000..7600ec89a0 --- /dev/null +++ b/ChangeLog.d/fix-ccm-finish.txt @@ -0,0 +1,6 @@ +Security + * Add tag length validation in mbedtls_ccm_finish() to prevent + out-of-bounds reads and mitigate potential application buffer + overflows where applications relied on the library to enforce + tag length constraints. + diff --git a/library/ccm.c b/library/ccm.c index 969c6c781b..6a4fbb0a0f 100644 --- a/library/ccm.c +++ b/library/ccm.c @@ -177,6 +177,7 @@ static int ccm_calculate_first_block_if_ready(mbedtls_ccm_context *ctx) ctx->plaintext_len = 0; return 0; } else { + ctx->state |= CCM_STATE__ERROR; return MBEDTLS_ERR_CCM_BAD_INPUT; } } @@ -480,6 +481,14 @@ int mbedtls_ccm_finish(mbedtls_ccm_context *ctx, return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; } + if (!(ctx->state & CCM_STATE__STARTED)) { + return MBEDTLS_ERR_CCM_BAD_INPUT; + } + + if (!(ctx->state & CCM_STATE__LENGTHS_SET)) { + return MBEDTLS_ERR_CCM_BAD_INPUT; + } + if (ctx->add_len > 0 && !(ctx->state & CCM_STATE__AUTH_DATA_FINISHED)) { return MBEDTLS_ERR_CCM_BAD_INPUT; } @@ -488,6 +497,10 @@ int mbedtls_ccm_finish(mbedtls_ccm_context *ctx, return MBEDTLS_ERR_CCM_BAD_INPUT; } + if (tag_len != ctx->tag_len) { + return MBEDTLS_ERR_CCM_BAD_INPUT; + } + /* * Authentication: reset counter and crypt/mask internal tag */ diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 0ef6c604ed..63b16f9284 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5507,6 +5507,10 @@ exit: static psa_status_t psa_aead_final_checks(const psa_aead_operation_t *operation) { + if (operation->alg == PSA_ALG_CCM && !operation->lengths_set) { + return PSA_ERROR_BAD_STATE; + } + if (operation->id == 0 || !operation->nonce_set) { return PSA_ERROR_BAD_STATE; } diff --git a/tests/suites/test_suite_ccm.data b/tests/suites/test_suite_ccm.data index 22150f7a71..b1930de50f 100644 --- a/tests/suites/test_suite_ccm.data +++ b/tests/suites/test_suite_ccm.data @@ -1746,6 +1746,72 @@ CCM* decrypt, instant finish AES-128 (P=0, N=13, A=0, T=16) depends_on:MBEDTLS_CCM_GCM_CAN_AES mbedtls_ccm_instant_finish:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_STAR_DECRYPT:"54caf96ef6d448734700aadab50faf7a":"a3803e752ae849c910d8da36af" +CCM finish check-boundary encrypt tag_len=8, finish_tag_len=32 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:8:32:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary encrypt tag_len=8, finish_tag_len=8 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:8:8:0 + +CCM finish check-boundary decrypt tag_len=8, finish_tag_len=32 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:8:32:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary decrypt tag_len=8, finish_tag_len=8 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:8:8:0 + +CCM finish check-boundary encrypt tag_len=0, finish_tag_len=0 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:0:0:MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED + +CCM finish check-boundary decrypt tag_len=0, finish_tag_len=0 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:0:0:MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED + +# CCM finish boundary matrix (encrypt) +CCM finish check-boundary encrypt tag_len=16, finish_tag_len=16 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:16:16:0 + +CCM finish check-boundary encrypt tag_len=8, finish_tag_len=16 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:8:16:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary encrypt tag_len=16, finish_tag_len=17 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:16:17:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary encrypt tag_len=16, finish_tag_len=4000 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:16:4000:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary encrypt tag_len=16, finish_tag_len=8 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:16:8:MBEDTLS_ERR_CCM_BAD_INPUT + +# CCM finish boundary matrix (decrypt) +CCM finish check-boundary decrypt tag_len=16, finish_tag_len=16 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:16:16:0 + +CCM finish check-boundary decrypt tag_len=8, finish_tag_len=16 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:8:16:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary decrypt tag_len=16, finish_tag_len=17 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:16:17:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary decrypt tag_len=16, finish_tag_len=4000 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:16:4000:MBEDTLS_ERR_CCM_BAD_INPUT + +CCM finish check-boundary decrypt tag_len=16, finish_tag_len=8 +depends_on:MBEDTLS_CCM_GCM_CAN_AES +mbedtls_ccm_finish_boundary:MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_DECRYPT:16:8:MBEDTLS_ERR_CCM_BAD_INPUT + CCM pass unexpected auth data, NIST VPT AES-128 #14 (P=13, N=13, A=32, T=16) depends_on:MBEDTLS_CCM_GCM_CAN_AES mbedtls_ccm_unexpected_ad::MBEDTLS_CIPHER_ID_AES:MBEDTLS_CCM_ENCRYPT:"d32088d50df9aba14d9022c870a0cb85":"e16c69861efc206e85aab1255e":"0eff7d7bcceb873c3203a8df74f4e91b04bd607ec11202f96cfeb99f5bcdb7aa" diff --git a/tests/suites/test_suite_ccm.function b/tests/suites/test_suite_ccm.function index 798be77013..75585f085b 100644 --- a/tests/suites/test_suite_ccm.function +++ b/tests/suites/test_suite_ccm.function @@ -1,5 +1,6 @@ /* BEGIN_HEADER */ #include "mbedtls/ccm.h" +#include "mbedtls/error.h" /* Use the multipart interface to process the encrypted data in two parts * and check that the output matches the expected output. @@ -906,3 +907,47 @@ exit: BLOCK_CIPHER_PSA_DONE(); } /* END_CASE */ + +/* BEGIN_CASE */ +void mbedtls_ccm_finish_boundary(int cipher_id, int mode, + int start_tag_len, int finish_tag_len, + int expected_finish_ret) +{ + unsigned char key[16]; + unsigned char iv[13]; + /* Test does not rely on data/iv.Using Arbitrary values */ + memset(key, 42, sizeof(key)); + memset(iv, 11, sizeof(iv)); + + mbedtls_ccm_context ctx; + unsigned char *tag = NULL; + + mbedtls_ccm_init(&ctx); + + BLOCK_CIPHER_PSA_INIT(); + TEST_EQUAL(mbedtls_ccm_setkey(&ctx, cipher_id, key, sizeof(key) * 8), 0); + + + /* For non-CCM* with tag_len=0, validate the mbedtls_ccm_set_lengths path + * when ccm_calculate_first_block_if_ready() fails. */ + if (start_tag_len == 0) { + TEST_EQUAL(0, mbedtls_ccm_set_lengths(&ctx, 0, 0, (size_t) start_tag_len)); + TEST_EQUAL(MBEDTLS_ERR_CCM_BAD_INPUT, + mbedtls_ccm_starts(&ctx, mode, iv, sizeof(iv))); + + /* Use the caller-provided tag length during setup, then finish with a + * different tag length to exercise boundary handling/API compliance */ + } else { + TEST_EQUAL(0, mbedtls_ccm_starts(&ctx, mode, iv, sizeof(iv))); + TEST_EQUAL(0, mbedtls_ccm_set_lengths(&ctx, 0, 0, (size_t) start_tag_len)); + } + + TEST_CALLOC(tag, finish_tag_len); + TEST_EQUAL(expected_finish_ret, + mbedtls_ccm_finish(&ctx, tag, finish_tag_len)); +exit: + mbedtls_free(tag); + mbedtls_ccm_free(&ctx); + BLOCK_CIPHER_PSA_DONE(); +} +/* END_CASE */ diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 4fc1a515b4..fb278ddbda 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -6490,11 +6490,94 @@ void aead_multipart_state_test(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); if (operation.alg == PSA_ALG_CCM) { - PSA_ASSERT(psa_aead_finish(&operation, final_data, + /* For CCM, finishing without setting lengths must fail. */ + TEST_EQUAL(psa_aead_finish(&operation, final_data, finish_output_size, &output_part_length, tag_buffer, tag_length, - &tag_size)); + &tag_size), + PSA_ERROR_BAD_STATE); + psa_aead_abort(&operation); + + /* For CCM, finishing after setting length without aead_update/ad must fail. */ + PSA_ASSERT(psa_aead_encrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + TEST_EQUAL(psa_aead_finish(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, tag_length, + &tag_size), + PSA_ERROR_INVALID_ARGUMENT); + psa_aead_abort(&operation); + + /* Valid path */ + PSA_ASSERT(psa_aead_encrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + PSA_ASSERT(psa_aead_update_ad(&operation, additional_data->x, + additional_data->len)); + PSA_ASSERT(psa_aead_update(&operation, input_data->x, + input_data->len, output_data, + output_size, &output_length)); + + TEST_EQUAL(psa_aead_finish(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, tag_length, + &tag_size), + PSA_SUCCESS); + psa_aead_abort(&operation); + + /* For CCM, verifying without setting lengths must fail. */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_BAD_STATE); + psa_aead_abort(&operation); + + /* For CCM, verifying after setting length without aead_update/ad must fail. */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_INVALID_ARGUMENT); + psa_aead_abort(&operation); + + /* Valid path */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + PSA_ASSERT(psa_aead_update_ad(&operation, additional_data->x, + additional_data->len)); + PSA_ASSERT(psa_aead_update(&operation, input_data->x, + input_data->len, output_data, + output_size, &output_length)); + + /* psa_driver_wrapper_aead_verify, which is called after psa_aead_final_checks + will return PSA_ERROR_INVALID_SIGNATURE */ + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_INVALID_SIGNATURE); } else { PSA_ASSERT(psa_aead_finish(&operation, final_data, finish_output_size, @@ -6562,11 +6645,97 @@ void aead_multipart_state_test(int key_type_arg, data_t *key_data, PSA_AEAD_NONCE_MAX_SIZE, &nonce_length)); if (operation.alg == PSA_ALG_CCM) { - PSA_ASSERT(psa_aead_finish(&operation, final_data, + /* For CCM, finishing without setting lengths must fail. */ + TEST_EQUAL(psa_aead_finish(&operation, final_data, finish_output_size, &output_part_length, tag_buffer, tag_length, - &tag_size)); + &tag_size), + PSA_ERROR_BAD_STATE); + psa_aead_abort(&operation); + + /* For CCM, finishing after setting length without aead_update/ad must fail. */ + PSA_ASSERT(psa_aead_encrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_generate_nonce(&operation, nonce_buffer, + PSA_AEAD_NONCE_MAX_SIZE, + &nonce_length)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + TEST_EQUAL(psa_aead_finish(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, tag_length, + &tag_size), + PSA_ERROR_INVALID_ARGUMENT); + psa_aead_abort(&operation); + + /* Valid path */ + PSA_ASSERT(psa_aead_encrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_generate_nonce(&operation, nonce_buffer, + PSA_AEAD_NONCE_MAX_SIZE, + &nonce_length)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + PSA_ASSERT(psa_aead_update_ad(&operation, additional_data->x, + additional_data->len)); + PSA_ASSERT(psa_aead_update(&operation, input_data->x, + input_data->len, output_data, + output_size, &output_length)); + + TEST_EQUAL(psa_aead_finish(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, tag_length, + &tag_size), + PSA_SUCCESS); + psa_aead_abort(&operation); + + /* For CCM, verifying without setting lengths must fail. */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_BAD_STATE); + psa_aead_abort(&operation); + + /* For CCM, verifying after setting length without aead_update/ad must fail. */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_INVALID_ARGUMENT); + psa_aead_abort(&operation); + + /* Valid path */ + PSA_ASSERT(psa_aead_decrypt_setup(&operation, key, alg)); + PSA_ASSERT(psa_aead_set_nonce(&operation, nonce->x, nonce->len)); + PSA_ASSERT(psa_aead_set_lengths(&operation, additional_data->len, + input_data->len)); + PSA_ASSERT(psa_aead_update_ad(&operation, additional_data->x, + additional_data->len)); + PSA_ASSERT(psa_aead_update(&operation, input_data->x, + input_data->len, output_data, + output_size, &output_length)); + + /* psa_driver_wrapper_aead_verify, which is called after psa_aead_final_checks + will return PSA_ERROR_INVALID_SIGNATURE */ + TEST_EQUAL(psa_aead_verify(&operation, final_data, + finish_output_size, + &output_part_length, + tag_buffer, + tag_length), + PSA_ERROR_INVALID_SIGNATURE); } else { PSA_ASSERT(psa_aead_finish(&operation, final_data, finish_output_size,