From e0215d78697938e4df9410e6e4657e2dc7ca9afd Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Mon, 17 Dec 2018 10:20:30 +0100 Subject: [PATCH 01/15] Add Cipher module parameter validation --- include/mbedtls/cipher.h | 186 ++++++++++------ library/cipher.c | 74 ++++++- tests/CMakeLists.txt | 1 + tests/suites/test_suite_cipher.function | 258 +++++++++++++++++++++++ tests/suites/test_suite_cipher.misc.data | 2 + 5 files changed, 441 insertions(+), 80 deletions(-) create mode 100644 tests/suites/test_suite_cipher.misc.data diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 58a5d63dd..4df10e802 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -336,11 +336,12 @@ const int *mbedtls_cipher_list( void ); * \brief This function retrieves the cipher-information * structure associated with the given cipher name. * - * \param cipher_name Name of the cipher to search for. + * \param cipher_name Name of the cipher to search for. This can be \c NULL. * * \return The cipher information structure associated with the * given \p cipher_name. - * \return NULL if the associated cipher information is not found. + * \return \c NULL if the associated cipher information is not found + * or if \p cipher_name is \c NULL. */ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_string( const char *cipher_name ); @@ -352,7 +353,7 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_string( const char *cipher * * \return The cipher information structure associated with the * given \p cipher_type. - * \return NULL if the associated cipher information is not found. + * \return \c NULL if the associated cipher information is not found. */ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_type( const mbedtls_cipher_type_t cipher_type ); @@ -368,7 +369,7 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_type( const mbedtls_cipher * * \return The cipher information structure associated with the * given \p cipher_id. - * \return NULL if the associated cipher information is not found. + * \return \c NULL if the associated cipher information is not found. */ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_values( const mbedtls_cipher_id_t cipher_id, int key_bitlen, @@ -376,6 +377,8 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_values( const mbedtls_ciph /** * \brief This function initializes a \p cipher_context as NONE. + * + * \param ctx The context to be initialized. This must not be \c NULL. */ void mbedtls_cipher_init( mbedtls_cipher_context_t *ctx ); @@ -383,6 +386,9 @@ void mbedtls_cipher_init( mbedtls_cipher_context_t *ctx ); * \brief This function frees and clears the cipher-specific * context of \p ctx. Freeing \p ctx itself remains the * responsibility of the caller. + * + * \param ctx The context to be freed. If this is \c NULL, the + * function has no effect. */ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); @@ -392,8 +398,8 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); * structure with the appropriate values. It also clears * the structure. * - * \param ctx The context to initialize. May not be NULL. - * \param cipher_info The cipher to use. + * \param ctx The context to initialize. This must be initialized. + * \param cipher_info The cipher to use. This may not be \c NULL. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on @@ -405,17 +411,19 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); * In future versions, the caller will be required to call * mbedtls_cipher_init() on the structure first. */ -int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_info_t *cipher_info ); +int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, + const mbedtls_cipher_info_t *cipher_info ); /** * \brief This function returns the block size of the given cipher. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The size of the blocks of the cipher. * \return 0 if \p ctx has not been initialized. */ -static inline unsigned int mbedtls_cipher_get_block_size( const mbedtls_cipher_context_t *ctx ) +static inline unsigned int mbedtls_cipher_get_block_size( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return 0; @@ -427,12 +435,13 @@ static inline unsigned int mbedtls_cipher_get_block_size( const mbedtls_cipher_c * \brief This function returns the mode of operation for * the cipher. For example, MBEDTLS_MODE_CBC. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The mode of operation. * \return #MBEDTLS_MODE_NONE if \p ctx has not been initialized. */ -static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( const mbedtls_cipher_context_t *ctx ) +static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return MBEDTLS_MODE_NONE; @@ -444,13 +453,14 @@ static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( const mbedtl * \brief This function returns the size of the IV or nonce * of the cipher, in Bytes. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The recommended IV size if no IV has been set. * \return \c 0 for ciphers not using an IV or a nonce. * \return The actual size if an IV has been set. */ -static inline int mbedtls_cipher_get_iv_size( const mbedtls_cipher_context_t *ctx ) +static inline int mbedtls_cipher_get_iv_size( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return 0; @@ -464,12 +474,13 @@ static inline int mbedtls_cipher_get_iv_size( const mbedtls_cipher_context_t *ct /** * \brief This function returns the type of the given cipher. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The type of the cipher. * \return #MBEDTLS_CIPHER_NONE if \p ctx has not been initialized. */ -static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( const mbedtls_cipher_context_t *ctx ) +static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return MBEDTLS_CIPHER_NONE; @@ -481,12 +492,13 @@ static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( const mbedtls_ciphe * \brief This function returns the name of the given cipher * as a string. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The name of the cipher. * \return NULL if \p ctx has not been not initialized. */ -static inline const char *mbedtls_cipher_get_name( const mbedtls_cipher_context_t *ctx ) +static inline const char *mbedtls_cipher_get_name( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return 0; @@ -497,13 +509,14 @@ static inline const char *mbedtls_cipher_get_name( const mbedtls_cipher_context_ /** * \brief This function returns the key length of the cipher. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The key length of the cipher in bits. * \return #MBEDTLS_KEY_LENGTH_NONE if ctx \p has not been * initialized. */ -static inline int mbedtls_cipher_get_key_bitlen( const mbedtls_cipher_context_t *ctx ) +static inline int mbedtls_cipher_get_key_bitlen( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return MBEDTLS_KEY_LENGTH_NONE; @@ -514,12 +527,13 @@ static inline int mbedtls_cipher_get_key_bitlen( const mbedtls_cipher_context_t /** * \brief This function returns the operation of the given cipher. * - * \param ctx The context of the cipher. Must be initialized. + * \param ctx The context of the cipher. This must be initialized. * * \return The type of operation: #MBEDTLS_ENCRYPT or #MBEDTLS_DECRYPT. * \return #MBEDTLS_OPERATION_NONE if \p ctx has not been initialized. */ -static inline mbedtls_operation_t mbedtls_cipher_get_operation( const mbedtls_cipher_context_t *ctx ) +static inline mbedtls_operation_t mbedtls_cipher_get_operation( + const mbedtls_cipher_context_t *ctx ) { if( NULL == ctx || NULL == ctx->cipher_info ) return MBEDTLS_OPERATION_NONE; @@ -530,11 +544,12 @@ static inline mbedtls_operation_t mbedtls_cipher_get_operation( const mbedtls_ci /** * \brief This function sets the key to use with the given context. * - * \param ctx The generic cipher context. May not be NULL. Must have - * been initialized using mbedtls_cipher_info_from_type() - * or mbedtls_cipher_info_from_string(). - * \param key The key to use. - * \param key_bitlen The key length to use, in bits. + * \param ctx The generic cipher context. This must be initialized + * using mbedtls_cipher_info_from_type() or + * mbedtls_cipher_info_from_string(). + * \param key The key to use. This must be a readable buffer of at + * least \p key_bitlen Bits. + * \param key_bitlen The key length to use, in Bits. * \param operation The operation that the key will be used for: * #MBEDTLS_ENCRYPT or #MBEDTLS_DECRYPT. * @@ -543,8 +558,10 @@ static inline mbedtls_operation_t mbedtls_cipher_get_operation( const mbedtls_ci * parameter-verification failure. * \return A cipher-specific error code on failure. */ -int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const unsigned char *key, - int key_bitlen, const mbedtls_operation_t operation ); +int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, + const unsigned char *key, + int key_bitlen, + const mbedtls_operation_t operation ); #if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING) /** @@ -553,7 +570,7 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const unsigned char *k * * The default passing mode is PKCS7 padding. * - * \param ctx The generic cipher context. + * \param ctx The generic cipher context. This must be initialized. * \param mode The padding mode. * * \return \c 0 on success. @@ -562,7 +579,8 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const unsigned char *k * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA if the cipher mode * does not support padding. */ -int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_cipher_padding_t mode ); +int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, + mbedtls_cipher_padding_t mode ); #endif /* MBEDTLS_CIPHER_MODE_WITH_PADDING */ /** @@ -572,8 +590,9 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_ciph * \note Some ciphers do not use IVs nor nonce. For these * ciphers, this function has no effect. * - * \param ctx The generic cipher context. - * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. + * \param ctx The generic cipher context. This must be initialized. + * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. This + * must be a readable buffer of at least \p iv_len Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. * @@ -582,12 +601,13 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_ciph * parameter-verification failure. */ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, - const unsigned char *iv, size_t iv_len ); + const unsigned char *iv, + size_t iv_len ); /** * \brief This function resets the cipher state. * - * \param ctx The generic cipher context. + * \param ctx The generic cipher context. This must be initialized. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on @@ -599,10 +619,12 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ); /** * \brief This function adds additional data for AEAD ciphers. * Currently supported with GCM and ChaCha20+Poly1305. - * Must be called exactly once, after mbedtls_cipher_reset(). + * This must be called exactly once, after + * mbedtls_cipher_reset(). * - * \param ctx The generic cipher context. - * \param ad The additional data to use. + * \param ctx The generic cipher context. This must be initialized. + * \param ad The additional data to use. If `ad_len > 0`, then this + * must be a readable buffer of at least \p ad_len Bytes. * \param ad_len the Length of \p ad. * * \return \c 0 on success. @@ -627,14 +649,16 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, * mbedtls_cipher_finish(), must have \p ilen as a * multiple of the block size of the cipher. * - * \param ctx The generic cipher context. - * \param input The buffer holding the input data. + * \param ctx The generic cipher context. This must be initialized. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. - * \param output The buffer for the output data. Must be able to hold at - * least \p ilen + block_size. Must not be the same buffer - * as input. + * \param output The buffer for the output data. This must be able to + * hold at least \p ilen + block_size. This must not be the + * same buffer as \p input. * \param olen The length of the output data, to be updated with the - * actual number of Bytes written. + * actual number of Bytes written. This must not be + * \c NULL. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on @@ -652,9 +676,11 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i * contained in it is padded to the size of * the last block, and written to the \p output buffer. * - * \param ctx The generic cipher context. - * \param output The buffer to write data to. Needs block_size available. + * \param ctx The generic cipher context. This must be initialized. + * \param output The buffer to write data to. This needs to be a writable + * buffer of at least \p block_size Bytes. * \param olen The length of the data written to the \p output buffer. + * This may not be \c NULL. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on @@ -672,10 +698,11 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, /** * \brief This function writes a tag for AEAD ciphers. * Currently supported with GCM and ChaCha20+Poly1305. - * Must be called after mbedtls_cipher_finish(). + * This must be called after mbedtls_cipher_finish(). * - * \param ctx The generic cipher context. - * \param tag The buffer to write the tag to. + * \param ctx The generic cipher context. This must be initialized. + * \param tag The buffer to write the tag to. This must be a readable + * boffer of at least \p tag_len Bytes. * \param tag_len The length of the tag to write. * * \return \c 0 on success. @@ -687,10 +714,11 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, /** * \brief This function checks the tag for AEAD ciphers. * Currently supported with GCM and ChaCha20+Poly1305. - * Must be called after mbedtls_cipher_finish(). + * This must be called after mbedtls_cipher_finish(). * - * \param ctx The generic cipher context. - * \param tag The buffer holding the tag. + * \param ctx The generic cipher context. This must be initialized. + * \param tag The buffer holding the tag. This must be a readable + * buffer of at least \p tag_len Bytes. * \param tag_len The length of the tag to check. * * \return \c 0 on success. @@ -704,18 +732,22 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, * \brief The generic all-in-one encryption/decryption function, * for all ciphers except AEAD constructs. * - * \param ctx The generic cipher context. + * \param ctx The generic cipher context. This must be initialized. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. + * If `iv_len > 0`, this must be a readable buffer of at + * least \p Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size * IV. - * \param input The buffer holding the input data. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. - * \param output The buffer for the output data. Must be able to hold at - * least \p ilen + block_size. Must not be the same buffer - * as input. + * \param output The buffer for the output data. This must be able to + * hold at least \p ilen + block_size. This must not be the + * same buffer as \p input. * \param olen The length of the output data, to be updated with the - * actual number of Bytes written. + * actual number of Bytes written. This must not be + * \c NULL. * * \note Some ciphers do not use IVs nor nonce. For these * ciphers, use \p iv = NULL and \p iv_len = 0. @@ -738,19 +770,26 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, /** * \brief The generic autenticated encryption (AEAD) function. * - * \param ctx The generic cipher context. + * \param ctx The generic cipher context. This must be initialized. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. + * This must be a readable buffer of at least \p iv_len + * Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. - * \param ad The additional data to authenticate. + * \param ad The additional data to authenticate. If `ad_len > 0`, + * this must be a readable buffer of at least \p ad_len + * Bytes. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. - * \param output The buffer for the output data. - * Must be able to hold at least \p ilen. + * \param output The buffer for the output data. This must be able to + * hold at least \p ilen. * \param olen The length of the output data, to be updated with the - * actual number of Bytes written. - * \param tag The buffer for the authentication tag. + * actual number of Bytes written. This must not be + * \c NULL. + * \param tag The buffer for the authentication tag. This must be a + * writable buffer of at least \p tag_len Bytes. * \param tag_len The desired length of the authentication tag. * * \return \c 0 on success. @@ -772,19 +811,26 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, * is zeroed out to prevent the unauthentic plaintext being * used, making this interface safer. * - * \param ctx The generic cipher context. + * \param ctx The generic cipher context. This must be initialized. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. + * This must be a readable buffer of at least \p iv_len + * Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. - * \param ad The additional data to be authenticated. + * \param ad The additional data to be authenticated. If `ad_len > 0`, + * this must be a readable buffer of at least \p ad_len + * Bytes. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. * \param output The buffer for the output data. - * Must be able to hold at least \p ilen. + * This must be able to hold at least \p ilen Bytes. * \param olen The length of the output data, to be updated with the - * actual number of Bytes written. - * \param tag The buffer holding the authentication tag. + * actual number of Bytes written. This must not be + * \c NULL. + * \param tag The buffer holding the authentication tag. This must be + * a readable buffer of at least \p tag_len Bytes. * \param tag_len The length of the authentication tag. * * \return \c 0 on success. diff --git a/library/cipher.c b/library/cipher.c index d7acf34ee..d2078f6f6 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -65,6 +65,11 @@ #define mbedtls_free free #endif +#define CIPHER_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ) +#define CIPHER_VALIDATE( cond ) \ + MBEDTLS_INTERNAL_VALIDATE( cond ) + #if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) /* Compare the contents of two buffers in constant time. * Returns 0 if the contents are bitwise identical, otherwise returns @@ -150,6 +155,7 @@ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_values( const mbedtls_ciph void mbedtls_cipher_init( mbedtls_cipher_context_t *ctx ) { + CIPHER_VALIDATE( ctx != NULL ); memset( ctx, 0, sizeof( mbedtls_cipher_context_t ) ); } @@ -199,9 +205,14 @@ int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_in return( 0 ); } -int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const unsigned char *key, - int key_bitlen, const mbedtls_operation_t operation ) +int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, + const unsigned char *key, + int key_bitlen, + const mbedtls_operation_t operation ) { + CIPHER_VALIDATE_RET( operation == MBEDTLS_ENCRYPT || + operation == MBEDTLS_DECRYPT ); + if( NULL == ctx || NULL == ctx->cipher_info ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); @@ -234,9 +245,11 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const unsigned char *k } int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, - const unsigned char *iv, size_t iv_len ) + const unsigned char *iv, + size_t iv_len ) { size_t actual_iv_size; + if( NULL == ctx || NULL == ctx->cipher_info ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); else if( NULL == iv && iv_len != 0 ) @@ -295,6 +308,8 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ) int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, const unsigned char *ad, size_t ad_len ) { + CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); + if( NULL == ctx || NULL == ctx->cipher_info ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); @@ -335,9 +350,13 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i size_t ilen, unsigned char *output, size_t *olen ) { int ret; - size_t block_size = 0; + size_t block_size; - if( NULL == ctx || NULL == ctx->cipher_info || NULL == olen ) + CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( output != NULL ); + CIPHER_VALIDATE_RET( olen != NULL ); + + if( NULL == ctx || NULL == ctx->cipher_info ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -745,7 +764,10 @@ static int get_no_padding( unsigned char *input, size_t input_len, int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, unsigned char *output, size_t *olen ) { - if( NULL == ctx || NULL == ctx->cipher_info || NULL == olen ) + CIPHER_VALIDATE_RET( output != NULL ); + CIPHER_VALIDATE_RET( olen != NULL ); + + if( NULL == ctx || NULL == ctx->cipher_info ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); *olen = 0; @@ -830,10 +852,13 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, } #if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING) -int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_cipher_padding_t mode ) +int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, + mbedtls_cipher_padding_t mode ) { - if( NULL == ctx || - MBEDTLS_MODE_CBC != ctx->cipher_info->mode ) + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + + if( MBEDTLS_MODE_CBC != ctx->cipher_info->mode ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -881,7 +906,9 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_ciph int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, unsigned char *tag, size_t tag_len ) { - if( NULL == ctx || NULL == ctx->cipher_info || NULL == tag ) + CIPHER_VALIDATE_RET( tag != NULL ); + + if( NULL == ctx || NULL == ctx->cipher_info ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( MBEDTLS_ENCRYPT != ctx->operation ) @@ -913,6 +940,8 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, unsigned char check_tag[16]; int ret; + CIPHER_VALIDATE_RET( tag != NULL ); + if( NULL == ctx || NULL == ctx->cipher_info || MBEDTLS_DECRYPT != ctx->operation ) { @@ -976,6 +1005,13 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, int ret; size_t finish_olen; + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); + CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( output != NULL ); + CIPHER_VALIDATE_RET( olen != NULL ); + if( ( ret = mbedtls_cipher_set_iv( ctx, iv, iv_len ) ) != 0 ) return( ret ); @@ -1004,6 +1040,15 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, unsigned char *output, size_t *olen, unsigned char *tag, size_t tag_len ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + CIPHER_VALIDATE_RET( iv != NULL ); + CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); + CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( output != NULL ); + CIPHER_VALIDATE_RET( olen != NULL ); + CIPHER_VALIDATE_RET( tag != NULL ); + #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { @@ -1051,6 +1096,15 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, unsigned char *output, size_t *olen, const unsigned char *tag, size_t tag_len ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + CIPHER_VALIDATE_RET( iv != NULL ); + CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); + CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( output != NULL ); + CIPHER_VALIDATE_RET( olen != NULL ); + CIPHER_VALIDATE_RET( tag != NULL ); + #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 58126bedc..d7a144270 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -72,6 +72,7 @@ add_test_suite(cipher cipher.chacha20) add_test_suite(cipher cipher.chachapoly) add_test_suite(cipher cipher.des) add_test_suite(cipher cipher.gcm) +add_test_suite(cipher cipher.misc) add_test_suite(cipher cipher.null) add_test_suite(cipher cipher.padding) add_test_suite(cmac) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index c5bce7e50..3d559a3c9 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -91,6 +91,264 @@ void cipher_null_args( ) } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void cipher_invalid_param( ) +{ + mbedtls_cipher_context_t invalid_ctx; + mbedtls_cipher_context_t valid_ctx; + + mbedtls_operation_t invalid_operation = 100; + mbedtls_cipher_padding_t valid_mode = MBEDTLS_PADDING_ZEROS; + unsigned char valid_buffer[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 }; + int valid_size = sizeof(valid_buffer); + int valid_bitlen = valid_size * 8; + size_t size_t_var; + + /* mbedtls_cipher_init() */ + TEST_VALID_PARAM( mbedtls_cipher_init( &invalid_ctx ) ); + TEST_VALID_PARAM( mbedtls_cipher_init( &valid_ctx ) ); + + TEST_INVALID_PARAM( mbedtls_cipher_init( NULL ) ); + + /* mbedtls_cipher_setkey() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setkey( &valid_ctx, + valid_buffer, + valid_bitlen, + invalid_operation ) ); + + /* mbedtls_cipher_set_padding_mode() */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_set_padding_mode( NULL, valid_mode ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_set_padding_mode( &invalid_ctx, + valid_mode ) ); + + /* mbedtls_cipher_update() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update( &valid_ctx, + NULL, valid_size, + valid_buffer, + &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update( &valid_ctx, + valid_buffer, valid_size, + NULL, + &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, + NULL ) ); + + /* mbedtls_cipher_finish() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_finish( &valid_ctx, + NULL, + &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_finish( &valid_ctx, + valid_buffer, + NULL ) ); + + /* mbedtls_cipher_write_tag() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_write_tag( &valid_ctx, + NULL, + valid_size ) ); + + /* mbedtls_cipher_check_tag() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_check_tag( &valid_ctx, + NULL, + valid_size ) ); + + /* mbedtls_cipher_crypt() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( NULL, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( &invalid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( &valid_ctx, + NULL, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( &valid_ctx, + valid_buffer, valid_size, + NULL, valid_size, + valid_buffer, &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + NULL, &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_crypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, NULL ) ); + + /* mbedtls_cipher_auth_encrypt() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( NULL, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &invalid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + NULL, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + valid_buffer, valid_size, + NULL, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + NULL, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + NULL, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, NULL, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_encrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + NULL, valid_size ) ); + + /* mbedtls_cipher_auth_decrypt() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( NULL, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &invalid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + NULL, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + valid_buffer, valid_size, + NULL, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + NULL, valid_size, + valid_buffer, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + NULL, &size_t_var, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, NULL, + valid_buffer, valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_auth_decrypt( &valid_ctx, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, valid_size, + valid_buffer, &size_t_var, + NULL, valid_size ) ); + + /* mbedtls_cipher_free() */ + TEST_VALID_PARAM( mbedtls_cipher_free( NULL ) ); +exit: + TEST_VALID_PARAM( mbedtls_cipher_free( &invalid_ctx ) ); + TEST_VALID_PARAM( mbedtls_cipher_free( &valid_ctx ) ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_AES_C */ void cipher_special_behaviours( ) { diff --git a/tests/suites/test_suite_cipher.misc.data b/tests/suites/test_suite_cipher.misc.data new file mode 100644 index 000000000..4ef257fe5 --- /dev/null +++ b/tests/suites/test_suite_cipher.misc.data @@ -0,0 +1,2 @@ +CIPHER - Invalid parameters +cipher_invalid_param: \ No newline at end of file From a539070f822cac577cf49c314573cdfdde14d7f6 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Mon, 17 Dec 2018 11:27:03 +0100 Subject: [PATCH 02/15] Make all parameter validation tests optional --- library/cipher.c | 48 +++---- tests/suites/test_suite_cipher.function | 172 +++++++++++++++++------- 2 files changed, 143 insertions(+), 77 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index d2078f6f6..c45a1a430 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -181,8 +181,8 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ) int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_info_t *cipher_info ) { - if( NULL == cipher_info || NULL == ctx ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( cipher_info != NULL ); memset( ctx, 0, sizeof( mbedtls_cipher_context_t ) ); @@ -210,12 +210,12 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, int key_bitlen, const mbedtls_operation_t operation ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + CIPHER_VALIDATE_RET( key != NULL ); CIPHER_VALIDATE_RET( operation == MBEDTLS_ENCRYPT || operation == MBEDTLS_DECRYPT ); - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - if( ( ctx->cipher_info->flags & MBEDTLS_CIPHER_VARIABLE_KEY_LEN ) == 0 && (int) ctx->cipher_info->key_bitlen != key_bitlen ) { @@ -250,10 +250,9 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, { size_t actual_iv_size; - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - else if( NULL == iv && iv_len != 0 ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); if( NULL == iv && iv_len == 0 ) ctx->iv_size = 0; @@ -296,8 +295,8 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); ctx->unprocessed_len = 0; @@ -308,11 +307,10 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ) int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, const unsigned char *ad, size_t ad_len ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { @@ -352,15 +350,12 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i int ret; size_t block_size; + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); - if( NULL == ctx || NULL == ctx->cipher_info ) - { - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - } - *olen = 0; block_size = mbedtls_cipher_get_block_size( ctx ); @@ -764,12 +759,11 @@ static int get_no_padding( unsigned char *input, size_t input_len, int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, unsigned char *output, size_t *olen ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - *olen = 0; if( MBEDTLS_MODE_CFB == ctx->cipher_info->mode || @@ -906,11 +900,10 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, unsigned char *tag, size_t tag_len ) { + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( tag != NULL ); - if( NULL == ctx || NULL == ctx->cipher_info ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - if( MBEDTLS_ENCRYPT != ctx->operation ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); @@ -940,10 +933,11 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, unsigned char check_tag[16]; int ret; + CIPHER_VALIDATE_RET( ctx != NULL ); + CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( tag != NULL ); - if( NULL == ctx || NULL == ctx->cipher_info || - MBEDTLS_DECRYPT != ctx->operation ) + if( MBEDTLS_DECRYPT != ctx->operation ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 3d559a3c9..dd997b089 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -25,9 +25,6 @@ void mbedtls_cipher_list( ) void cipher_null_args( ) { mbedtls_cipher_context_t ctx; - const mbedtls_cipher_info_t *info = mbedtls_cipher_info_from_type( *( mbedtls_cipher_list() ) ); - unsigned char buf[1] = { 0 }; - size_t olen; mbedtls_cipher_init( &ctx ); @@ -41,53 +38,6 @@ void cipher_null_args( ) TEST_ASSERT( mbedtls_cipher_get_iv_size( &ctx ) == 0 ); TEST_ASSERT( mbedtls_cipher_info_from_string( NULL ) == NULL ); - - TEST_ASSERT( mbedtls_cipher_setup( &ctx, NULL ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_setup( NULL, info ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_cipher_setkey( NULL, buf, 0, MBEDTLS_ENCRYPT ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_setkey( &ctx, buf, 0, MBEDTLS_ENCRYPT ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_cipher_set_iv( NULL, buf, 0 ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_set_iv( &ctx, buf, 0 ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_cipher_reset( NULL ) == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_reset( &ctx ) == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - -#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) - TEST_ASSERT( mbedtls_cipher_update_ad( NULL, buf, 0 ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_update_ad( &ctx, buf, 0 ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); -#endif - - TEST_ASSERT( mbedtls_cipher_update( NULL, buf, 0, buf, &olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_update( &ctx, buf, 0, buf, &olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_cipher_finish( NULL, buf, &olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_finish( &ctx, buf, &olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - -#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) - TEST_ASSERT( mbedtls_cipher_write_tag( NULL, buf, olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_write_tag( &ctx, buf, olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - - TEST_ASSERT( mbedtls_cipher_check_tag( NULL, buf, olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_cipher_check_tag( &ctx, buf, olen ) - == MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); -#endif } /* END_CASE */ @@ -97,11 +47,15 @@ void cipher_invalid_param( ) mbedtls_cipher_context_t invalid_ctx; mbedtls_cipher_context_t valid_ctx; + mbedtls_operation_t valid_operation = MBEDTLS_ENCRYPT; mbedtls_operation_t invalid_operation = 100; mbedtls_cipher_padding_t valid_mode = MBEDTLS_PADDING_ZEROS; unsigned char valid_buffer[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 }; int valid_size = sizeof(valid_buffer); int valid_bitlen = valid_size * 8; + const mbedtls_cipher_info_t *valid_info = mbedtls_cipher_info_from_type( + *( mbedtls_cipher_list() ) ); + size_t size_t_var; /* mbedtls_cipher_init() */ @@ -110,7 +64,33 @@ void cipher_invalid_param( ) TEST_INVALID_PARAM( mbedtls_cipher_init( NULL ) ); + /* mbedtls_cipher_setup() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setup( NULL, valid_info ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setup( &valid_ctx, NULL ) ); + /* mbedtls_cipher_setkey() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setkey( NULL, + valid_buffer, + valid_bitlen, + valid_operation ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setkey( &invalid_ctx, + valid_buffer, + valid_bitlen, + valid_operation ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_setkey( &valid_ctx, + NULL, + valid_bitlen, + valid_operation ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setkey( &valid_ctx, @@ -118,14 +98,72 @@ void cipher_invalid_param( ) valid_bitlen, invalid_operation ) ); + /* mbedtls_cipher_set_iv() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_set_iv( NULL, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_set_iv( &invalid_ctx, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_set_iv( &valid_ctx, + NULL, + valid_size ) ); + + /* mbedtls_cipher_reset() */ + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_reset( NULL ) ); + TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_reset( &invalid_ctx ) ); + +#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) + /* mbedtls_cipher_update_ad() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update_ad( NULL, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update_ad( &invalid_ctx, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update_ad( &valid_ctx, + NULL, + valid_size ) ); +#endif /* defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) */ + +#if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING) /* mbedtls_cipher_set_padding_mode() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_padding_mode( NULL, valid_mode ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_padding_mode( &invalid_ctx, valid_mode ) ); +#endif /* mbedtls_cipher_update() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update( NULL, + valid_buffer, + valid_size, + valid_buffer, + &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_update( &invalid_ctx, + valid_buffer, + valid_size, + valid_buffer, + &size_t_var ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_update( &valid_ctx, @@ -146,6 +184,16 @@ void cipher_invalid_param( ) NULL ) ); /* mbedtls_cipher_finish() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_finish( NULL, + valid_buffer, + &size_t_var ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_finish( &invalid_ctx, + valid_buffer, + &size_t_var ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_finish( &valid_ctx, @@ -157,7 +205,18 @@ void cipher_invalid_param( ) valid_buffer, NULL ) ); +#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) /* mbedtls_cipher_write_tag() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_write_tag( NULL, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_write_tag( &invalid_ctx, + valid_buffer, + valid_size ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_write_tag( &valid_ctx, @@ -165,11 +224,22 @@ void cipher_invalid_param( ) valid_size ) ); /* mbedtls_cipher_check_tag() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_check_tag( NULL, + valid_buffer, + valid_size ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + mbedtls_cipher_check_tag( &invalid_ctx, + valid_buffer, + valid_size ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_check_tag( &valid_ctx, NULL, valid_size ) ); +#endif /* defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) */ /* mbedtls_cipher_crypt() */ TEST_INVALID_PARAM_RET( @@ -209,6 +279,7 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, NULL ) ); +#if defined(MBEDTLS_CIPHER_MODE_AEAD) /* mbedtls_cipher_auth_encrypt() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, @@ -340,6 +411,7 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, &size_t_var, NULL, valid_size ) ); +#endif /* defined(MBEDTLS_CIPHER_MODE_AEAD) */ /* mbedtls_cipher_free() */ TEST_VALID_PARAM( mbedtls_cipher_free( NULL ) ); From d5913bc115c8850ac05bec7dba635b014110f80e Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 18 Dec 2018 14:59:16 +0100 Subject: [PATCH 03/15] Improve documentation of the parameter validation in the Cipher module --- include/mbedtls/cipher.h | 103 +++++++++++++++++++++++---------------- 1 file changed, 61 insertions(+), 42 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 4df10e802..1e81ac256 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -336,12 +336,12 @@ const int *mbedtls_cipher_list( void ); * \brief This function retrieves the cipher-information * structure associated with the given cipher name. * - * \param cipher_name Name of the cipher to search for. This can be \c NULL. + * \param cipher_name Name of the cipher to search for. This must not be + * \c NULL. * * \return The cipher information structure associated with the * given \p cipher_name. - * \return \c NULL if the associated cipher information is not found - * or if \p cipher_name is \c NULL. + * \return \c NULL if the associated cipher information is not found. */ const mbedtls_cipher_info_t *mbedtls_cipher_info_from_string( const char *cipher_name ); @@ -388,7 +388,8 @@ void mbedtls_cipher_init( mbedtls_cipher_context_t *ctx ); * responsibility of the caller. * * \param ctx The context to be freed. If this is \c NULL, the - * function has no effect. + * function has no effect, otherwise this must point to an + * initialized context. */ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); @@ -419,8 +420,8 @@ int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, * * \param ctx The context of the cipher. This must be initialized. * - * \return The size of the blocks of the cipher. - * \return 0 if \p ctx has not been initialized. + * \return The block size of the underlying cipher. + * \return \c 0 if \p ctx has not been initialized. */ static inline unsigned int mbedtls_cipher_get_block_size( const mbedtls_cipher_context_t *ctx ) @@ -544,9 +545,8 @@ static inline mbedtls_operation_t mbedtls_cipher_get_operation( /** * \brief This function sets the key to use with the given context. * - * \param ctx The generic cipher context. This must be initialized - * using mbedtls_cipher_info_from_type() or - * mbedtls_cipher_info_from_string(). + * \param ctx The generic cipher context. This must be initialized and + * bound to a cipher information structure. * \param key The key to use. This must be a readable buffer of at * least \p key_bitlen Bits. * \param key_bitlen The key length to use, in Bits. @@ -570,7 +570,8 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, * * The default passing mode is PKCS7 padding. * - * \param ctx The generic cipher context. This must be initialized. + * \param ctx The generic cipher context. This must be initialized and + * bound to a cipher information structure. * \param mode The padding mode. * * \return \c 0 on success. @@ -590,9 +591,11 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, * \note Some ciphers do not use IVs nor nonce. For these * ciphers, this function has no effect. * - * \param ctx The generic cipher context. This must be initialized. - * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. This - * must be a readable buffer of at least \p iv_len Bytes. + * \param ctx The generic cipher context. This must be initialized and + * bound to a cipher information structure. + * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. If + * `iv_len > 0`, this may be \c NULL, otherwise this must be a + * readable buffer of at least \p iv_len Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. * @@ -624,8 +627,9 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ); * * \param ctx The generic cipher context. This must be initialized. * \param ad The additional data to use. If `ad_len > 0`, then this - * must be a readable buffer of at least \p ad_len Bytes. - * \param ad_len the Length of \p ad. + * must be a readable buffer of at least \p ad_len Bytes, + * otherwise this may be \c NULL. + * \param ad_len the Length of \p ad Bytes. * * \return \c 0 on success. * \return A specific error code on failure. @@ -649,12 +653,14 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, * mbedtls_cipher_finish(), must have \p ilen as a * multiple of the block size of the cipher. * - * \param ctx The generic cipher context. This must be initialized. + * \param ctx The generic cipher context. This must be initialized and + * bound to a key. * \param input The buffer holding the input data. This must be a - * readable buffer of at least \p ilen Bytes. + * readable buffer of at least \p ilen Bytes. If + * `ilen == 0`, this may be \c NULL. * \param ilen The length of the input data. * \param output The buffer for the output data. This must be able to - * hold at least \p ilen + block_size. This must not be the + * hold at least `ilen + block_size`. This must not be the * same buffer as \p input. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be @@ -676,7 +682,8 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i * contained in it is padded to the size of * the last block, and written to the \p output buffer. * - * \param ctx The generic cipher context. This must be initialized. + * \param ctx The generic cipher context. This must be initialized and + * bound to a key. * \param output The buffer to write data to. This needs to be a writable * buffer of at least \p block_size Bytes. * \param olen The length of the data written to the \p output buffer. @@ -700,9 +707,13 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, * Currently supported with GCM and ChaCha20+Poly1305. * This must be called after mbedtls_cipher_finish(). * - * \param ctx The generic cipher context. This must be initialized. - * \param tag The buffer to write the tag to. This must be a readable - * boffer of at least \p tag_len Bytes. + * \param ctx The generic cipher context. This must be initialized, + * bound to a key, and have just completed a cipher + * operation through mbedtls_cipher_finish() the tag for + * which should be written. + * \param tag The buffer to write the tag to. This must be a writable + * buffer of at least \p tag_len Bytes. If `tag_len == 0`, + * this may be \c NULL. * \param tag_len The length of the tag to write. * * \return \c 0 on success. @@ -717,8 +728,9 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, * This must be called after mbedtls_cipher_finish(). * * \param ctx The generic cipher context. This must be initialized. - * \param tag The buffer holding the tag. This must be a readable - * buffer of at least \p tag_len Bytes. + * \param tag The buffer holding the tag. If `tag_len > 0`, then this + * must be a readable buffer of at least \p tag_len Bytes, + * otherwise this may be \c NULL. * \param tag_len The length of the tag to check. * * \return \c 0 on success. @@ -735,15 +747,16 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, * \param ctx The generic cipher context. This must be initialized. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. * If `iv_len > 0`, this must be a readable buffer of at - * least \p Bytes. + * least \p iv_len Bytes, otherwise this may be \c NULL. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size * IV. - * \param input The buffer holding the input data. This must be a - * readable buffer of at least \p ilen Bytes. - * \param ilen The length of the input data. + * \param input The buffer holding the input data. If `ilen > 0`, then + * this must be a readable buffer of at least \p ilen + * Bytes, otherwise this may be \c NULL. + * \param ilen The length of the input data in Bytes. * \param output The buffer for the output data. This must be able to - * hold at least \p ilen + block_size. This must not be the + * hold at least `ilen + block_size`. This must not be the * same buffer as \p input. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be @@ -770,7 +783,8 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, /** * \brief The generic autenticated encryption (AEAD) function. * - * \param ctx The generic cipher context. This must be initialized. + * \param ctx The generic cipher context. This must be initialized and + * bound to a key. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. * This must be a readable buffer of at least \p iv_len * Bytes. @@ -778,18 +792,20 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, * This parameter is discarded by ciphers with fixed-size IV. * \param ad The additional data to authenticate. If `ad_len > 0`, * this must be a readable buffer of at least \p ad_len - * Bytes. + * Bytes, otherwise this may be \c NULL. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. This must be a - * readable buffer of at least \p ilen Bytes. + * \param input The buffer holding the input data. If `ilen > 0`, then + * this must be a readable buffer of at least \p ilen + * Bytes, otherwise this may be \c NULL. * \param ilen The length of the input data. * \param output The buffer for the output data. This must be able to - * hold at least \p ilen. + * hold at least \p ilen Bytes. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be * \c NULL. - * \param tag The buffer for the authentication tag. This must be a - * writable buffer of at least \p tag_len Bytes. + * \param tag The buffer for the authentication tag. If `tag_len > 0`, + * then this must be a writable buffer of at least + * \p tag_len Bytes, otherwise this may be \c NULL. * \param tag_len The desired length of the authentication tag. * * \return \c 0 on success. @@ -811,7 +827,8 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, * is zeroed out to prevent the unauthentic plaintext being * used, making this interface safer. * - * \param ctx The generic cipher context. This must be initialized. + * \param ctx The generic cipher context. This must be initialized and + * and bound to a key. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. * This must be a readable buffer of at least \p iv_len * Bytes. @@ -819,18 +836,20 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, * This parameter is discarded by ciphers with fixed-size IV. * \param ad The additional data to be authenticated. If `ad_len > 0`, * this must be a readable buffer of at least \p ad_len - * Bytes. + * Bytes, otherwise this may be \c NULL. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. This must be a - * readable buffer of at least \p ilen Bytes. + * \param input The buffer holding the input data. If `ilen > 0`, then + * this must be a readable buffer of at least \p ilen + * Bytes, otherwise, this may be \c NULL. * \param ilen The length of the input data. * \param output The buffer for the output data. * This must be able to hold at least \p ilen Bytes. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be * \c NULL. - * \param tag The buffer holding the authentication tag. This must be - * a readable buffer of at least \p tag_len Bytes. + * \param tag The buffer holding the authentication tag. If + * `tag_len > 0`, then this must be a readable buffer of at + * least \p tag_len Bytes, otherwise this can be \c NULL. * \param tag_len The length of the authentication tag. * * \return \c 0 on success. From c29d94c7bfbe47c5c298720e4e0e970d415b1225 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 18 Dec 2018 15:09:56 +0100 Subject: [PATCH 04/15] Account for optional NULL buffer arguments in the Cipher module --- library/cipher.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index c45a1a430..14ff37111 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -254,9 +254,6 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); - if( NULL == iv && iv_len == 0 ) - ctx->iv_size = 0; - /* avoid buffer overflow in ctx->iv */ if( iv_len > MBEDTLS_MAX_IV_LENGTH ) return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); @@ -352,7 +349,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); - CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); @@ -902,7 +899,7 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, { CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); - CIPHER_VALIDATE_RET( tag != NULL ); + CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( MBEDTLS_ENCRYPT != ctx->operation ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); @@ -935,7 +932,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); - CIPHER_VALIDATE_RET( tag != NULL ); + CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( MBEDTLS_DECRYPT != ctx->operation ) { @@ -1002,7 +999,7 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); - CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); @@ -1038,10 +1035,10 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); - CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); - CIPHER_VALIDATE_RET( tag != NULL ); + CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) @@ -1094,10 +1091,10 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); - CIPHER_VALIDATE_RET( input != NULL ); + CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); - CIPHER_VALIDATE_RET( tag != NULL ); + CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) From 90b8d4a11e36928d9258985f3aa914e9cd2eda1a Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 18 Dec 2018 16:12:34 +0100 Subject: [PATCH 05/15] Include static cipher functions in the parameter validation scheme --- include/mbedtls/cipher.h | 35 ++++++----- tests/suites/test_suite_cipher.function | 64 ++++++++++++++------- tests/suites/test_suite_cipher.padding.data | 3 - 3 files changed, 65 insertions(+), 37 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 1e81ac256..3c6077b04 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -36,6 +36,7 @@ #endif #include +#include "mbedtls/platform_util.h" #if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CCM_C) || defined(MBEDTLS_CHACHAPOLY_C) #define MBEDTLS_CIPHER_MODE_AEAD @@ -426,8 +427,8 @@ int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, static inline unsigned int mbedtls_cipher_get_block_size( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return 0; + MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); + MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); return ctx->cipher_info->block_size; } @@ -444,8 +445,8 @@ static inline unsigned int mbedtls_cipher_get_block_size( static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return MBEDTLS_MODE_NONE; + MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, MBEDTLS_MODE_NONE ); + MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, MBEDTLS_MODE_NONE ); return ctx->cipher_info->mode; } @@ -463,8 +464,8 @@ static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( static inline int mbedtls_cipher_get_iv_size( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return 0; + MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); + MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); if( ctx->iv_size != 0 ) return (int) ctx->iv_size; @@ -483,8 +484,10 @@ static inline int mbedtls_cipher_get_iv_size( static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return MBEDTLS_CIPHER_NONE; + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx != NULL, MBEDTLS_CIPHER_NONE ); + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx->cipher_info != NULL, MBEDTLS_CIPHER_NONE ); return ctx->cipher_info->type; } @@ -501,8 +504,8 @@ static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( static inline const char *mbedtls_cipher_get_name( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return 0; + MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); + MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); return ctx->cipher_info->name; } @@ -519,8 +522,10 @@ static inline const char *mbedtls_cipher_get_name( static inline int mbedtls_cipher_get_key_bitlen( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return MBEDTLS_KEY_LENGTH_NONE; + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx != NULL, MBEDTLS_KEY_LENGTH_NONE ); + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx->cipher_info != NULL, MBEDTLS_KEY_LENGTH_NONE ); return (int) ctx->cipher_info->key_bitlen; } @@ -536,8 +541,10 @@ static inline int mbedtls_cipher_get_key_bitlen( static inline mbedtls_operation_t mbedtls_cipher_get_operation( const mbedtls_cipher_context_t *ctx ) { - if( NULL == ctx || NULL == ctx->cipher_info ) - return MBEDTLS_OPERATION_NONE; + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx != NULL, MBEDTLS_OPERATION_NONE ); + MBEDTLS_INTERNAL_VALIDATE_RET( + ctx->cipher_info != NULL, MBEDTLS_OPERATION_NONE ); return ctx->operation; } diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index dd997b089..fba32fdd6 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -21,26 +21,6 @@ void mbedtls_cipher_list( ) } /* END_CASE */ -/* BEGIN_CASE */ -void cipher_null_args( ) -{ - mbedtls_cipher_context_t ctx; - - mbedtls_cipher_init( &ctx ); - - TEST_ASSERT( mbedtls_cipher_get_block_size( NULL ) == 0 ); - TEST_ASSERT( mbedtls_cipher_get_block_size( &ctx ) == 0 ); - - TEST_ASSERT( mbedtls_cipher_get_cipher_mode( NULL ) == MBEDTLS_MODE_NONE ); - TEST_ASSERT( mbedtls_cipher_get_cipher_mode( &ctx ) == MBEDTLS_MODE_NONE ); - - TEST_ASSERT( mbedtls_cipher_get_iv_size( NULL ) == 0 ); - TEST_ASSERT( mbedtls_cipher_get_iv_size( &ctx ) == 0 ); - - TEST_ASSERT( mbedtls_cipher_info_from_string( NULL ) == NULL ); -} -/* END_CASE */ - /* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ void cipher_invalid_param( ) { @@ -72,6 +52,50 @@ void cipher_invalid_param( ) MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setup( &valid_ctx, NULL ) ); + /* mbedtls_cipher_get_block_size() */ + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_block_size( NULL ) ); + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_block_size( &invalid_ctx ) ); + + /* mbedtls_cipher_get_cipher_mode() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_MODE_NONE, + mbedtls_cipher_get_cipher_mode( NULL ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_MODE_NONE, + mbedtls_cipher_get_cipher_mode( &invalid_ctx ) ); + + /* mbedtls_cipher_get_iv_size() */ + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_iv_size( NULL ) ); + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_iv_size( &invalid_ctx ) ); + + /* mbedtls_cipher_get_type() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_CIPHER_NONE, + mbedtls_cipher_get_type( NULL ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_CIPHER_NONE, + mbedtls_cipher_get_type( &invalid_ctx ) ); + + /* mbedtls_cipher_get_name() */ + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_name( NULL ) ); + TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_name( &invalid_ctx ) ); + + /* mbedtls_cipher_get_key_bitlen() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_KEY_LENGTH_NONE, + mbedtls_cipher_get_key_bitlen( NULL ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_KEY_LENGTH_NONE, + mbedtls_cipher_get_key_bitlen( &invalid_ctx ) ); + + /* mbedtls_cipher_get_operation() */ + TEST_INVALID_PARAM_RET( + MBEDTLS_OPERATION_NONE, + mbedtls_cipher_get_operation( NULL ) ); + TEST_INVALID_PARAM_RET( + MBEDTLS_OPERATION_NONE, + mbedtls_cipher_get_operation( &invalid_ctx ) ); + /* mbedtls_cipher_setkey() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, diff --git a/tests/suites/test_suite_cipher.padding.data b/tests/suites/test_suite_cipher.padding.data index 1c0ba0980..dc4c9d70b 100644 --- a/tests/suites/test_suite_cipher.padding.data +++ b/tests/suites/test_suite_cipher.padding.data @@ -1,9 +1,6 @@ Cipher list mbedtls_cipher_list: -Cipher null/uninitialised arguments -cipher_null_args: - Set padding with AES-CBC depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7 set_padding:MBEDTLS_CIPHER_AES_128_CBC:MBEDTLS_PADDING_PKCS7:0 From 5b01f8b3ae97b889ae7f68e15b5c6c65cd33f36c Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 18 Dec 2018 16:15:12 +0100 Subject: [PATCH 06/15] Add a new line at the end of the test data file --- tests/suites/test_suite_cipher.misc.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_cipher.misc.data b/tests/suites/test_suite_cipher.misc.data index 4ef257fe5..07783c300 100644 --- a/tests/suites/test_suite_cipher.misc.data +++ b/tests/suites/test_suite_cipher.misc.data @@ -1,2 +1,2 @@ CIPHER - Invalid parameters -cipher_invalid_param: \ No newline at end of file +cipher_invalid_param: From d409285cfa2c88a547f61811ebac5a9fcedd720a Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Tue, 18 Dec 2018 17:02:05 +0100 Subject: [PATCH 07/15] Add a change log entry --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 0ead78009..839cefdd5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -47,6 +47,8 @@ API Changes in favour a new generic error MBEDTLS_ERR_CAMELLIA_BAD_INPUT_DATA. * Deprecate the Blowfish error MBEDTLS_ERR_BLOWFISH_INVALID_KEY_LENGTH in favour of a new generic error MBEDTLS_ERR_BLOWFISH_BAD_INPUT_DATA. + * Add validation checks for input parameters to functions in the Cipher + module. New deprecations * Deprecate mbedtls_ctr_drbg_update and mbedtls_hmac_drbg_update From 6df25e793009466a857140127732dce01db9b686 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 11:22:40 +0100 Subject: [PATCH 08/15] Increase strictness of NULL parameter validity in Cipher's doxygen --- include/mbedtls/cipher.h | 60 ++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 3c6077b04..dc7644396 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -600,9 +600,8 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, * * \param ctx The generic cipher context. This must be initialized and * bound to a cipher information structure. - * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. If - * `iv_len > 0`, this may be \c NULL, otherwise this must be a - * readable buffer of at least \p iv_len Bytes. + * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. This + * must be a readable buffer of at least \p iv_len Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. * @@ -633,9 +632,8 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ); * mbedtls_cipher_reset(). * * \param ctx The generic cipher context. This must be initialized. - * \param ad The additional data to use. If `ad_len > 0`, then this - * must be a readable buffer of at least \p ad_len Bytes, - * otherwise this may be \c NULL. + * \param ad The additional data to use. This must be a readable + * buffer of at least \p ad_len Bytes. * \param ad_len the Length of \p ad Bytes. * * \return \c 0 on success. @@ -663,8 +661,7 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, * \param ctx The generic cipher context. This must be initialized and * bound to a key. * \param input The buffer holding the input data. This must be a - * readable buffer of at least \p ilen Bytes. If - * `ilen == 0`, this may be \c NULL. + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. * \param output The buffer for the output data. This must be able to * hold at least `ilen + block_size`. This must not be the @@ -719,8 +716,7 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, * operation through mbedtls_cipher_finish() the tag for * which should be written. * \param tag The buffer to write the tag to. This must be a writable - * buffer of at least \p tag_len Bytes. If `tag_len == 0`, - * this may be \c NULL. + * buffer of at least \p tag_len Bytes. * \param tag_len The length of the tag to write. * * \return \c 0 on success. @@ -735,9 +731,8 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, * This must be called after mbedtls_cipher_finish(). * * \param ctx The generic cipher context. This must be initialized. - * \param tag The buffer holding the tag. If `tag_len > 0`, then this - * must be a readable buffer of at least \p tag_len Bytes, - * otherwise this may be \c NULL. + * \param tag The buffer holding the tag. This must be a readable + * buffer of at least \p tag_len Bytes. * \param tag_len The length of the tag to check. * * \return \c 0 on success. @@ -753,14 +748,13 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, * * \param ctx The generic cipher context. This must be initialized. * \param iv The IV to use, or NONCE_COUNTER for CTR-mode ciphers. - * If `iv_len > 0`, this must be a readable buffer of at - * least \p iv_len Bytes, otherwise this may be \c NULL. + * This must be a readable buffer of at least \p iv_len + * Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size * IV. - * \param input The buffer holding the input data. If `ilen > 0`, then - * this must be a readable buffer of at least \p ilen - * Bytes, otherwise this may be \c NULL. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data in Bytes. * \param output The buffer for the output data. This must be able to * hold at least `ilen + block_size`. This must not be the @@ -797,22 +791,19 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, * Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. - * \param ad The additional data to authenticate. If `ad_len > 0`, - * this must be a readable buffer of at least \p ad_len - * Bytes, otherwise this may be \c NULL. + * \param ad The additional data to authenticate. This must be a + * readable buffer of at least \p ad_len Bytes. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. If `ilen > 0`, then - * this must be a readable buffer of at least \p ilen - * Bytes, otherwise this may be \c NULL. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. * \param output The buffer for the output data. This must be able to * hold at least \p ilen Bytes. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be * \c NULL. - * \param tag The buffer for the authentication tag. If `tag_len > 0`, - * then this must be a writable buffer of at least - * \p tag_len Bytes, otherwise this may be \c NULL. + * \param tag The buffer for the authentication tag. This must be a + * writable buffer of at least \p tag_len Bytes. * \param tag_len The desired length of the authentication tag. * * \return \c 0 on success. @@ -841,22 +832,19 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, * Bytes. * \param iv_len The IV length for ciphers with variable-size IV. * This parameter is discarded by ciphers with fixed-size IV. - * \param ad The additional data to be authenticated. If `ad_len > 0`, - * this must be a readable buffer of at least \p ad_len - * Bytes, otherwise this may be \c NULL. + * \param ad The additional data to be authenticated. This must be a + * readable buffer of at least \p ad_len Bytes. * \param ad_len The length of \p ad. - * \param input The buffer holding the input data. If `ilen > 0`, then - * this must be a readable buffer of at least \p ilen - * Bytes, otherwise, this may be \c NULL. + * \param input The buffer holding the input data. This must be a + * readable buffer of at least \p ilen Bytes. * \param ilen The length of the input data. * \param output The buffer for the output data. * This must be able to hold at least \p ilen Bytes. * \param olen The length of the output data, to be updated with the * actual number of Bytes written. This must not be * \c NULL. - * \param tag The buffer holding the authentication tag. If - * `tag_len > 0`, then this must be a readable buffer of at - * least \p tag_len Bytes, otherwise this can be \c NULL. + * \param tag The buffer holding the authentication tag. This must be + * a readable buffer of at least \p tag_len Bytes. * \param tag_len The length of the authentication tag. * * \return \c 0 on success. From 95070a828660f8964bd4bd6b52162481873d685d Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 14:48:37 +0100 Subject: [PATCH 09/15] Make some cipher parameter validation unconditional --- include/mbedtls/cipher.h | 24 +++--- library/cipher.c | 39 ++++++--- tests/suites/test_suite_cipher.function | 101 +++++++++++------------- 3 files changed, 88 insertions(+), 76 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index dc7644396..520055409 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -428,7 +428,8 @@ static inline unsigned int mbedtls_cipher_get_block_size( const mbedtls_cipher_context_t *ctx ) { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); - MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); + if( ctx->cipher_info == NULL ) + return 0; return ctx->cipher_info->block_size; } @@ -446,7 +447,8 @@ static inline mbedtls_cipher_mode_t mbedtls_cipher_get_cipher_mode( const mbedtls_cipher_context_t *ctx ) { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, MBEDTLS_MODE_NONE ); - MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, MBEDTLS_MODE_NONE ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_MODE_NONE; return ctx->cipher_info->mode; } @@ -465,7 +467,8 @@ static inline int mbedtls_cipher_get_iv_size( const mbedtls_cipher_context_t *ctx ) { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); - MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); + if( ctx->cipher_info == NULL ) + return 0; if( ctx->iv_size != 0 ) return (int) ctx->iv_size; @@ -486,8 +489,8 @@ static inline mbedtls_cipher_type_t mbedtls_cipher_get_type( { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, MBEDTLS_CIPHER_NONE ); - MBEDTLS_INTERNAL_VALIDATE_RET( - ctx->cipher_info != NULL, MBEDTLS_CIPHER_NONE ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_CIPHER_NONE; return ctx->cipher_info->type; } @@ -505,7 +508,8 @@ static inline const char *mbedtls_cipher_get_name( const mbedtls_cipher_context_t *ctx ) { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, 0 ); - MBEDTLS_INTERNAL_VALIDATE_RET( ctx->cipher_info != NULL, 0 ); + if( ctx->cipher_info == NULL ) + return 0; return ctx->cipher_info->name; } @@ -524,8 +528,8 @@ static inline int mbedtls_cipher_get_key_bitlen( { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, MBEDTLS_KEY_LENGTH_NONE ); - MBEDTLS_INTERNAL_VALIDATE_RET( - ctx->cipher_info != NULL, MBEDTLS_KEY_LENGTH_NONE ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_KEY_LENGTH_NONE; return (int) ctx->cipher_info->key_bitlen; } @@ -543,8 +547,8 @@ static inline mbedtls_operation_t mbedtls_cipher_get_operation( { MBEDTLS_INTERNAL_VALIDATE_RET( ctx != NULL, MBEDTLS_OPERATION_NONE ); - MBEDTLS_INTERNAL_VALIDATE_RET( - ctx->cipher_info != NULL, MBEDTLS_OPERATION_NONE ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_OPERATION_NONE; return ctx->operation; } diff --git a/library/cipher.c b/library/cipher.c index 14ff37111..b37fc768c 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -182,7 +182,8 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ) int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_info_t *cipher_info ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( cipher_info != NULL ); + if( cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; memset( ctx, 0, sizeof( mbedtls_cipher_context_t ) ); @@ -211,10 +212,11 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, const mbedtls_operation_t operation ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( key != NULL ); CIPHER_VALIDATE_RET( operation == MBEDTLS_ENCRYPT || operation == MBEDTLS_DECRYPT ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; if( ( ctx->cipher_info->flags & MBEDTLS_CIPHER_VARIABLE_KEY_LEN ) == 0 && (int) ctx->cipher_info->key_bitlen != key_bitlen ) @@ -251,8 +253,9 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, size_t actual_iv_size; CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; /* avoid buffer overflow in ctx->iv */ if( iv_len > MBEDTLS_MAX_IV_LENGTH ) @@ -293,7 +296,8 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; ctx->unprocessed_len = 0; @@ -305,8 +309,9 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, const unsigned char *ad, size_t ad_len ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) @@ -348,10 +353,11 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i size_t block_size; CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; *olen = 0; block_size = mbedtls_cipher_get_block_size( ctx ); @@ -757,9 +763,10 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, unsigned char *output, size_t *olen ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; *olen = 0; @@ -847,7 +854,8 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_cipher_padding_t mode ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; if( MBEDTLS_MODE_CBC != ctx->cipher_info->mode ) { @@ -898,8 +906,9 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, unsigned char *tag, size_t tag_len ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; if( MBEDTLS_ENCRYPT != ctx->operation ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); @@ -931,8 +940,9 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, int ret; CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; if( MBEDTLS_DECRYPT != ctx->operation ) { @@ -997,11 +1007,12 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, size_t finish_olen; CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; if( ( ret = mbedtls_cipher_set_iv( ctx, iv, iv_len ) ) != 0 ) return( ret ); @@ -1032,13 +1043,14 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, unsigned char *tag, size_t tag_len ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) @@ -1088,13 +1100,14 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, const unsigned char *tag, size_t tag_len ) { CIPHER_VALIDATE_RET( ctx != NULL ); - CIPHER_VALIDATE_RET( ctx->cipher_info != NULL ); CIPHER_VALIDATE_RET( iv != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); + if( ctx->cipher_info == NULL ) + return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index fba32fdd6..16327c387 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -48,53 +48,49 @@ void cipher_invalid_param( ) TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setup( NULL, valid_info ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, - mbedtls_cipher_setup( &valid_ctx, NULL ) ); + TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, NULL ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); /* mbedtls_cipher_get_block_size() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_block_size( NULL ) ); - TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_block_size( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_block_size( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_cipher_mode() */ TEST_INVALID_PARAM_RET( MBEDTLS_MODE_NONE, mbedtls_cipher_get_cipher_mode( NULL ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_MODE_NONE, - mbedtls_cipher_get_cipher_mode( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_cipher_mode( &invalid_ctx ) == + MBEDTLS_MODE_NONE ); /* mbedtls_cipher_get_iv_size() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_iv_size( NULL ) ); - TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_iv_size( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_iv_size( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_type() */ TEST_INVALID_PARAM_RET( MBEDTLS_CIPHER_NONE, mbedtls_cipher_get_type( NULL ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_CIPHER_NONE, - mbedtls_cipher_get_type( &invalid_ctx ) ); + TEST_ASSERT( + mbedtls_cipher_get_type( &invalid_ctx ) == + MBEDTLS_CIPHER_NONE); /* mbedtls_cipher_get_name() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_name( NULL ) ); - TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_name( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_name( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_key_bitlen() */ TEST_INVALID_PARAM_RET( MBEDTLS_KEY_LENGTH_NONE, mbedtls_cipher_get_key_bitlen( NULL ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_KEY_LENGTH_NONE, - mbedtls_cipher_get_key_bitlen( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_key_bitlen( &invalid_ctx ) == + MBEDTLS_KEY_LENGTH_NONE ); /* mbedtls_cipher_get_operation() */ TEST_INVALID_PARAM_RET( MBEDTLS_OPERATION_NONE, mbedtls_cipher_get_operation( NULL ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_OPERATION_NONE, - mbedtls_cipher_get_operation( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_get_operation( &invalid_ctx ) == + MBEDTLS_OPERATION_NONE ); /* mbedtls_cipher_setkey() */ TEST_INVALID_PARAM_RET( @@ -103,12 +99,12 @@ void cipher_invalid_param( ) valid_buffer, valid_bitlen, valid_operation ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_setkey( &invalid_ctx, valid_buffer, valid_bitlen, - valid_operation ) ); + valid_operation ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setkey( &valid_ctx, @@ -128,11 +124,11 @@ void cipher_invalid_param( ) mbedtls_cipher_set_iv( NULL, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_set_iv( &invalid_ctx, valid_buffer, - valid_size ) ); + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_iv( &valid_ctx, @@ -142,8 +138,8 @@ void cipher_invalid_param( ) /* mbedtls_cipher_reset() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_reset( NULL ) ); - TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, - mbedtls_cipher_reset( &invalid_ctx ) ); + TEST_ASSERT( mbedtls_cipher_reset( &invalid_ctx ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) /* mbedtls_cipher_update_ad() */ @@ -152,11 +148,11 @@ void cipher_invalid_param( ) mbedtls_cipher_update_ad( NULL, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_update_ad( &invalid_ctx, valid_buffer, - valid_size ) ); + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_update_ad( &valid_ctx, @@ -168,9 +164,8 @@ void cipher_invalid_param( ) /* mbedtls_cipher_set_padding_mode() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_padding_mode( NULL, valid_mode ) ); - TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, - mbedtls_cipher_set_padding_mode( &invalid_ctx, - valid_mode ) ); + TEST_ASSERT( mbedtls_cipher_set_padding_mode( &invalid_ctx, valid_mode ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #endif /* mbedtls_cipher_update() */ @@ -181,13 +176,13 @@ void cipher_invalid_param( ) valid_size, valid_buffer, &size_t_var ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_update( &invalid_ctx, valid_buffer, valid_size, valid_buffer, - &size_t_var ) ); + &size_t_var ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_update( &valid_ctx, @@ -213,11 +208,11 @@ void cipher_invalid_param( ) mbedtls_cipher_finish( NULL, valid_buffer, &size_t_var ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_finish( &invalid_ctx, valid_buffer, - &size_t_var ) ); + &size_t_var ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_finish( &valid_ctx, @@ -236,11 +231,11 @@ void cipher_invalid_param( ) mbedtls_cipher_write_tag( NULL, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_write_tag( &invalid_ctx, valid_buffer, - valid_size ) ); + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_write_tag( &valid_ctx, @@ -253,11 +248,11 @@ void cipher_invalid_param( ) mbedtls_cipher_check_tag( NULL, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_check_tag( &invalid_ctx, valid_buffer, - valid_size ) ); + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_check_tag( &valid_ctx, @@ -272,12 +267,12 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, &size_t_var ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_crypt( &invalid_ctx, valid_buffer, valid_size, valid_buffer, valid_size, - valid_buffer, &size_t_var ) ); + valid_buffer, &size_t_var ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_crypt( &valid_ctx, @@ -313,14 +308,14 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, &size_t_var, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_auth_encrypt( &invalid_ctx, valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, &size_t_var, - valid_buffer, valid_size ) ); + valid_buffer, valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_auth_encrypt( &valid_ctx, @@ -379,14 +374,14 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, &size_t_var, valid_buffer, valid_size ) ); - TEST_INVALID_PARAM_RET( - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, + TEST_ASSERT( mbedtls_cipher_auth_decrypt( &invalid_ctx, valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, &size_t_var, - valid_buffer, valid_size ) ); + valid_buffer, valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_auth_decrypt( &valid_ctx, From e4b8d28ca78ccc0b3a1df43dcd8696bd1fd4c245 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 16:52:33 +0100 Subject: [PATCH 10/15] Remove imprecise clause from documenting comment --- include/mbedtls/cipher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 520055409..922b6c32c 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -401,7 +401,7 @@ void mbedtls_cipher_free( mbedtls_cipher_context_t *ctx ); * the structure. * * \param ctx The context to initialize. This must be initialized. - * \param cipher_info The cipher to use. This may not be \c NULL. + * \param cipher_info The cipher to use. * * \return \c 0 on success. * \return #MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA on From 1a9df6bcb7d17d32d027659671da2727c9f5548c Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 16:52:45 +0100 Subject: [PATCH 11/15] Improve style in the Cipher module --- library/cipher.c | 69 ++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index b37fc768c..abf268f31 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -86,7 +86,7 @@ static int mbedtls_constant_time_memcmp( const void *v1, const void *v2, size_t for( diff = 0, i = 0; i < len; i++ ) diff |= p1[i] ^ p2[i]; - return (int)diff; + return( (int)diff ); } #endif /* MBEDTLS_GCM_C || MBEDTLS_CHACHAPOLY_C */ @@ -183,7 +183,7 @@ int mbedtls_cipher_setup( mbedtls_cipher_context_t *ctx, const mbedtls_cipher_in { CIPHER_VALIDATE_RET( ctx != NULL ); if( cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); memset( ctx, 0, sizeof( mbedtls_cipher_context_t ) ); @@ -216,7 +216,7 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( operation == MBEDTLS_ENCRYPT || operation == MBEDTLS_DECRYPT ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( ( ctx->cipher_info->flags & MBEDTLS_CIPHER_VARIABLE_KEY_LEN ) == 0 && (int) ctx->cipher_info->key_bitlen != key_bitlen ) @@ -235,13 +235,13 @@ int mbedtls_cipher_setkey( mbedtls_cipher_context_t *ctx, MBEDTLS_MODE_OFB == ctx->cipher_info->mode || MBEDTLS_MODE_CTR == ctx->cipher_info->mode ) { - return ctx->cipher_info->base->setkey_enc_func( ctx->cipher_ctx, key, - ctx->key_bitlen ); + return( ctx->cipher_info->base->setkey_enc_func( ctx->cipher_ctx, key, + ctx->key_bitlen ) ); } if( MBEDTLS_DECRYPT == operation ) - return ctx->cipher_info->base->setkey_dec_func( ctx->cipher_ctx, key, - ctx->key_bitlen ); + return( ctx->cipher_info->base->setkey_dec_func( ctx->cipher_ctx, key, + ctx->key_bitlen ) ); return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -255,7 +255,7 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( iv_len == 0 || iv != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); /* avoid buffer overflow in ctx->iv */ if( iv_len > MBEDTLS_MAX_IV_LENGTH ) @@ -297,7 +297,7 @@ int mbedtls_cipher_reset( mbedtls_cipher_context_t *ctx ) { CIPHER_VALIDATE_RET( ctx != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); ctx->unprocessed_len = 0; @@ -311,13 +311,13 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( ad_len == 0 || ad != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) { - return mbedtls_gcm_starts( (mbedtls_gcm_context *) ctx->cipher_ctx, ctx->operation, - ctx->iv, ctx->iv_size, ad, ad_len ); + return( mbedtls_gcm_starts( (mbedtls_gcm_context *) ctx->cipher_ctx, ctx->operation, + ctx->iv, ctx->iv_size, ad, ad_len ) ); } #endif @@ -337,8 +337,8 @@ int mbedtls_cipher_update_ad( mbedtls_cipher_context_t *ctx, if ( result != 0 ) return( result ); - return mbedtls_chachapoly_update_aad( (mbedtls_chachapoly_context*) ctx->cipher_ctx, - ad, ad_len ); + return( mbedtls_chachapoly_update_aad( (mbedtls_chachapoly_context*) ctx->cipher_ctx, + ad, ad_len ) ); } #endif @@ -357,7 +357,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); *olen = 0; block_size = mbedtls_cipher_get_block_size( ctx ); @@ -382,8 +382,8 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i if( ctx->cipher_info->mode == MBEDTLS_MODE_GCM ) { *olen = ilen; - return mbedtls_gcm_update( (mbedtls_gcm_context *) ctx->cipher_ctx, ilen, input, - output ); + return( mbedtls_gcm_update( (mbedtls_gcm_context *) ctx->cipher_ctx, ilen, input, + output ) ); } #endif @@ -391,14 +391,14 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i if ( ctx->cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) { *olen = ilen; - return mbedtls_chachapoly_update( (mbedtls_chachapoly_context*) ctx->cipher_ctx, - ilen, input, output ); + return( mbedtls_chachapoly_update( (mbedtls_chachapoly_context*) ctx->cipher_ctx, + ilen, input, output ) ); } #endif if ( 0 == block_size ) { - return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); } if( input == output && @@ -461,7 +461,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i { if( 0 == block_size ) { - return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); } /* Encryption: only cache partial blocks @@ -766,7 +766,7 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); *olen = 0; @@ -835,8 +835,8 @@ int mbedtls_cipher_finish( mbedtls_cipher_context_t *ctx, /* Set output size for decryption */ if( MBEDTLS_DECRYPT == ctx->operation ) - return ctx->get_padding( output, mbedtls_cipher_get_block_size( ctx ), - olen ); + return( ctx->get_padding( output, mbedtls_cipher_get_block_size( ctx ), + olen ) ); /* Set output size for encryption */ *olen = mbedtls_cipher_get_block_size( ctx ); @@ -854,10 +854,8 @@ int mbedtls_cipher_set_padding_mode( mbedtls_cipher_context_t *ctx, mbedtls_cipher_padding_t mode ) { CIPHER_VALIDATE_RET( ctx != NULL ); - if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; - if( MBEDTLS_MODE_CBC != ctx->cipher_info->mode ) + if( NULL == ctx->cipher_info || MBEDTLS_MODE_CBC != ctx->cipher_info->mode ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -908,14 +906,15 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( MBEDTLS_ENCRYPT != ctx->operation ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) - return mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, tag, tag_len ); + return( mbedtls_gcm_finish( (mbedtls_gcm_context *) ctx->cipher_ctx, + tag, tag_len ) ); #endif #if defined(MBEDTLS_CHACHAPOLY_C) @@ -925,8 +924,8 @@ int mbedtls_cipher_write_tag( mbedtls_cipher_context_t *ctx, if ( tag_len != 16U ) return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - return mbedtls_chachapoly_finish( (mbedtls_chachapoly_context*) ctx->cipher_ctx, - tag ); + return( mbedtls_chachapoly_finish( (mbedtls_chachapoly_context*) ctx->cipher_ctx, + tag ) ); } #endif @@ -942,7 +941,7 @@ int mbedtls_cipher_check_tag( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ctx != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( MBEDTLS_DECRYPT != ctx->operation ) { @@ -1012,7 +1011,7 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( ( ret = mbedtls_cipher_set_iv( ctx, iv, iv_len ) ) != 0 ) return( ret ); @@ -1050,7 +1049,7 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) @@ -1107,7 +1106,7 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); if( ctx->cipher_info == NULL ) - return MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA; + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) From 516897a44a19c862227e08b8bc5a7ef95a3d4a72 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 17:07:02 +0100 Subject: [PATCH 12/15] Remove unnecessary parameter validation from the Cipher module --- library/cipher.c | 6 ------ tests/suites/test_suite_cipher.function | 22 ---------------------- 2 files changed, 28 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index abf268f31..273997577 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1010,8 +1010,6 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( ilen == 0 || input != NULL ); CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); - if( ctx->cipher_info == NULL ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); if( ( ret = mbedtls_cipher_set_iv( ctx, iv, iv_len ) ) != 0 ) return( ret ); @@ -1048,8 +1046,6 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); - if( ctx->cipher_info == NULL ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) @@ -1105,8 +1101,6 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, CIPHER_VALIDATE_RET( output != NULL ); CIPHER_VALIDATE_RET( olen != NULL ); CIPHER_VALIDATE_RET( tag_len == 0 || tag != NULL ); - if( ctx->cipher_info == NULL ) - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) if( MBEDTLS_MODE_GCM == ctx->cipher_info->mode ) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 16327c387..6073421a9 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -267,12 +267,6 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, valid_size, valid_buffer, &size_t_var ) ); - TEST_ASSERT( - mbedtls_cipher_crypt( &invalid_ctx, - valid_buffer, valid_size, - valid_buffer, valid_size, - valid_buffer, &size_t_var ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_crypt( &valid_ctx, @@ -308,14 +302,6 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, &size_t_var, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_auth_encrypt( &invalid_ctx, - valid_buffer, valid_size, - valid_buffer, valid_size, - valid_buffer, valid_size, - valid_buffer, &size_t_var, - valid_buffer, valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_auth_encrypt( &valid_ctx, @@ -374,14 +360,6 @@ void cipher_invalid_param( ) valid_buffer, valid_size, valid_buffer, &size_t_var, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_auth_decrypt( &invalid_ctx, - valid_buffer, valid_size, - valid_buffer, valid_size, - valid_buffer, valid_size, - valid_buffer, &size_t_var, - valid_buffer, valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_auth_decrypt( &valid_ctx, From a85edd9415af509f949184a64a26c39235863ef6 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 18:06:35 +0100 Subject: [PATCH 13/15] Split the unconditional and conditional parameter validation tests --- tests/suites/test_suite_cipher.function | 183 +++++++++++++++-------- tests/suites/test_suite_cipher.misc.data | 7 +- 2 files changed, 127 insertions(+), 63 deletions(-) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 6073421a9..4e616d3ef 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -21,10 +21,129 @@ void mbedtls_cipher_list( ) } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ -void cipher_invalid_param( ) +/* BEGIN_CASE */ +void cipher_invalid_param_unconditional( ) { + mbedtls_cipher_context_t valid_ctx; mbedtls_cipher_context_t invalid_ctx; + mbedtls_operation_t valid_operation = MBEDTLS_ENCRYPT; + mbedtls_cipher_padding_t valid_mode = MBEDTLS_PADDING_ZEROS; + unsigned char valid_buffer[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07 }; + int valid_size = sizeof(valid_buffer); + int valid_bitlen = valid_size * 8; + const mbedtls_cipher_info_t *valid_info = mbedtls_cipher_info_from_type( + *( mbedtls_cipher_list() ) ); + size_t size_t_var; + + mbedtls_cipher_init( &valid_ctx ); + mbedtls_cipher_setup( &valid_ctx, valid_info ); + mbedtls_cipher_init( &invalid_ctx ); + + /* mbedtls_cipher_setup() */ + TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, NULL ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + + /* mbedtls_cipher_get_block_size() */ + TEST_ASSERT( mbedtls_cipher_get_block_size( &invalid_ctx ) == 0 ); + + /* mbedtls_cipher_get_cipher_mode() */ + TEST_ASSERT( mbedtls_cipher_get_cipher_mode( &invalid_ctx ) == + MBEDTLS_MODE_NONE ); + + /* mbedtls_cipher_get_iv_size() */ + TEST_ASSERT( mbedtls_cipher_get_iv_size( &invalid_ctx ) == 0 ); + + /* mbedtls_cipher_get_type() */ + TEST_ASSERT( + mbedtls_cipher_get_type( &invalid_ctx ) == + MBEDTLS_CIPHER_NONE); + + /* mbedtls_cipher_get_name() */ + TEST_ASSERT( mbedtls_cipher_get_name( &invalid_ctx ) == 0 ); + + /* mbedtls_cipher_get_key_bitlen() */ + TEST_ASSERT( mbedtls_cipher_get_key_bitlen( &invalid_ctx ) == + MBEDTLS_KEY_LENGTH_NONE ); + + /* mbedtls_cipher_get_operation() */ + TEST_ASSERT( mbedtls_cipher_get_operation( &invalid_ctx ) == + MBEDTLS_OPERATION_NONE ); + + /* mbedtls_cipher_setkey() */ + TEST_ASSERT( + mbedtls_cipher_setkey( &invalid_ctx, + valid_buffer, + valid_bitlen, + valid_operation ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + + /* mbedtls_cipher_set_iv() */ + TEST_ASSERT( + mbedtls_cipher_set_iv( &invalid_ctx, + valid_buffer, + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + + /* mbedtls_cipher_reset() */ + TEST_ASSERT( mbedtls_cipher_reset( &invalid_ctx ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + +#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) + /* mbedtls_cipher_update_ad() */ + TEST_ASSERT( + mbedtls_cipher_update_ad( &invalid_ctx, + valid_buffer, + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); +#endif /* defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) */ + +#if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING) + /* mbedtls_cipher_set_padding_mode() */ + TEST_ASSERT( mbedtls_cipher_set_padding_mode( &invalid_ctx, valid_mode ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); +#endif + + /* mbedtls_cipher_update() */ + TEST_ASSERT( + mbedtls_cipher_update( &invalid_ctx, + valid_buffer, + valid_size, + valid_buffer, + &size_t_var ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + + /* mbedtls_cipher_finish() */ + TEST_ASSERT( + mbedtls_cipher_finish( &invalid_ctx, + valid_buffer, + &size_t_var ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + +#if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) + /* mbedtls_cipher_write_tag() */ + TEST_ASSERT( + mbedtls_cipher_write_tag( &invalid_ctx, + valid_buffer, + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + + /* mbedtls_cipher_check_tag() */ + TEST_ASSERT( + mbedtls_cipher_check_tag( &invalid_ctx, + valid_buffer, + valid_size ) == + MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); +#endif /* defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) */ + +exit: + mbedtls_cipher_free( &invalid_ctx ); + mbedtls_cipher_free( &valid_ctx ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_CHECK_PARAMS:!MBEDTLS_PARAM_FAILED_ALT */ +void cipher_invalid_param_conditional( ) +{ mbedtls_cipher_context_t valid_ctx; mbedtls_operation_t valid_operation = MBEDTLS_ENCRYPT; @@ -39,58 +158,43 @@ void cipher_invalid_param( ) size_t size_t_var; /* mbedtls_cipher_init() */ - TEST_VALID_PARAM( mbedtls_cipher_init( &invalid_ctx ) ); TEST_VALID_PARAM( mbedtls_cipher_init( &valid_ctx ) ); - TEST_INVALID_PARAM( mbedtls_cipher_init( NULL ) ); /* mbedtls_cipher_setup() */ + TEST_VALID_PARAM( mbedtls_cipher_setup( &valid_ctx, valid_info ) ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setup( NULL, valid_info ) ); - TEST_ASSERT( mbedtls_cipher_setup( &valid_ctx, NULL ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); /* mbedtls_cipher_get_block_size() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_block_size( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_block_size( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_cipher_mode() */ TEST_INVALID_PARAM_RET( MBEDTLS_MODE_NONE, mbedtls_cipher_get_cipher_mode( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_cipher_mode( &invalid_ctx ) == - MBEDTLS_MODE_NONE ); /* mbedtls_cipher_get_iv_size() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_iv_size( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_iv_size( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_type() */ TEST_INVALID_PARAM_RET( MBEDTLS_CIPHER_NONE, mbedtls_cipher_get_type( NULL ) ); - TEST_ASSERT( - mbedtls_cipher_get_type( &invalid_ctx ) == - MBEDTLS_CIPHER_NONE); /* mbedtls_cipher_get_name() */ TEST_INVALID_PARAM_RET( 0, mbedtls_cipher_get_name( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_name( &invalid_ctx ) == 0 ); /* mbedtls_cipher_get_key_bitlen() */ TEST_INVALID_PARAM_RET( MBEDTLS_KEY_LENGTH_NONE, mbedtls_cipher_get_key_bitlen( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_key_bitlen( &invalid_ctx ) == - MBEDTLS_KEY_LENGTH_NONE ); /* mbedtls_cipher_get_operation() */ TEST_INVALID_PARAM_RET( MBEDTLS_OPERATION_NONE, mbedtls_cipher_get_operation( NULL ) ); - TEST_ASSERT( mbedtls_cipher_get_operation( &invalid_ctx ) == - MBEDTLS_OPERATION_NONE ); /* mbedtls_cipher_setkey() */ TEST_INVALID_PARAM_RET( @@ -99,12 +203,6 @@ void cipher_invalid_param( ) valid_buffer, valid_bitlen, valid_operation ) ); - TEST_ASSERT( - mbedtls_cipher_setkey( &invalid_ctx, - valid_buffer, - valid_bitlen, - valid_operation ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_setkey( &valid_ctx, @@ -124,11 +222,6 @@ void cipher_invalid_param( ) mbedtls_cipher_set_iv( NULL, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_set_iv( &invalid_ctx, - valid_buffer, - valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_iv( &valid_ctx, @@ -138,8 +231,6 @@ void cipher_invalid_param( ) /* mbedtls_cipher_reset() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_reset( NULL ) ); - TEST_ASSERT( mbedtls_cipher_reset( &invalid_ctx ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #if defined(MBEDTLS_GCM_C) || defined(MBEDTLS_CHACHAPOLY_C) /* mbedtls_cipher_update_ad() */ @@ -148,11 +239,6 @@ void cipher_invalid_param( ) mbedtls_cipher_update_ad( NULL, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_update_ad( &invalid_ctx, - valid_buffer, - valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_update_ad( &valid_ctx, @@ -164,8 +250,6 @@ void cipher_invalid_param( ) /* mbedtls_cipher_set_padding_mode() */ TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_set_padding_mode( NULL, valid_mode ) ); - TEST_ASSERT( mbedtls_cipher_set_padding_mode( &invalid_ctx, valid_mode ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); #endif /* mbedtls_cipher_update() */ @@ -176,13 +260,6 @@ void cipher_invalid_param( ) valid_size, valid_buffer, &size_t_var ) ); - TEST_ASSERT( - mbedtls_cipher_update( &invalid_ctx, - valid_buffer, - valid_size, - valid_buffer, - &size_t_var ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_update( &valid_ctx, @@ -208,11 +285,6 @@ void cipher_invalid_param( ) mbedtls_cipher_finish( NULL, valid_buffer, &size_t_var ) ); - TEST_ASSERT( - mbedtls_cipher_finish( &invalid_ctx, - valid_buffer, - &size_t_var ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_finish( &valid_ctx, @@ -231,11 +303,6 @@ void cipher_invalid_param( ) mbedtls_cipher_write_tag( NULL, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_write_tag( &invalid_ctx, - valid_buffer, - valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_write_tag( &valid_ctx, @@ -248,11 +315,6 @@ void cipher_invalid_param( ) mbedtls_cipher_check_tag( NULL, valid_buffer, valid_size ) ); - TEST_ASSERT( - mbedtls_cipher_check_tag( &invalid_ctx, - valid_buffer, - valid_size ) == - MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); TEST_INVALID_PARAM_RET( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA, mbedtls_cipher_check_tag( &valid_ctx, @@ -413,7 +475,6 @@ void cipher_invalid_param( ) /* mbedtls_cipher_free() */ TEST_VALID_PARAM( mbedtls_cipher_free( NULL ) ); exit: - TEST_VALID_PARAM( mbedtls_cipher_free( &invalid_ctx ) ); TEST_VALID_PARAM( mbedtls_cipher_free( &valid_ctx ) ); } /* END_CASE */ diff --git a/tests/suites/test_suite_cipher.misc.data b/tests/suites/test_suite_cipher.misc.data index 07783c300..25bfd407d 100644 --- a/tests/suites/test_suite_cipher.misc.data +++ b/tests/suites/test_suite_cipher.misc.data @@ -1,2 +1,5 @@ -CIPHER - Invalid parameters -cipher_invalid_param: +CIPHER - Conditional invalid parameter checks +cipher_invalid_param_conditional: + +CIPHER - Unconditional invalid parameter checks +cipher_invalid_param_unconditional: From fb54360f8cf551117be19a5868d3482df083af23 Mon Sep 17 00:00:00 2001 From: k-stachowiak Date: Wed, 19 Dec 2018 18:34:21 +0100 Subject: [PATCH 14/15] Prevent unused variable in some configurations --- tests/suites/test_suite_cipher.function | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 4e616d3ef..773c792ca 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -35,6 +35,8 @@ void cipher_invalid_param_unconditional( ) *( mbedtls_cipher_list() ) ); size_t size_t_var; + (void)valid_mode; /* In some configurations this is unused */ + mbedtls_cipher_init( &valid_ctx ); mbedtls_cipher_setup( &valid_ctx, valid_info ); mbedtls_cipher_init( &invalid_ctx ); @@ -157,6 +159,8 @@ void cipher_invalid_param_conditional( ) size_t size_t_var; + (void)valid_mode; /* In some configurations this is unused */ + /* mbedtls_cipher_init() */ TEST_VALID_PARAM( mbedtls_cipher_init( &valid_ctx ) ); TEST_INVALID_PARAM( mbedtls_cipher_init( NULL ) ); From 01d4b76b7eb1c0f1d2a436de37541a933417bafc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 20 Dec 2018 12:09:07 +0100 Subject: [PATCH 15/15] Remove faulty cipher_finish calls from nist_kw The calls to cipher_finish didn't actually do anything: - the cipher mode is always ECB - in that case cipher_finish() only sets *olen to zero, and returns either 0 or an error depending on whether there was pending data - olen is a local variable in the caller, so setting it to zero right before returning is not essential - the return value of cipher_finis() was not checked by the caller so that's not useful either - the cipher layer does not have ALT implementations so the behaviour described above is unconditional on ALT implementations (in particular, cipher_finish() can't be useful to hardware as (with ECB) it doesn't call any functions from lower-level modules that could release resources for example) Since the calls are causing issues with parameter validation, and were no serving any functional purpose, it's simpler to just remove them. --- library/nist_kw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/nist_kw.c b/library/nist_kw.c index 176af9fe0..317a2426a 100644 --- a/library/nist_kw.c +++ b/library/nist_kw.c @@ -311,7 +311,7 @@ cleanup: } mbedtls_platform_zeroize( inbuff, KW_SEMIBLOCK_LENGTH * 2 ); mbedtls_platform_zeroize( outbuff, KW_SEMIBLOCK_LENGTH * 2 ); - mbedtls_cipher_finish( &ctx->cipher_ctx, NULL, &olen ); + return( ret ); } @@ -528,7 +528,7 @@ cleanup: mbedtls_platform_zeroize( &bad_padding, sizeof( bad_padding) ); mbedtls_platform_zeroize( &diff, sizeof( diff ) ); mbedtls_platform_zeroize( A, sizeof( A ) ); - mbedtls_cipher_finish( &ctx->cipher_ctx, NULL, &olen ); + return( ret ); }