Merge pull request #1031 from gilles-peskine-arm/cmac-blksize_max

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.

include/mbedtls/cmac.h

@@ -5,6 +5,7 @@
  *
  * The Cipher-based Message Authentication Code (CMAC) Mode for
  * Authentication is defined in <em>RFC-4493: The AES-CMAC Algorithm</em>.
+ * It is supported with AES and DES.
  */
 /*
  *  Copyright The Mbed TLS Contributors
@@ -38,12 +39,30 @@ 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. */
+#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
 
+#if !defined(MBEDTLS_DEPRECATED_REMOVED)
+/** 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
+#endif /* MBEDTLS_DEPRECATED_REMOVED */
+
 #if !defined(MBEDTLS_CMAC_ALT)
 
 /**
@@ -51,11 +70,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);

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 K1[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
-    unsigned char K2[MBEDTLS_CIPHER_BLKSIZE_MAX];
+    unsigned char K2[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
-    unsigned char M_last[MBEDTLS_CIPHER_BLKSIZE_MAX];
+    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 K1[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
-    unsigned char K2[MBEDTLS_CIPHER_BLKSIZE_MAX];
+    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) {

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);

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

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_key[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
-    unsigned char test_data[MBEDTLS_CIPHER_BLKSIZE_MAX];
+    unsigned char test_data[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
-    unsigned char test_output[MBEDTLS_CIPHER_BLKSIZE_MAX];
+    unsigned char test_output[MBEDTLS_CMAC_MAX_BLOCK_SIZE];
 
     mbedtls_cipher_init(&ctx);
 
@@ -111,6 +111,12 @@ 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);
 
     memset(buf, 0x2A, sizeof(buf));
     TEST_ASSERT((result == mbedtls_cipher_cmac(cipher_info, key, key_size,
@@ -129,7 +135,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 */
 
@@ -208,7 +214,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 */