From 3b4d6c69254f13bcff93c3d28da1581dd6d65897 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 18:14:08 +0000 Subject: [PATCH 1/8] Document parameter preconditions for Blowfish module --- include/mbedtls/blowfish.h | 119 +++++++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 43 deletions(-) diff --git a/include/mbedtls/blowfish.h b/include/mbedtls/blowfish.h index 922d8f82e..d904269c0 100644 --- a/include/mbedtls/blowfish.h +++ b/include/mbedtls/blowfish.h @@ -76,40 +76,51 @@ mbedtls_blowfish_context; #endif /* MBEDTLS_BLOWFISH_ALT */ /** - * \brief Initialize Blowfish context + * \brief Initialize a Blowfish context. * - * \param ctx Blowfish context to be initialized + * \param ctx The Blowfish context to be initialized. + * Must not be \c NULL. */ void mbedtls_blowfish_init( mbedtls_blowfish_context *ctx ); /** - * \brief Clear Blowfish context + * \brief Clear a Blowfish context. * - * \param ctx Blowfish context to be cleared + * \param ctx The Blowfish context to be cleared. + * 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 Blowfish context. */ void mbedtls_blowfish_free( mbedtls_blowfish_context *ctx ); /** - * \brief Blowfish key schedule + * \brief Perform a Blowfish key schedule. * - * \param ctx Blowfish context to be initialized - * \param key encryption key - * \param keybits must be between 32 and 448 bits + * \param ctx The Blowfish context to perform the key schedule on. + * \param key The encryption key. Must be a readable buffer of + * length \p keybits Bits. + * \param keybits The length of \p key in Bits. Must be between + * \c 32 and \c 448 and a multiple of \c 8. * - * \return 0 if successful, or MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, const unsigned char *key, unsigned int keybits ); /** - * \brief Blowfish-ECB block encryption/decryption + * \brief Perform a Blowfish-ECB block encryption/decryption. * - * \param ctx Blowfish context - * \param mode MBEDTLS_BLOWFISH_ENCRYPT or MBEDTLS_BLOWFISH_DECRYPT - * \param input 8-byte input block - * \param output 8-byte output block + * \param ctx The Blowfish context to use. This must be initialized + * and bound to a key. + * \param mode The mode of operation. Possible values are + * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or + * #MBEDTLS_BLOWFISH_DECRYPT for decryption. + * \param input The input block. Must be a readable buffer of size 8 Bytes. + * \param input The output block. Must be a writable buffer of size 8 Bytes. * - * \return 0 if successful + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, int mode, @@ -118,7 +129,7 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CBC) /** - * \brief Blowfish-CBC buffer encryption/decryption + * \brief Perform a Blowfish-CBC buffer encryption/decryption * Length should be a multiple of the block * size (8 bytes) * @@ -130,15 +141,21 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, * IV, you should either save it manually or use the cipher * module instead. * - * \param ctx Blowfish context - * \param mode MBEDTLS_BLOWFISH_ENCRYPT or MBEDTLS_BLOWFISH_DECRYPT - * \param length length of the input data - * \param iv initialization vector (updated after use) - * \param input buffer holding the input data - * \param output buffer holding the output data + * \param ctx The Blowfish context to use. This must be initialized + * and bound to a key. + * \param mode The mode of operation. Possible values are + * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or + * #MBEDTLS_BLOWFISH_DECRYPT for decryption. + * \param length The length of the input data in Bytes. + * \param iv The initialization vector. This must be an RW buffer + * of length \c 8 Bytes. It is updated by this function. + * \param input The input data. Must be a readable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \param output The output data. Must be a writable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. * - * \return 0 if successful, or - * MBEDTLS_ERR_BLOWFISH_INVALID_INPUT_LENGTH + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, int mode, @@ -150,7 +167,7 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CFB) /** - * \brief Blowfish CFB buffer encryption/decryption. + * \brief Perform a Blowfish CFB buffer encryption/decryption. * * \note Upon exit, the content of the IV is updated so that you can * call the function same function again on the following @@ -160,15 +177,25 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, * IV, you should either save it manually or use the cipher * module instead. * - * \param ctx Blowfish context - * \param mode MBEDTLS_BLOWFISH_ENCRYPT or MBEDTLS_BLOWFISH_DECRYPT - * \param length length of the input data - * \param iv_off offset in IV (updated after use) - * \param iv initialization vector (updated after use) - * \param input buffer holding the input data - * \param output buffer holding the output data + * \param ctx The Blowfish context to use. This must be initialized + * and bound to a key. + * \param mode The mode of operation. Possible values are + * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or + * #MBEDTLS_BLOWFISH_DECRYPT for decryption. + * \param length The length of the input data in Bytes. + * \param iv_off The offset in the initialiation vector. + * The value pointed to must be smaller than \c 8. + * It is updated by this function to support the aforementioned + * streaming usage. + * \param iv The initialization vector. Must be an RW buffer of + * size \c 8 Bytes. It is updated after use. + * \param input The input data. Must be a readable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \param output The output data. Must be a writable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. * - * \return 0 if successful + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, int mode, @@ -181,7 +208,7 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CTR) /** - * \brief Blowfish-CTR buffer encryption/decryption + * \brief Perform a Blowfish-CTR buffer encryption/decryption. * * \warning You must never reuse a nonce value with the same key. Doing so * would void the encryption for the two messages encrypted with @@ -224,18 +251,24 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, * content must not be written to insecure storage and should be * securely discarded as soon as it's no longer needed. * - * \param ctx Blowfish context - * \param length The length of the data + * \param ctx The Blowfish context to use. This must be initialized + * and bound to a key. + * \param length The length of the input data in Bytes. * \param nc_off The offset in the current stream_block (for resuming * within current cipher stream). The offset pointer to - * should be 0 at the start of a stream. - * \param nonce_counter The 64-bit nonce and counter. - * \param stream_block The saved stream-block for resuming. Is overwritten - * by the function. - * \param input The input data stream - * \param output The output data stream + * should be \c 0 at the start of a stream and must be + * smaller than \c 8. It is updated by this function. + * \param nonce_counter The 64-bit nonce and counter. This must point to an RW + * buffer of length \c 8 Bytes. + * \param stream_block The saved stream-block for resuming. This must point to + * an RW buffer of length \c 8 Bytes. + * \param input The input data. Must be a readable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \param output The output data. Must be a writable buffer of length + * \p length Bytes. If \p length if \c 0, it may be \c NULL. * - * \return 0 if successful + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_blowfish_crypt_ctr( mbedtls_blowfish_context *ctx, size_t length, From 541aa69de40e022cd73f216d0f27e86550f2cd3c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 18:46:09 +0000 Subject: [PATCH 2/8] Implement parameter validation for Blowfish module --- library/blowfish.c | 56 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/library/blowfish.c b/library/blowfish.c index b3448c20b..cbf923824 100644 --- a/library/blowfish.c +++ b/library/blowfish.c @@ -40,6 +40,12 @@ #if !defined(MBEDTLS_BLOWFISH_ALT) +/* Parameter validation macros */ +#define BLOWFISH_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA ) +#define BLOWFISH_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + /* * 32-bit integer manipulation macros (big endian) */ @@ -153,6 +159,7 @@ static void blowfish_dec( mbedtls_blowfish_context *ctx, uint32_t *xl, uint32_t void mbedtls_blowfish_init( mbedtls_blowfish_context *ctx ) { + BLOWFISH_VALIDATE( ctx != NULL ); memset( ctx, 0, sizeof( mbedtls_blowfish_context ) ); } @@ -167,14 +174,18 @@ void mbedtls_blowfish_free( mbedtls_blowfish_context *ctx ) /* * Blowfish key schedule */ -int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, const unsigned char *key, - unsigned int keybits ) +int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, + const unsigned char *key, + unsigned int keybits ) { unsigned int i, j, k; uint32_t data, datal, datar; + BLOWFISH_VALIDATE_RET( ctx != NULL ); + BLOWFISH_VALIDATE_RET( key != NULL ); - if( keybits < MBEDTLS_BLOWFISH_MIN_KEY_BITS || keybits > MBEDTLS_BLOWFISH_MAX_KEY_BITS || - ( keybits % 8 ) ) + if( keybits < MBEDTLS_BLOWFISH_MIN_KEY_BITS || + keybits > MBEDTLS_BLOWFISH_MAX_KEY_BITS || + keybits % 8 != 0 ) { return( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA ); } @@ -231,6 +242,11 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, unsigned char output[MBEDTLS_BLOWFISH_BLOCKSIZE] ) { uint32_t X0, X1; + BLOWFISH_VALIDATE_RET( ctx != NULL ); + BLOWFISH_VALIDATE_RET( mode == MBEDTLS_BLOWFISH_ENCRYPT || + mode == MBEDTLS_BLOWFISH_DECRYPT ); + BLOWFISH_VALIDATE_RET( input != NULL ); + BLOWFISH_VALIDATE_RET( output != NULL ); GET_UINT32_BE( X0, input, 0 ); GET_UINT32_BE( X1, input, 4 ); @@ -263,6 +279,12 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, { int i; unsigned char temp[MBEDTLS_BLOWFISH_BLOCKSIZE]; + BLOWFISH_VALIDATE_RET( ctx != NULL ); + BLOWFISH_VALIDATE_RET( mode == MBEDTLS_BLOWFISH_ENCRYPT || + mode == MBEDTLS_BLOWFISH_DECRYPT ); + BLOWFISH_VALIDATE_RET( iv != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || input != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || output != NULL ); if( length % MBEDTLS_BLOWFISH_BLOCKSIZE ) return( MBEDTLS_ERR_BLOWFISH_INVALID_INPUT_LENGTH ); @@ -317,7 +339,19 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, unsigned char *output ) { int c; - size_t n = *iv_off; + size_t n; + + BLOWFISH_VALIDATE_RET( ctx != NULL ); + BLOWFISH_VALIDATE_RET( mode == MBEDTLS_BLOWFISH_ENCRYPT || + mode == MBEDTLS_BLOWFISH_DECRYPT ); + BLOWFISH_VALIDATE_RET( iv != NULL ); + BLOWFISH_VALIDATE_RET( iv_off != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || input != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || output != NULL ); + + n = *iv_off; + if( n >= 8 ) + return( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA ); if( mode == MBEDTLS_BLOWFISH_DECRYPT ) { @@ -365,7 +399,17 @@ int mbedtls_blowfish_crypt_ctr( mbedtls_blowfish_context *ctx, unsigned char *output ) { int c, i; - size_t n = *nc_off; + size_t n; + BLOWFISH_VALIDATE_RET( ctx != NULL ); + BLOWFISH_VALIDATE_RET( nonce_counter != NULL ); + BLOWFISH_VALIDATE_RET( stream_block != NULL ); + BLOWFISH_VALIDATE_RET( nc_off != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || input != NULL ); + BLOWFISH_VALIDATE_RET( length == 0 || output != NULL ); + + n = *nc_off; + if( n >= 8 ) + return( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA ); while( length-- ) { From e38b4cd661c51c39ab6eb31bef9b5b8244680f46 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Dec 2018 18:46:19 +0000 Subject: [PATCH 3/8] Test parameter validation for Blowfish module --- tests/suites/test_suite_blowfish.data | 3 + tests/suites/test_suite_blowfish.function | 148 ++++++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/tests/suites/test_suite_blowfish.data b/tests/suites/test_suite_blowfish.data index d4e8791c1..e1a614ca7 100644 --- a/tests/suites/test_suite_blowfish.data +++ b/tests/suites/test_suite_blowfish.data @@ -1,3 +1,6 @@ +Blowfish parameter validation +blowfish_invalid_param: + BLOWFISH-ECB Encrypt SSLeay reference #1 blowfish_encrypt_ecb:"0000000000000000":"0000000000000000":"4ef997456198dd78":0 diff --git a/tests/suites/test_suite_blowfish.function b/tests/suites/test_suite_blowfish.function index 189e23dc6..028ae1a20 100644 --- a/tests/suites/test_suite_blowfish.function +++ b/tests/suites/test_suite_blowfish.function @@ -7,6 +7,154 @@ * END_DEPENDENCIES */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void blowfish_invalid_param( ) +{ + mbedtls_blowfish_context ctx; + unsigned char buf[16] = { 0 }; + size_t off; + ((void) off); + + TEST_INVALID_PARAM( mbedtls_blowfish_init( NULL ) ); + TEST_VALID_PARAM( mbedtls_blowfish_free( NULL ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_setkey( NULL, + buf, + 128 ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_setkey( &ctx, + NULL, + 128 ) ); + + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ecb( NULL, + MBEDTLS_BLOWFISH_ENCRYPT, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ecb( &ctx, + 42, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ecb( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ecb( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + buf, NULL ) ); + +#if defined(MBEDTLS_CIPHER_MODE_CBC) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cbc( NULL, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + buf, buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cbc( &ctx, + 42, + sizeof( buf ), + buf, buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cbc( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + NULL, buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cbc( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + buf, NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cbc( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + buf, buf, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CBC */ + +#if defined(MBEDTLS_CIPHER_MODE_CFB) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( NULL, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + &off, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( &ctx, + 42, + sizeof( buf ), + &off, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + NULL, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + &off, NULL, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + &off, buf, + NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_cfb64( &ctx, + MBEDTLS_BLOWFISH_ENCRYPT, + sizeof( buf ), + &off, buf, + buf, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CFB */ + +#if defined(MBEDTLS_CIPHER_MODE_CTR) + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( NULL, + sizeof( buf ), + &off, + buf, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( &ctx, + sizeof( buf ), + NULL, + buf, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( &ctx, + sizeof( buf ), + &off, + NULL, buf, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( &ctx, + sizeof( buf ), + &off, + buf, NULL, + buf, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( &ctx, + sizeof( buf ), + &off, + buf, buf, + NULL, buf ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, + mbedtls_blowfish_crypt_ctr( &ctx, + sizeof( buf ), + &off, + buf, buf, + buf, NULL ) ); +#endif /* MBEDTLS_CIPHER_MODE_CTR */ + +exit: + return; +} +/* END_CASE */ + /* BEGIN_CASE */ void blowfish_encrypt_ecb( data_t * key_str, data_t * src_str, data_t * hex_dst_string, int setkey_result ) From 49acc64c695e2b1b88f31b770f23df83d0f2b34f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 09:24:51 +0000 Subject: [PATCH 4/8] Minor improvements to Blowfish documentation and tests --- include/mbedtls/blowfish.h | 35 +++++++++++---------- tests/suites/test_suite_blowfish.function | 37 ++++++++++++----------- 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/include/mbedtls/blowfish.h b/include/mbedtls/blowfish.h index d904269c0..e40e17c65 100644 --- a/include/mbedtls/blowfish.h +++ b/include/mbedtls/blowfish.h @@ -79,7 +79,7 @@ mbedtls_blowfish_context; * \brief Initialize a Blowfish context. * * \param ctx The Blowfish context to be initialized. - * Must not be \c NULL. + * This must not be \c NULL. */ void mbedtls_blowfish_init( mbedtls_blowfish_context *ctx ); @@ -97,9 +97,9 @@ void mbedtls_blowfish_free( mbedtls_blowfish_context *ctx ); * \brief Perform a Blowfish key schedule. * * \param ctx The Blowfish context to perform the key schedule on. - * \param key The encryption key. Must be a readable buffer of + * \param key The encryption key. This must be a readable buffer of * length \p keybits Bits. - * \param keybits The length of \p key in Bits. Must be between + * \param keybits The length of \p key in Bits. This must be between * \c 32 and \c 448 and a multiple of \c 8. * * \return \c 0 if successful. @@ -116,8 +116,8 @@ int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, const unsigned char * \param mode The mode of operation. Possible values are * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or * #MBEDTLS_BLOWFISH_DECRYPT for decryption. - * \param input The input block. Must be a readable buffer of size 8 Bytes. - * \param input The output block. Must be a writable buffer of size 8 Bytes. + * \param input The input block. This must be a readable buffer of size 8 Bytes. + * \param input The output block. This must be a writable buffer of size 8 Bytes. * * \return \c 0 if successful. * \return A negative error code on failure. @@ -129,9 +129,7 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CBC) /** - * \brief Perform a Blowfish-CBC buffer encryption/decryption - * Length should be a multiple of the block - * size (8 bytes) + * \brief Perform a Blowfish-CBC buffer encryption/decryption. * * \note Upon exit, the content of the IV is updated so that you can * call the function same function again on the following @@ -146,12 +144,13 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, * \param mode The mode of operation. Possible values are * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or * #MBEDTLS_BLOWFISH_DECRYPT for decryption. - * \param length The length of the input data in Bytes. + * \param length The length of the input data in Bytes. This must be + * multiple of \c 8. * \param iv The initialization vector. This must be an RW buffer * of length \c 8 Bytes. It is updated by this function. - * \param input The input data. Must be a readable buffer of length + * \param input The input data. This must be a readable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. - * \param output The output data. Must be a writable buffer of length + * \param output The output data. This must be a writable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. * * \return \c 0 if successful. @@ -187,11 +186,11 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, * The value pointed to must be smaller than \c 8. * It is updated by this function to support the aforementioned * streaming usage. - * \param iv The initialization vector. Must be an RW buffer of + * \param iv The initialization vector. This must be an RW buffer of * size \c 8 Bytes. It is updated after use. - * \param input The input data. Must be a readable buffer of length + * \param input The input data. This must be a readable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. - * \param output The output data. Must be a writable buffer of length + * \param output The output data. This must be a writable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. * * \return \c 0 if successful. @@ -262,10 +261,10 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, * buffer of length \c 8 Bytes. * \param stream_block The saved stream-block for resuming. This must point to * an RW buffer of length \c 8 Bytes. - * \param input The input data. Must be a readable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. - * \param output The output data. Must be a writable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \param input The input data. This must be a readable buffer of length + * \p length Bytes. If \p length is \c 0, it may be \c NULL. + * \param output The output data. This must be a writable buffer of length + * \p length Bytes. If \p length is \c 0, it may be \c NULL. * * \return \c 0 if successful. * \return A negative error code on failure. diff --git a/tests/suites/test_suite_blowfish.function b/tests/suites/test_suite_blowfish.function index 028ae1a20..1d1422a4f 100644 --- a/tests/suites/test_suite_blowfish.function +++ b/tests/suites/test_suite_blowfish.function @@ -12,6 +12,9 @@ void blowfish_invalid_param( ) { mbedtls_blowfish_context ctx; unsigned char buf[16] = { 0 }; + size_t const valid_keylength = sizeof( buf ) * 8; + size_t valid_mode = MBEDTLS_BLOWFISH_ENCRYPT; + size_t invalid_mode = 42; size_t off; ((void) off); @@ -21,53 +24,53 @@ void blowfish_invalid_param( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_setkey( NULL, buf, - 128 ) ); + valid_keylength ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_setkey( &ctx, NULL, - 128 ) ); + valid_keylength ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_ecb( NULL, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_ecb( &ctx, - 42, + invalid_mode, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_ecb( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, NULL, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_ecb( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, buf, NULL ) ); #if defined(MBEDTLS_CIPHER_MODE_CBC) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cbc( NULL, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), buf, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cbc( &ctx, - 42, + invalid_mode, sizeof( buf ), buf, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cbc( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), NULL, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cbc( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), buf, NULL, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cbc( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), buf, buf, NULL ) ); #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -75,37 +78,37 @@ void blowfish_invalid_param( ) #if defined(MBEDTLS_CIPHER_MODE_CFB) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( NULL, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), &off, buf, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( &ctx, - 42, + invalid_mode, sizeof( buf ), &off, buf, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), NULL, buf, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), &off, NULL, buf, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), &off, buf, NULL, buf ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA, mbedtls_blowfish_crypt_cfb64( &ctx, - MBEDTLS_BLOWFISH_ENCRYPT, + valid_mode, sizeof( buf ), &off, buf, buf, NULL ) ); From f947c0a2dd7d29fcbe36e9cc83129579a3282d89 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 14:17:10 +0000 Subject: [PATCH 5/8] Move testing of mbedtls_blowfish_free() to separate test case It should be tested regardless of the setting of MBEDTLS_CHECK_PARAMS. --- tests/suites/test_suite_blowfish.data | 5 ++++- tests/suites/test_suite_blowfish.function | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_blowfish.data b/tests/suites/test_suite_blowfish.data index e1a614ca7..fd172d3b2 100644 --- a/tests/suites/test_suite_blowfish.data +++ b/tests/suites/test_suite_blowfish.data @@ -1,4 +1,7 @@ -Blowfish parameter validation +BLOWFISH - Valid parameters +blowfish_valid_param: + +BLOWFISH - Invalid parameters blowfish_invalid_param: BLOWFISH-ECB Encrypt SSLeay reference #1 diff --git a/tests/suites/test_suite_blowfish.function b/tests/suites/test_suite_blowfish.function index 1d1422a4f..7a93cd139 100644 --- a/tests/suites/test_suite_blowfish.function +++ b/tests/suites/test_suite_blowfish.function @@ -7,6 +7,13 @@ * END_DEPENDENCIES */ +/* BEGIN_CASE */ +void blowfish_valid_param( ) +{ + TEST_VALID_PARAM( mbedtls_blowfish_free( NULL ) ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void blowfish_invalid_param( ) { From 3d9a3490f85d826c3a4c819acc61cdae4c9354b0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 17 Dec 2018 15:15:42 +0000 Subject: [PATCH 6/8] Improve Blowfish documentation --- include/mbedtls/blowfish.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/blowfish.h b/include/mbedtls/blowfish.h index e40e17c65..74d516dc9 100644 --- a/include/mbedtls/blowfish.h +++ b/include/mbedtls/blowfish.h @@ -88,8 +88,8 @@ void mbedtls_blowfish_init( mbedtls_blowfish_context *ctx ); * * \param ctx The Blowfish context to be cleared. * 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 Blowfish context. + * returns immediately. If it is not \c NULL, it must + * point to an initialized Blowfish context. */ void mbedtls_blowfish_free( mbedtls_blowfish_context *ctx ); @@ -146,7 +146,7 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, * #MBEDTLS_BLOWFISH_DECRYPT for decryption. * \param length The length of the input data in Bytes. This must be * multiple of \c 8. - * \param iv The initialization vector. This must be an RW buffer + * \param iv The initialization vector. This must be a read/write buffer * of length \c 8 Bytes. It is updated by this function. * \param input The input data. This must be a readable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. @@ -183,10 +183,10 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, * #MBEDTLS_BLOWFISH_DECRYPT for decryption. * \param length The length of the input data in Bytes. * \param iv_off The offset in the initialiation vector. - * The value pointed to must be smaller than \c 8. + * The value pointed to must be smaller than \c 8 Bytes. * It is updated by this function to support the aforementioned * streaming usage. - * \param iv The initialization vector. This must be an RW buffer of + * \param iv The initialization vector. This must be a read/write buffer of * size \c 8 Bytes. It is updated after use. * \param input The input data. This must be a readable buffer of length * \p length Bytes. If \p length if \c 0, it may be \c NULL. @@ -257,10 +257,10 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, * within current cipher stream). The offset pointer to * should be \c 0 at the start of a stream and must be * smaller than \c 8. It is updated by this function. - * \param nonce_counter The 64-bit nonce and counter. This must point to an RW - * buffer of length \c 8 Bytes. + * \param nonce_counter The 64-bit nonce and counter. This must point to a + * read/write buffer of length \c 8 Bytes. * \param stream_block The saved stream-block for resuming. This must point to - * an RW buffer of length \c 8 Bytes. + * a read/write buffer of length \c 8 Bytes. * \param input The input data. This must be a readable buffer of length * \p length Bytes. If \p length is \c 0, it may be \c NULL. * \param output The output data. This must be a writable buffer of length From 20376d631d54b40be2165280de12aeb8c409c5ce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 18 Dec 2018 17:47:39 +0000 Subject: [PATCH 7/8] Don't promise that passing NULL input to Blowfish works It seems to work, but we don't test it currently, so we shouldn't promise it. --- include/mbedtls/blowfish.h | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/blowfish.h b/include/mbedtls/blowfish.h index 74d516dc9..cd53840ed 100644 --- a/include/mbedtls/blowfish.h +++ b/include/mbedtls/blowfish.h @@ -116,8 +116,10 @@ int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, const unsigned char * \param mode The mode of operation. Possible values are * #MBEDTLS_BLOWFISH_ENCRYPT for encryption, or * #MBEDTLS_BLOWFISH_DECRYPT for decryption. - * \param input The input block. This must be a readable buffer of size 8 Bytes. - * \param input The output block. This must be a writable buffer of size 8 Bytes. + * \param input The input block. This must be a readable buffer + * of size \c 8 Bytes. + * \param output The output block. This must be a writable buffer + * of size \c 8 Bytes. * * \return \c 0 if successful. * \return A negative error code on failure. @@ -149,9 +151,9 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, * \param iv The initialization vector. This must be a read/write buffer * of length \c 8 Bytes. It is updated by this function. * \param input The input data. This must be a readable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \p length Bytes. * \param output The output data. This must be a writable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \p length Bytes. * * \return \c 0 if successful. * \return A negative error code on failure. @@ -186,12 +188,12 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, * The value pointed to must be smaller than \c 8 Bytes. * It is updated by this function to support the aforementioned * streaming usage. - * \param iv The initialization vector. This must be a read/write buffer of - * size \c 8 Bytes. It is updated after use. + * \param iv The initialization vector. This must be a read/write buffer + * of size \c 8 Bytes. It is updated after use. * \param input The input data. This must be a readable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \p length Bytes. * \param output The output data. This must be a writable buffer of length - * \p length Bytes. If \p length if \c 0, it may be \c NULL. + * \p length Bytes. * * \return \c 0 if successful. * \return A negative error code on failure. @@ -261,10 +263,10 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, * read/write buffer of length \c 8 Bytes. * \param stream_block The saved stream-block for resuming. This must point to * a read/write buffer of length \c 8 Bytes. - * \param input The input data. This must be a readable buffer of length - * \p length Bytes. If \p length is \c 0, it may be \c NULL. - * \param output The output data. This must be a writable buffer of length - * \p length Bytes. If \p length is \c 0, it may be \c NULL. + * \param input The input data. This must be a readable buffer of + * length \p length Bytes. + * \param output The output data. This must be a writable buffer of + * length \p length Bytes. * * \return \c 0 if successful. * \return A negative error code on failure. From ed54128fdb5da3cf6d37d09669873bafe4c69155 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 19 Dec 2018 15:48:37 +0000 Subject: [PATCH 8/8] Minor Blowfish documentation improvements --- include/mbedtls/blowfish.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/blowfish.h b/include/mbedtls/blowfish.h index cd53840ed..f01573dca 100644 --- a/include/mbedtls/blowfish.h +++ b/include/mbedtls/blowfish.h @@ -94,7 +94,7 @@ void mbedtls_blowfish_init( mbedtls_blowfish_context *ctx ); void mbedtls_blowfish_free( mbedtls_blowfish_context *ctx ); /** - * \brief Perform a Blowfish key schedule. + * \brief Perform a Blowfish key schedule operation. * * \param ctx The Blowfish context to perform the key schedule on. * \param key The encryption key. This must be a readable buffer of @@ -109,7 +109,7 @@ int mbedtls_blowfish_setkey( mbedtls_blowfish_context *ctx, const unsigned char unsigned int keybits ); /** - * \brief Perform a Blowfish-ECB block encryption/decryption. + * \brief Perform a Blowfish-ECB block encryption/decryption operation. * * \param ctx The Blowfish context to use. This must be initialized * and bound to a key. @@ -131,7 +131,7 @@ int mbedtls_blowfish_crypt_ecb( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CBC) /** - * \brief Perform a Blowfish-CBC buffer encryption/decryption. + * \brief Perform a Blowfish-CBC buffer encryption/decryption operation. * * \note Upon exit, the content of the IV is updated so that you can * call the function same function again on the following @@ -168,7 +168,7 @@ int mbedtls_blowfish_crypt_cbc( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CFB) /** - * \brief Perform a Blowfish CFB buffer encryption/decryption. + * \brief Perform a Blowfish CFB buffer encryption/decryption operation. * * \note Upon exit, the content of the IV is updated so that you can * call the function same function again on the following @@ -209,7 +209,7 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, #if defined(MBEDTLS_CIPHER_MODE_CTR) /** - * \brief Perform a Blowfish-CTR buffer encryption/decryption. + * \brief Perform a Blowfish-CTR buffer encryption/decryption operation. * * \warning You must never reuse a nonce value with the same key. Doing so * would void the encryption for the two messages encrypted with @@ -256,7 +256,7 @@ int mbedtls_blowfish_crypt_cfb64( mbedtls_blowfish_context *ctx, * and bound to a key. * \param length The length of the input data in Bytes. * \param nc_off The offset in the current stream_block (for resuming - * within current cipher stream). The offset pointer to + * within current cipher stream). The offset pointer * should be \c 0 at the start of a stream and must be * smaller than \c 8. It is updated by this function. * \param nonce_counter The 64-bit nonce and counter. This must point to a