From 4f4d4b2c4031d93f649c33a1ddb1060b6a147b98 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:34:31 +0200 Subject: [PATCH 1/7] Test consistency of cipher max-size macros Signed-off-by: Gilles Peskine --- tests/suites/test_suite_cipher.function | 3 +++ tests/suites/test_suite_cmac.function | 2 ++ 2 files changed, 5 insertions(+) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index f8420458f2..01b3f2c7a5 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -63,6 +63,9 @@ static int check_cipher_info(mbedtls_cipher_type_t type, key_bitlen == 192 || key_bitlen == 256); } + TEST_LE_U(key_bitlen, MBEDTLS_MAX_KEY_LENGTH * 8); + TEST_LE_U(block_size, MBEDTLS_MAX_BLOCK_LENGTH); + TEST_LE_U(iv_size, MBEDTLS_MAX_IV_LENGTH); if (strstr(info->name, "-ECB") != NULL) { TEST_ASSERT(iv_size == 0); diff --git a/tests/suites/test_suite_cmac.function b/tests/suites/test_suite_cmac.function index 9624e8fea0..4597550502 100644 --- a/tests/suites/test_suite_cmac.function +++ b/tests/suites/test_suite_cmac.function @@ -111,6 +111,8 @@ void mbedtls_cmac_setkey(int cipher_type, int key_size, int result) TEST_ASSERT((cipher_info = mbedtls_cipher_info_from_type(cipher_type)) != NULL); + TEST_LE_U(mbedtls_cipher_info_get_block_size(cipher_info), + MBEDTLS_CIPHER_BLKSIZE_MAX); memset(buf, 0x2A, sizeof(buf)); TEST_ASSERT((result == mbedtls_cipher_cmac(cipher_info, key, key_size, From 9d1689bbbe38033a27fbacc4109ed8b15b10cc97 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:38:43 +0200 Subject: [PATCH 2/7] Add not-supported test case for ARIA and for other Camellia key sizes Signed-off-by: Gilles Peskine --- tests/suites/test_suite_cmac.data | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_cmac.data b/tests/suites/test_suite_cmac.data index 3ca5e542d0..248a41505f 100644 --- a/tests/suites/test_suite_cmac.data +++ b/tests/suites/test_suite_cmac.data @@ -29,9 +29,29 @@ CMAC init #6 AES-0: bad key size depends_on:MBEDTLS_AES_C mbedtls_cmac_setkey:MBEDTLS_CIPHER_AES_128_ECB:0:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA -CMAC init #7 Camellia: wrong cipher +CMAC init Camellia-128: wrong cipher depends_on:MBEDTLS_CAMELLIA_C -mbedtls_cmac_setkey:MBEDTLS_CIPHER_CAMELLIA_192_ECB:128:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA +mbedtls_cmac_setkey:MBEDTLS_CIPHER_CAMELLIA_128_ECB:128:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +CMAC init Camellia-192: wrong cipher +depends_on:MBEDTLS_CAMELLIA_C +mbedtls_cmac_setkey:MBEDTLS_CIPHER_CAMELLIA_192_ECB:192:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +CMAC init Camellia-256: wrong cipher +depends_on:MBEDTLS_CAMELLIA_C +mbedtls_cmac_setkey:MBEDTLS_CIPHER_CAMELLIA_256_ECB:256:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +CMAC init #8 ARIA-128: wrong cipher +depends_on:MBEDTLS_ARIA_C +mbedtls_cmac_setkey:MBEDTLS_CIPHER_ARIA_128_ECB:128:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +CMAC init #8 ARIA-192: wrong cipher +depends_on:MBEDTLS_ARIA_C +mbedtls_cmac_setkey:MBEDTLS_CIPHER_ARIA_192_ECB:192:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +CMAC init #8 ARIA-256: wrong cipher +depends_on:MBEDTLS_ARIA_C +mbedtls_cmac_setkey:MBEDTLS_CIPHER_ARIA_256_ECB:256:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA CMAC Single Blocks #1 - Empty block, no updates depends_on:MBEDTLS_AES_C From 16bb83cb57f99538d9159711e39f8a4627706ee5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:42:26 +0200 Subject: [PATCH 3/7] Explicitly document that Camellia and ARIA aren't supported In particular, this explains why the definition of MBEDTLS_CIPHER_BLKSIZE_MAX is correct. Signed-off-by: Gilles Peskine --- include/mbedtls/cmac.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/mbedtls/cmac.h b/include/mbedtls/cmac.h index 3125e702e6..bb020800e2 100644 --- a/include/mbedtls/cmac.h +++ b/include/mbedtls/cmac.h @@ -5,6 +5,7 @@ * * The Cipher-based Message Authentication Code (CMAC) Mode for * Authentication is defined in RFC-4493: The AES-CMAC Algorithm. + * It is supported with AES and DES. */ /* * Copyright The Mbed TLS Contributors @@ -38,6 +39,7 @@ extern "C" { #define MBEDTLS_AES_BLOCK_SIZE 16 #define MBEDTLS_DES3_BLOCK_SIZE 8 +/* We don't support Camellia or ARIA in this module */ #if defined(MBEDTLS_AES_C) #define MBEDTLS_CIPHER_BLKSIZE_MAX 16 /**< The longest block used by CMAC is that of AES. */ #else From 7282a9e1a076a06d6216a0c41d6faca56689c2d6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:49:02 +0200 Subject: [PATCH 4/7] Replacement for MBEDTLS_CIPHER_BLKSIZE_MAX Prepare to rename this constant by MBEDTLS_CMAC_MAX_BLOCK_SIZE. The old name was misleading since it looked like it covered all cipher support, not just CMAC support, but CMAC doesn't support Camellia or ARIA so the two are different. This commit introduces the new constant. Subsequent commits will replace internal uses of MBEDTLS_CIPHER_BLKSIZE_MAX and deprecate it. Signed-off-by: Gilles Peskine --- include/mbedtls/cmac.h | 19 +++++++++++++++++-- tests/suites/test_suite_cmac.function | 2 ++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/cmac.h b/include/mbedtls/cmac.h index bb020800e2..5f0a7cde84 100644 --- a/include/mbedtls/cmac.h +++ b/include/mbedtls/cmac.h @@ -41,11 +41,26 @@ extern "C" { /* We don't support Camellia or ARIA in this module */ #if defined(MBEDTLS_AES_C) -#define MBEDTLS_CIPHER_BLKSIZE_MAX 16 /**< The longest block used by CMAC is that of AES. */ +#define MBEDTLS_CMAC_MAX_BLOCK_SIZE 16 /**< The longest block used by CMAC is that of AES. */ #else -#define MBEDTLS_CIPHER_BLKSIZE_MAX 8 /**< The longest block used by CMAC is that of 3DES. */ +#define MBEDTLS_CMAC_MAX_BLOCK_SIZE 8 /**< The longest block used by CMAC is that of 3DES. */ #endif +/** The longest block supported by the cipher module. + * + * \deprecated + * For the maximum block size of a cipher supported by the CMAC module, + * use #MBEDTLS_CMAC_MAX_BLOCK_SIZE. + * For the maximum block size of a cipher supported by the cipher module, + * use #MBEDTLS_MAX_BLOCK_LENGTH. + */ +/* Before Mbed TLS 3.5, this was the maximum block size supported by the CMAC + * module, so it didn't take Camellia or ARIA into account. Since the name + * of the macro doesn't even convey "CMAC", this was misleading. Now the size + * is sufficient for any cipher, but the name is defined in cmac.h for + * backward compatibility. */ +#define MBEDTLS_CIPHER_BLKSIZE_MAX MBEDTLS_MAX_BLOCK_LENGTH + #if !defined(MBEDTLS_CMAC_ALT) /** diff --git a/tests/suites/test_suite_cmac.function b/tests/suites/test_suite_cmac.function index 4597550502..71b5f00667 100644 --- a/tests/suites/test_suite_cmac.function +++ b/tests/suites/test_suite_cmac.function @@ -113,6 +113,8 @@ void mbedtls_cmac_setkey(int cipher_type, int key_size, int result) != NULL); TEST_LE_U(mbedtls_cipher_info_get_block_size(cipher_info), MBEDTLS_CIPHER_BLKSIZE_MAX); + TEST_LE_U(mbedtls_cipher_info_get_block_size(cipher_info), + MBEDTLS_CMAC_MAX_BLOCK_SIZE); memset(buf, 0x2A, sizeof(buf)); TEST_ASSERT((result == mbedtls_cipher_cmac(cipher_info, key, key_size, From 9e930e2887f1945bf9bbecf63be66435679ab144 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:52:54 +0200 Subject: [PATCH 5/7] Rename MBEDTLS_CIPHER_BLKSIZE_MAX internally Replace all occurrences of MBEDTLS_CIPHER_BLKSIZE_MAX by the new name with the same semantics MBEDTLS_CMAC_MAX_BLOCK_SIZE, except when defining or testing the old name. Signed-off-by: Gilles Peskine --- include/mbedtls/cmac.h | 4 ++-- library/cmac.c | 18 +++++++++--------- tests/suites/test_suite_cmac.function | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/cmac.h b/include/mbedtls/cmac.h index 5f0a7cde84..672aeba6e7 100644 --- a/include/mbedtls/cmac.h +++ b/include/mbedtls/cmac.h @@ -68,11 +68,11 @@ extern "C" { */ struct mbedtls_cmac_context_t { /** The internal state of the CMAC algorithm. */ - unsigned char MBEDTLS_PRIVATE(state)[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char MBEDTLS_PRIVATE(state)[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; /** Unprocessed data - either data that was not block aligned and is still * pending processing, or the final block. */ - unsigned char MBEDTLS_PRIVATE(unprocessed_block)[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char MBEDTLS_PRIVATE(unprocessed_block)[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; /** The length of data pending processing. */ size_t MBEDTLS_PRIVATE(unprocessed_len); diff --git a/library/cmac.c b/library/cmac.c index 7d90ad2f5f..e5ac72e189 100644 --- a/library/cmac.c +++ b/library/cmac.c @@ -114,7 +114,7 @@ static int cmac_generate_subkeys(mbedtls_cipher_context_t *ctx, unsigned char *K1, unsigned char *K2) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char L[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char L[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; size_t olen, block_size; mbedtls_platform_zeroize(L, sizeof(L)); @@ -152,7 +152,7 @@ exit: * We can't use the padding option from the cipher layer, as it only works for * CBC and we use ECB mode, and anyway we need to XOR K1 or K2 in addition. */ -static void cmac_pad(unsigned char padded_block[MBEDTLS_CIPHER_BLKSIZE_MAX], +static void cmac_pad(unsigned char padded_block[MBEDTLS_CMAC_MAX_BLOCK_SIZE], size_t padded_block_len, const unsigned char *last_block, size_t last_block_len) @@ -283,9 +283,9 @@ int mbedtls_cipher_cmac_finish(mbedtls_cipher_context_t *ctx, { mbedtls_cmac_context_t *cmac_ctx; unsigned char *state, *last_block; - unsigned char K1[MBEDTLS_CIPHER_BLKSIZE_MAX]; - unsigned char K2[MBEDTLS_CIPHER_BLKSIZE_MAX]; - unsigned char M_last[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char K1[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; + unsigned char K2[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; + unsigned char M_last[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t olen, block_size; @@ -332,7 +332,7 @@ exit: mbedtls_platform_zeroize(cmac_ctx->unprocessed_block, sizeof(cmac_ctx->unprocessed_block)); - mbedtls_platform_zeroize(state, MBEDTLS_CIPHER_BLKSIZE_MAX); + mbedtls_platform_zeroize(state, MBEDTLS_CMAC_MAX_BLOCK_SIZE); return ret; } @@ -746,8 +746,8 @@ static int cmac_test_subkeys(int verbose, int i, ret = 0; mbedtls_cipher_context_t ctx; const mbedtls_cipher_info_t *cipher_info; - unsigned char K1[MBEDTLS_CIPHER_BLKSIZE_MAX]; - unsigned char K2[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char K1[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; + unsigned char K2[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; cipher_info = mbedtls_cipher_info_from_type(cipher_type); if (cipher_info == NULL) { @@ -841,7 +841,7 @@ static int cmac_test_wth_cipher(int verbose, { const mbedtls_cipher_info_t *cipher_info; int i, ret = 0; - unsigned char output[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char output[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; cipher_info = mbedtls_cipher_info_from_type(cipher_type); if (cipher_info == NULL) { diff --git a/tests/suites/test_suite_cmac.function b/tests/suites/test_suite_cmac.function index 71b5f00667..1b6fed0225 100644 --- a/tests/suites/test_suite_cmac.function +++ b/tests/suites/test_suite_cmac.function @@ -20,9 +20,9 @@ void mbedtls_cmac_null_args() { mbedtls_cipher_context_t ctx; const mbedtls_cipher_info_t *cipher_info; - unsigned char test_key[MBEDTLS_CIPHER_BLKSIZE_MAX]; - unsigned char test_data[MBEDTLS_CIPHER_BLKSIZE_MAX]; - unsigned char test_output[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char test_key[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; + unsigned char test_data[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; + unsigned char test_output[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; mbedtls_cipher_init(&ctx); @@ -133,7 +133,7 @@ void mbedtls_cmac_multiple_blocks(int cipher_type, data_t *key, { const mbedtls_cipher_info_t *cipher_info; mbedtls_cipher_context_t ctx; - unsigned char output[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char output[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; /* Convert the test parameters to binary data */ @@ -212,7 +212,7 @@ void mbedtls_cmac_multiple_operations_same_key(int cipher_type, { const mbedtls_cipher_info_t *cipher_info; mbedtls_cipher_context_t ctx; - unsigned char output[MBEDTLS_CIPHER_BLKSIZE_MAX]; + unsigned char output[MBEDTLS_CMAC_MAX_BLOCK_SIZE]; /* Convert the test parameters to binary data */ From c453e2e7e835415608733fc6873c7419defc5b38 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:54:38 +0200 Subject: [PATCH 6/7] Officially deprecate MBEDTLS_CIPHER_BLKSIZE_MAX Signed-off-by: Gilles Peskine --- include/mbedtls/cmac.h | 2 ++ tests/suites/test_suite_cmac.function | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/mbedtls/cmac.h b/include/mbedtls/cmac.h index 672aeba6e7..b2aca5d041 100644 --- a/include/mbedtls/cmac.h +++ b/include/mbedtls/cmac.h @@ -46,6 +46,7 @@ extern "C" { #define MBEDTLS_CMAC_MAX_BLOCK_SIZE 8 /**< The longest block used by CMAC is that of 3DES. */ #endif +#if !defined(MBEDTLS_DEPRECATED_REMOVED) /** The longest block supported by the cipher module. * * \deprecated @@ -60,6 +61,7 @@ extern "C" { * is sufficient for any cipher, but the name is defined in cmac.h for * backward compatibility. */ #define MBEDTLS_CIPHER_BLKSIZE_MAX MBEDTLS_MAX_BLOCK_LENGTH +#endif /* MBEDTLS_DEPRECATED_REMOVED */ #if !defined(MBEDTLS_CMAC_ALT) diff --git a/tests/suites/test_suite_cmac.function b/tests/suites/test_suite_cmac.function index 1b6fed0225..2d7bcd1ab5 100644 --- a/tests/suites/test_suite_cmac.function +++ b/tests/suites/test_suite_cmac.function @@ -111,8 +111,10 @@ void mbedtls_cmac_setkey(int cipher_type, int key_size, int result) TEST_ASSERT((cipher_info = mbedtls_cipher_info_from_type(cipher_type)) != NULL); +#if !defined(MBEDTLS_DEPRECATED_REMOVED) TEST_LE_U(mbedtls_cipher_info_get_block_size(cipher_info), MBEDTLS_CIPHER_BLKSIZE_MAX); +#endif /* MBEDTLS_DEPRECATED_REMOVED */ TEST_LE_U(mbedtls_cipher_info_get_block_size(cipher_info), MBEDTLS_CMAC_MAX_BLOCK_SIZE); From 69661415610509fbd6891c3e809adbc0f981e999 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 14 Jun 2023 17:59:48 +0200 Subject: [PATCH 7/7] Changelog entry for the MBEDTLS_CIPHER_BLKSIZE_MAX deprecation Signed-off-by: Gilles Peskine --- ChangeLog.d/MBEDTLS_CIPHER_BLKSIZE_MAX.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 ChangeLog.d/MBEDTLS_CIPHER_BLKSIZE_MAX.txt diff --git a/ChangeLog.d/MBEDTLS_CIPHER_BLKSIZE_MAX.txt b/ChangeLog.d/MBEDTLS_CIPHER_BLKSIZE_MAX.txt new file mode 100644 index 0000000000..e4e564cdb9 --- /dev/null +++ b/ChangeLog.d/MBEDTLS_CIPHER_BLKSIZE_MAX.txt @@ -0,0 +1,13 @@ +New deprecations + * MBEDTLS_CIPHER_BLKSIZE_MAX is deprecated in favor of + MBEDTLS_MAX_BLOCK_LENGTH (if you intended what the name suggests: + maximum size of any supported block cipher) or the new name + MBEDTLS_CMAC_MAX_BLOCK_SIZE (if you intended the actual semantics: + maximum size of a block cipher supported by the CMAC module). + +Security + * In configurations with ARIA or Camellia but not AES, the value of + MBEDTLS_CIPHER_BLKSIZE_MAX was 8, rather than 16 as the name might + suggest. This did not affect any library code, because this macro was + only used in relation with CMAC which does not support these ciphers. + This may affect application code that uses this macro.