From 68e3dff3f1924f308610d32253eb34ccb78ba8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 12 Dec 2018 12:48:04 +0100 Subject: [PATCH 01/15] Add parameter validation XTS setkey functions --- include/mbedtls/aes.h | 4 ++++ library/aes.c | 6 ++++++ tests/suites/test_suite_aes.function | 28 ++++++++++++++++++++-------- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 11edc0fab..197d4db10 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -197,8 +197,10 @@ int mbedtls_aes_setkey_dec( mbedtls_aes_context *ctx, const unsigned char *key, * sets the encryption key. * * \param ctx The AES XTS context to which the key should be bound. + * It must be initialized. * \param key The encryption key. This is comprised of the XTS key1 * concatenated with the XTS key2. + * This must be a readable buffer of size \p keybits bits. * \param keybits The size of \p key passed in bits. Valid options are: * @@ -215,8 +217,10 @@ int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, * sets the decryption key. * * \param ctx The AES XTS context to which the key should be bound. + * It must be initialized. * \param key The decryption key. This is comprised of the XTS key1 * concatenated with the XTS key2. + * This must be a readable buffer of size \p keybits bits. * \param keybits The size of \p key passed in bits. Valid options are: * diff --git a/library/aes.c b/library/aes.c index cc1e5ceb4..4d9a56a5c 100644 --- a/library/aes.c +++ b/library/aes.c @@ -771,6 +771,9 @@ int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, const unsigned char *key1, *key2; unsigned int key1bits, key2bits; + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( key != NULL ); + ret = mbedtls_aes_xts_decode_keys( key, keybits, &key1, &key1bits, &key2, &key2bits ); if( ret != 0 ) @@ -793,6 +796,9 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, const unsigned char *key1, *key2; unsigned int key1bits, key2bits; + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( key != NULL ); + ret = mbedtls_aes_xts_decode_keys( key, keybits, &key1, &key1bits, &key2, &key2bits ); if( ret != 0 ) diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index 131565060..576e5be08 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -215,7 +215,7 @@ void aes_crypt_xts_size( int size, int retval ) void aes_crypt_xts_keysize( int size, int retval ) { mbedtls_aes_xts_context ctx; - const unsigned char *key = NULL; + const unsigned char key[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; size_t key_len = size; mbedtls_aes_xts_init( &ctx ); @@ -374,26 +374,38 @@ exit: /* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void aes_invalid_param( ) { - mbedtls_aes_context dummy_ctx; + mbedtls_aes_context aes_ctx; +#if defined(MBEDTLS_CIPHER_MODE_XTS) + mbedtls_aes_xts_context xts_ctx; +#endif const unsigned char key[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; TEST_INVALID_PARAM( mbedtls_aes_init( NULL ) ); - +#if defined(MBEDTLS_CIPHER_MODE_XTS) TEST_INVALID_PARAM( mbedtls_aes_xts_init( NULL ) ); +#endif - /* mbedtls_aes_setkey_enc() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_setkey_enc( NULL, key, 128 ) ); - TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, - mbedtls_aes_setkey_enc( &dummy_ctx, NULL, 128 ) ); + mbedtls_aes_setkey_enc( &aes_ctx, NULL, 128 ) ); - /* mbedtls_aes_setkey_dec() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_setkey_dec( NULL, key, 128 ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_setkey_dec( &aes_ctx, NULL, 128 ) ); + +#if defined(MBEDTLS_CIPHER_MODE_XTS) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_xts_setkey_enc( NULL, key, 128 ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_xts_setkey_enc( &xts_ctx, NULL, 128 ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, - mbedtls_aes_setkey_dec( &dummy_ctx, NULL, 128 ) ); + mbedtls_aes_xts_setkey_dec( NULL, key, 128 ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_xts_setkey_dec( &xts_ctx, NULL, 128 ) ); +#endif } /* END_CASE */ From 1aca2605713e3242f72297932409dd58ecfb782f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 12 Dec 2018 12:56:55 +0100 Subject: [PATCH 02/15] Add parameter validation for mbedtls_aes_crypt_ecb() --- include/mbedtls/aes.h | 7 +++++-- library/aes.c | 6 ++++++ tests/suites/test_suite_aes.function | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 197d4db10..90d5bba26 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -246,10 +246,13 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, * call to this API with the same context. * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. - * \param input The 16-Byte buffer holding the input data. - * \param output The 16-Byte buffer holding the output data. + * \param input The buffer holding the input data. + * It must be readable and at least 16 Bytes long. + * \param output The buffer where the output data will be written. + * It must be writeable and at least 16 Bytes long. * \return \c 0 on success. */ diff --git a/library/aes.c b/library/aes.c index 4d9a56a5c..9f2074483 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1006,6 +1006,12 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || + mode == MBEDTLS_AES_DECRYPT ); + #if defined(MBEDTLS_AESNI_C) && defined(MBEDTLS_HAVE_X86_64) if( mbedtls_aesni_has_support( MBEDTLS_AESNI_AES ) ) return( mbedtls_aesni_crypt_ecb( ctx, mode, input, output ) ); diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index 576e5be08..0d0e51519 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -379,6 +379,8 @@ void aes_invalid_param( ) mbedtls_aes_xts_context xts_ctx; #endif const unsigned char key[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; + const unsigned char in[16] = { 0 }; + unsigned char out[16]; TEST_INVALID_PARAM( mbedtls_aes_init( NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_XTS) @@ -406,6 +408,20 @@ void aes_invalid_param( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_xts_setkey_dec( &xts_ctx, NULL, 128 ) ); #endif + + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ecb( NULL, + MBEDTLS_AES_ENCRYPT, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ecb( &aes_ctx, + 42, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ecb( &aes_ctx, + MBEDTLS_AES_ENCRYPT, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ecb( &aes_ctx, + MBEDTLS_AES_ENCRYPT, in, NULL ) ); } /* END_CASE */ From 3178d1a997b4a2c7b706f1537005f8e2b54a4fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 12 Dec 2018 13:05:00 +0100 Subject: [PATCH 03/15] Add param validation for mbedtls_aes_crypt_cbc() --- include/mbedtls/aes.h | 6 +++- library/aes.c | 8 ++++++ tests/suites/test_suite_aes.function | 41 +++++++++++++++++++++++++-- tests/suites/test_suite_aes.rest.data | 8 +++--- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 90d5bba26..0f8934f72 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -275,7 +275,7 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * mbedtls_aes_setkey_enc() or mbedtls_aes_setkey_dec() must be called * before the first call to this API with the same context. * - * \note This function operates on aligned blocks, that is, the input size + * \note This function operates on full blocks, that is, the input size * must be a multiple of the AES block size of 16 Bytes. * * \note Upon exit, the content of the IV is updated so that you can @@ -287,13 +287,17 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. * \param length The length of the input data in Bytes. This must be a * multiple of the block size (16 Bytes). * \param iv Initialization vector (updated after use). + * It must be a readable and writeable buffer of 16 Bytes. * \param input The buffer holding the input data. + * It must be readable and of size \p length. * \param output The buffer holding the output data. + * It must be writeable and of size \p length. * * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH diff --git a/library/aes.c b/library/aes.c index 9f2074483..2da86c713 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1049,6 +1049,14 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, int i; unsigned char temp[16]; + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || + mode == MBEDTLS_AES_DECRYPT ); + AES_VALIDATE_RET( iv != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + + if( length % 16 ) return( MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index 0d0e51519..d21a41dd5 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -372,7 +372,7 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ -void aes_invalid_param( ) +void aes_check_params( ) { mbedtls_aes_context aes_ctx; #if defined(MBEDTLS_CIPHER_MODE_XTS) @@ -422,17 +422,54 @@ void aes_invalid_param( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_ecb( &aes_ctx, MBEDTLS_AES_ENCRYPT, in, NULL ) ); + +#if defined(MBEDTLS_CIPHER_MODE_CBC) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cbc( NULL, + MBEDTLS_AES_ENCRYPT, 16, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cbc( &aes_ctx, + 42, 16, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cbc( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cbc( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + out, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cbc( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + out, in, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CBC */ } /* END_CASE */ /* BEGIN_CASE */ -void aes_valid_param( ) +void aes_misc_params( ) { + mbedtls_aes_context aes_ctx; + const unsigned char in[16] = { 0 }; + unsigned char out[16]; + /* These calls accept NULL */ TEST_VALID_PARAM( mbedtls_aes_free( NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_XTS) TEST_VALID_PARAM( mbedtls_aes_xts_free( NULL ) ); #endif + +#if defined(MBEDTLS_CIPHER_MODE_CBC) + TEST_ASSERT( mbedtls_aes_crypt_cbc( &aes_ctx, MBEDTLS_AES_ENCRYPT, + 15, out, in, out ) + == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); + TEST_ASSERT( mbedtls_aes_crypt_cbc( &aes_ctx, MBEDTLS_AES_ENCRYPT, + 17, out, in, out ) + == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); +#endif + } /* END_CASE */ diff --git a/tests/suites/test_suite_aes.rest.data b/tests/suites/test_suite_aes.rest.data index a5d843de4..6a76b43eb 100644 --- a/tests/suites/test_suite_aes.rest.data +++ b/tests/suites/test_suite_aes.rest.data @@ -10,11 +10,11 @@ aes_encrypt_cbc:"000000000000000000000000000000000000000000000000000000000000000 AES-256-CBC Decrypt (Invalid input length) aes_decrypt_cbc:"0000000000000000000000000000000000000000000000000000000000000000":"00000000000000000000000000000000":"623a52fcea5d443e48d9181ab32c74":"":MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH -AES - Invalid parameters -aes_invalid_param: +AES - Optional Parameter Validation (MBEDTLS_CHECK_PARAMS) +aes_check_params: -AES - Valid parameters -aes_valid_param: +AES - Mandatory Parameter Validation and Valid Parameters +aes_misc_params: AES Selftest depends_on:MBEDTLS_SELF_TEST From 191af1313ae192c8f0a9bf5eb3c892eefa42605f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 13 Dec 2018 10:15:30 +0100 Subject: [PATCH 04/15] Add param validation for mbedtls_aes_crypt_xts() --- include/mbedtls/aes.h | 1 + library/aes.c | 6 ++++ tests/suites/test_suite_aes.function | 52 +++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 0f8934f72..1bfa434c0 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -325,6 +325,7 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, * returns #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH. * * \param ctx The AES XTS context to use for AES XTS operations. + * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. * \param length The length of a data unit in bytes. This can be any diff --git a/library/aes.c b/library/aes.c index 2da86c713..c15022b91 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1182,6 +1182,12 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, unsigned char prev_tweak[16]; unsigned char tmp[16]; + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || + mode == MBEDTLS_AES_DECRYPT ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + /* Data units must be at least 16 bytes long. */ if( length < 16 ) return MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH; diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index d21a41dd5..bcffe37b6 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -194,8 +194,8 @@ exit: void aes_crypt_xts_size( int size, int retval ) { mbedtls_aes_xts_context ctx; - const unsigned char *src = NULL; - unsigned char *output = NULL; + const unsigned char src[16] = { 0 }; + unsigned char output[16]; unsigned char data_unit[16]; size_t length = size; @@ -203,10 +203,8 @@ void aes_crypt_xts_size( int size, int retval ) memset( data_unit, 0x00, sizeof( data_unit ) ); - /* Note that this function will most likely crash on failure, as NULL - * parameters will be used. In the passing case, the length check in - * mbedtls_aes_crypt_xts() will prevent any accesses to parameters by - * exiting the function early. */ + /* Valid pointers are passed for builds with MBEDTLS_CHECK_PARAMS, as + * otherwise we wouldn't get to the size check we're interested in. */ TEST_ASSERT( mbedtls_aes_crypt_xts( &ctx, MBEDTLS_AES_ENCRYPT, length, data_unit, src, output ) == retval ); } /* END_CASE */ @@ -445,6 +443,29 @@ void aes_check_params( ) MBEDTLS_AES_ENCRYPT, 16, out, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_CBC */ + +#if defined(MBEDTLS_CIPHER_MODE_XTS) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_xts( NULL, + MBEDTLS_AES_ENCRYPT, 16, + in, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_xts( &xts_ctx, + 42, 16, + in, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_xts( &xts_ctx, + MBEDTLS_AES_ENCRYPT, 16, + NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_xts( &xts_ctx, + MBEDTLS_AES_ENCRYPT, 16, + in, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_xts( &xts_ctx, + MBEDTLS_AES_ENCRYPT, 16, + in, in, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_XTS */ } /* END_CASE */ @@ -452,6 +473,9 @@ void aes_check_params( ) void aes_misc_params( ) { mbedtls_aes_context aes_ctx; +#if defined(MBEDTLS_CIPHER_MODE_XTS) + mbedtls_aes_xts_context xts_ctx; +#endif const unsigned char in[16] = { 0 }; unsigned char out[16]; @@ -463,13 +487,25 @@ void aes_misc_params( ) #if defined(MBEDTLS_CIPHER_MODE_CBC) TEST_ASSERT( mbedtls_aes_crypt_cbc( &aes_ctx, MBEDTLS_AES_ENCRYPT, - 15, out, in, out ) + 15, + out, in, out ) == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); TEST_ASSERT( mbedtls_aes_crypt_cbc( &aes_ctx, MBEDTLS_AES_ENCRYPT, - 17, out, in, out ) + 17, + out, in, out ) == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); #endif +#if defined(MBEDTLS_CIPHER_MODE_XTS) + TEST_ASSERT( mbedtls_aes_crypt_xts( &xts_ctx, MBEDTLS_AES_ENCRYPT, + 15, + in, in, out ) + == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); + TEST_ASSERT( mbedtls_aes_crypt_xts( &xts_ctx, MBEDTLS_AES_ENCRYPT, + (1 << 24) + 1, + in, in, out ) + == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); +#endif } /* END_CASE */ From 1677cca54b51f0c82c6ec24825443ae0fc592a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 13 Dec 2018 10:27:13 +0100 Subject: [PATCH 05/15] Add parameter validation for AES-CFB functions --- include/mbedtls/aes.h | 9 +++++ library/aes.c | 18 +++++++++- tests/suites/test_suite_aes.function | 50 ++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 1bfa434c0..4cc4d143d 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -380,13 +380,18 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, * * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. * \param length The length of the input data. * \param iv_off The offset in IV (updated after use). + * It must point to a valid \c size_t. * \param iv The initialization vector (updated after use). + * It must be a readable and writeable buffer of 16 Bytes. * \param input The buffer holding the input data. + * It must be readable and of size \p length. * \param output The buffer holding the output data. + * It must be writeable and of size \p length. * * \return \c 0 on success. */ @@ -421,12 +426,16 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT * \param length The length of the input data. * \param iv The initialization vector (updated after use). + * It must be a readable and writeable buffer of 16 Bytes. * \param input The buffer holding the input data. + * It must be readable and of size \p length. * \param output The buffer holding the output data. + * It must be writeable and of size \p length. * * \return \c 0 on success. */ diff --git a/library/aes.c b/library/aes.c index c15022b91..b70529011 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1287,7 +1287,17 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, unsigned char *output ) { int c; - size_t n = *iv_off; + size_t n; + + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || + mode == MBEDTLS_AES_DECRYPT ); + AES_VALIDATE_RET( iv_off != NULL ); + AES_VALIDATE_RET( iv != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + + n = *iv_off; if( mode == MBEDTLS_AES_DECRYPT ) { @@ -1334,6 +1344,12 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, unsigned char c; unsigned char ov[17]; + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || + mode == MBEDTLS_AES_DECRYPT ); + AES_VALIDATE_RET( iv != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); while( length-- ) { memcpy( ov, iv, 16 ); diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index bcffe37b6..f581cbe7f 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -379,6 +379,7 @@ void aes_check_params( ) const unsigned char key[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; const unsigned char in[16] = { 0 }; unsigned char out[16]; + size_t size; TEST_INVALID_PARAM( mbedtls_aes_init( NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_XTS) @@ -466,6 +467,55 @@ void aes_check_params( ) MBEDTLS_AES_ENCRYPT, 16, in, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_XTS */ + +#if defined(MBEDTLS_CIPHER_MODE_CFB) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( NULL, + MBEDTLS_AES_ENCRYPT, 16, + &size, out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( &aes_ctx, + 42, 16, + &size, out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + NULL, out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + &size, NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + &size, out, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb128( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + &size, out, in, NULL ) ); + + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb8( NULL, + MBEDTLS_AES_ENCRYPT, 16, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb8( &aes_ctx, + 42, 16, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb8( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb8( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + out, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_cfb8( &aes_ctx, + MBEDTLS_AES_ENCRYPT, 16, + out, in, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CFB */ } /* END_CASE */ From 8e41eb718727b7bdfcd8fb96afa412d98a9267ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 13 Dec 2018 11:00:56 +0100 Subject: [PATCH 06/15] Add parameter validation for AES-OFB --- include/mbedtls/aes.h | 5 +++++ library/aes.c | 10 +++++++++- tests/suites/test_suite_aes.function | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 4cc4d143d..93522e6ab 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -480,11 +480,16 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, * will compromise security. * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param length The length of the input data. * \param iv_off The offset in IV (updated after use). + * It must point to a valid \c size_t. * \param iv The initialization vector (updated after use). + * It must be a readable and writeable buffer of 16 Bytes. * \param input The buffer holding the input data. + * It must be readable and of size \p length. * \param output The buffer holding the output data. + * It must be writeable and of size \p length. * * \return \c 0 on success. */ diff --git a/library/aes.c b/library/aes.c index b70529011..52fc74c47 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1382,7 +1382,15 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, unsigned char *output ) { int ret = 0; - size_t n = *iv_off; + size_t n; + + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( iv_off != NULL ); + AES_VALIDATE_RET( iv != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + + n = *iv_off; while( length-- ) { diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index f581cbe7f..d585ffbc8 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -516,6 +516,24 @@ void aes_check_params( ) MBEDTLS_AES_ENCRYPT, 16, out, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_CFB */ + +#if defined(MBEDTLS_CIPHER_MODE_OFB) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ofb( NULL, 16, + &size, out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ofb( &aes_ctx, 16, + NULL, out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ofb( &aes_ctx, 16, + &size, NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ofb( &aes_ctx, 16, + &size, out, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ofb( &aes_ctx, 16, + &size, out, in, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_OFB */ } /* END_CASE */ From 2bc535be86e4833c3932457317a207df732cb8e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 13 Dec 2018 11:08:36 +0100 Subject: [PATCH 07/15] Add parameter validation for AES-CTR --- include/mbedtls/aes.h | 6 ++++++ library/aes.c | 11 ++++++++++- tests/suites/test_suite_aes.function | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 93522e6ab..ae80e9df2 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -561,15 +561,21 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, * securely discarded as soon as it's no longer needed. * * \param ctx The AES context to use for encryption or decryption. + * It must be initialized and bound to a key. * \param length The length of the input data. * \param nc_off The offset in the current \p stream_block, for * resuming within the current cipher stream. The * offset pointer should be 0 at the start of a stream. + * It must point to a valid \c size_t. * \param nonce_counter The 128-bit nonce and counter. + * It must be a readable-writeable buffer of 16 Bytes. * \param stream_block The saved stream block for resuming. This is * overwritten by the function. + * It must be a readable-writeable buffer of 16 Bytes. * \param input The buffer holding the input data. + * It must be readable and of size \p length. * \param output The buffer holding the output data. + * It must be writeable and of size \p length. * * \return \c 0 on success. */ diff --git a/library/aes.c b/library/aes.c index 52fc74c47..818c5991b 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1425,7 +1425,16 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, unsigned char *output ) { int c, i; - size_t n = *nc_off; + size_t n; + + AES_VALIDATE_RET( ctx != NULL ); + AES_VALIDATE_RET( nc_off != NULL ); + AES_VALIDATE_RET( nonce_counter != NULL ); + AES_VALIDATE_RET( stream_block != NULL ); + AES_VALIDATE_RET( input != NULL ); + AES_VALIDATE_RET( output != NULL ); + + n = *nc_off; if ( n > 0x0F ) return( MBEDTLS_ERR_AES_BAD_INPUT_DATA ); diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index d585ffbc8..07040e590 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -534,6 +534,27 @@ void aes_check_params( ) mbedtls_aes_crypt_ofb( &aes_ctx, 16, &size, out, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_OFB */ + +#if defined(MBEDTLS_CIPHER_MODE_CTR) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( NULL, 16, &size, out, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( &aes_ctx, 16, NULL, out, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( &aes_ctx, 16, &size, NULL, + out, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( &aes_ctx, 16, &size, out, + NULL, in, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( &aes_ctx, 16, &size, out, + out, NULL, out ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, + mbedtls_aes_crypt_ctr( &aes_ctx, 16, &size, out, + out, in, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CTR */ } /* END_CASE */ From ad54c49e750ddfa8f75059cc9fc06018d3670582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 13 Dec 2018 11:15:26 +0100 Subject: [PATCH 08/15] Document AES accelerator functions as internal --- include/mbedtls/aes.h | 4 ++-- include/mbedtls/aesni.h | 46 ++++++++++++++++++++++++++++----------- include/mbedtls/padlock.h | 34 +++++++++++++++++++---------- 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index ae80e9df2..d21427e7d 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -628,7 +628,7 @@ int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, * \brief Deprecated internal AES block encryption function * without return value. * - * \deprecated Superseded by mbedtls_aes_encrypt_ext() in 2.5.0. + * \deprecated Superseded by mbedtls_internal_aes_encrypt() in 2.5.0. * * \param ctx The AES context to use for encryption. * \param input Plaintext block. @@ -642,7 +642,7 @@ MBEDTLS_DEPRECATED void mbedtls_aes_encrypt( mbedtls_aes_context *ctx, * \brief Deprecated internal AES block decryption function * without return value. * - * \deprecated Superseded by mbedtls_aes_decrypt_ext() in 2.5.0. + * \deprecated Superseded by mbedtls_internal_aes_decrypt() in 2.5.0. * * \param ctx The AES context to use for decryption. * \param input Ciphertext block. diff --git a/include/mbedtls/aesni.h b/include/mbedtls/aesni.h index 746baa0e1..b490cbebf 100644 --- a/include/mbedtls/aesni.h +++ b/include/mbedtls/aesni.h @@ -2,6 +2,9 @@ * \file aesni.h * * \brief AES-NI for hardware AES acceleration on some Intel processors + * + * \warning These functions are only for internal use by other library + * functions; you must not call them directly. */ /* * Copyright (C) 2006-2015, ARM Limited, All Rights Reserved @@ -42,7 +45,10 @@ extern "C" { #endif /** - * \brief AES-NI features detection routine + * \brief Internal AES-NI features detection routine + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param what The feature to detect * (MBEDTLS_AESNI_AES or MBEDTLS_AESNI_CLMUL) @@ -52,7 +58,10 @@ extern "C" { int mbedtls_aesni_has_support( unsigned int what ); /** - * \brief AES-NI AES-ECB block en(de)cryption + * \brief Internal AES-NI AES-ECB block en(de)cryption + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param ctx AES context * \param mode MBEDTLS_AES_ENCRYPT or MBEDTLS_AES_DECRYPT @@ -62,12 +71,15 @@ int mbedtls_aesni_has_support( unsigned int what ); * \return 0 on success (cannot fail) */ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, - int mode, - const unsigned char input[16], - unsigned char output[16] ); + int mode, + const unsigned char input[16], + unsigned char output[16] ); /** - * \brief GCM multiplication: c = a * b in GF(2^128) + * \brief Internal GCM multiplication: c = a * b in GF(2^128) + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param c Result * \param a First operand @@ -77,21 +89,29 @@ int mbedtls_aesni_crypt_ecb( mbedtls_aes_context *ctx, * elements of GF(2^128) as per the GCM spec. */ void mbedtls_aesni_gcm_mult( unsigned char c[16], - const unsigned char a[16], - const unsigned char b[16] ); + const unsigned char a[16], + const unsigned char b[16] ); /** - * \brief Compute decryption round keys from encryption round keys + * \brief Internal round key inversion. + * Compute decryption round keys from encryption round keys + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param invkey Round keys for the equivalent inverse cipher * \param fwdkey Original round keys (for encryption) * \param nr Number of rounds (that is, number of round keys minus one) */ void mbedtls_aesni_inverse_key( unsigned char *invkey, - const unsigned char *fwdkey, int nr ); + const unsigned char *fwdkey, + int nr ); /** - * \brief Perform key expansion (for encryption) + * \brief Internal key expansion (for encryption) + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param rk Destination buffer where the round keys are written * \param key Encryption key @@ -100,8 +120,8 @@ void mbedtls_aesni_inverse_key( unsigned char *invkey, * \return 0 if successful, or MBEDTLS_ERR_AES_INVALID_KEY_LENGTH */ int mbedtls_aesni_setkey_enc( unsigned char *rk, - const unsigned char *key, - size_t bits ); + const unsigned char *key, + size_t bits ); #ifdef __cplusplus } diff --git a/include/mbedtls/padlock.h b/include/mbedtls/padlock.h index 677936ebf..95e2bfce1 100644 --- a/include/mbedtls/padlock.h +++ b/include/mbedtls/padlock.h @@ -3,6 +3,9 @@ * * \brief VIA PadLock ACE for HW encryption/decryption supported by some * processors + * + * \warning These functions are only for internal use by other library + * functions; you must not call them directly. */ /* * Copyright (C) 2006-2015, ARM Limited, All Rights Reserved @@ -57,7 +60,10 @@ extern "C" { #endif /** - * \brief PadLock detection routine + * \brief Internal PadLock detection routine + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param feature The feature to detect * @@ -66,7 +72,10 @@ extern "C" { int mbedtls_padlock_has_support( int feature ); /** - * \brief PadLock AES-ECB block en(de)cryption + * \brief Internal PadLock AES-ECB block en(de)cryption + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param ctx AES context * \param mode MBEDTLS_AES_ENCRYPT or MBEDTLS_AES_DECRYPT @@ -76,12 +85,15 @@ int mbedtls_padlock_has_support( int feature ); * \return 0 if success, 1 if operation failed */ int mbedtls_padlock_xcryptecb( mbedtls_aes_context *ctx, - int mode, - const unsigned char input[16], - unsigned char output[16] ); + int mode, + const unsigned char input[16], + unsigned char output[16] ); /** - * \brief PadLock AES-CBC buffer en(de)cryption + * \brief Internal PadLock AES-CBC buffer en(de)cryption + * + * \note This function is only for internal use by other library + * functions; you must not call it directly. * * \param ctx AES context * \param mode MBEDTLS_AES_ENCRYPT or MBEDTLS_AES_DECRYPT @@ -93,11 +105,11 @@ int mbedtls_padlock_xcryptecb( mbedtls_aes_context *ctx, * \return 0 if success, 1 if operation failed */ int mbedtls_padlock_xcryptcbc( mbedtls_aes_context *ctx, - int mode, - size_t length, - unsigned char iv[16], - const unsigned char *input, - unsigned char *output ); + int mode, + size_t length, + unsigned char iv[16], + const unsigned char *input, + unsigned char *output ); #ifdef __cplusplus } From b66e7dbcc1063c7b9df4ebed4e309662b7cac3d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 09:57:18 +0100 Subject: [PATCH 09/15] Fix some documentation markup/wording issues --- include/mbedtls/aes.h | 54 +++++++++++++++++++-------------------- include/mbedtls/aesni.h | 14 +++++----- include/mbedtls/padlock.h | 4 +-- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index d21427e7d..b42e564ef 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -250,9 +250,9 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. * \param input The buffer holding the input data. - * It must be readable and at least 16 Bytes long. + * It must be readable and at least \c 16 Bytes long. * \param output The buffer where the output data will be written. - * It must be writeable and at least 16 Bytes long. + * It must be writeable and at least \c 16 Bytes long. * \return \c 0 on success. */ @@ -276,7 +276,7 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * before the first call to this API with the same context. * * \note This function operates on full blocks, that is, the input size - * must be a multiple of the AES block size of 16 Bytes. + * must be a multiple of the AES block size of \c 16 Bytes. * * \note Upon exit, the content of the IV is updated so that you can * call the same function again on the next @@ -291,13 +291,13 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. * \param length The length of the input data in Bytes. This must be a - * multiple of the block size (16 Bytes). + * multiple of the block size (\c 16 Bytes). * \param iv Initialization vector (updated after use). - * It must be a readable and writeable buffer of 16 Bytes. + * It must be a readable and writeable buffer of \c 16 Bytes. * \param input The buffer holding the input data. - * It must be readable and of size \p length. + * It must be readable and of size \p length Bytes. * \param output The buffer holding the output data. - * It must be writeable and of size \p length. + * It must be writeable and of size \p length Bytes. * * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH @@ -328,7 +328,7 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. - * \param length The length of a data unit in bytes. This can be any + * \param length The length of a data unit in Bytes. This can be any * length between 16 bytes and 2^24 bytes inclusive * (between 1 and 2^20 block cipher blocks). * \param data_unit The address of the data unit encoded as an array of 16 @@ -336,15 +336,15 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, * is typically the index of the block device sector that * contains the data. * \param input The buffer holding the input data (which is an entire - * data unit). This function reads \p length bytes from \p + * data unit). This function reads \p length Bytes from \p * input. * \param output The buffer holding the output data (which is an entire - * data unit). This function writes \p length bytes to \p + * data unit). This function writes \p length Bytes to \p * output. * * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH if \p length is - * smaller than an AES block in size (16 bytes) or if \p + * smaller than an AES block in size (16 Bytes) or if \p * length is larger than 2^20 blocks (16 MiB). */ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, @@ -383,15 +383,15 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, * It must be initialized and bound to a key. * \param mode The AES operation: #MBEDTLS_AES_ENCRYPT or * #MBEDTLS_AES_DECRYPT. - * \param length The length of the input data. + * \param length The length of the input data in Bytes. * \param iv_off The offset in IV (updated after use). * It must point to a valid \c size_t. * \param iv The initialization vector (updated after use). - * It must be a readable and writeable buffer of 16 Bytes. + * It must be a readable and writeable buffer of \c 16 Bytes. * \param input The buffer holding the input data. - * It must be readable and of size \p length. + * It must be readable and of size \p length Bytes. * \param output The buffer holding the output data. - * It must be writeable and of size \p length. + * It must be writeable and of size \p length Bytes. * * \return \c 0 on success. */ @@ -431,11 +431,11 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * #MBEDTLS_AES_DECRYPT * \param length The length of the input data. * \param iv The initialization vector (updated after use). - * It must be a readable and writeable buffer of 16 Bytes. + * It must be a readable and writeable buffer of \c 16 Bytes. * \param input The buffer holding the input data. - * It must be readable and of size \p length. + * It must be readable and of size \p length Bytes. * \param output The buffer holding the output data. - * It must be writeable and of size \p length. + * It must be writeable and of size \p length Bytes. * * \return \c 0 on success. */ @@ -485,11 +485,11 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, * \param iv_off The offset in IV (updated after use). * It must point to a valid \c size_t. * \param iv The initialization vector (updated after use). - * It must be a readable and writeable buffer of 16 Bytes. + * It must be a readable and writeable buffer of \c 16 Bytes. * \param input The buffer holding the input data. - * It must be readable and of size \p length. + * It must be readable and of size \p length Bytes. * \param output The buffer holding the output data. - * It must be writeable and of size \p length. + * It must be writeable and of size \p length Bytes. * * \return \c 0 on success. */ @@ -568,14 +568,14 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, * offset pointer should be 0 at the start of a stream. * It must point to a valid \c size_t. * \param nonce_counter The 128-bit nonce and counter. - * It must be a readable-writeable buffer of 16 Bytes. + * It must be a readable-writeable buffer of \c 16 Bytes. * \param stream_block The saved stream block for resuming. This is * overwritten by the function. - * It must be a readable-writeable buffer of 16 Bytes. + * It must be a readable-writeable buffer of \c 16 Bytes. * \param input The buffer holding the input data. - * It must be readable and of size \p length. + * It must be readable and of size \p length Bytes. * \param output The buffer holding the output data. - * It must be writeable and of size \p length. + * It must be writeable and of size \p length Bytes. * * \return \c 0 on success. */ @@ -628,7 +628,7 @@ int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, * \brief Deprecated internal AES block encryption function * without return value. * - * \deprecated Superseded by mbedtls_internal_aes_encrypt() in 2.5.0. + * \deprecated Superseded by mbedtls_internal_aes_encrypt() * * \param ctx The AES context to use for encryption. * \param input Plaintext block. @@ -642,7 +642,7 @@ MBEDTLS_DEPRECATED void mbedtls_aes_encrypt( mbedtls_aes_context *ctx, * \brief Deprecated internal AES block decryption function * without return value. * - * \deprecated Superseded by mbedtls_internal_aes_decrypt() in 2.5.0. + * \deprecated Superseded by mbedtls_internal_aes_decrypt() * * \param ctx The AES context to use for decryption. * \param input Ciphertext block. diff --git a/include/mbedtls/aesni.h b/include/mbedtls/aesni.h index b490cbebf..0196f49b8 100644 --- a/include/mbedtls/aesni.h +++ b/include/mbedtls/aesni.h @@ -3,8 +3,8 @@ * * \brief AES-NI for hardware AES acceleration on some Intel processors * - * \warning These functions are only for internal use by other library - * functions; you must not call them directly. + * \warning These functions are only for internal use by other library + * functions; you must not call them directly. */ /* * Copyright (C) 2006-2015, ARM Limited, All Rights Reserved @@ -45,7 +45,7 @@ extern "C" { #endif /** - * \brief Internal AES-NI features detection routine + * \brief Internal function to detect the AES-NI feature in CPUs. * * \note This function is only for internal use by other library * functions; you must not call it directly. @@ -58,7 +58,7 @@ extern "C" { int mbedtls_aesni_has_support( unsigned int what ); /** - * \brief Internal AES-NI AES-ECB block en(de)cryption + * \brief Internal AES-NI AES-ECB block encryption and decryption * * \note This function is only for internal use by other library * functions; you must not call it directly. @@ -93,8 +93,8 @@ void mbedtls_aesni_gcm_mult( unsigned char c[16], const unsigned char b[16] ); /** - * \brief Internal round key inversion. - * Compute decryption round keys from encryption round keys + * \brief Internal round key inversion. This function computes + * decryption round keys from the encryption round keys. * * \note This function is only for internal use by other library * functions; you must not call it directly. @@ -108,7 +108,7 @@ void mbedtls_aesni_inverse_key( unsigned char *invkey, int nr ); /** - * \brief Internal key expansion (for encryption) + * \brief Internal key expansion for encryption * * \note This function is only for internal use by other library * functions; you must not call it directly. diff --git a/include/mbedtls/padlock.h b/include/mbedtls/padlock.h index 95e2bfce1..7a5d083a9 100644 --- a/include/mbedtls/padlock.h +++ b/include/mbedtls/padlock.h @@ -4,8 +4,8 @@ * \brief VIA PadLock ACE for HW encryption/decryption supported by some * processors * - * \warning These functions are only for internal use by other library - * functions; you must not call them directly. + * \warning These functions are only for internal use by other library + * functions; you must not call them directly. */ /* * Copyright (C) 2006-2015, ARM Limited, All Rights Reserved From ab6b9758d6b1431e4adc4bc8852540c5515e72af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 09:58:18 +0100 Subject: [PATCH 10/15] Improve constant naming in test functions --- tests/suites/test_suite_aes.function | 52 +++++++++++++++------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index 07040e590..3762ba4ec 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -380,6 +380,8 @@ void aes_check_params( ) const unsigned char in[16] = { 0 }; unsigned char out[16]; size_t size; + const int valid_mode = MBEDTLS_AES_ENCRYPT; + const int invalid_mode = 42; TEST_INVALID_PARAM( mbedtls_aes_init( NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_XTS) @@ -411,109 +413,109 @@ void aes_check_params( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_ecb( NULL, - MBEDTLS_AES_ENCRYPT, in, out ) ); + valid_mode, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_ecb( &aes_ctx, - 42, in, out ) ); + invalid_mode, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_ecb( &aes_ctx, - MBEDTLS_AES_ENCRYPT, NULL, out ) ); + valid_mode, NULL, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_ecb( &aes_ctx, - MBEDTLS_AES_ENCRYPT, in, NULL ) ); + valid_mode, in, NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_CBC) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cbc( NULL, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cbc( &aes_ctx, - 42, 16, + invalid_mode, 16, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cbc( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, NULL, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cbc( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, NULL, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cbc( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_CBC */ #if defined(MBEDTLS_CIPHER_MODE_XTS) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_xts( NULL, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, in, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_xts( &xts_ctx, - 42, 16, + invalid_mode, 16, in, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_xts( &xts_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, NULL, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_xts( &xts_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, in, NULL, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_xts( &xts_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, in, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_XTS */ #if defined(MBEDTLS_CIPHER_MODE_CFB) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( NULL, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, &size, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( &aes_ctx, - 42, 16, + invalid_mode, 16, &size, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, NULL, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, &size, NULL, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, &size, out, NULL, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb128( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, &size, out, in, NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb8( NULL, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb8( &aes_ctx, - 42, 16, + invalid_mode, 16, out, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb8( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, NULL, in, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb8( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, NULL, out ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_AES_BAD_INPUT_DATA, mbedtls_aes_crypt_cfb8( &aes_ctx, - MBEDTLS_AES_ENCRYPT, 16, + valid_mode, 16, out, in, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_CFB */ From eb6d3968b16210954c772510c6eafefeb381ea5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 09:59:35 +0100 Subject: [PATCH 11/15] Fix some whitespace issues in aes.c --- library/aes.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/library/aes.c b/library/aes.c index 818c5991b..e48a2a6ec 100644 --- a/library/aes.c +++ b/library/aes.c @@ -575,7 +575,6 @@ int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, { aes_gen_tables(); aes_init_done = 1; - } #endif @@ -1002,9 +1001,9 @@ void mbedtls_aes_decrypt( mbedtls_aes_context *ctx, * AES-ECB block encryption/decryption */ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, - int mode, - const unsigned char input[16], - unsigned char output[16] ) + int mode, + const unsigned char input[16], + unsigned char output[16] ) { AES_VALIDATE_RET( ctx != NULL ); AES_VALIDATE_RET( input != NULL ); @@ -1056,7 +1055,6 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, AES_VALIDATE_RET( input != NULL ); AES_VALIDATE_RET( output != NULL ); - if( length % 16 ) return( MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); @@ -1335,11 +1333,11 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * AES-CFB8 buffer encryption/decryption */ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, - int mode, - size_t length, - unsigned char iv[16], - const unsigned char *input, - unsigned char *output ) + int mode, + size_t length, + unsigned char iv[16], + const unsigned char *input, + unsigned char *output ) { unsigned char c; unsigned char ov[17]; From 998a358529faac5b6bfded2d79590bdf2d610579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 10:03:13 +0100 Subject: [PATCH 12/15] Make a check more explicit in aes.c The check was already done later when calling ECB, (as evidenced by the tests passing, which have a call with data_unit set to NULL), but it's more readable to have it here too, and more helpful when debugging. --- library/aes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/aes.c b/library/aes.c index e48a2a6ec..f6dc9963e 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1183,6 +1183,7 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, AES_VALIDATE_RET( ctx != NULL ); AES_VALIDATE_RET( mode == MBEDTLS_AES_ENCRYPT || mode == MBEDTLS_AES_DECRYPT ); + AES_VALIDATE_RET( data_unit != NULL ); AES_VALIDATE_RET( input != NULL ); AES_VALIDATE_RET( output != NULL ); From 5b89c0927366631597ab9a48f430ffd6c603846a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 10:03:30 +0100 Subject: [PATCH 13/15] Add check for iv_off in AES-CFB128 and AES-OFB The check is mandatory as skipping it results in buffer overread of arbitrary size. --- library/aes.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/aes.c b/library/aes.c index f6dc9963e..1c743f95d 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1298,6 +1298,9 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, n = *iv_off; + if( n > 16 ) + return( MBEDTLS_ERR_AES_BAD_INPUT_DATA ); + if( mode == MBEDTLS_AES_DECRYPT ) { while( length-- ) @@ -1391,6 +1394,9 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, n = *iv_off; + if( n > 16 ) + return( MBEDTLS_ERR_AES_BAD_INPUT_DATA ); + while( length-- ) { if( n == 0 ) From e55e103bfe31855aa2a35212f4c670371b384b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 12:09:02 +0100 Subject: [PATCH 14/15] Fix off-by-one in iv_off check and add tests --- library/aes.c | 4 ++-- tests/suites/test_suite_aes.function | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/library/aes.c b/library/aes.c index 1c743f95d..0543cd781 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1298,7 +1298,7 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, n = *iv_off; - if( n > 16 ) + if( n > 15 ) return( MBEDTLS_ERR_AES_BAD_INPUT_DATA ); if( mode == MBEDTLS_AES_DECRYPT ) @@ -1394,7 +1394,7 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, n = *iv_off; - if( n > 16 ) + if( n > 15 ) return( MBEDTLS_ERR_AES_BAD_INPUT_DATA ); while( length-- ) diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index 3762ba4ec..f74183d8e 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -569,6 +569,7 @@ void aes_misc_params( ) #endif const unsigned char in[16] = { 0 }; unsigned char out[16]; + size_t size; /* These calls accept NULL */ TEST_VALID_PARAM( mbedtls_aes_free( NULL ) ); @@ -597,6 +598,19 @@ void aes_misc_params( ) in, in, out ) == MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH ); #endif + +#if defined(MBEDTLS_CIPHER_MODE_CFB) + size = 16; + TEST_ASSERT( mbedtls_aes_crypt_cfb128( &aes_ctx, MBEDTLS_AES_ENCRYPT, 16, + &size, out, in, out ) + == MBEDTLS_ERR_AES_BAD_INPUT_DATA ); +#endif + +#if defined(MBEDTLS_CIPHER_MODE_OFB) + size = 16; + TEST_ASSERT( mbedtls_aes_crypt_ofb( &aes_ctx, 16, &size, out, in, out ) + == MBEDTLS_ERR_AES_BAD_INPUT_DATA ); +#endif } /* END_CASE */ From 488d9309fc4f74f9bbdd666cc9bf89c8c9d7c8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 18 Dec 2018 13:05:49 +0100 Subject: [PATCH 15/15] Fix unused param warnings in test function --- tests/suites/test_suite_aes.function | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index f74183d8e..da8c1e935 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -563,13 +563,21 @@ void aes_check_params( ) /* BEGIN_CASE */ void aes_misc_params( ) { +#if defined(MBEDTLS_CIPHER_MODE_CBC) || \ + defined(MBEDTLS_CIPHER_MODE_XTS) || \ + defined(MBEDTLS_CIPHER_MODE_CFB) || \ + defined(MBEDTLS_CIPHER_MODE_OFB) mbedtls_aes_context aes_ctx; + const unsigned char in[16] = { 0 }; + unsigned char out[16]; +#endif #if defined(MBEDTLS_CIPHER_MODE_XTS) mbedtls_aes_xts_context xts_ctx; #endif - const unsigned char in[16] = { 0 }; - unsigned char out[16]; +#if defined(MBEDTLS_CIPHER_MODE_CFB) || \ + defined(MBEDTLS_CIPHER_MODE_OFB) size_t size; +#endif /* These calls accept NULL */ TEST_VALID_PARAM( mbedtls_aes_free( NULL ) );