From 7b8571fcb54ce4115076451c5a69cb0f8e1b99be Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Jul 2021 21:02:36 +0200 Subject: [PATCH 01/14] New macro MBEDTLS_CHECK_RETURN Put this macro before a function declaration to indicate that its result must be checked. This commit supports GCC-like compilers and MSVC >=2012. Signed-off-by: Gilles Peskine --- include/mbedtls/aes.h | 1 + include/mbedtls/des.h | 1 + include/mbedtls/platform_util.h | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index ab8793cb5..a74e41343 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 diff --git a/include/mbedtls/des.h b/include/mbedtls/des.h index 6bfe65491..29b96dbb2 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 diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index fbc2a0d1c..b4ffb471e 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -132,6 +132,26 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; #endif /* MBEDTLS_DEPRECATED_WARNING */ #endif /* MBEDTLS_DEPRECATED_REMOVED */ +/** \def MBEDTLS_CHECK_RETURN + * + * This macro appearing at the beginning of the declaration of a function + * indicates that its return value should be checked. + * + * 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) +#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 + /** * \brief Securely zeroize a buffer * From 377a310da44b7f7001c7a6081b8700d80b2963d6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 7 Jul 2021 21:08:28 +0200 Subject: [PATCH 02/14] Catch failures of AES or DES operations Declare all AES and DES functions that return int as needing to have their result checked, and do check the result in our code. A DES or AES block operation can fail in alternative implementations of mbedtls_internal_aes_encrypt() (under MBEDTLS_AES_ENCRYPT_ALT), mbedtls_internal_aes_decrypt() (under MBEDTLS_AES_DECRYPT_ALT), mbedtls_des_crypt_ecb() (under MBEDTLS_DES_CRYPT_ECB_ALT), mbedtls_des3_crypt_ecb() (under MBEDTLS_DES3_CRYPT_ECB_ALT). A failure can happen if the accelerator peripheral is in a bad state. Several block modes were not catching the error. This commit does the following code changes, grouped together to avoid having an intermediate commit where the build fails: * Add MBEDTLS_CHECK_RETURN to all functions returning int in aes.h and des.h. * Fix all places where this causes a GCC warning, indicating that our code was not properly checking the result of an AES operation: * In library code: on failure, goto exit and return ret. * In pkey programs: goto exit. * In the benchmark program: exit (not ideal since there's no error message, but it's what the code currently does for failures). * In test code: TEST_ASSERT. * Changelog entry. Signed-off-by: Gilles Peskine --- ChangeLog.d/check-return.txt | 12 +++++ include/mbedtls/aes.h | 14 ++++++ include/mbedtls/des.h | 13 +++++ library/aes.c | 48 ++++++++++++++---- library/des.c | 75 +++++++++++++++++++--------- programs/pkey/dh_client.c | 8 ++- programs/pkey/dh_server.c | 8 ++- programs/test/benchmark.c | 10 ++-- tests/suites/test_suite_aes.function | 12 ++--- tests/suites/test_suite_des.function | 24 ++++----- 10 files changed, 164 insertions(+), 60 deletions(-) create mode 100644 ChangeLog.d/check-return.txt diff --git a/ChangeLog.d/check-return.txt b/ChangeLog.d/check-return.txt new file mode 100644 index 000000000..062b8a172 --- /dev/null +++ b/ChangeLog.d/check-return.txt @@ -0,0 +1,12 @@ +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. + +Changes + * Warn if errors from AES or DES functions are ignored. This is currently + supported on GCC-like compilers and on MSVC and can be configured by + setting MBEDTLS_CHECK_RETURN in config.h. diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index a74e41343..4662260cb 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -175,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 int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -193,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 int mbedtls_aes_setkey_dec( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -213,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 int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -233,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 int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -261,6 +265,7 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, int mode, const unsigned char input[16], @@ -308,6 +313,7 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH * on failure. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, int mode, size_t length, @@ -352,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 int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, int mode, size_t length, @@ -400,6 +407,7 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, int mode, size_t length, @@ -444,6 +452,7 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, int mode, size_t length, @@ -498,6 +507,7 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, size_t length, size_t *iv_off, @@ -584,6 +594,7 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, size_t length, size_t *nc_off, @@ -604,6 +615,7 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -619,6 +631,7 @@ int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ +MBEDTLS_CHECK_RETURN int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -668,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 int mbedtls_aes_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ diff --git a/include/mbedtls/des.h b/include/mbedtls/des.h index 29b96dbb2..a82ecf843 100644 --- a/include/mbedtls/des.h +++ b/include/mbedtls/des.h @@ -147,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 int mbedtls_des_key_check_key_parity( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -160,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 int mbedtls_des_key_check_weak( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -174,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 int mbedtls_des_setkey_enc( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -188,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 int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -198,6 +202,7 @@ int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MB * * \return 0 */ +MBEDTLS_CHECK_RETURN int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -209,6 +214,7 @@ int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -220,6 +226,7 @@ int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -231,6 +238,7 @@ int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, * * \return 0 */ +MBEDTLS_CHECK_RETURN int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -247,6 +255,7 @@ int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -274,6 +283,7 @@ int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ +MBEDTLS_CHECK_RETURN int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, int mode, size_t length, @@ -291,6 +301,7 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, * * \return 0 if successful */ +MBEDTLS_CHECK_RETURN int mbedtls_des3_crypt_ecb( mbedtls_des3_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -316,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 int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, int mode, size_t length, @@ -346,6 +358,7 @@ void mbedtls_des_setkey( uint32_t SK[32], * * \return 0 if successful, or 1 if the test failed */ +MBEDTLS_CHECK_RETURN int mbedtls_des_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ diff --git a/library/aes.c b/library/aes.c index 3f616427a..e4bcfa023 100644 --- a/library/aes.c +++ b/library/aes.c @@ -1052,6 +1052,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 ); @@ -1081,7 +1082,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] ); @@ -1100,7 +1103,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; @@ -1108,8 +1113,10 @@ int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, length -= 16; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -1292,6 +1299,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 ); @@ -1312,7 +1320,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] ); @@ -1326,7 +1338,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++ ); @@ -1335,8 +1351,10 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, } *iv_off = n; + ret = 0; - return( 0 ); +exit: + return( ret ); } /* @@ -1349,6 +1367,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]; @@ -1361,7 +1380,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; @@ -1373,8 +1394,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 */ @@ -1436,6 +1459,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 ); @@ -1453,7 +1477,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 ) @@ -1466,8 +1492,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 eddf55e78..9ff419046 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 @@ -665,6 +666,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 ) @@ -677,7 +679,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; @@ -690,7 +694,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] ); @@ -702,8 +708,10 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, length -= 8; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -764,6 +772,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 ) @@ -776,7 +785,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; @@ -789,7 +800,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] ); @@ -801,8 +814,10 @@ int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, length -= 8; } } + ret = 0; - return( 0 ); +exit: + return( ret ); } #endif /* MBEDTLS_CIPHER_MODE_CBC */ @@ -895,39 +910,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 && @@ -970,41 +989,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 @@ -1014,9 +1037,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 ); @@ -1050,6 +1075,8 @@ exit: mbedtls_des_free( &ctx ); mbedtls_des3_free( &ctx3 ); + if( ret != 0 ) + ret = 1; return( ret ); } 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/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 ); From 1ef29fcf477cd215d01c249d2162bbc624108e79 Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Fri, 6 Aug 2021 13:56:54 +0200 Subject: [PATCH 03/14] Add MBEDTLS_CHECK_RETURN description to config.h Signed-off-by: Mateusz Starzyk Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index e2f460c8e..b9e57eb19 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3695,6 +3695,18 @@ */ //#define MBEDTLS_PARAM_FAILED( cond ) assert( cond ) +/** \def MBEDTLS_CHECK_RETURN + * + * This macro appearing at the beginning of the declaration of a function + * indicates that its return value should be checked. + * + * Default implementation resides in platform_util.h. + * You can override default implementation by defining your own. + * Custom implementation can be empty, which will disable checking + * of functions' return values. + */ +//#define MBEDTLS_CHECK_RETURN + /* PSA options */ /** * Use HMAC_DRBG with the specified hash algorithm for HMAC_DRBG for the From ee0a4435f7a78d79d44d0d1b6bea6cb19c5bc2d1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Sep 2021 17:28:59 +0200 Subject: [PATCH 04/14] Define indirection macros MBEDTLS_CHECK_RETURN_xxx Define macros MBEDTLS_CHECK_RETURN_CRITICAL, MBEDTLS_CHECK_RETURN_TYPICAL and MBEDTLS_CHECK_RETURN_OPTIONAL so that we can indicate on a function-by-function basis whether checking the function's return value is almost always necessary (CRITICAL), typically necessary in portable applications but unnecessary in some reasonable cases (TYPICAL), or typically unnecessary (OPTIONAL). Update the documentation of MBEDTLS_CHECK_RETURN accordingly. This is split between the user documentation (Doxygen, in config.h) and the internal documentation (non-Doxygen, in platform_util.h, of minor importance since the macro isn't meant to be used directly). Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 15 +++++---- include/mbedtls/platform_util.h | 58 +++++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index b9e57eb19..d2357dc0f 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3697,13 +3697,16 @@ /** \def MBEDTLS_CHECK_RETURN * - * This macro appearing at the beginning of the declaration of a function - * indicates that its return value should be checked. + * 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. * - * Default implementation resides in platform_util.h. - * You can override default implementation by defining your own. - * Custom implementation can be empty, which will disable checking - * of functions' return values. + * 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 diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index b4ffb471e..4d9fcb6df 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -132,14 +132,12 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; #endif /* MBEDTLS_DEPRECATED_WARNING */ #endif /* MBEDTLS_DEPRECATED_REMOVED */ -/** \def MBEDTLS_CHECK_RETURN +/* Implementation of the check-return facility. + * See the user documentation in config.h. * - * This macro appearing at the beginning of the declaration of a function - * indicates that its return value should be checked. - * - * 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). + * 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__) @@ -152,6 +150,52 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; #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. + * + * \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). + */ +#define MBEDTLS_CHECK_RETURN_TYPICAL MBEDTLS_CHECK_RETURN + +/** 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 + /** * \brief Securely zeroize a buffer * From ce555e4fad05652f237c587ab41c59a5b0c6e335 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Sep 2021 17:35:37 +0200 Subject: [PATCH 05/14] Change DES and AES functions to MBEDTLS_CHECK_RETURN_TYPICAL For all of these functions, the only possible failures are a hardware accelerator (not possible unless using an ALT implementation), an internal error or runtime corruption. Exception: the self-tests, which serve little purpose if their status isn't tested. Signed-off-by: Gilles Peskine --- include/mbedtls/aes.h | 28 ++++++++++++++-------------- include/mbedtls/des.h | 26 +++++++++++++------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index 4662260cb..94ee58a83 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -175,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -194,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_setkey_dec( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -215,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_xts_setkey_enc( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -236,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, const unsigned char *key, unsigned int keybits ); @@ -265,7 +265,7 @@ int mbedtls_aes_xts_setkey_dec( mbedtls_aes_xts_context *ctx, * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, int mode, const unsigned char input[16], @@ -313,7 +313,7 @@ int mbedtls_aes_crypt_ecb( mbedtls_aes_context *ctx, * \return #MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH * on failure. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cbc( mbedtls_aes_context *ctx, int mode, size_t length, @@ -358,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, int mode, size_t length, @@ -407,7 +407,7 @@ int mbedtls_aes_crypt_xts( mbedtls_aes_xts_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, int mode, size_t length, @@ -452,7 +452,7 @@ int mbedtls_aes_crypt_cfb128( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, int mode, size_t length, @@ -507,7 +507,7 @@ int mbedtls_aes_crypt_cfb8( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, size_t length, size_t *iv_off, @@ -594,7 +594,7 @@ int mbedtls_aes_crypt_ofb( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, size_t length, size_t *nc_off, @@ -615,7 +615,7 @@ int mbedtls_aes_crypt_ctr( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -631,7 +631,7 @@ int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, * * \return \c 0 on success. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ); @@ -681,7 +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 +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_aes_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ diff --git a/include/mbedtls/des.h b/include/mbedtls/des.h index a82ecf843..325aab536 100644 --- a/include/mbedtls/des.h +++ b/include/mbedtls/des.h @@ -147,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_key_check_key_parity( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -161,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_key_check_weak( const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -176,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_setkey_enc( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -191,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE] ); /** @@ -202,7 +202,7 @@ int mbedtls_des_setkey_dec( mbedtls_des_context *ctx, const unsigned char key[MB * * \return 0 */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -214,7 +214,7 @@ int mbedtls_des3_set2key_enc( mbedtls_des3_context *ctx, * * \return 0 */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 2] ); @@ -226,7 +226,7 @@ int mbedtls_des3_set2key_dec( mbedtls_des3_context *ctx, * * \return 0 */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -238,7 +238,7 @@ int mbedtls_des3_set3key_enc( mbedtls_des3_context *ctx, * * \return 0 */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, const unsigned char key[MBEDTLS_DES_KEY_SIZE * 3] ); @@ -255,7 +255,7 @@ int mbedtls_des3_set3key_dec( mbedtls_des3_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -283,7 +283,7 @@ int mbedtls_des_crypt_ecb( mbedtls_des_context *ctx, * security risk. We recommend considering stronger ciphers * instead. */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, int mode, size_t length, @@ -301,7 +301,7 @@ int mbedtls_des_crypt_cbc( mbedtls_des_context *ctx, * * \return 0 if successful */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_crypt_ecb( mbedtls_des3_context *ctx, const unsigned char input[8], unsigned char output[8] ); @@ -327,7 +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 +MBEDTLS_CHECK_RETURN_TYPICAL int mbedtls_des3_crypt_cbc( mbedtls_des3_context *ctx, int mode, size_t length, @@ -358,7 +358,7 @@ void mbedtls_des_setkey( uint32_t SK[32], * * \return 0 if successful, or 1 if the test failed */ -MBEDTLS_CHECK_RETURN +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_des_self_test( int verbose ); #endif /* MBEDTLS_SELF_TEST */ From 6b5c0f0e441ee62ca6f670b045e0074c874996a8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Sep 2021 17:43:46 +0200 Subject: [PATCH 06/14] Better default for MBEDTLS_CHECK_RETURN in config.h An empty expansion is possible, but as documented its effect is to disable the feature, so that isn't a good example. Instead, use the GCC implementation as the default: it's plausible that it could work even on compilers that don't advertise themselves as sufficiently GCC-like to define __GNUC__, and if not it gives users a concrete idea of what the macro is supposed to do. Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d2357dc0f..40b20bd9d 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3708,7 +3708,7 @@ * If the implementation here is empty, this will effectively disable the * checking of functions' return values. */ -//#define MBEDTLS_CHECK_RETURN +//#define MBEDTLS_CHECK_RETURN __attribute__(__warn_unused_result__) /* PSA options */ /** From e568ebade1ab199e3dcb4f1384cad8dd5b5cf913 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Sep 2021 17:46:12 +0200 Subject: [PATCH 07/14] Use reserved identifier for warn_unused_result This is normally equivalent, but works even if some other header defines a macro called warn_unused_result. Signed-off-by: Gilles Peskine --- include/mbedtls/platform_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 4d9fcb6df..22ccc8979 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -141,7 +141,7 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; */ #if !defined(MBEDTLS_CHECK_RETURN) #if defined(__GNUC__) -#define MBEDTLS_CHECK_RETURN __attribute__((warn_unused_result)) +#define MBEDTLS_CHECK_RETURN __attribute__((__warn_unused_result__)) #elif defined(_MSC_VER) && _MSC_VER >= 1700 #include #define MBEDTLS_CHECK_RETURN _Check_return_ From 8472a10594a49e428439888cd1ff561aa0065e21 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Sep 2021 18:07:36 +0200 Subject: [PATCH 08/14] New configuration option MBEDTLS_CHECK_RETURN_WARNING MBEDTLS_CHECK_RETURN_TYPICAL defaults off, but is enabled if MBEDTLS_CHECK_RETURN_WARNING is enabled at compile time. (MBEDTLS_CHECK_RETURN_CRITICAL is always enabled.) The default is off so that a plausible program that builds with one version of Mbed TLS in the default configuration will still build under the next version. Signed-off-by: Gilles Peskine --- ChangeLog.d/check-return.txt | 13 +++++++++---- include/mbedtls/config.h | 23 +++++++++++++++++++++++ include/mbedtls/platform_util.h | 7 ++++++- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ChangeLog.d/check-return.txt b/ChangeLog.d/check-return.txt index 062b8a172..045b1805e 100644 --- a/ChangeLog.d/check-return.txt +++ b/ChangeLog.d/check-return.txt @@ -6,7 +6,12 @@ Bugfix where this function cannot fail, or full-module replacements with MBEDTLS_AES_ALT or MBEDTLS_DES_ALT. Reported by Armelle Duboc in #1092. -Changes - * Warn if errors from AES or DES functions are ignored. This is currently - supported on GCC-like compilers and on MSVC and can be configured by - setting MBEDTLS_CHECK_RETURN in config.h. +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/config.h b/include/mbedtls/config.h index 40b20bd9d..f5fe085eb 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 * diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 22ccc8979..7e04c4f22 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -173,7 +173,8 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; * 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. + * #MBEDTLS_CHECK_RETURN is implemented for the compiler in use and + * #MBEDTLS_CHECK_RETURN_WARNING is enabled in the compile-time configuration. * * \note The use of this macro is a work in progress. * This macro will be added to more functions in the future. @@ -181,7 +182,11 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; * 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 * From 15a7420d3c317604a937c3fe954f69ef73e10cc1 Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Thu, 5 Aug 2021 13:56:48 +0200 Subject: [PATCH 09/14] Silence warnings about unused return value This macro is introduced here for use in deprecated functions. It may also be useful in user code, so it is in a public header. Signed-off-by: Mateusz Starzyk Signed-off-by: Gilles Peskine --- include/mbedtls/platform_util.h | 7 +++++++ library/aes.c | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index 7e04c4f22..a90c10a4a 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -201,6 +201,13 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; */ #define MBEDTLS_CHECK_RETURN_OPTIONAL +/** \def MBEDTLS_IGNORE_RETURN + * + * Silences warning about unused return value given by functions + * with \c MBEDTLS_CHECK_RETURN attribute. + */ +#define MBEDTLS_IGNORE_RETURN(result) if( result ) {} + /** * \brief Securely zeroize a buffer * diff --git a/library/aes.c b/library/aes.c index e4bcfa023..609f852d5 100644 --- a/library/aes.c +++ b/library/aes.c @@ -926,7 +926,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 */ @@ -999,7 +999,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 */ From 9110809c46b6be092b7aa0db5537049efbd59ed0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Sep 2021 18:53:36 +0200 Subject: [PATCH 10/14] Fix mistake in the sample implementation of MBEDTLS_CHECK_RETURN Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index f5fe085eb..dadd8a30c 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3731,7 +3731,7 @@ * If the implementation here is empty, this will effectively disable the * checking of functions' return values. */ -//#define MBEDTLS_CHECK_RETURN __attribute__(__warn_unused_result__) +//#define MBEDTLS_CHECK_RETURN __attribute__((__warn_unused_result__)) /* PSA options */ /** From 327cb72e76dd8803f4d98edd4e75b87400bee3b1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Sep 2021 18:54:51 +0200 Subject: [PATCH 11/14] Cleaner implementation of MBEDTLS_IGNORE_RETURN The previous implementation was misparsed in constructs like `if (condition) MBEDTLS_IGNORE_RETURN(...); else ...;`. Implement it as an expression, tested with GCC, Clang and MSVC. Signed-off-by: Gilles Peskine --- include/mbedtls/platform_util.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index a90c10a4a..7e8046a47 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -205,8 +205,15 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; * * Silences warning about unused return value given by functions * with \c MBEDTLS_CHECK_RETURN attribute. +/* 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) if( result ) {} +#define MBEDTLS_IGNORE_RETURN(result) ( (void) !( result ) ) /** * \brief Securely zeroize a buffer From c2779328bf65cfb9c2d4dda02aafc93cd64f9014 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Sep 2021 18:56:17 +0200 Subject: [PATCH 12/14] Make MBEDTLS_IGNORE_RETURN configurable Signed-off-by: Gilles Peskine --- include/mbedtls/config.h | 8 ++++++++ include/mbedtls/platform_util.h | 10 ++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index dadd8a30c..a5366123e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -3733,6 +3733,14 @@ */ //#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/platform_util.h b/include/mbedtls/platform_util.h index 7e8046a47..e08644d38 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -176,6 +176,9 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; * #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 @@ -203,8 +206,10 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; /** \def MBEDTLS_IGNORE_RETURN * - * Silences warning about unused return value given by functions - * with \c MBEDTLS_CHECK_RETURN attribute. + * 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. @@ -214,6 +219,7 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425#c34 */ #define MBEDTLS_IGNORE_RETURN(result) ( (void) !( result ) ) +#endif /** * \brief Securely zeroize a buffer From 8ad54fa0b42f626b8a7a9cb243df6900817b8a1e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Sep 2021 19:22:48 +0200 Subject: [PATCH 13/14] Update files generated from config.h Signed-off-by: Gilles Peskine --- library/version_features.c | 3 +++ programs/test/query_config.c | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+) 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/test/query_config.c b/programs/test/query_config.c index 9760f626c..0dc8fa0c7 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 ) { From c79e4abaefc9d7ed7893421240f9e8a5e7642e0c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 30 Sep 2021 20:34:29 +0200 Subject: [PATCH 14/14] Fix typo in comment Signed-off-by: Gilles Peskine --- include/mbedtls/platform_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index e08644d38..f982db8c0 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -211,7 +211,7 @@ MBEDTLS_DEPRECATED typedef int mbedtls_deprecated_numeric_constant_t; */ #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 + * (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