From 30346f639edf6a4c67e9068ed9253c9655448c5d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Jun 2019 16:44:19 +0200 Subject: [PATCH] Make it easier to define MBEDTLS_PARAM_FAILED as assert Introduce a new configuration option MBEDTLS_CHECK_PARAMS_ASSERT, which is disabled by default. When this option is enabled, MBEDTLS_PARAM_FAILED defaults to assert rather than to a call to mbedtls_param_failed, and is included. This fixes #2671 (no easy way to make MBEDTLS_PARAM_FAILED assert) without breaking backward compatibility. With this change, `config.pl full` runs tests with MBEDTLS_PARAM_FAILED set to assert, so the tests will fail if a validation check fails, and programs don't need to provide their own definition of mbedtls_param_failed(). --- ChangeLog | 2 + include/mbedtls/config.h | 68 ++++++++++++++++++++++++--------- include/mbedtls/platform_util.h | 11 ++++++ library/version_features.c | 3 ++ programs/ssl/query_config.c | 8 ++++ 5 files changed, 73 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index cec77d2af..13034fd6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -57,6 +57,8 @@ Changes * Change wording in the `mbedtls_ssl_conf_max_frag_len()`'s documentation to improve clarity. Fixes #2258. * Replace multiple uses of MD2 by SHA-256 in X.509 test suite. Fixes #821. + * Make it easier to define MBEDTLS_PARAM_FAILED as assert (which config.h + suggests). #2671 = mbed TLS 2.16.1 branch released 2019-03-19 diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 654f9725e..2ab19804c 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -276,28 +276,52 @@ * For example, when a function accepts as input a pointer to a buffer that may * contain untrusted data, and its documentation mentions that this pointer * must not be NULL: - * - the pointer is checked to be non-NULL only if this option is enabled - * - the content of the buffer is always validated + * - The pointer is checked to be non-NULL only if this option is enabled. + * - The content of the buffer is always validated. * * When this flag is defined, if a library function receives a parameter that - * is invalid, it will: - * - invoke the macro MBEDTLS_PARAM_FAILED() which by default expands to a - * call to the function mbedtls_param_failed() - * - immediately return (with a specific error code unless the function - * returns void and can't communicate an error). + * is invalid: + * 1. The function will invoke the macro MBEDTLS_PARAM_FAILED(). + * 2. If MBEDTLS_PARAM_FAILED() did not terminate the program, the function + * will immediately return. If the function returns an Mbed TLS error code, + * the error code in this case is MBEDTLS_ERR_xxx_BAD_INPUT_DATA. * - * When defining this flag, you also need to: - * - either provide a definition of the function mbedtls_param_failed() in - * your application (see platform_util.h for its prototype) as the library - * calls that function, but does not provide a default definition for it, - * - or provide a different definition of the macro MBEDTLS_PARAM_FAILED() - * below if the above mechanism is not flexible enough to suit your needs. - * See the documentation of this macro later in this file. + * When defining this flag, you also need to arrange a definition for + * MBEDTLS_PARAM_FAILED(). You can do this by any of the following methods: + * - By default, the library defines MBEDTLS_PARAM_FAILED() to call a + * function mbedtls_param_failed(), but the library does not define this + * function. If you do not make any other arrangements, you must provide + * the function mbedtls_param_failed() in your application. + * See `platform_util.h` for its prototype. + * - If you enable the macro #MBEDTLS_CHECK_PARAMS_ASSERT, then the + * library defines #MBEDTLS_PARAM_FAILED(\c cond) to be `assert(cond)`. + * You can still supply an alternative definition of + * MBEDTLS_PARAM_FAILED(), which may call `assert`. + * - If you define a macro MBEDTLS_PARAM_FAILED() before including `config.h` + * or you uncomment the definition of MBEDTLS_PARAM_FAILED() in `config.h`, + * the library will call the macro that you defined and will not supply + * its own version. Note that if MBEDTLS_PARAM_FAILED() calls `assert`, + * you need to enable #MBEDTLS_CHECK_PARAMS_ASSERT so that library source + * files include ``. * * Uncomment to enable validation of application-controlled parameters. */ //#define MBEDTLS_CHECK_PARAMS +/** + * \def MBEDTLS_CHECK_PARAMS_ASSERT + * + * Allow MBEDTLS_PARAM_FAILED() to call `assert`, and make it default to + * `assert`. This macro is only used if #MBEDTLS_CHECK_PARAMS is defined. + * + * If this macro is not defined, then MBEDTLS_PARAM_FAILED() defaults to + * calling a function mbedtls_param_failed(). See the documentation of + * #MBEDTLS_CHECK_PARAMS for details. + * + * Uncomment to allow MBEDTLS_PARAM_FAILED() to call `assert`. + */ +//#define MBEDTLS_CHECK_PARAMS_ASSERT + /* \} name SECTION: System support */ /** @@ -3060,13 +3084,16 @@ /** * \brief This macro is invoked by the library when an invalid parameter - * is detected that is only checked with MBEDTLS_CHECK_PARAMS + * is detected that is only checked with #MBEDTLS_CHECK_PARAMS * (see the documentation of that option for context). * - * When you leave this undefined here, a default definition is - * provided that invokes the function mbedtls_param_failed(), - * which is declared in platform_util.h for the benefit of the - * library, but that you need to define in your application. + * When you leave this undefined here, the library provides + * a default definition. If the macro #MBEDTLS_CHECK_PARAMS_ASSERT + * is defined, the default definition is `assert(cond)`, + * otherwise the default definition calls a function + * mbedtls_param_failed(). This function is declared in + * `platform_util.h` for the benefit of the library, but + * you need to define in your application. * * When you define this here, this replaces the default * definition in platform_util.h (which no longer declares the @@ -3075,6 +3102,9 @@ * particular, that all the necessary declarations are visible * from within the library - you can ensure that by providing * them in this file next to the macro definition). + * If you define this macro to call `assert`, also define + * #MBEDTLS_CHECK_PARAMS_ASSERT so that library source files + * include ``. * * Note that you may define this macro to expand to nothing, in * which case you don't have to worry about declarations or diff --git a/include/mbedtls/platform_util.h b/include/mbedtls/platform_util.h index dba6d4598..09d096518 100644 --- a/include/mbedtls/platform_util.h +++ b/include/mbedtls/platform_util.h @@ -43,6 +43,12 @@ extern "C" { #if defined(MBEDTLS_CHECK_PARAMS) +#if defined(MBEDTLS_CHECK_PARAMS_ASSERT) +/* Allow the user to define MBEDTLS_PARAM_FAILED to something like assert + * (which is what our config.h suggests). */ +#include +#endif /* MBEDTLS_CHECK_PARAMS_ASSERT */ + #if defined(MBEDTLS_PARAM_FAILED) /** An alternative definition of MBEDTLS_PARAM_FAILED has been set in config.h. * @@ -50,6 +56,11 @@ extern "C" { * MBEDTLS_PARAM_FAILED() will expand to a call to mbedtls_param_failed(). */ #define MBEDTLS_PARAM_FAILED_ALT + +#elif defined(MBEDTLS_CHECK_PARAMS_ASSERT) +#define MBEDTLS_PARAM_FAILED( cond ) assert( cond ) +#define MBEDTLS_PARAM_FAILED_ALT + #else /* MBEDTLS_PARAM_FAILED */ #define MBEDTLS_PARAM_FAILED( cond ) \ mbedtls_param_failed( #cond, __FILE__, __LINE__ ) diff --git a/library/version_features.c b/library/version_features.c index 24143d052..a99ee808d 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -87,6 +87,9 @@ static const char *features[] = { #if defined(MBEDTLS_CHECK_PARAMS) "MBEDTLS_CHECK_PARAMS", #endif /* MBEDTLS_CHECK_PARAMS */ +#if defined(MBEDTLS_CHECK_PARAMS_ASSERT) + "MBEDTLS_CHECK_PARAMS_ASSERT", +#endif /* MBEDTLS_CHECK_PARAMS_ASSERT */ #if defined(MBEDTLS_TIMING_ALT) "MBEDTLS_TIMING_ALT", #endif /* MBEDTLS_TIMING_ALT */ diff --git a/programs/ssl/query_config.c b/programs/ssl/query_config.c index 6e281977e..bfb92720b 100644 --- a/programs/ssl/query_config.c +++ b/programs/ssl/query_config.c @@ -274,6 +274,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_CHECK_PARAMS */ +#if defined(MBEDTLS_CHECK_PARAMS_ASSERT) + if( strcmp( "MBEDTLS_CHECK_PARAMS_ASSERT", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_CHECK_PARAMS_ASSERT ); + return( 0 ); + } +#endif /* MBEDTLS_CHECK_PARAMS_ASSERT */ + #if defined(MBEDTLS_TIMING_ALT) if( strcmp( "MBEDTLS_TIMING_ALT", config ) == 0 ) {