From 139d8313d904a27d7d1d60cbb32faf1b487ee954 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 21:29:27 +0000 Subject: [PATCH 01/10] Document parameter preconditions for the ARIA module --- include/mbedtls/aria.h | 51 +++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/aria.h b/include/mbedtls/aria.h index c80c9fd26..54db1a637 100644 --- a/include/mbedtls/aria.h +++ b/include/mbedtls/aria.h @@ -91,14 +91,15 @@ mbedtls_aria_context; * It must be the first API called before using * the context. * - * \param ctx The ARIA context to initialize. + * \param ctx The ARIA context to initialize. Must not be \c NULL. */ void mbedtls_aria_init( mbedtls_aria_context *ctx ); /** * \brief This function releases and clears the specified ARIA context. * - * \param ctx The ARIA context to clear. + * \param ctx The ARIA context to clear. May be \c NULL, in which + * case this function is a no-op. */ void mbedtls_aria_free( mbedtls_aria_context *ctx ); @@ -106,14 +107,16 @@ void mbedtls_aria_free( mbedtls_aria_context *ctx ); * \brief This function sets the encryption key. * * \param ctx The ARIA context to which the key should be bound. - * \param key The encryption key. + * Must be initialized. + * \param key The encryption key. Must be a readable buffer + * of size \p keybits bits. * \param keybits The size of data passed in bits. Valid options are: * * - * \return \c 0 on success or #MBEDTLS_ERR_ARIA_BAD_INPUT_DATA - * on failure. + * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_setkey_enc( mbedtls_aria_context *ctx, const unsigned char *key, @@ -123,13 +126,16 @@ int mbedtls_aria_setkey_enc( mbedtls_aria_context *ctx, * \brief This function sets the decryption key. * * \param ctx The ARIA context to which the key should be bound. - * \param key The decryption key. + * Must be initialized. + * \param key The decryption key. Must be a readable buffer + * of size \p keybits bits. * \param keybits The size of data passed. Valid options are: * * - * \return \c 0 on success, or #MBEDTLS_ERR_ARIA_BAD_INPUT_DATA on failure. + * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_setkey_dec( mbedtls_aria_context *ctx, const unsigned char *key, @@ -148,10 +154,12 @@ int mbedtls_aria_setkey_dec( mbedtls_aria_context *ctx, * call to this API with the same context. * * \param ctx The ARIA context to use for encryption or decryption. + * Must be initialized. * \param input The 16-Byte buffer holding the input data. * \param output The 16-Byte buffer holding the output data. * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, const unsigned char input[MBEDTLS_ARIA_BLOCKSIZE], @@ -183,16 +191,20 @@ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, * * * \param ctx The ARIA context to use for encryption or decryption. + * Must be initialized. * \param mode The ARIA operation: #MBEDTLS_ARIA_ENCRYPT or * #MBEDTLS_ARIA_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). + * Must be a readable buffer of size 16 Bytes. * \param input The buffer holding the input data. + * May be \c NULL if `length == 0`. * \param output The buffer holding the output data. + * May be \c NULL if `length == 0`. * - * \return \c 0 on success, or #MBEDTLS_ERR_ARIA_INVALID_INPUT_LENGTH - * on failure. + * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_crypt_cbc( mbedtls_aria_context *ctx, int mode, @@ -227,15 +239,21 @@ int mbedtls_aria_crypt_cbc( mbedtls_aria_context *ctx, * * * \param ctx The ARIA context to use for encryption or decryption. + * Must be initialized. * \param mode The ARIA operation: #MBEDTLS_ARIA_ENCRYPT or * #MBEDTLS_ARIA_DECRYPT. * \param length The length of the input data. * \param iv_off The offset in IV (updated after use). + * Must not be larger than 15. * \param iv The initialization vector (updated after use). + * Must be a readable buffer of size 16 Bytes. * \param input The buffer holding the input data. + * May be \c NULL if `length == 0`. * \param output The buffer holding the output data. + * May be \c NULL if `length == 0`. * * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, int mode, @@ -305,17 +323,24 @@ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, * securely discarded as soon as it's no longer needed. * * \param ctx The ARIA context to use for encryption or decryption. + * Must be initialized. * \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. - * \param nonce_counter The 128-bit nonce and counter. - * \param stream_block The saved stream block for resuming. This is - * overwritten by the function. + * Must not be larger than 15. + * \param nonce_counter The 128-bit nonce and counter. Must point to + * an RW-buffer of length 16 bytes. + * \param stream_block The saved stream block for resuming. Must point to + * an RW-buffer of length 16 bytes. + * This is overwritten by the function. * \param input The buffer holding the input data. + * May be \c NULL if `length == 0`. * \param output The buffer holding the output data. + * May be \c NULL if `length == 0`. * - * \return \c 0 on success. + * \return \c 0 on success. + * \return A negative error code on failure. */ int mbedtls_aria_crypt_ctr( mbedtls_aria_context *ctx, size_t length, From b54ae0bc0d16f349f3f8fec15b0e3168c2abdede Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 21:51:32 +0000 Subject: [PATCH 02/10] Implement parameter validation for ARIA module --- library/aria.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/library/aria.c b/library/aria.c index 6d87941c1..aff66d667 100644 --- a/library/aria.c +++ b/library/aria.c @@ -55,6 +55,12 @@ #define inline __inline #endif +/* Parameter validation macros */ +#define ARIA_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_ARIA_BAD_INPUT_DATA ) +#define ARIA_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + /* * 32-bit integer manipulation macros (little endian) */ @@ -449,6 +455,8 @@ int mbedtls_aria_setkey_enc( mbedtls_aria_context *ctx, int i; uint32_t w[4][4], *w2; + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( key != NULL ); if( keybits != 128 && keybits != 192 && keybits != 256 ) return( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA ); @@ -503,6 +511,8 @@ int mbedtls_aria_setkey_dec( mbedtls_aria_context *ctx, const unsigned char *key, unsigned int keybits ) { int i, j, k, ret; + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( key != NULL ); ret = mbedtls_aria_setkey_enc( ctx, key, keybits ); if( ret != 0 ) @@ -539,6 +549,9 @@ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, int i; uint32_t a, b, c, d; + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( input != NULL ); + ARIA_VALIDATE_RET( output != NULL ); GET_UINT32_LE( a, input, 0 ); GET_UINT32_LE( b, input, 4 ); @@ -586,6 +599,7 @@ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, /* Initialize context */ void mbedtls_aria_init( mbedtls_aria_context *ctx ) { + ARIA_VALIDATE( ctx != NULL ); memset( ctx, 0, sizeof( mbedtls_aria_context ) ); } @@ -612,6 +626,13 @@ int mbedtls_aria_crypt_cbc( mbedtls_aria_context *ctx, int i; unsigned char temp[MBEDTLS_ARIA_BLOCKSIZE]; + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( mode == MBEDTLS_ARIA_ENCRYPT || + mode == MBEDTLS_ARIA_DECRYPT ); + ARIA_VALIDATE_RET( length == 0 || input != NULL ); + ARIA_VALIDATE_RET( length == 0 || output != NULL ); + ARIA_VALIDATE_RET( iv != NULL ); + if( length % MBEDTLS_ARIA_BLOCKSIZE ) return( MBEDTLS_ERR_ARIA_INVALID_INPUT_LENGTH ); @@ -665,7 +686,23 @@ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, unsigned char *output ) { unsigned char c; - size_t n = *iv_off; + size_t n; + + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( mode == MBEDTLS_ARIA_ENCRYPT || + mode == MBEDTLS_ARIA_DECRYPT ); + ARIA_VALIDATE_RET( length == 0 || input != NULL ); + ARIA_VALIDATE_RET( length == 0 || output != NULL ); + ARIA_VALIDATE_RET( iv != NULL ); + ARIA_VALIDATE_RET( iv_off != NULL ); + + n = *iv_off; + + /* An overly large value of n can lead to an unlimited + * buffer overflow. Therefore, guard against this + * outside of parameter validation. */ + if( n >= MBEDTLS_ARIA_BLOCKSIZE ) + return( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA ); if( mode == MBEDTLS_ARIA_DECRYPT ) { @@ -713,7 +750,21 @@ int mbedtls_aria_crypt_ctr( mbedtls_aria_context *ctx, unsigned char *output ) { int c, i; - size_t n = *nc_off; + size_t n; + + ARIA_VALIDATE_RET( ctx != NULL ); + ARIA_VALIDATE_RET( length == 0 || input != NULL ); + ARIA_VALIDATE_RET( length == 0 || output != NULL ); + ARIA_VALIDATE_RET( nonce_counter != NULL ); + ARIA_VALIDATE_RET( stream_block != NULL ); + ARIA_VALIDATE_RET( nc_off != NULL ); + + n = *nc_off; + /* An overly large value of n can lead to an unlimited + * buffer overflow. Therefore, guard against this + * outside of parameter validation. */ + if( n >= MBEDTLS_ARIA_BLOCKSIZE ) + return( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA ); while( length-- ) { From 9e45c1607ec439a23592dd1d35539bf10ccc91e8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 11 Dec 2018 21:51:38 +0000 Subject: [PATCH 03/10] Test parameter validation for ARIA module --- tests/suites/test_suite_aria.data | 3 + tests/suites/test_suite_aria.function | 179 ++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/tests/suites/test_suite_aria.data b/tests/suites/test_suite_aria.data index 8cb2d2aa3..5a92ee9c7 100644 --- a/tests/suites/test_suite_aria.data +++ b/tests/suites/test_suite_aria.data @@ -1,3 +1,6 @@ +Parameter validation +aria_invalid_param: + ARIA-128-ECB Encrypt - RFC 5794 aria_encrypt_ecb:"000102030405060708090a0b0c0d0e0f":"00112233445566778899aabbccddeeff":"d718fbd6ab644c739da95f3be6451778":0 diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function index 4e39078ff..586b2d37f 100644 --- a/tests/suites/test_suite_aria.function +++ b/tests/suites/test_suite_aria.function @@ -16,6 +16,185 @@ * END_DEPENDENCIES */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void aria_invalid_param( ) +{ + mbedtls_aria_context ctx; + unsigned char key[128 / 8] = { 0 }; + unsigned char input[MBEDTLS_ARIA_BLOCKSIZE] = { 0 }; + unsigned char output[MBEDTLS_ARIA_BLOCKSIZE] = { 0 }; + unsigned char iv[MBEDTLS_ARIA_BLOCKSIZE] = { 0 }; + size_t iv_off = 0; + + TEST_INVALID_PARAM( mbedtls_aria_init( NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_setkey_enc( NULL, key, + sizeof( key ) ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_setkey_enc( &ctx, NULL, + sizeof( key ) ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_setkey_dec( NULL, key, + sizeof( key ) ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_setkey_dec( &ctx, NULL, + sizeof( key ) ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ecb( NULL, input, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ecb( &ctx, NULL, output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ecb( &ctx, input, NULL ) ); + +#if defined(MBEDTLS_CIPHER_MODE_CBC) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cbc( NULL, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cbc( &ctx, + 42 /* invalid mode */, + sizeof( input ), + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cbc( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + NULL, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cbc( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + iv, + NULL, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cbc( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + iv, + input, + NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CBC */ + +#if defined(MBEDTLS_CIPHER_MODE_CFB) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( NULL, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + &iv_off, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( &ctx, + 42, /* invalid mode */ + sizeof( input ), + &iv_off, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + NULL, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + &iv_off, + NULL, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + &iv_off, + iv, + NULL, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_cfb128( &ctx, + MBEDTLS_ARIA_ENCRYPT, + sizeof( input ), + &iv_off, + iv, + input, + NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CFB */ + +#if defined(MBEDTLS_CIPHER_MODE_CTR) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( NULL, + sizeof( input ), + &iv_off, + iv, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( &ctx, + sizeof( input ), + NULL, + iv, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( &ctx, + sizeof( input ), + &iv_off, + NULL, + iv, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( &ctx, + sizeof( input ), + &iv_off, + iv, + NULL, + input, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( &ctx, + sizeof( input ), + &iv_off, + iv, + iv, + NULL, + output ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, + mbedtls_aria_crypt_ctr( &ctx, + sizeof( input ), + &iv_off, + iv, + iv, + input, + NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CTR */ + +exit: + return; + +} +/* END_CASE */ + /* BEGIN_CASE */ void aria_encrypt_ecb( char *hex_key_string, char *hex_src_string, char *hex_dst_string, int setkey_result ) From 02d524c05c37c85f51f1662ab0685a5bf7059bdb Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 09:18:37 +0000 Subject: [PATCH 04/10] Minor ARIA documentation improvements --- include/mbedtls/aria.h | 69 ++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 33 deletions(-) diff --git a/include/mbedtls/aria.h b/include/mbedtls/aria.h index 54db1a637..bd99b9fb8 100644 --- a/include/mbedtls/aria.h +++ b/include/mbedtls/aria.h @@ -91,15 +91,16 @@ mbedtls_aria_context; * It must be the first API called before using * the context. * - * \param ctx The ARIA context to initialize. Must not be \c NULL. + * \param ctx The ARIA context to initialize. This must not be \c NULL. */ void mbedtls_aria_init( mbedtls_aria_context *ctx ); /** * \brief This function releases and clears the specified ARIA context. * - * \param ctx The ARIA context to clear. May be \c NULL, in which - * case this function is a no-op. + * \param ctx The ARIA context to clear. This may be \c NULL, in which + * case this function is a no-op. If it is not \c NULL, + * it must point to an initialized ARIA context. */ void mbedtls_aria_free( mbedtls_aria_context *ctx ); @@ -107,10 +108,10 @@ void mbedtls_aria_free( mbedtls_aria_context *ctx ); * \brief This function sets the encryption key. * * \param ctx The ARIA context to which the key should be bound. - * Must be initialized. - * \param key The encryption key. Must be a readable buffer - * of size \p keybits bits. - * \param keybits The size of data passed in bits. Valid options are: + * This must be initialized. + * \param key The encryption key. This must be a readable buffer + * of size \p keybits Bits. + * \param keybits The size of \p key in Bits. Valid options are: *
  • 128 bits
    • *
  • 192 bits
    • *
  • 256 bits
@@ -126,9 +127,9 @@ int mbedtls_aria_setkey_enc( mbedtls_aria_context *ctx, * \brief This function sets the decryption key. * * \param ctx The ARIA context to which the key should be bound. - * Must be initialized. - * \param key The decryption key. Must be a readable buffer - * of size \p keybits bits. + * This must be initialized. + * \param key The decryption key. This must be a readable buffer + * of size \p keybits Bits. * \param keybits The size of data passed. Valid options are: *
  • 128 bits
    • *
  • 192 bits
    • @@ -154,7 +155,7 @@ int mbedtls_aria_setkey_dec( mbedtls_aria_context *ctx, * call to this API with the same context. * * \param ctx The ARIA context to use for encryption or decryption. - * Must be initialized. + * This must be initialized and bound to a key. * \param input The 16-Byte buffer holding the input data. * \param output The 16-Byte buffer holding the output data. @@ -191,17 +192,18 @@ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, * * * \param ctx The ARIA context to use for encryption or decryption. - * Must be initialized. - * \param mode The ARIA operation: #MBEDTLS_ARIA_ENCRYPT or - * #MBEDTLS_ARIA_DECRYPT. + * This must be initialized and bound to a key. + * \param mode The mode of operation. This must be either + * #MBEDTLS_ARIA_ENCRYPT for encryption, or + * #MBEDTLS_ARIA_DECRYPT for decryption. * \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). - * Must be a readable buffer of size 16 Bytes. + * This must be a readable buffer of size 16 Bytes. * \param input The buffer holding the input data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * \param output The buffer holding the output data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * * \return \c 0 on success. * \return A negative error code on failure. @@ -239,18 +241,19 @@ int mbedtls_aria_crypt_cbc( mbedtls_aria_context *ctx, * * * \param ctx The ARIA context to use for encryption or decryption. - * Must be initialized. - * \param mode The ARIA operation: #MBEDTLS_ARIA_ENCRYPT or - * #MBEDTLS_ARIA_DECRYPT. - * \param length The length of the input data. + * This must be initialized and bound to a key. + * \param mode The mode of operation. This must be either + * #MBEDTLS_ARIA_ENCRYPT for encryption, or + * #MBEDTLS_ARIA_DECRYPT for decryption. + * \param length The length of the input data \p input in Bytes. * \param iv_off The offset in IV (updated after use). - * Must not be larger than 15. + * This must not be larger than 15. * \param iv The initialization vector (updated after use). - * Must be a readable buffer of size 16 Bytes. + * This must be a readable buffer of size 16 Bytes. * \param input The buffer holding the input data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * \param output The buffer holding the output data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * * \return \c 0 on success. * \return A negative error code on failure. @@ -323,21 +326,21 @@ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, * securely discarded as soon as it's no longer needed. * * \param ctx The ARIA context to use for encryption or decryption. - * Must be initialized. - * \param length The length of the input data. + * This must be initialized and bound to a key. + * \param length The length of the input data \p input in Bytes. * \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. - * Must not be larger than 15. - * \param nonce_counter The 128-bit nonce and counter. Must point to - * an RW-buffer of length 16 bytes. - * \param stream_block The saved stream block for resuming. Must point to + * This must not be larger than 15. + * \param nonce_counter The 128-bit nonce and counter. This must point to * an RW-buffer of length 16 bytes. + * \param stream_block The saved stream block for resuming. This must + * point to an RW-buffer of length 16 bytes. * This is overwritten by the function. * \param input The buffer holding the input data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * \param output The buffer holding the output data. - * May be \c NULL if `length == 0`. + * This may be \c NULL if `length == 0`. * * \return \c 0 on success. * \return A negative error code on failure. From b0de9f5b03b0ab64657b9ca13d8862c80168e9b7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 12:06:38 +0000 Subject: [PATCH 05/10] Test that mbedtls_aria_free() accepts NULL parameter --- tests/suites/test_suite_aria.function | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function index 586b2d37f..d392cfd81 100644 --- a/tests/suites/test_suite_aria.function +++ b/tests/suites/test_suite_aria.function @@ -27,6 +27,7 @@ void aria_invalid_param( ) size_t iv_off = 0; TEST_INVALID_PARAM( mbedtls_aria_init( NULL ) ); + TEST_VALID_PARAM( mbedtls_aria_free( NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, mbedtls_aria_setkey_enc( NULL, key, From 2f87504cb788907f1d9f320a82e48d0ca4ee8167 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 12:06:51 +0000 Subject: [PATCH 06/10] Minor ARIA documentation improvements --- include/mbedtls/aria.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/aria.h b/include/mbedtls/aria.h index bd99b9fb8..3db43766a 100644 --- a/include/mbedtls/aria.h +++ b/include/mbedtls/aria.h @@ -99,7 +99,7 @@ void mbedtls_aria_init( mbedtls_aria_context *ctx ); * \brief This function releases and clears the specified ARIA context. * * \param ctx The ARIA context to clear. This may be \c NULL, in which - * case this function is a no-op. If it is not \c NULL, + * case this function returns immediately. If it is not \c NULL, * it must point to an initialized ARIA context. */ void mbedtls_aria_free( mbedtls_aria_context *ctx ); @@ -328,14 +328,14 @@ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, * \param ctx The ARIA context to use for encryption or decryption. * This must be initialized and bound to a key. * \param length The length of the input data \p input in Bytes. - * \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. - * This must not be larger than 15. + * \param nc_off The offset in Bytes in the current \p stream_block, + * for resuming within the current cipher stream. The + * offset pointer should be \c 0 at the start of a + * stream. This must not be larger than \c 15 Bytes. * \param nonce_counter The 128-bit nonce and counter. This must point to - * an RW-buffer of length 16 bytes. + * a read/write buffer of length \c 16 bytes. * \param stream_block The saved stream block for resuming. This must - * point to an RW-buffer of length 16 bytes. + * point to a read/write buffer of length \c 16 bytes. * This is overwritten by the function. * \param input The buffer holding the input data. * This may be \c NULL if `length == 0`. From fac1d44d62f833b82afe6fb743a55f87c80cf854 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 12:07:01 +0000 Subject: [PATCH 07/10] Fix style in ARIA parameter validation tests --- tests/suites/test_suite_aria.function | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function index d392cfd81..8ceb5bb08 100644 --- a/tests/suites/test_suite_aria.function +++ b/tests/suites/test_suite_aria.function @@ -33,15 +33,15 @@ void aria_invalid_param( ) mbedtls_aria_setkey_enc( NULL, key, sizeof( key ) ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, - mbedtls_aria_setkey_enc( &ctx, NULL, - sizeof( key ) ) ); + mbedtls_aria_setkey_enc( &ctx, NULL, + sizeof( key ) ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, - mbedtls_aria_setkey_dec( NULL, key, - sizeof( key ) ) ); + mbedtls_aria_setkey_dec( NULL, key, + sizeof( key ) ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, - mbedtls_aria_setkey_dec( &ctx, NULL, - sizeof( key ) ) ); + mbedtls_aria_setkey_dec( &ctx, NULL, + sizeof( key ) ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, mbedtls_aria_crypt_ecb( NULL, input, output ) ); From 14b91e8e228fb3cd766842e894215db126ed6291 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 14:13:36 +0000 Subject: [PATCH 08/10] Move testing of mbedtls_aria_free() to separate test The test that mbedtls_aria_free() accepts NULL parameters can be performed even if MBEDTLS_CHECK_PARAMS is unset, but was previously included in the test case aria_invalid_params() which is only executed if MBEDTLS_CHECK_PARAMS is set. --- tests/suites/test_suite_aria.data | 5 ++++- tests/suites/test_suite_aria.function | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_aria.data b/tests/suites/test_suite_aria.data index 5a92ee9c7..2da0b30c2 100644 --- a/tests/suites/test_suite_aria.data +++ b/tests/suites/test_suite_aria.data @@ -1,4 +1,7 @@ -Parameter validation +ARIA - Valid parameters +aria_valid_param: + +ARIA - Invalid parameters aria_invalid_param: ARIA-128-ECB Encrypt - RFC 5794 diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function index 8ceb5bb08..6e29e535f 100644 --- a/tests/suites/test_suite_aria.function +++ b/tests/suites/test_suite_aria.function @@ -16,6 +16,13 @@ * END_DEPENDENCIES */ +/* BEGIN_CASE */ +void aria_valid_param( ) +{ + TEST_VALID_PARAM( mbedtls_aria_free( NULL ) ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void aria_invalid_param( ) { @@ -27,7 +34,6 @@ void aria_invalid_param( ) size_t iv_off = 0; TEST_INVALID_PARAM( mbedtls_aria_init( NULL ) ); - TEST_VALID_PARAM( mbedtls_aria_free( NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA, mbedtls_aria_setkey_enc( NULL, key, From 938a15e58408f636e40e10cb553a6befc006ba92 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 16:43:45 +0000 Subject: [PATCH 09/10] Leave behaviour on NULL input unspecified in ARIA We allow a NULL input buffer if the input length is zero, but we don't test it. As long as that's the case, we shouldn't promise to support it. --- include/mbedtls/aria.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/aria.h b/include/mbedtls/aria.h index 3db43766a..1e8956ed1 100644 --- a/include/mbedtls/aria.h +++ b/include/mbedtls/aria.h @@ -200,10 +200,10 @@ int mbedtls_aria_crypt_ecb( mbedtls_aria_context *ctx, * multiple of the block size (16 Bytes). * \param iv Initialization vector (updated after use). * This must be a readable buffer of size 16 Bytes. - * \param input The buffer holding the input data. - * This may be \c NULL if `length == 0`. - * \param output The buffer holding the output data. - * This may be \c NULL if `length == 0`. + * \param input The buffer holding the input data. This must + * be a readable buffer of length \p length Bytes. + * \param output The buffer holding the output data. This must + * be a writable buffer of length \p length Bytes. * * \return \c 0 on success. * \return A negative error code on failure. @@ -250,10 +250,10 @@ int mbedtls_aria_crypt_cbc( mbedtls_aria_context *ctx, * This must not be larger than 15. * \param iv The initialization vector (updated after use). * This must be a readable buffer of size 16 Bytes. - * \param input The buffer holding the input data. - * This may be \c NULL if `length == 0`. - * \param output The buffer holding the output data. - * This may be \c NULL if `length == 0`. + * \param input The buffer holding the input data. This must + * be a readable buffer of length \p length Bytes. + * \param output The buffer holding the output data. This must + * be a writable buffer of length \p length Bytes. * * \return \c 0 on success. * \return A negative error code on failure. @@ -337,10 +337,10 @@ int mbedtls_aria_crypt_cfb128( mbedtls_aria_context *ctx, * \param stream_block The saved stream block for resuming. This must * point to a read/write buffer of length \c 16 bytes. * This is overwritten by the function. - * \param input The buffer holding the input data. - * This may be \c NULL if `length == 0`. - * \param output The buffer holding the output data. - * This may be \c NULL if `length == 0`. + * \param input The buffer holding the input data. This must + * be a readable buffer of length \p length Bytes. + * \param output The buffer holding the output data. This must + * be a writable buffer of length \p length Bytes. * * \return \c 0 on success. * \return A negative error code on failure. From 0294072c0987ccac1894fb90ff2c8fa804690187 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 18:18:45 +0000 Subject: [PATCH 10/10] Avoid unused variable warning in ARIA param validation test --- tests/suites/test_suite_aria.function | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/suites/test_suite_aria.function b/tests/suites/test_suite_aria.function index 6e29e535f..7e35f154b 100644 --- a/tests/suites/test_suite_aria.function +++ b/tests/suites/test_suite_aria.function @@ -33,6 +33,9 @@ void aria_invalid_param( ) unsigned char iv[MBEDTLS_ARIA_BLOCKSIZE] = { 0 }; size_t iv_off = 0; + ((void) iv_off); + ((void) iv); + TEST_INVALID_PARAM( mbedtls_aria_init( NULL ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_ARIA_BAD_INPUT_DATA,