diff --git a/ChangeLog.d/check-return.txt b/ChangeLog.d/check-return.txt new file mode 100644 index 000000000..045b1805e --- /dev/null +++ b/ChangeLog.d/check-return.txt @@ -0,0 +1,17 @@ +Bugfix + * Failures of alternative implementations of AES or DES single-block + functions enabled with MBEDTLS_AES_ENCRYPT_ALT, MBEDTLS_AES_DECRYPT_ALT, + MBEDTLS_DES_CRYPT_ECB_ALT or MBEDTLS_DES3_CRYPT_ECB_ALT were ignored. + This does not concern the implementation provided with Mbed TLS, + where this function cannot fail, or full-module replacements with + MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092. + +Features + * Warn if errors from certain functions are ignored. This is currently + supported on GCC-like compilers and on MSVC and can be configured through + the macro MBEDTLS_CHECK_RETURN. The warnings are always enabled + (where supported) for critical functions where ignoring the return + value is almost always a bug. Enable the new configuration option + MBEDTLS_CHECK_RETURN_WARNING to get warnings for other functions. This + is currently implemented in the AES and DES modules, and will be extended + to other modules in the future. diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index ab8793cb5..94ee58a83 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -45,6 +45,7 @@ #else #include MBEDTLS_CONFIG_FILE #endif +#include "mbedtls/platform_util.h" #include #include @@ -174,6 +175,7 @@ void mbedtls_aes_xts_free( mbedtls_aes_xts_context *ctx ); * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_KEY_LENGTH on failure. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -192,6 +194,7 @@ int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_KEY_LENGTH on failure. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_setkey_dec( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -212,6 +215,7 @@ int mbedtls_aes_setkey_dec( mbedtls_aes_context *ctx, const unsigned char *key, * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_KEY_LENGTH on failure. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -232,6 +236,7 @@ int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, * \return \c 0 on success. * \return #MBEDTLS_ERR_AES_INVALID_KEY_LENGTH on failure. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -260,6 +265,7 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, int mode, const unsigned char input[16], @@ -307,6 +313,7 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH * on failure. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, int mode, size_t length, @@ -351,6 +358,7 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, * smaller than an AES block in size (16 Bytes) or if \p * length is larger than 2^20 blocks (16 MiB). */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, int mode, size_t length, @@ -399,6 +407,7 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, int mode, size_t length, @@ -443,6 +452,7 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, int mode, size_t length, @@ -497,6 +507,7 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, size_t length, size_t *iv_off, @@ -583,6 +594,7 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, size_t length, size_t *nc_off, @@ -603,6 +615,7 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -618,6 +631,7 @@ int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -667,6 +681,7 @@ MBEDTLS_DEPRECATED void mbedtls_aes_decrypt( mbedtls_aes_context *ctx, * \return \c 0 on success. * \return \c 1 on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_aes_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 2730967b0..87b4e9192 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -616,6 +616,29 @@ */ //#define MBEDTLS_CAMELLIA_SMALL_MEMORY +/** + * \def MBEDTLS_CHECK_RETURN_WARNING + * + * If this macro is defined, emit a compile-time warning if application code + * calls a function without checking its return value, but the return value + * should generally be checked in portable applications. + * + * This is only supported on platforms where #MBEDTLS_CHECK_RETURN is + * implemented. Otherwise this option has no effect. + * + * Uncomment to get warnings on using fallible functions without checking + * their return value. + * + * \note This feature is a work in progress. + * Warnings will be added to more functions in the future. + * + * \note A few functions are considered critical, and ignoring the return + * value of these functions will trigger a warning even if this + * macro is not defined. To completely disable return value check + * warnings, define #MBEDTLS_CHECK_RETURN with an empty expansion. + */ +//#define MBEDTLS_CHECK_RETURN_WARNING + /** * \def MBEDTLS_CIPHER_MODE_CBC * @@ -3689,6 +3712,29 @@ */ //#define MBEDTLS_PARAM_FAILED( cond ) assert( cond ) +/** \def MBEDTLS_CHECK_RETURN + * + * This macro is used at the beginning of the declaration of a function + * to indicate that its return value should be checked. It should + * instruct the compiler to emit a warning or an error if the function + * is called without checking its return value. + * + * There is a default implementation for popular compilers in platform_util.h. + * You can override the default implementation by defining your own here. + * + * If the implementation here is empty, this will effectively disable the + * checking of functions' return values. + */ +//#define MBEDTLS_CHECK_RETURN __attribute__((__warn_unused_result__)) + +/** \def MBEDTLS_IGNORE_RETURN + * + * This macro requires one argument, which should be a C function call. + * If that function call would cause a #MBEDTLS_CHECK_RETURN warning, this + * warning is suppressed. + */ +//#define MBEDTLS_IGNORE_RETURN( result ) ((void) !(result)) + /* PSA options */ /** * Use HMAC_DRBG with the specified hash algorithm for HMAC_DRBG for the diff --git a/include/mbedtls/des.h b/include/mbedtls/des.h index 6bfe65491..325aab536 100644 --- a/include/mbedtls/des.h +++ b/include/mbedtls/des.h @@ -32,6 +32,7 @@ #else #include MBEDTLS_CONFIG_FILE #endif +#include "mbedtls/platform_util.h" #include #include @@ -146,6 +147,7 @@ void mbedtls_des_key_set_parity( unsigned char key[MBEDTLS_DES_KEY_SIZE] ); * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_key_check_key_parity( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -159,6 +161,7 @@ int mbedtls_des_key_check_key_parity( const unsigned char key[MBEDTLS_DES_KEY_SI * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_key_check_weak( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -173,6 +176,7 @@ int mbedtls_des_key_check_weak( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_setkey_enc( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -187,6 +191,7 @@ int mbedtls_des_setkey_enc( mbedtls_des_context *ctx, const unsigned char key[MB * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -197,6 +202,7 @@ int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MB * * \return 0 */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -208,6 +214,7 @@ int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -219,6 +226,7 @@ int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -230,6 +238,7 @@ int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -246,6 +255,7 @@ int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -273,6 +283,7 @@ int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, int mode, size_t length, @@ -290,6 +301,7 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, * * \return 0 if successful */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_crypt_ecb( mbedtls_des3_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -315,6 +327,7 @@ int mbedtls_des3_crypt_ecb( mbedtls_des3_context *ctx, * * \return 0 if successful, or MBEDTLS_ERR_DES_INVALID_INPUT_LENGTH */ +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, int mode, size_t length, @@ -345,6 +358,7 @@ void mbedtls_des_setkey( uint32_t SK[32], * * \return 0 if successful, or 1 if the test failed */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_des_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index fbc2a0d1c..f982db8c0 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -132,6 +132,95 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; #endif /* MBEDTLS_DEPRECATED_WARNING */ #endif /* MBEDTLS_DEPRECATED_REMOVED */ +/* Implementation of the check-return facility. + * See the user documentation in config.h. + * + * Do not use this macro directly to annotate function: instead, + * use one of MBEDTLS_CHECK_RETURN_CRITICAL or MBEDTLS_CHECK_RETURN_TYPICAL + * depending on how important it is to check the return value. + */ +#if !defined(MBEDTLS_CHECK_RETURN) +#if defined(__GNUC__) +#define MBEDTLS_CHECK_RETURN __attribute__((__warn_unused_result__)) +#elif defined(_MSC_VER) && _MSC_VER >= 1700 +#include +#define MBEDTLS_CHECK_RETURN _Check_return_ +#else +#define MBEDTLS_CHECK_RETURN +#endif +#endif + +/** Critical-failure function + * + * This macro appearing at the beginning of the declaration of a function + * indicates that its return value should be checked in all applications. + * Omitting the check is very likely to indicate a bug in the application + * and will result in a compile-time warning if #MBEDTLS_CHECK_RETURN + * is implemented for the compiler in use. + * + * \note The use of this macro is a work in progress. + * This macro may be added to more functions in the future. + * Such an extension is not considered an API break, provided that + * there are near-unavoidable circumstances under which the function + * can fail. For example, signature/MAC/AEAD verification functions, + * and functions that require a random generator, are considered + * return-check-critical. + */ +#define MBEDTLS_CHECK_RETURN_CRITICAL MBEDTLS_CHECK_RETURN + +/** Ordinary-failure function + * + * This macro appearing at the beginning of the declaration of a function + * indicates that its return value should be generally be checked in portable + * applications. Omitting the check will result in a compile-time warning if + * #MBEDTLS_CHECK_RETURN is implemented for the compiler in use and + * #MBEDTLS_CHECK_RETURN_WARNING is enabled in the compile-time configuration. + * + * You can use #MBEDTLS_IGNORE_RETURN to explicitly ignore the return value + * of a function that is annotated with #MBEDTLS_CHECK_RETURN. + * + * \note The use of this macro is a work in progress. + * This macro will be added to more functions in the future. + * Eventually this should appear before most functions returning + * an error code (as \c int in the \c mbedtls_xxx API or + * as ::psa_status_t in the \c psa_xxx API). + */ +#if defined(MBEDTLS_CHECK_RETURN_WARNING) +#define MBEDTLS_CHECK_RETURN_TYPICAL MBEDTLS_CHECK_RETURN +#else +#define MBEDTLS_CHECK_RETURN_TYPICAL +#endif + +/** Benign-failure function + * + * This macro appearing at the beginning of the declaration of a function + * indicates that it is rarely useful to check its return value. + * + * This macro has an empty expansion. It exists for documentation purposes: + * a #MBEDTLS_CHECK_RETURN_OPTIONAL annotation indicates that the function + * has been analyzed for return-check usefuless, whereas the lack of + * an annotation indicates that the function has not been analyzed and its + * return-check usefulness is unknown. + */ +#define MBEDTLS_CHECK_RETURN_OPTIONAL + +/** \def MBEDTLS_IGNORE_RETURN + * + * Call this macro with one argument, a function call, to suppress a warning + * from #MBEDTLS_CHECK_RETURN due to that function call. + */ +#if !defined(MBEDTLS_IGNORE_RETURN) +/* GCC doesn't silence the warning with just (void)(result). + * (void)!(result) is known to work up at least up to GCC 10, as well + * as with Clang and MSVC. + * + * https://gcc.gnu.org/onlinedocs/gcc-3.4.6/gcc/Non_002dbugs.html + * https://stackoverflow.com/questions/40576003/ignoring-warning-wunused-result + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34 + */ +#define MBEDTLS_IGNORE_RETURN(result) ( (void) !( result ) ) +#endif + /** * \brief Securely zeroize a buffer * diff --git a/library/aes.c b/library/aes.c index 544b5834f..31824e75c 100644 --- a/library/aes.c +++ b/library/aes.c @@ -903,7 +903,7 @@ void mbedtls_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { - mbedtls_internal_aes_encrypt( ctx, input, output ); + MBEDTLS_IGNORE_RETURN( mbedtls_internal_aes_encrypt( ctx, input, output ) ); } #endif /* !MBEDTLS_DEPRECATED_REMOVED */ @@ -976,7 +976,7 @@ void mbedtls_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { - mbedtls_internal_aes_decrypt( ctx, input, output ); + MBEDTLS_IGNORE_RETURN( mbedtls_internal_aes_decrypt( ctx, input, output ) ); } #endif /* !MBEDTLS_DEPRECATED_REMOVED */ @@ -1029,6 +1029,7 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, unsigned char *output ) { int i; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char temp[16]; AES_VALIDATE_RET( ctx != NULL ); @@ -1058,7 +1059,9 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, while( length > 0 ) { memcpy( temp, input, 16 ); - mbedtls_aes_crypt_ecb( ctx, mode, input, output ); + ret = mbedtls_aes_crypt_ecb( ctx, mode, input, output ); + if( ret != 0 ) + goto exit; for( i = 0; i < 16; i++ ) output[i] = (unsigned char)( output[i] ^ iv[i] ); @@ -1077,7 +1080,9 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, for( i = 0; i < 16; i++ ) output[i] = (unsigned char)( input[i] ^ iv[i] ); - mbedtls_aes_crypt_ecb( ctx, mode, output, output ); + ret = mbedtls_aes_crypt_ecb( ctx, mode, output, output ); + if( ret != 0 ) + goto exit; memcpy( iv, output, 16 ); input += 16; @@ -1085,8 +1090,10 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, length -= 16; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -1240,6 +1247,7 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, unsigned char *output ) { int c; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t n; AES_VALIDATE_RET( ctx != NULL ); @@ -1260,7 +1268,11 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, while( length-- ) { if( n == 0 ) - mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + { + ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + if( ret != 0 ) + goto exit; + } c = *input++; *output++ = (unsigned char)( c ^ iv[n] ); @@ -1274,7 +1286,11 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, while( length-- ) { if( n == 0 ) - mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + { + ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + if( ret != 0 ) + goto exit; + } iv[n] = *output++ = (unsigned char)( iv[n] ^ *input++ ); @@ -1283,8 +1299,10 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, } *iv_off = n; + ret = 0; - return( 0 ); +exit: + return( ret ); } /* @@ -1297,6 +1315,7 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, const unsigned char *input, unsigned char *output ) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char c; unsigned char ov[17]; @@ -1309,7 +1328,9 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, while( length-- ) { memcpy( ov, iv, 16 ); - mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv ); + if( ret != 0 ) + goto exit; if( mode == MBEDTLS_AES_DECRYPT ) ov[16] = *input; @@ -1321,8 +1342,10 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, memcpy( iv, ov + 1, 16 ); } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CFB */ @@ -1384,6 +1407,7 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, unsigned char *output ) { int c, i; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t n; AES_VALIDATE_RET( ctx != NULL ); @@ -1401,7 +1425,9 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, while( length-- ) { if( n == 0 ) { - mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, nonce_counter, stream_block ); + ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, nonce_counter, stream_block ); + if( ret != 0 ) + goto exit; for( i = 16; i > 0; i-- ) if( ++nonce_counter[i - 1] != 0 ) @@ -1414,8 +1440,10 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, } *nc_off = n; + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CTR */ diff --git a/library/des.c b/library/des.c index 7f90faa04..91d22b5d9 100644 --- a/library/des.c +++ b/library/des.c @@ -28,6 +28,7 @@ #if defined(MBEDTLS_DES_C) #include "mbedtls/des.h" +#include "mbedtls/error.h" #include "mbedtls/platform_util.h" #include @@ -642,6 +643,7 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, unsigned char *output ) { int i; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char temp[8]; if( length % 8 ) @@ -654,7 +656,9 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, for( i = 0; i < 8; i++ ) output[i] = (unsigned char)( input[i] ^ iv[i] ); - mbedtls_des_crypt_ecb( ctx, output, output ); + ret = mbedtls_des_crypt_ecb( ctx, output, output ); + if( ret != 0 ) + goto exit; memcpy( iv, output, 8 ); input += 8; @@ -667,7 +671,9 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, while( length > 0 ) { memcpy( temp, input, 8 ); - mbedtls_des_crypt_ecb( ctx, input, output ); + ret = mbedtls_des_crypt_ecb( ctx, input, output ); + if( ret != 0 ) + goto exit; for( i = 0; i < 8; i++ ) output[i] = (unsigned char)( output[i] ^ iv[i] ); @@ -679,8 +685,10 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, length -= 8; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -741,6 +749,7 @@ int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, unsigned char *output ) { int i; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char temp[8]; if( length % 8 ) @@ -753,7 +762,9 @@ int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, for( i = 0; i < 8; i++ ) output[i] = (unsigned char)( input[i] ^ iv[i] ); - mbedtls_des3_crypt_ecb( ctx, output, output ); + ret = mbedtls_des3_crypt_ecb( ctx, output, output ); + if( ret != 0 ) + goto exit; memcpy( iv, output, 8 ); input += 8; @@ -766,7 +777,9 @@ int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, while( length > 0 ) { memcpy( temp, input, 8 ); - mbedtls_des3_crypt_ecb( ctx, input, output ); + ret = mbedtls_des3_crypt_ecb( ctx, input, output ); + if( ret != 0 ) + goto exit; for( i = 0; i < 8; i++ ) output[i] = (unsigned char)( output[i] ^ iv[i] ); @@ -778,8 +791,10 @@ int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, length -= 8; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -872,39 +887,43 @@ int mbedtls_des_self_test( int verbose ) switch( i ) { case 0: - mbedtls_des_setkey_dec( &ctx, des3_test_keys ); + ret = mbedtls_des_setkey_dec( &ctx, des3_test_keys ); break; case 1: - mbedtls_des_setkey_enc( &ctx, des3_test_keys ); + ret = mbedtls_des_setkey_enc( &ctx, des3_test_keys ); break; case 2: - mbedtls_des3_set2key_dec( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set2key_dec( &ctx3, des3_test_keys ); break; case 3: - mbedtls_des3_set2key_enc( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set2key_enc( &ctx3, des3_test_keys ); break; case 4: - mbedtls_des3_set3key_dec( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set3key_dec( &ctx3, des3_test_keys ); break; case 5: - mbedtls_des3_set3key_enc( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set3key_enc( &ctx3, des3_test_keys ); break; default: return( 1 ); } + if( ret != 0 ) + goto exit; for( j = 0; j < 100; j++ ) { if( u == 0 ) - mbedtls_des_crypt_ecb( &ctx, buf, buf ); + ret = mbedtls_des_crypt_ecb( &ctx, buf, buf ); else - mbedtls_des3_crypt_ecb( &ctx3, buf, buf ); + ret = mbedtls_des3_crypt_ecb( &ctx3, buf, buf ); + if( ret != 0 ) + goto exit; } if( ( v == MBEDTLS_DES_DECRYPT && @@ -947,41 +966,45 @@ int mbedtls_des_self_test( int verbose ) switch( i ) { case 0: - mbedtls_des_setkey_dec( &ctx, des3_test_keys ); + ret = mbedtls_des_setkey_dec( &ctx, des3_test_keys ); break; case 1: - mbedtls_des_setkey_enc( &ctx, des3_test_keys ); + ret = mbedtls_des_setkey_enc( &ctx, des3_test_keys ); break; case 2: - mbedtls_des3_set2key_dec( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set2key_dec( &ctx3, des3_test_keys ); break; case 3: - mbedtls_des3_set2key_enc( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set2key_enc( &ctx3, des3_test_keys ); break; case 4: - mbedtls_des3_set3key_dec( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set3key_dec( &ctx3, des3_test_keys ); break; case 5: - mbedtls_des3_set3key_enc( &ctx3, des3_test_keys ); + ret = mbedtls_des3_set3key_enc( &ctx3, des3_test_keys ); break; default: return( 1 ); } + if( ret != 0 ) + goto exit; if( v == MBEDTLS_DES_DECRYPT ) { for( j = 0; j < 100; j++ ) { if( u == 0 ) - mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf ); + ret = mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf ); else - mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf ); + ret = mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf ); + if( ret != 0 ) + goto exit; } } else @@ -991,9 +1014,11 @@ int mbedtls_des_self_test( int verbose ) unsigned char tmp[8]; if( u == 0 ) - mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf ); + ret = mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf ); else - mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf ); + ret = mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf ); + if( ret != 0 ) + goto exit; memcpy( tmp, prv, 8 ); memcpy( prv, buf, 8 ); @@ -1027,6 +1052,8 @@ exit: mbedtls_des_free( &ctx ); mbedtls_des3_free( &ctx3 ); + if( ret != 0 ) + ret = 1; return( ret ); } diff --git a/library/version_features.c b/library/version_features.c index f665a2375..40c95201b 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -267,6 +267,9 @@ static const char * const features[] = { #if defined(MBEDTLS_CAMELLIA_SMALL_MEMORY) "MBEDTLS_CAMELLIA_SMALL_MEMORY", #endif /* MBEDTLS_CAMELLIA_SMALL_MEMORY */ +#if defined(MBEDTLS_CHECK_RETURN_WARNING) + "MBEDTLS_CHECK_RETURN_WARNING", +#endif /* MBEDTLS_CHECK_RETURN_WARNING */ #if defined(MBEDTLS_CIPHER_MODE_CBC) "MBEDTLS_CIPHER_MODE_CBC", #endif /* MBEDTLS_CIPHER_MODE_CBC */ diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index d6e4990a9..f6c622608 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -274,7 +274,9 @@ int main( void ) mbedtls_printf( "...\n . Receiving and decrypting the ciphertext" ); fflush( stdout ); - mbedtls_aes_setkey_dec( &aes, buf, 256 ); + ret = mbedtls_aes_setkey_dec( &aes, buf, 256 ); + if( ret != 0 ) + goto exit; memset( buf, 0, sizeof( buf ) ); @@ -284,7 +286,9 @@ int main( void ) goto exit; } - mbedtls_aes_crypt_ecb( &aes, MBEDTLS_AES_DECRYPT, buf, buf ); + ret = mbedtls_aes_crypt_ecb( &aes, MBEDTLS_AES_DECRYPT, buf, buf ); + if( ret != 0 ) + goto exit; buf[16] = '\0'; mbedtls_printf( "\n . Plaintext is \"%s\"\n\n", (char *) buf ); diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index dccf0951c..8f032a3b2 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -295,9 +295,13 @@ int main( void ) mbedtls_printf( "...\n . Encrypting and sending the ciphertext" ); fflush( stdout ); - mbedtls_aes_setkey_enc( &aes, buf, 256 ); + ret = mbedtls_aes_setkey_enc( &aes, buf, 256 ); + if( ret != 0 ) + goto exit; memcpy( buf, PLAINTEXT, 16 ); - mbedtls_aes_crypt_ecb( &aes, MBEDTLS_AES_ENCRYPT, buf, buf ); + ret = mbedtls_aes_crypt_ecb( &aes, MBEDTLS_AES_ENCRYPT, buf, buf ); + if( ret != 0 ) + goto exit; if( ( ret = mbedtls_net_send( &client_fd, buf, 16 ) ) != 16 ) { diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index 9c5911ba2..88c9e65c5 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -449,7 +449,8 @@ int main( int argc, char *argv[] ) { mbedtls_des3_context des3; mbedtls_des3_init( &des3 ); - mbedtls_des3_set3key_enc( &des3, tmp ); + if( mbedtls_des3_set3key_enc( &des3, tmp ) != 0 ) + mbedtls_exit( 1 ); TIME_AND_TSC( "3DES", mbedtls_des3_crypt_cbc( &des3, MBEDTLS_DES_ENCRYPT, BUFSIZE, tmp, buf, buf ) ); mbedtls_des3_free( &des3 ); @@ -459,7 +460,8 @@ int main( int argc, char *argv[] ) { mbedtls_des_context des; mbedtls_des_init( &des ); - mbedtls_des_setkey_enc( &des, tmp ); + if( mbedtls_des_setkey_enc( &des, tmp ) != 0 ) + mbedtls_exit( 1 ); TIME_AND_TSC( "DES", mbedtls_des_crypt_cbc( &des, MBEDTLS_DES_ENCRYPT, BUFSIZE, tmp, buf, buf ) ); mbedtls_des_free( &des ); @@ -497,7 +499,7 @@ int main( int argc, char *argv[] ) memset( buf, 0, sizeof( buf ) ); memset( tmp, 0, sizeof( tmp ) ); - mbedtls_aes_setkey_enc( &aes, tmp, keysize ); + CHECK_AND_CONTINUE( mbedtls_aes_setkey_enc( &aes, tmp, keysize ) ); TIME_AND_TSC( title, mbedtls_aes_crypt_cbc( &aes, MBEDTLS_AES_ENCRYPT, BUFSIZE, tmp, buf, buf ) ); @@ -518,7 +520,7 @@ int main( int argc, char *argv[] ) memset( buf, 0, sizeof( buf ) ); memset( tmp, 0, sizeof( tmp ) ); - mbedtls_aes_xts_setkey_enc( &ctx, tmp, keysize * 2 ); + CHECK_AND_CONTINUE( mbedtls_aes_xts_setkey_enc( &ctx, tmp, keysize * 2 ) ); TIME_AND_TSC( title, mbedtls_aes_crypt_xts( &ctx, MBEDTLS_AES_ENCRYPT, BUFSIZE, diff --git a/programs/test/query_config.c b/programs/test/query_config.c index 9b9d0c0f7..5dfe38323 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -770,6 +770,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_CAMELLIA_SMALL_MEMORY */ +#if defined(MBEDTLS_CHECK_RETURN_WARNING) + if( strcmp( "MBEDTLS_CHECK_RETURN_WARNING", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_CHECK_RETURN_WARNING ); + return( 0 ); + } +#endif /* MBEDTLS_CHECK_RETURN_WARNING */ + #if defined(MBEDTLS_CIPHER_MODE_CBC) if( strcmp( "MBEDTLS_CIPHER_MODE_CBC", config ) == 0 ) { @@ -2642,6 +2650,22 @@ int query_config( const char *config ) } #endif /* MBEDTLS_PLATFORM_NV_SEED_WRITE_MACRO */ +#if defined(MBEDTLS_CHECK_RETURN) + if( strcmp( "MBEDTLS_CHECK_RETURN", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_CHECK_RETURN ); + return( 0 ); + } +#endif /* MBEDTLS_CHECK_RETURN */ + +#if defined(MBEDTLS_IGNORE_RETURN) + if( strcmp( "MBEDTLS_IGNORE_RETURN", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_IGNORE_RETURN ); + return( 0 ); + } +#endif /* MBEDTLS_IGNORE_RETURN */ + #if defined(MBEDTLS_PSA_HMAC_DRBG_MD_TYPE) if( strcmp( "MBEDTLS_PSA_HMAC_DRBG_MD_TYPE", config ) == 0 ) { diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index a90277d97..5a64099fb 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -67,7 +67,7 @@ void aes_encrypt_cbc( data_t * key_str, data_t * iv_str, mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cbc( &ctx, MBEDTLS_AES_ENCRYPT, src_str->len, iv_str->x, src_str->x, output ) == cbc_result ); if( cbc_result == 0 ) { @@ -92,7 +92,7 @@ void aes_decrypt_cbc( data_t * key_str, data_t * iv_str, memset(output, 0x00, 100); mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_dec( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_dec( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cbc( &ctx, MBEDTLS_AES_DECRYPT, src_str->len, iv_str->x, src_str->x, output ) == cbc_result ); if( cbc_result == 0) { @@ -244,7 +244,7 @@ void aes_encrypt_cfb128( data_t * key_str, data_t * iv_str, mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cfb128( &ctx, MBEDTLS_AES_ENCRYPT, 16, &iv_offset, iv_str->x, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, 16, dst->len ) == 0 ); @@ -266,7 +266,7 @@ void aes_decrypt_cfb128( data_t * key_str, data_t * iv_str, mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cfb128( &ctx, MBEDTLS_AES_DECRYPT, 16, &iv_offset, iv_str->x, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, 16, dst->len ) == 0 ); @@ -287,7 +287,7 @@ void aes_encrypt_cfb8( data_t * key_str, data_t * iv_str, mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cfb8( &ctx, MBEDTLS_AES_ENCRYPT, src_str->len, iv_str->x, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, @@ -309,7 +309,7 @@ void aes_decrypt_cfb8( data_t * key_str, data_t * iv_str, mbedtls_aes_init( &ctx ); - mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &ctx, key_str->x, key_str->len * 8 ) == 0 ); TEST_ASSERT( mbedtls_aes_crypt_cfb8( &ctx, MBEDTLS_AES_DECRYPT, src_str->len, iv_str->x, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, diff --git a/tests/suites/test_suite_des.function b/tests/suites/test_suite_des.function index 5b249355b..7256fb537 100644 --- a/tests/suites/test_suite_des.function +++ b/tests/suites/test_suite_des.function @@ -24,7 +24,7 @@ void des_encrypt_ecb( data_t * key_str, data_t * src_str, data_t * dst ) mbedtls_des_init( &ctx ); - mbedtls_des_setkey_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des_setkey_enc( &ctx, key_str->x ) == 0 ); TEST_ASSERT( mbedtls_des_crypt_ecb( &ctx, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, 8, dst->len ) == 0 ); @@ -44,7 +44,7 @@ void des_decrypt_ecb( data_t * key_str, data_t * src_str, data_t * dst ) mbedtls_des_init( &ctx ); - mbedtls_des_setkey_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des_setkey_dec( &ctx, key_str->x ) == 0 ); TEST_ASSERT( mbedtls_des_crypt_ecb( &ctx, src_str->x, output ) == 0 ); TEST_ASSERT( mbedtls_test_hexcmp( output, dst->x, 8, dst->len ) == 0 ); @@ -65,7 +65,7 @@ void des_encrypt_cbc( data_t * key_str, data_t * iv_str, mbedtls_des_init( &ctx ); - mbedtls_des_setkey_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des_setkey_enc( &ctx, key_str->x ) == 0 ); TEST_ASSERT( mbedtls_des_crypt_cbc( &ctx, MBEDTLS_DES_ENCRYPT, src_str->len, iv_str->x, src_str->x, output ) == cbc_result ); if( cbc_result == 0 ) { @@ -91,7 +91,7 @@ void des_decrypt_cbc( data_t * key_str, data_t * iv_str, mbedtls_des_init( &ctx ); - mbedtls_des_setkey_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des_setkey_dec( &ctx, key_str->x ) == 0 ); TEST_ASSERT( mbedtls_des_crypt_cbc( &ctx, MBEDTLS_DES_DECRYPT, src_str->len, iv_str->x, src_str->x, output ) == cbc_result ); if( cbc_result == 0 ) { @@ -117,9 +117,9 @@ void des3_encrypt_ecb( int key_count, data_t * key_str, if( key_count == 2 ) - mbedtls_des3_set2key_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set2key_enc( &ctx, key_str->x ) == 0 ); else if( key_count == 3 ) - mbedtls_des3_set3key_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set3key_enc( &ctx, key_str->x ) == 0 ); else TEST_ASSERT( 0 ); @@ -144,9 +144,9 @@ void des3_decrypt_ecb( int key_count, data_t * key_str, if( key_count == 2 ) - mbedtls_des3_set2key_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set2key_dec( &ctx, key_str->x ) == 0 ); else if( key_count == 3 ) - mbedtls_des3_set3key_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set3key_dec( &ctx, key_str->x ) == 0 ); else TEST_ASSERT( 0 ); @@ -172,9 +172,9 @@ void des3_encrypt_cbc( int key_count, data_t * key_str, if( key_count == 2 ) - mbedtls_des3_set2key_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set2key_enc( &ctx, key_str->x ) == 0 ); else if( key_count == 3 ) - mbedtls_des3_set3key_enc( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set3key_enc( &ctx, key_str->x ) == 0 ); else TEST_ASSERT( 0 ); @@ -205,9 +205,9 @@ void des3_decrypt_cbc( int key_count, data_t * key_str, if( key_count == 2 ) - mbedtls_des3_set2key_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set2key_dec( &ctx, key_str->x ) == 0 ); else if( key_count == 3 ) - mbedtls_des3_set3key_dec( &ctx, key_str->x ); + TEST_ASSERT( mbedtls_des3_set3key_dec( &ctx, key_str->x ) == 0 ); else TEST_ASSERT( 0 );