From 1a3f9edc08e527953eff5a1a3a500ddd1e429807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 19 May 2020 12:38:31 +0200 Subject: [PATCH 01/21] Add config.h option MBEDTLS_ECP_NO_INTERNAL_RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No effect so far, except on dependency checking, as the feature it's meant to disable isn't implemented yet (so the descriptions in config.h and the ChangeLog entry are anticipation for now). Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/ecp-internal-rng.txt | 5 +++++ include/mbedtls/check_config.h | 8 ++++++++ include/mbedtls/config.h | 22 ++++++++++++++++++++++ library/version_features.c | 3 +++ programs/test/query_config.c | 8 ++++++++ scripts/config.py | 1 + tests/scripts/all.sh | 18 ++++++++++++++++++ 7 files changed, 65 insertions(+) create mode 100644 ChangeLog.d/ecp-internal-rng.txt diff --git a/ChangeLog.d/ecp-internal-rng.txt b/ChangeLog.d/ecp-internal-rng.txt new file mode 100644 index 000000000..bf11a7391 --- /dev/null +++ b/ChangeLog.d/ecp-internal-rng.txt @@ -0,0 +1,5 @@ +Changes + * The ECP module, enabled by `MBEDTLS_ECP_C`, now depends on + `MBEDTLS_CTR_DRBG_C` or `MBEDTLS_HMAC_DRBG_C` for some side-channel + coutermeasures. If side channels are not a concern, this dependency can + be avoided by enabling the new option `MBEDTLS_ECP_NO_INTERNAL_RNG`. diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index e2e45ac98..f2148a8b5 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -156,6 +156,14 @@ #error "MBEDTLS_ECP_C defined, but not all prerequisites" #endif +#if defined(MBEDTLS_ECP_C) && !( \ + defined(MBEDTLS_ECP_ALT) || \ + defined(MBEDTLS_CTR_DRBG_C) || \ + defined(MBEDTLS_HMAC_DRBG_C) || \ + defined(MBEDTLS_ECP_NO_INTERNAL_RNG)) +#error "MBEDTLS_ECP_C requires a DRBG module unless MBEDTLS_ECP_NO_INTERNAL_RNG is defined or an alternative implementation is used" +#endif + #if defined(MBEDTLS_PK_PARSE_C) && !defined(MBEDTLS_ASN1_PARSE_C) #error "MBEDTLS_PK_PARSE_C defined, but not all prerequesites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 60a3aee55..e00c546e5 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -781,6 +781,28 @@ */ #define MBEDTLS_ECP_NIST_OPTIM +/** + * \def MBEDTLS_ECP_NO_INTERNAL_RNG + * + * When this option is disabled, mbedtls_ecp_mul() will make use of an + * internal RNG when called with a NULL \c f_rng argument, in order to protect + * against some side-channel attacks. + * + * This protection introduces a dependency of the ECP module on one of the + * DRBG modules. For very constrained implementations that don't require this + * protection (for example, because you're only doing signature verification, + * so not manipulating any secret, or because local/physical side-channel + * attacks are outside your threat model), it might be desirable to get rid of + * that dependency. + * + * \warning Enabling this option makes some uses of ECP vulnerable to some + * side-channel attacks. Only enable it if you know that's not a problem for + * your use case. + * + * Uncomment this macro to disable some counter-measures in ECP. + */ +//#define MBEDTLS_ECP_NO_INTERNAL_RNG + /** * \def MBEDTLS_ECP_RESTARTABLE * diff --git a/library/version_features.c b/library/version_features.c index adc61a1fe..16a0cd0e8 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -354,6 +354,9 @@ static const char * const features[] = { #if defined(MBEDTLS_ECP_NIST_OPTIM) "MBEDTLS_ECP_NIST_OPTIM", #endif /* MBEDTLS_ECP_NIST_OPTIM */ +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + "MBEDTLS_ECP_NO_INTERNAL_RNG", +#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ #if defined(MBEDTLS_ECP_RESTARTABLE) "MBEDTLS_ECP_RESTARTABLE", #endif /* MBEDTLS_ECP_RESTARTABLE */ diff --git a/programs/test/query_config.c b/programs/test/query_config.c index 062dce6c1..98b065bfe 100644 --- a/programs/test/query_config.c +++ b/programs/test/query_config.c @@ -986,6 +986,14 @@ int query_config( const char *config ) } #endif /* MBEDTLS_ECP_NIST_OPTIM */ +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + if( strcmp( "MBEDTLS_ECP_NO_INTERNAL_RNG", config ) == 0 ) + { + MACRO_EXPANSION_TO_STR( MBEDTLS_ECP_NO_INTERNAL_RNG ); + return( 0 ); + } +#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ + #if defined(MBEDTLS_ECP_RESTARTABLE) if( strcmp( "MBEDTLS_ECP_RESTARTABLE", config ) == 0 ) { diff --git a/scripts/config.py b/scripts/config.py index 7f94587f6..3d297dc3d 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -173,6 +173,7 @@ EXCLUDE_FROM_FULL = frozenset([ 'MBEDTLS_DEPRECATED_REMOVED', # conflicts with deprecated options 'MBEDTLS_DEPRECATED_WARNING', # conflicts with deprecated options 'MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED', # influences the use of ECDH in TLS + 'MBEDTLS_ECP_NO_INTERNAL_RNG', # removes a feature 'MBEDTLS_ECP_RESTARTABLE', # incompatible with USE_PSA_CRYPTO 'MBEDTLS_ENTROPY_FORCE_SHA256', # interacts with CTR_DRBG_128_BIT_KEY 'MBEDTLS_HAVE_SSE2', # hardware dependency diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d911d493a..44c5fa27d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -865,6 +865,24 @@ component_test_no_hmac_drbg () { # so there's little value in running those lengthy tests here. } +component_test_ecp_no_internal_rng () { + msg "build: Default plus ECP_NO_INTERNAL_RNG minus DRBG modules" + scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG + scripts/config.py unset MBEDTLS_CTR_DRBG_C + scripts/config.py unset MBEDTLS_HMAC_DRBG_C + scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG + scripts/config.py unset MBEDTLS_PSA_CRYPTO_C # requires a DRBG + scripts/config.py unset MBEDTLS_PSA_CRYPTO_STORAGE_C # requires PSA Crypto + + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: ECP_NO_INTERNAL_RNG, no DRBG module" + make test + + # no SSL tests as they all depend on having a DRBG +} + component_test_new_ecdh_context () { msg "build: new ECDH context (ASan build)" # ~ 6 min scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT From c52a43c2bdcd5f118d9608013039f1e9e2ff372d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 22 May 2020 12:12:36 +0200 Subject: [PATCH 02/21] Implement use of internal DRBG for ecp_mul() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The case of MBEDTLS_ECP_RESTARTABLE isn't handled correctly yet: in that case the DRBG instance should persist when resuming the operation. This will be addressed in the next commit. When both CTR_DRBG and HMAC_DRBG are available, CTR_DRBG is preferred since both are suitable but CTR_DRBG tends to be faster and I needed a tie-breaker. There are currently three possible cases to test: - NO_INTERNAL_RNG is set -> tested in test_ecp_no_internal_rng - it's unset and CTR_DRBG is available -> tested in the default config - it's unset and CTR_DRBG is disabled -> tested in test_ecp_internal_rng_no_ctr_drbg Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/md.h | 2 + library/ecp.c | 137 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/include/mbedtls/md.h b/include/mbedtls/md.h index 0b0ec91ff..7e70778ce 100644 --- a/include/mbedtls/md.h +++ b/include/mbedtls/md.h @@ -104,6 +104,8 @@ typedef struct mbedtls_md_context_t * \brief This function returns the list of digests supported by the * generic digest module. * + * \note The list starts with the strongest available hashes. + * * \return A statically allocated array of digests. Each element * in the returned list is an integer belonging to the * message-digest enumeration #mbedtls_md_type_t. diff --git a/library/ecp.c b/library/ecp.c index 9522edf77..dc2b59244 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -105,6 +105,16 @@ #include "mbedtls/ecp_internal.h" +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) +#if defined(MBEDTLS_CTR_DRBG_C) +#include "mbedtls/ctr_drbg.h" +#elif defined(MBEDTLS_HMAC_DRBG_C) +#include "mbedtls/hmac_drbg.h" +#else +#error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." +#endif +#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ + #if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \ !defined(inline) && !defined(__cplusplus) #define inline __inline @@ -118,6 +128,113 @@ static unsigned long add_count, dbl_count, mul_count; #endif +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) +/* + * Currently ecp_mul() takes a RNG function as an argument, used for + * side-channel protection, but it can be NULL. The initial reasonning was + * that people will pass non-NULL RNG when they care about side-channels, but + * unfortunately we have some APIs that call ecp_mul() with a NULL RNG, with + * no opportunity for the user to do anything about it. + * + * The obvious strategies for addressing that include: + * - change those APIs so that they take RNG arguments; + * - require a global RNG to be available to all crypto modules. + * + * Unfortunately those would break compatibility. So what we do instead is + * have our own internal DRBG instance, seeded from the secret scalar. + * + * The following is a light-weight abstraction layer for doing that with + * CTR_DRBG or HMAC_DRBG. + */ + +#if defined(MBEDTLS_CTR_DRBG_C) +/* DRBG context type */ +typedef mbedtls_ctr_drbg_context ecp_drbg_context; + +/* DRBG context init */ +static inline void ecp_drbg_init( ecp_drbg_context *ctx ) +{ + mbedtls_ctr_drbg_init( ctx ); +} + +/* DRBG context free */ +static inline void ecp_drbg_free( ecp_drbg_context *ctx ) +{ + mbedtls_ctr_drbg_free( ctx ); +} + +/* DRBG function */ +static inline int ecp_drbg_random( void *p_rng, + unsigned char *output, size_t output_len ) +{ + return( mbedtls_ctr_drbg_random( p_rng, output, output_len ) ); +} + +/* + * Since CTR_DRBG doesn't have a seed_buf() function the way HMAC_DRBG does, + * we need to pass an entropy function when seeding. So we use a dummy + * function for that, and pass the actual entropy as customisation string. + * (During seeding of CTR_DRBG the entropy input and customisation string are + * concatenated before being used to update the secret state.) + */ +static int ecp_ctr_drbg_null_entropy(void *ctx, unsigned char *out, size_t len) +{ + (void) ctx; + memset( out, 0, len ); + return( 0 ); +} + +/* DRBG context seeding */ +static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) +{ + const unsigned char *secret_p = (const unsigned char *) secret->p; + const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); + + return( mbedtls_ctr_drbg_seed( ctx, ecp_ctr_drbg_null_entropy, NULL, + secret_p, secret_size ) ); +} + +#elif defined(MBEDTLS_HMAC_DRBG_C) +/* DRBG context type */ +typedef mbedtls_hmac_drbg_context ecp_drbg_context; + +/* DRBG context init */ +static inline void ecp_drbg_init( ecp_drbg_context *ctx ) +{ + mbedtls_hmac_drbg_init( ctx ); +} + +/* DRBG context free */ +static inline void ecp_drbg_free( ecp_drbg_context *ctx ) +{ + mbedtls_hmac_drbg_free( ctx ); +} + +/* DRBG function */ +static inline int ecp_drbg_random( void *p_rng, + unsigned char *output, size_t output_len ) +{ + return( mbedtls_hmac_drbg_random( p_rng, output, output_len ) ); +} + +/* DRBG context seeding */ +static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) +{ + const unsigned char *secret_p = (const unsigned char *) secret->p; + const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); + + /* The list starts with strong hashes */ + const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; + const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); + + return( mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_p, secret_size ) ); +} + +#else +#error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." +#endif /* DRBG modules */ +#endif /* MBEDTLS_ECP_NO_INTERNAL_RNG */ + #if defined(MBEDTLS_ECP_RESTARTABLE) /* * Maximum number of "basic operations" to be done in a row. @@ -2443,12 +2560,19 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, int ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA; #if defined(MBEDTLS_ECP_INTERNAL_ALT) char is_grp_capable = 0; +#endif +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_context drbg_ctx; #endif ECP_VALIDATE_RET( grp != NULL ); ECP_VALIDATE_RET( R != NULL ); ECP_VALIDATE_RET( m != NULL ); ECP_VALIDATE_RET( P != NULL ); +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_init( &drbg_ctx ); +#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ + #if defined(MBEDTLS_ECP_RESTARTABLE) /* reset ops count for this call if top-level */ if( rs_ctx != NULL && rs_ctx->depth++ == 0 ) @@ -2460,6 +2584,15 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MBEDTLS_MPI_CHK( mbedtls_internal_ecp_init( grp ) ); #endif /* MBEDTLS_ECP_INTERNAL_ALT */ +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + if( f_rng == NULL ) + { + MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + f_rng = &ecp_drbg_random; + p_rng = &drbg_ctx; + } +#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ + #if defined(MBEDTLS_ECP_RESTARTABLE) /* skip argument check when restarting */ if( rs_ctx == NULL || rs_ctx->rsm == NULL ) @@ -2490,6 +2623,10 @@ cleanup: mbedtls_internal_ecp_free( grp ); #endif /* MBEDTLS_ECP_INTERNAL_ALT */ +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_free( &drbg_ctx ); +#endif + #if defined(MBEDTLS_ECP_RESTARTABLE) if( rs_ctx != NULL ) rs_ctx->depth--; From f2a9fcff6211f51204aa77cea3d682a3984d31a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 3 Jun 2020 12:11:56 +0200 Subject: [PATCH 03/21] Move internal drbg init to specific mul functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While it seems cleaner and more convenient to set it in the top-level mbedtls_ecp_mul() function, the existence of the restartable option changes things - when it's enabled the drbg context needs to be saved in the restart context (more precisely in the restart_mul sub-context), which can only be done when it's allocated, which is in the curve-specific mul function. This commit only internal drbg management from mbedtls_ecp_mul() to ecp_mul_mxz() and ecp_mul_comb(), without modifying behaviour (even internal), and a future commit will modify the ecp_mul_comb() version to handle restart properly. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 59 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index dc2b59244..0af3f4746 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2221,11 +2221,25 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char w, p_eq_g, i; size_t d; - unsigned char T_size, T_ok; - mbedtls_ecp_point *T; + unsigned char T_size = 0, T_ok = 0; + mbedtls_ecp_point *T = NULL; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_context drbg_ctx; + + ecp_drbg_init( &drbg_ctx ); +#endif ECP_RS_ENTER( rsm ); +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + if( f_rng == NULL ) + { + MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + f_rng = &ecp_drbg_random; + p_rng = &drbg_ctx; + } +#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ + /* Is P the base point ? */ #if MBEDTLS_ECP_FIXED_POINT_OPTIM == 1 p_eq_g = ( mbedtls_mpi_cmp_mpi( &P->Y, &grp->G.Y ) == 0 && @@ -2297,6 +2311,10 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, cleanup: +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_free( &drbg_ctx ); +#endif + /* does T belong to the group? */ if( T == grp->T ) T = NULL; @@ -2487,9 +2505,22 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, unsigned char b; mbedtls_ecp_point RP; mbedtls_mpi PX; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_context drbg_ctx; + ecp_drbg_init( &drbg_ctx ); +#endif mbedtls_ecp_point_init( &RP ); mbedtls_mpi_init( &PX ); +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + if( f_rng == NULL ) + { + MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + f_rng = &ecp_drbg_random; + p_rng = &drbg_ctx; + } +#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ + /* Save PX and read from P before writing to R, in case P == R */ MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &PX, &P->X ) ); MBEDTLS_MPI_CHK( mbedtls_ecp_copy( &RP, P ) ); @@ -2542,6 +2573,10 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MBEDTLS_MPI_CHK( ecp_normalize_mxz( grp, R ) ); cleanup: +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_free( &drbg_ctx ); +#endif + mbedtls_ecp_point_free( &RP ); mbedtls_mpi_free( &PX ); return( ret ); @@ -2560,19 +2595,12 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, int ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA; #if defined(MBEDTLS_ECP_INTERNAL_ALT) char is_grp_capable = 0; -#endif -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_context drbg_ctx; #endif ECP_VALIDATE_RET( grp != NULL ); ECP_VALIDATE_RET( R != NULL ); ECP_VALIDATE_RET( m != NULL ); ECP_VALIDATE_RET( P != NULL ); -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_init( &drbg_ctx ); -#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ - #if defined(MBEDTLS_ECP_RESTARTABLE) /* reset ops count for this call if top-level */ if( rs_ctx != NULL && rs_ctx->depth++ == 0 ) @@ -2584,15 +2612,6 @@ int mbedtls_ecp_mul_restartable( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MBEDTLS_MPI_CHK( mbedtls_internal_ecp_init( grp ) ); #endif /* MBEDTLS_ECP_INTERNAL_ALT */ -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - if( f_rng == NULL ) - { - MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); - f_rng = &ecp_drbg_random; - p_rng = &drbg_ctx; - } -#endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ - #if defined(MBEDTLS_ECP_RESTARTABLE) /* skip argument check when restarting */ if( rs_ctx == NULL || rs_ctx->rsm == NULL ) @@ -2623,10 +2642,6 @@ cleanup: mbedtls_internal_ecp_free( grp ); #endif /* MBEDTLS_ECP_INTERNAL_ALT */ -#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - ecp_drbg_free( &drbg_ctx ); -#endif - #if defined(MBEDTLS_ECP_RESTARTABLE) if( rs_ctx != NULL ) rs_ctx->depth--; From 53fb66db12fa3ff7a8cd5bfe7f5707cb55aa626a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 09:43:14 +0200 Subject: [PATCH 04/21] Add support for RESTARTABLE with internal RNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we draw pseudo-random numbers at the beginning and end of the main loop. With ECP_RESTARTABLE, it's possible that between those two occasions we returned from the multiplication function, hence lost our internal DRBG context that lives in this function's stack frame. This would result in the same pseudo-random numbers being used for blinding in multiple places. While it's not immediately clear that this would give rise to an attack, it's also absolutely not clear that it doesn't. So let's avoid that by using a DRBG context that lives inside the restart context and persists across return/resume cycles. That way the RESTARTABLE case uses exactly the same pseudo-random numbers as the non-restartable case. Testing and compile-time options: - The case ECP_RESTARTABLE && !ECP_NO_INTERNAL_RNG is already tested by component_test_no_use_psa_crypto_full_cmake_asan. - The case ECP_RESTARTABLE && ECP_NO_INTERNAL_RNG didn't have a pre-existing test so a component is added. Testing and runtime options: when ECP_RESTARTABLE is enabled, the test suites already contain cases where restart happens and cases where it doesn't (because the operation is short enough or because restart is disabled (NULL restart context)). Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 34 ++++++++++++++++++++++++++++++++-- tests/scripts/all.sh | 19 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 0af3f4746..b8ce020be 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -282,6 +282,10 @@ struct mbedtls_ecp_restart_mul ecp_rsm_comb_core, /* ecp_mul_comb_core() */ ecp_rsm_final_norm, /* do the final normalization */ } state; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_context drbg_ctx; + unsigned char drbg_seeded; +#endif }; /* @@ -294,6 +298,10 @@ static void ecp_restart_rsm_init( mbedtls_ecp_restart_mul_ctx *ctx ) ctx->T = NULL; ctx->T_size = 0; ctx->state = ecp_rsm_init; +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_init( &ctx->drbg_ctx ); + ctx->drbg_seeded = 0; +#endif } /* @@ -315,6 +323,10 @@ static void ecp_restart_rsm_free( mbedtls_ecp_restart_mul_ctx *ctx ) mbedtls_free( ctx->T ); } +#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + ecp_drbg_free( &ctx->drbg_ctx ); +#endif + ecp_restart_rsm_init( ctx ); } @@ -2234,9 +2246,27 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng == NULL ) { - MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + /* Adjust pointers */ f_rng = &ecp_drbg_random; - p_rng = &drbg_ctx; +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx != NULL && rs_ctx->rsm != NULL ) + p_rng = &rs_ctx->rsm->drbg_ctx; + else +#endif + p_rng = &drbg_ctx; + + /* Initialize internal DRBG if necessary */ +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx == NULL || rs_ctx->rsm == NULL || + rs_ctx->rsm->drbg_seeded == 0 ) +#endif + { + MBEDTLS_MPI_CHK( ecp_drbg_seed( p_rng, m ) ); + } +#if defined(MBEDTLS_ECP_RESTARTABLE) + if( rs_ctx != NULL && rs_ctx->rsm != NULL ) + rs_ctx->rsm->drbg_seeded = 1; +#endif } #endif /* !MBEDTLS_ECP_NO_INTERNAL_RNG */ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 44c5fa27d..34c03c3fa 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -883,6 +883,25 @@ component_test_ecp_no_internal_rng () { # no SSL tests as they all depend on having a DRBG } +component_test_ecp_restartable_no_internal_rng () { + msg "build: Default plus ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG" + scripts/config.py set MBEDTLS_ECP_NO_INTERNAL_RNG + scripts/config.py set MBEDTLS_ECP_RESTARTABLE + scripts/config.py unset MBEDTLS_CTR_DRBG_C + scripts/config.py unset MBEDTLS_HMAC_DRBG_C + scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # requires HMAC_DRBG + scripts/config.py unset MBEDTLS_PSA_CRYPTO_C # requires CTR_DRBG + scripts/config.py unset MBEDTLS_PSA_CRYPTO_STORAGE_C # requires PSA Crypto + + CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . + make + + msg "test: ECP_RESTARTABLE and ECP_NO_INTERNAL_RNG, no DRBG module" + make test + + # no SSL tests as they all depend on having a DRBG +} + component_test_new_ecdh_context () { msg "build: new ECDH context (ASan build)" # ~ 6 min scripts/config.py unset MBEDTLS_ECDH_LEGACY_CONTEXT From 71d56678d1cdf8ca97201d92e4a8aab0371710a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 10:20:12 +0200 Subject: [PATCH 05/21] Update documentation about optional f_rng parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ecp.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 4c05b4fd0..875e1f8d3 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -846,6 +846,9 @@ int mbedtls_ecp_tls_write_group( const mbedtls_ecp_group *grp, * intermediate results to prevent potential timing attacks * targeting these results. We recommend always providing * a non-NULL \p f_rng. The overhead is negligible. + * Note: unless #MBEDTLS_ECP_NO_INTERNAL_RNG is defined, when + * \p f_rng is NULL, an internal RNG (seeded from the value + * of \p m) will be used instead. * * \param grp The ECP group to use. * This must be initialized and have group parameters From c7211784875025addffe0ef2c9f9593c8a5e105f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 10:31:06 +0200 Subject: [PATCH 06/21] Add Security ChangeLog entry for lack of blinding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/ecp-internal-rng.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog.d/ecp-internal-rng.txt b/ChangeLog.d/ecp-internal-rng.txt index bf11a7391..c0419acad 100644 --- a/ChangeLog.d/ecp-internal-rng.txt +++ b/ChangeLog.d/ecp-internal-rng.txt @@ -3,3 +3,13 @@ Changes `MBEDTLS_CTR_DRBG_C` or `MBEDTLS_HMAC_DRBG_C` for some side-channel coutermeasures. If side channels are not a concern, this dependency can be avoided by enabling the new option `MBEDTLS_ECP_NO_INTERNAL_RNG`. + +Security + * Fix side channel in mbedtls_ecp_check_pub_priv() and + mbedtls_pk_parse_key() / mbedtls_pk_parse_keyfile() (when loading a + private key that didn't include the uncompressed public key), as well as + mbedtls_ecp_mul() / mbedtls_ecp_mul_restartable() when called with a NULL + f_rng argument. An attacker with access to precise enough timing and + memory access information (typically an untrusted operating system + attacking a secure enclave) could fully recover the ECC private key. + Found and reported by Alejandro Cabrera Aldaya and Billy Brumley. From 22b1de309758e972a7b62bc1474a23ed05c4ca40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 10:43:29 +0200 Subject: [PATCH 07/21] Skip redundant checks for NULL f_rng MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unless MBEDTLS_ECP_NO_INTERNAL_RNG is defined, it's no longer possible for f_rng to be NULL at the places that randomize coordinates. Eliminate the NULL check in this case: - it makes it clearer to reviewers that randomization always happens (unless the user opted out at compile time) - a NULL check in a place where it's easy to prove the value is never NULL might upset or confuse static analyzers (including humans) - removing the check saves a bit of code size Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ecp.c b/library/ecp.c index b8ce020be..2dda15658 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2026,7 +2026,9 @@ static int ecp_mul_comb_core( const mbedtls_ecp_group *grp, mbedtls_ecp_point *R i = d; MBEDTLS_MPI_CHK( ecp_select_comb( grp, R, T, T_size, x[i] ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_lset( &R->Z, 1 ) ); +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != 0 ) +#endif MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, R, f_rng, p_rng ) ); } @@ -2159,7 +2161,9 @@ final_norm: * * Avoid the leak by randomizing coordinates before we normalize them. */ +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != 0 ) +#endif MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, RR, f_rng, p_rng ) ); MBEDTLS_ECP_BUDGET( MBEDTLS_ECP_OPS_INV ); @@ -2564,7 +2568,9 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MOD_ADD( RP.X ); /* Randomize coordinates of the starting point */ +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != NULL ) +#endif MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, &RP, f_rng, p_rng ) ); /* Loop invariant: R = result so far, RP = R + P */ @@ -2597,7 +2603,9 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, * * Avoid the leak by randomizing coordinates before we normalize them. */ +#if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng != NULL ) +#endif MBEDTLS_MPI_CHK( ecp_randomize_mxz( grp, R, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( ecp_normalize_mxz( grp, R ) ); From d53ef2ffd16490993010076f35f110bd65323caa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 4 Jun 2020 12:32:14 +0200 Subject: [PATCH 08/21] Use HMAC_DRBG by default for ECP internal DRBG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It results in smaller code than using CTR_DRBG (64 bytes smaller on ARMv6-M with arm-none-eabi-gcc 7.3.1), so let's use this by default when both are available. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 84 ++++++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 2dda15658..eb921618a 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -106,10 +106,10 @@ #include "mbedtls/ecp_internal.h" #if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) -#if defined(MBEDTLS_CTR_DRBG_C) -#include "mbedtls/ctr_drbg.h" -#elif defined(MBEDTLS_HMAC_DRBG_C) +#if defined(MBEDTLS_HMAC_DRBG_C) #include "mbedtls/hmac_drbg.h" +#elif defined(MBEDTLS_CTR_DRBG_C) +#include "mbedtls/ctr_drbg.h" #else #error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." #endif @@ -144,10 +144,48 @@ static unsigned long add_count, dbl_count, mul_count; * have our own internal DRBG instance, seeded from the secret scalar. * * The following is a light-weight abstraction layer for doing that with - * CTR_DRBG or HMAC_DRBG. + * HMAC_DRBG (first choice) or CTR_DRBG. */ -#if defined(MBEDTLS_CTR_DRBG_C) +#if defined(MBEDTLS_HMAC_DRBG_C) + +/* DRBG context type */ +typedef mbedtls_hmac_drbg_context ecp_drbg_context; + +/* DRBG context init */ +static inline void ecp_drbg_init( ecp_drbg_context *ctx ) +{ + mbedtls_hmac_drbg_init( ctx ); +} + +/* DRBG context free */ +static inline void ecp_drbg_free( ecp_drbg_context *ctx ) +{ + mbedtls_hmac_drbg_free( ctx ); +} + +/* DRBG function */ +static inline int ecp_drbg_random( void *p_rng, + unsigned char *output, size_t output_len ) +{ + return( mbedtls_hmac_drbg_random( p_rng, output, output_len ) ); +} + +/* DRBG context seeding */ +static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) +{ + const unsigned char *secret_p = (const unsigned char *) secret->p; + const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); + + /* The list starts with strong hashes */ + const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; + const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); + + return( mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_p, secret_size ) ); +} + +#elif defined(MBEDTLS_CTR_DRBG_C) + /* DRBG context type */ typedef mbedtls_ctr_drbg_context ecp_drbg_context; @@ -194,42 +232,6 @@ static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) secret_p, secret_size ) ); } -#elif defined(MBEDTLS_HMAC_DRBG_C) -/* DRBG context type */ -typedef mbedtls_hmac_drbg_context ecp_drbg_context; - -/* DRBG context init */ -static inline void ecp_drbg_init( ecp_drbg_context *ctx ) -{ - mbedtls_hmac_drbg_init( ctx ); -} - -/* DRBG context free */ -static inline void ecp_drbg_free( ecp_drbg_context *ctx ) -{ - mbedtls_hmac_drbg_free( ctx ); -} - -/* DRBG function */ -static inline int ecp_drbg_random( void *p_rng, - unsigned char *output, size_t output_len ) -{ - return( mbedtls_hmac_drbg_random( p_rng, output, output_len ) ); -} - -/* DRBG context seeding */ -static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) -{ - const unsigned char *secret_p = (const unsigned char *) secret->p; - const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); - - /* The list starts with strong hashes */ - const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; - const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); - - return( mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_p, secret_size ) ); -} - #else #error "Invalid configuration detected. Include check_config.h to ensure that the configuration is valid." #endif /* DRBG modules */ From 9b8d34edd48877972c2f446b183eca2212de63ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 8 Jun 2020 09:53:20 +0200 Subject: [PATCH 09/21] Avoid superflous randomization with restartable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Checking the budget only after the randomization is done means sometimes we were randomizing first, then noticing we ran out of budget, return, come back and randomize again before we finally normalize. While this is fine from a correctness and security perspective, it's a minor inefficiency, and can also be disconcerting while debugging, so we might as well avoid it. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp.c b/library/ecp.c index eb921618a..6c856dc3d 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2151,6 +2151,7 @@ static int ecp_mul_comb_after_precomp( const mbedtls_ecp_group *grp, rs_ctx->rsm->state = ecp_rsm_final_norm; final_norm: + MBEDTLS_ECP_BUDGET( MBEDTLS_ECP_OPS_INV ); #endif /* * Knowledge of the jacobian coordinates may leak the last few bits of the @@ -2168,7 +2169,6 @@ final_norm: #endif MBEDTLS_MPI_CHK( ecp_randomize_jac( grp, RR, f_rng, p_rng ) ); - MBEDTLS_ECP_BUDGET( MBEDTLS_ECP_OPS_INV ); MBEDTLS_MPI_CHK( ecp_normalize_jac( grp, RR ) ); #if defined(MBEDTLS_ECP_RESTARTABLE) From 25705e6757cfa052d007c7dede8c16577206f283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 10 Jun 2020 09:18:25 +0200 Subject: [PATCH 10/21] Fix typo in a comment Co-authored-by: Janos Follath --- library/ecp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp.c b/library/ecp.c index 6c856dc3d..13408799e 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -131,7 +131,7 @@ static unsigned long add_count, dbl_count, mul_count; #if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) /* * Currently ecp_mul() takes a RNG function as an argument, used for - * side-channel protection, but it can be NULL. The initial reasonning was + * side-channel protection, but it can be NULL. The initial reasoning was * that people will pass non-NULL RNG when they care about side-channels, but * unfortunately we have some APIs that call ecp_mul() with a NULL RNG, with * no opportunity for the user to do anything about it. From f4e3fc91336296f97ce89c7b144c28b3056550e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 12 Jun 2020 11:14:35 +0200 Subject: [PATCH 11/21] Use starts/finish around Lucky 13 dummy compressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #3246 Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/l13-hw-accel.txt | 7 +++++++ library/ssl_msg.c | 11 +++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/l13-hw-accel.txt diff --git a/ChangeLog.d/l13-hw-accel.txt b/ChangeLog.d/l13-hw-accel.txt new file mode 100644 index 000000000..53c79243b --- /dev/null +++ b/ChangeLog.d/l13-hw-accel.txt @@ -0,0 +1,7 @@ +Security + * Fix issue in Lucky 13 counter-measure that could make it ineffective when + hardware accelerators were used (using one of the MBEDTLS_SHAxxx_ALT + macros). This would cause the original Lucky 13 attack to be possible in + those configurations, allowing an active network attacker to recover + plaintext after repeated timing measurements under some conditions. + Reported and fix suggested by Luc Perneel in #3246. diff --git a/library/ssl_msg.c b/library/ssl_msg.c index ae8d07653..7fc4bf01d 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -1578,6 +1578,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, * linking an extra division function in some builds). */ size_t j, extra_run = 0; + /* This size is enough to server either as input to + * md_process() or as output to md_finish() */ unsigned char tmp[MBEDTLS_MD_MAX_BLOCK_SIZE]; /* @@ -1633,10 +1635,15 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, ssl_read_memory( data + rec->data_len, padlen ); mbedtls_md_hmac_finish( &transform->md_ctx_dec, mac_expect ); - /* Call mbedtls_md_process at least once due to cache attacks - * that observe whether md_process() was called of not */ + /* Dummy calls to compression function. + * Call mbedtls_md_process at least once due to cache attacks + * that observe whether md_process() was called of not. + * Respect the usual start-(process|update)-finish sequence for + * the sake of hardware accelerators that might require it. */ + mbedtls_md_starts( &transform->md_ctx_dec ); for( j = 0; j < extra_run + 1; j++ ) mbedtls_md_process( &transform->md_ctx_dec, tmp ); + mbedtls_md_finish( &transform->md_ctx_dec, tmp ); mbedtls_md_hmac_reset( &transform->md_ctx_dec ); From 4539a45cbff9dbf273121addb505ee2d0e75a557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 18 Jun 2020 12:27:56 +0200 Subject: [PATCH 12/21] Use fixed-length encoding for internal RNG seed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTR-DRBG and HMAC-DRBG may used the seed differently depending on its length. To avoid leaks, pass them a constant-length seed. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 13408799e..be9e42d91 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -172,16 +172,24 @@ static inline int ecp_drbg_random( void *p_rng, } /* DRBG context seeding */ -static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) +static int ecp_drbg_seed( ecp_drbg_context *ctx, + const mbedtls_mpi *secret, size_t secret_len ) { - const unsigned char *secret_p = (const unsigned char *) secret->p; - const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); - + int ret; + unsigned char secret_bytes[MBEDTLS_ECP_MAX_BYTES]; /* The list starts with strong hashes */ const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); - return( mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_p, secret_size ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, + secret_bytes, secret_len ) ); + + ret = mbedtls_hmac_drbg_seed_buf( ctx, md_info, secret_bytes, secret_len ); + +cleanup: + mbedtls_platform_zeroize( secret_bytes, secret_len ); + + return( ret ); } #elif defined(MBEDTLS_CTR_DRBG_C) @@ -223,13 +231,22 @@ static int ecp_ctr_drbg_null_entropy(void *ctx, unsigned char *out, size_t len) } /* DRBG context seeding */ -static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_mpi *secret ) +static int ecp_drbg_seed( ecp_drbg_context *ctx, + const mbedtls_mpi *secret, size_t secret_len ) { - const unsigned char *secret_p = (const unsigned char *) secret->p; - const size_t secret_size = secret->n * sizeof( mbedtls_mpi_uint ); + int ret; + unsigned char secret_bytes[MBEDTLS_ECP_MAX_BYTES]; - return( mbedtls_ctr_drbg_seed( ctx, ecp_ctr_drbg_null_entropy, NULL, - secret_p, secret_size ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, + secret_bytes, secret_len ) ); + + ret = mbedtls_ctr_drbg_seed( ctx, ecp_ctr_drbg_null_entropy, NULL, + secret_bytes, secret_len ); + +cleanup: + mbedtls_platform_zeroize( secret_bytes, secret_len ); + + return( ret ); } #else @@ -2267,7 +2284,8 @@ static int ecp_mul_comb( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, rs_ctx->rsm->drbg_seeded == 0 ) #endif { - MBEDTLS_MPI_CHK( ecp_drbg_seed( p_rng, m ) ); + const size_t m_len = ( grp->nbits + 7 ) / 8; + MBEDTLS_MPI_CHK( ecp_drbg_seed( p_rng, m, m_len ) ); } #if defined(MBEDTLS_ECP_RESTARTABLE) if( rs_ctx != NULL && rs_ctx->rsm != NULL ) @@ -2551,7 +2569,8 @@ static int ecp_mul_mxz( mbedtls_ecp_group *grp, mbedtls_ecp_point *R, #if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if( f_rng == NULL ) { - MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m ) ); + const size_t m_len = ( grp->nbits + 7 ) / 8; + MBEDTLS_MPI_CHK( ecp_drbg_seed( &drbg_ctx, m, m_len ) ); f_rng = &ecp_drbg_random; p_rng = &drbg_ctx; } From 1215c54754bc3a40efafc74d65c5155923934c22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 19 Jun 2020 11:59:49 +0200 Subject: [PATCH 13/21] Add length check in ecp_drbg_seed() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While this is a static function, so right now we know we don't need the check, things may change in the future, so better be on the safe side. Signed-off-by: Manuel Pégourié-Gonnard --- library/ecp.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/library/ecp.c b/library/ecp.c index be9e42d91..7b205160a 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -181,6 +181,12 @@ static int ecp_drbg_seed( ecp_drbg_context *ctx, const mbedtls_md_type_t md_type = mbedtls_md_list()[0]; const mbedtls_md_info_t *md_info = mbedtls_md_info_from_type( md_type ); + if( secret_len > MBEDTLS_ECP_MAX_BYTES ) + { + ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; + goto cleanup; + } + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, secret_bytes, secret_len ) ); @@ -237,6 +243,12 @@ static int ecp_drbg_seed( ecp_drbg_context *ctx, int ret; unsigned char secret_bytes[MBEDTLS_ECP_MAX_BYTES]; + if( secret_len > MBEDTLS_ECP_MAX_BYTES ) + { + ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; + goto cleanup; + } + MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( secret, secret_bytes, secret_len ) ); From f8f5026a3b0a0692741e6b24b93c4d6e902e71c1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 25 Jun 2020 11:48:11 +0100 Subject: [PATCH 14/21] Add ChangeLog entry for #3147: MSVC flags Signed-off-by: Janos Follath --- ChangeLog.d/align-msvc-flags.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/align-msvc-flags.txt diff --git a/ChangeLog.d/align-msvc-flags.txt b/ChangeLog.d/align-msvc-flags.txt new file mode 100644 index 000000000..02de3fd34 --- /dev/null +++ b/ChangeLog.d/align-msvc-flags.txt @@ -0,0 +1,3 @@ +Changes + * Align MSVC error flag with GCC and Clang. Contributed by Carlos Gomes + Martinho. #3147 From 8a43bd1d20b438344ea0448847989483087c5fcb Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 25 Jun 2020 11:48:27 +0100 Subject: [PATCH 15/21] Add ChangeLog entry for #3217: avoid re-assignment Signed-off-by: Janos Follath --- ChangeLog.d/remove-extra-assignment.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/remove-extra-assignment.txt diff --git a/ChangeLog.d/remove-extra-assignment.txt b/ChangeLog.d/remove-extra-assignment.txt new file mode 100644 index 000000000..a98c42347 --- /dev/null +++ b/ChangeLog.d/remove-extra-assignment.txt @@ -0,0 +1,3 @@ +Changes + * Remove superfluous assignment in mbedtls_ssl_parse_certificate(). Reported + in #3182 and fix submitted by irwir. #3217 From 3ec2e4a464df942c14b369acf5f7913d8ff94ad1 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 25 Jun 2020 11:53:33 +0100 Subject: [PATCH 16/21] Add ChangeLog entry for #3239: win2k net support Signed-off-by: Janos Follath --- ChangeLog.d/add_win2k_net_socket.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/add_win2k_net_socket.txt diff --git a/ChangeLog.d/add_win2k_net_socket.txt b/ChangeLog.d/add_win2k_net_socket.txt new file mode 100644 index 000000000..9842c5d87 --- /dev/null +++ b/ChangeLog.d/add_win2k_net_socket.txt @@ -0,0 +1,2 @@ +Features + * Add support for Windows 2000 in net_sockets. Contributed by opatomic. #3239 From 0b849818d38918415d1c9a735fb79d0d90f544e4 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 25 Jun 2020 13:08:25 +0100 Subject: [PATCH 17/21] Add ChangeLog entry for #3311: fix uninitialised variable Signed-off-by: Janos Follath --- ChangeLog.d/make-cpp-check-happy.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 ChangeLog.d/make-cpp-check-happy.txt diff --git a/ChangeLog.d/make-cpp-check-happy.txt b/ChangeLog.d/make-cpp-check-happy.txt new file mode 100644 index 000000000..2ae74cb0a --- /dev/null +++ b/ChangeLog.d/make-cpp-check-happy.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix false positive uninitialised variable reported by cpp-check. + Contributed by Sander Visser in #3311. From a805c4d3288facd050851924d39c41f3fac9e373 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Thu, 25 Jun 2020 13:21:15 +0100 Subject: [PATCH 18/21] Add ChangeLog entry for #3319: fix typo in test Signed-off-by: Janos Follath --- ChangeLog.d/fix-typo-in-xts-test.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/fix-typo-in-xts-test.txt diff --git a/ChangeLog.d/fix-typo-in-xts-test.txt b/ChangeLog.d/fix-typo-in-xts-test.txt new file mode 100644 index 000000000..4711b8cf9 --- /dev/null +++ b/ChangeLog.d/fix-typo-in-xts-test.txt @@ -0,0 +1,2 @@ +Changes + * Fix typo in XTS tests. Reported and fix submitted by Kxuan. #3319 From 1959010c4be11a62d99e95354d9244e8bf3df581 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 26 Jun 2020 11:26:02 +0100 Subject: [PATCH 19/21] Assemble changelog Executed scripts/assemble_changelog.py and manually fixed style where it diverged from the instructions in ChangeLog.d/00README.md. Manually added ChangeLog.d/bugfix_PR3405 which didn't have the .txt extension as prescribed in ChangeLog.d/00README.md and deleted it afterwards. Signed-off-by: Janos Follath --- ChangeLog | 123 ++++++++++++++++++ ChangeLog.d/add_win2k_net_socket.txt | 2 - ChangeLog.d/align-msvc-flags.txt | 3 - ChangeLog.d/bugfix.txt | 4 - ChangeLog.d/bugfix_PR2855.txt | 2 - ChangeLog.d/bugfix_PR3333.txt | 2 - ChangeLog.d/bugfix_PR3405 | 5 - ChangeLog.d/bugfix_PR3421.txt | 2 - ChangeLog.d/bugfix_PR3422.txt | 2 - ChangeLog.d/ecp-internal-rng.txt | 15 --- ChangeLog.d/error-asn1.txt | 2 - ChangeLog.d/error_const.txt | 6 - ChangeLog.d/fix-ecp-mul-memory-leak.txt | 3 - ChangeLog.d/fix-ecp_double_add_mxz.txt | 4 - .../fix-gcc-format-signedness-warnings.txt | 4 - .../fix-masked-hw-record-init-error.txt | 3 - ...fix-null-ptr-deref-in-mbedtls_ssl_free.txt | 3 - ...n-ascii-string-in-mbedtls_x509_dn_gets.txt | 3 - ChangeLog.d/fix-typo-in-xts-test.txt | 2 - .../inline-mbedtls_gcc_group_to_psa.txt | 4 - ChangeLog.d/l13-hw-accel.txt | 7 - ChangeLog.d/make-cpp-check-happy.txt | 3 - ChangeLog.d/max_pathlen.txt | 5 - ChangeLog.d/md_switch.txt | 3 - ChangeLog.d/midipix-support.txt | 2 - ChangeLog.d/montmul-cmp-branch.txt | 6 - ...x509_crt_parse_der_with_ext_cb_routine.txt | 5 - .../pass-unsupported-policies-to-callback.txt | 4 - ChangeLog.d/psa-lifetime-locations.txt | 8 -- ChangeLog.d/remove-extra-assignment.txt | 3 - ChangeLog.d/ssl_context_info.txt | 3 - ChangeLog.d/ssl_write_certificate_request.txt | 3 - ChangeLog.d/sysctl-arnd-support.txt | 2 - ChangeLog.d/tests-common-code.txt | 5 - ChangeLog.d/unified-exit-in-examples.txt | 4 - ChangeLog.d/uniformize_bounds_checks.txt | 9 -- ChangeLog.d/use-find-python3-cmake.txt | 2 - 37 files changed, 123 insertions(+), 145 deletions(-) delete mode 100644 ChangeLog.d/add_win2k_net_socket.txt delete mode 100644 ChangeLog.d/align-msvc-flags.txt delete mode 100644 ChangeLog.d/bugfix.txt delete mode 100644 ChangeLog.d/bugfix_PR2855.txt delete mode 100644 ChangeLog.d/bugfix_PR3333.txt delete mode 100644 ChangeLog.d/bugfix_PR3405 delete mode 100644 ChangeLog.d/bugfix_PR3421.txt delete mode 100644 ChangeLog.d/bugfix_PR3422.txt delete mode 100644 ChangeLog.d/ecp-internal-rng.txt delete mode 100644 ChangeLog.d/error-asn1.txt delete mode 100644 ChangeLog.d/error_const.txt delete mode 100644 ChangeLog.d/fix-ecp-mul-memory-leak.txt delete mode 100644 ChangeLog.d/fix-ecp_double_add_mxz.txt delete mode 100644 ChangeLog.d/fix-gcc-format-signedness-warnings.txt delete mode 100644 ChangeLog.d/fix-masked-hw-record-init-error.txt delete mode 100644 ChangeLog.d/fix-null-ptr-deref-in-mbedtls_ssl_free.txt delete mode 100644 ChangeLog.d/fix-print-non-ascii-string-in-mbedtls_x509_dn_gets.txt delete mode 100644 ChangeLog.d/fix-typo-in-xts-test.txt delete mode 100644 ChangeLog.d/inline-mbedtls_gcc_group_to_psa.txt delete mode 100644 ChangeLog.d/l13-hw-accel.txt delete mode 100644 ChangeLog.d/make-cpp-check-happy.txt delete mode 100644 ChangeLog.d/max_pathlen.txt delete mode 100644 ChangeLog.d/md_switch.txt delete mode 100644 ChangeLog.d/midipix-support.txt delete mode 100644 ChangeLog.d/montmul-cmp-branch.txt delete mode 100644 ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt delete mode 100644 ChangeLog.d/pass-unsupported-policies-to-callback.txt delete mode 100644 ChangeLog.d/psa-lifetime-locations.txt delete mode 100644 ChangeLog.d/remove-extra-assignment.txt delete mode 100644 ChangeLog.d/ssl_context_info.txt delete mode 100644 ChangeLog.d/ssl_write_certificate_request.txt delete mode 100644 ChangeLog.d/sysctl-arnd-support.txt delete mode 100644 ChangeLog.d/tests-common-code.txt delete mode 100644 ChangeLog.d/unified-exit-in-examples.txt delete mode 100644 ChangeLog.d/uniformize_bounds_checks.txt delete mode 100644 ChangeLog.d/use-find-python3-cmake.txt diff --git a/ChangeLog b/ChangeLog index 062a1ad32..e18ef0bd5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,128 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Default behavior changes + * In the experimental PSA secure element interface, change the encoding of + key lifetimes to encode a persistence level and the location. Although C + prototypes do not effectively change, code calling + psa_register_se_driver() must be modified to pass the driver's location + instead of the keys' lifetime. If the library is upgraded on an existing + device, keys created with the old lifetime value will not be readable or + removable through Mbed TLS after the upgrade. + +Features + * New functions in the error module return constant strings for + high- and low-level error codes, complementing mbedtls_strerror() + which constructs a string for any error code, including compound + ones, but requires a writable buffer. Contributed by Gaurav Aggarwal + in #3176. + * The new utility programs/ssl/ssl_context_info prints a human-readable + dump of an SSL context saved with mbedtls_ssl_context_save(). + * Add support for midipix, a POSIX layer for Microsoft Windows. + * Add new mbedtls_x509_crt_parse_der_with_ext_cb() routine which allows + parsing unsupported certificate extensions via user provided callback. + Contributed by Nicola Di Lieto in #3243 as + a solution to #3241. + * Pass the "certificate policies" extension to the callback supplied to + mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported + policies (#3419). + * Added support to entropy_poll for the kern.arandom syscall supported on + some BSD systems. Contributed by Nia Alarie in #3423. + * Add support for Windows 2000 in net_sockets. Contributed by opatomic. #3239 + +Security + * Fix a side channel vulnerability in modular exponentiation that could + reveal an RSA private key used in a secure enclave. Noticed by Sangho Lee, + Ming-Wei Shih, Prasun Gera, Taesoo Kim and Hyesoon Kim (Georgia Institute + of Technology); and Marcus Peinado (Microsoft Research). Reported by Raoul + Strackx (Fortanix) in #3394. + * Fix side channel in mbedtls_ecp_check_pub_priv() and + mbedtls_pk_parse_key() / mbedtls_pk_parse_keyfile() (when loading a + private key that didn't include the uncompressed public key), as well as + mbedtls_ecp_mul() / mbedtls_ecp_mul_restartable() when called with a NULL + f_rng argument. An attacker with access to precise enough timing and + memory access information (typically an untrusted operating system + attacking a secure enclave) could fully recover the ECC private key. + Found and reported by Alejandro Cabrera Aldaya and Billy Brumley. + * Fix issue in Lucky 13 counter-measure that could make it ineffective when + hardware accelerators were used (using one of the MBEDTLS_SHAxxx_ALT + macros). This would cause the original Lucky 13 attack to be possible in + those configurations, allowing an active network attacker to recover + plaintext after repeated timing measurements under some conditions. + Reported and fix suggested by Luc Perneel in #3246. + +Bugfix + * Fix the Visual Studio Release x64 build configuration for mbedtls itself. + Completes a previous fix in Mbed TLS 2.19 that only fixed the build for + the example programs. Reported in #1430 and fix contributed by irwir. + * Fix undefined behavior in X.509 certificate parsing if the + pathLenConstraint basic constraint value is equal to INT_MAX. + The actual effect with almost every compiler is the intended + behavior, so this is unlikely to be exploitable anywhere. #3192 + * Fix issue with a detected HW accelerated record error not being exposed + due to shadowed variable. Contributed by Sander Visser in #3310. + * Avoid NULL pointer dereferencing if mbedtls_ssl_free() is called with a + NULL pointer argument. Contributed by Sander Visser in #3312. + * Fix potential linker errors on dual world platforms by inlining + mbedtls_gcc_group_to_psa(). This allows the pk.c module to link separately + from psa_crypto.c. Fixes #3300. + * Remove dead code in X.509 certificate parsing. Contributed by irwir in + #2855. + * Include asn1.h in error.c. Fixes #3328 reported by David Hu. + * Fix potential memory leaks in ecp_randomize_jac() and ecp_randomize_mxz() + when PRNG function fails. Contributed by Jonas Lejeune in #3318. + * Remove unused macros from MSVC projects. Reported in #3297 and fix + submitted in #3333 by irwir. + * Add additional bounds checks in ssl_write_client_hello() preventing + output buffer overflow if the configuration declared a buffer that was + too small. + * Set _POSIX_C_SOURCE to at least 200112L in C99 code. Reported in #3420 and + fix submitted in #3421 by Nia Alarie. + * Fix building library/net_sockets.c and the ssl_mail_client program on + NetBSD. Contributed by Nia Alarie in #3422. + * Fix false positive uninitialised variable reported by cpp-check. + Contributed by Sander Visser in #3311. + * Update iv and len context pointers manually when reallocating buffers + using the MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH feature. This caused issues + when receiving a connection with CID, when these fields were shifted + in ssl_parse_record_header(). + +Changes + * Fix warnings about signedness issues in format strings. The build is now + clean of -Wformat-signedness warnings. Contributed by Kenneth Soerensen + in #3153. + * Fix minor performance issue in operations on Curve25519 caused by using a + suboptimal modular reduction in one place. Found and fix contributed by + Aurelien Jarno in #3209. + * Combine identical cases in switch statements in md.c. Contributed + by irwir in #3208. + * Simplify a bounds check in ssl_write_certificate_request(). Contributed + by irwir in #3150. + * Unify the example programs termination to call mbedtls_exit() instead of + using a return command. This has been done to enable customization of the + behavior in bare metal environments. + * Fix mbedtls_x509_dn_gets to escape non-ASCII characters as "?". + Contributed by Koh M. Nakagawa in #3326. + * Use FindPython3 when cmake version >= 3.15.0 + * Abort the ClientHello writing function as soon as some extension doesn't + fit into the record buffer. Previously, such extensions were silently + dropped. As a consequence, the TLS handshake now fails when the output + buffer is not large enough to hold the ClientHello. + * The unit tests now rely on header files in tests/include/test and source + files in tests/src. When building with make or cmake, the files in + tests/src are compiled and the resulting object linked into each test + executable. + * The ECP module, enabled by `MBEDTLS_ECP_C`, now depends on + `MBEDTLS_CTR_DRBG_C` or `MBEDTLS_HMAC_DRBG_C` for some side-channel + coutermeasures. If side channels are not a concern, this dependency can + be avoided by enabling the new option `MBEDTLS_ECP_NO_INTERNAL_RNG`. + * Align MSVC error flag with GCC and Clang. Contributed by Carlos Gomes + Martinho. #3147 + * Remove superfluous assignment in mbedtls_ssl_parse_certificate(). Reported + in #3182 and fix submitted by irwir. #3217 + * Fix typo in XTS tests. Reported and fix submitted by Kxuan. #3319 + = mbed TLS 2.22.0 branch released 2020-04-14 New deprecations diff --git a/ChangeLog.d/add_win2k_net_socket.txt b/ChangeLog.d/add_win2k_net_socket.txt deleted file mode 100644 index 9842c5d87..000000000 --- a/ChangeLog.d/add_win2k_net_socket.txt +++ /dev/null @@ -1,2 +0,0 @@ -Features - * Add support for Windows 2000 in net_sockets. Contributed by opatomic. #3239 diff --git a/ChangeLog.d/align-msvc-flags.txt b/ChangeLog.d/align-msvc-flags.txt deleted file mode 100644 index 02de3fd34..000000000 --- a/ChangeLog.d/align-msvc-flags.txt +++ /dev/null @@ -1,3 +0,0 @@ -Changes - * Align MSVC error flag with GCC and Clang. Contributed by Carlos Gomes - Martinho. #3147 diff --git a/ChangeLog.d/bugfix.txt b/ChangeLog.d/bugfix.txt deleted file mode 100644 index 922bd318b..000000000 --- a/ChangeLog.d/bugfix.txt +++ /dev/null @@ -1,4 +0,0 @@ -Bugfix - * Fix the Visual Studio Release x64 build configuration for mbedtls itself. - Completes a previous fix in Mbed TLS 2.19 that only fixed the build for - the example programs. Reported in #1430 and fix contributed by irwir. diff --git a/ChangeLog.d/bugfix_PR2855.txt b/ChangeLog.d/bugfix_PR2855.txt deleted file mode 100644 index 6e29710ec..000000000 --- a/ChangeLog.d/bugfix_PR2855.txt +++ /dev/null @@ -1,2 +0,0 @@ -Bugfix - * Remove dead code in X.509 certificate parsing. Contributed by irwir in #2855. diff --git a/ChangeLog.d/bugfix_PR3333.txt b/ChangeLog.d/bugfix_PR3333.txt deleted file mode 100644 index 90766ac71..000000000 --- a/ChangeLog.d/bugfix_PR3333.txt +++ /dev/null @@ -1,2 +0,0 @@ -Bugfix - * Remove unused macros from MSVC projects. Reported in #3297 and fix submitted in #3333 by irwir. diff --git a/ChangeLog.d/bugfix_PR3405 b/ChangeLog.d/bugfix_PR3405 deleted file mode 100644 index 73c57c081..000000000 --- a/ChangeLog.d/bugfix_PR3405 +++ /dev/null @@ -1,5 +0,0 @@ -Bugfix - * Update iv and len context pointers manually when reallocating buffers - using the MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH feature. This caused issues - when receiving a connection with CID, when these fields were shifted - in ssl_parse_record_header(). diff --git a/ChangeLog.d/bugfix_PR3421.txt b/ChangeLog.d/bugfix_PR3421.txt deleted file mode 100644 index b52dee00f..000000000 --- a/ChangeLog.d/bugfix_PR3421.txt +++ /dev/null @@ -1,2 +0,0 @@ -Bugfix - * Set _POSIX_C_SOURCE to at least 200112L in C99 code. Reported in #3420 and fix submitted in #3421 by Nia Alarie. diff --git a/ChangeLog.d/bugfix_PR3422.txt b/ChangeLog.d/bugfix_PR3422.txt deleted file mode 100644 index dfe152c36..000000000 --- a/ChangeLog.d/bugfix_PR3422.txt +++ /dev/null @@ -1,2 +0,0 @@ -Bugfix - * Fix building library/net_sockets.c and the ssl_mail_client program on NetBSD. Contributed by Nia Alarie in #3422. diff --git a/ChangeLog.d/ecp-internal-rng.txt b/ChangeLog.d/ecp-internal-rng.txt deleted file mode 100644 index c0419acad..000000000 --- a/ChangeLog.d/ecp-internal-rng.txt +++ /dev/null @@ -1,15 +0,0 @@ -Changes - * The ECP module, enabled by `MBEDTLS_ECP_C`, now depends on - `MBEDTLS_CTR_DRBG_C` or `MBEDTLS_HMAC_DRBG_C` for some side-channel - coutermeasures. If side channels are not a concern, this dependency can - be avoided by enabling the new option `MBEDTLS_ECP_NO_INTERNAL_RNG`. - -Security - * Fix side channel in mbedtls_ecp_check_pub_priv() and - mbedtls_pk_parse_key() / mbedtls_pk_parse_keyfile() (when loading a - private key that didn't include the uncompressed public key), as well as - mbedtls_ecp_mul() / mbedtls_ecp_mul_restartable() when called with a NULL - f_rng argument. An attacker with access to precise enough timing and - memory access information (typically an untrusted operating system - attacking a secure enclave) could fully recover the ECC private key. - Found and reported by Alejandro Cabrera Aldaya and Billy Brumley. diff --git a/ChangeLog.d/error-asn1.txt b/ChangeLog.d/error-asn1.txt deleted file mode 100644 index c165696fd..000000000 --- a/ChangeLog.d/error-asn1.txt +++ /dev/null @@ -1,2 +0,0 @@ -Bugfix - * Include asn1.h in error.c. Fixes #3328 reported by David Hu. diff --git a/ChangeLog.d/error_const.txt b/ChangeLog.d/error_const.txt deleted file mode 100644 index e0086b74c..000000000 --- a/ChangeLog.d/error_const.txt +++ /dev/null @@ -1,6 +0,0 @@ -Features - * New functions in the error module return constant strings for - high- and low-level error codes, complementing mbedtls_strerror() - which constructs a string for any error code, including compound - ones, but requires a writable buffer. Contributed by Gaurav Aggarwal - in #3176. diff --git a/ChangeLog.d/fix-ecp-mul-memory-leak.txt b/ChangeLog.d/fix-ecp-mul-memory-leak.txt deleted file mode 100644 index e82cadc2d..000000000 --- a/ChangeLog.d/fix-ecp-mul-memory-leak.txt +++ /dev/null @@ -1,3 +0,0 @@ -Bugfix - * Fix potential memory leaks in ecp_randomize_jac() and ecp_randomize_mxz() - when PRNG function fails. Contributed by Jonas Lejeune in #3318. diff --git a/ChangeLog.d/fix-ecp_double_add_mxz.txt b/ChangeLog.d/fix-ecp_double_add_mxz.txt deleted file mode 100644 index 91531b2bb..000000000 --- a/ChangeLog.d/fix-ecp_double_add_mxz.txt +++ /dev/null @@ -1,4 +0,0 @@ -Changes - * Fix minor performance issue in operations on Curve25519 caused by using a - suboptimal modular reduction in one place. Found and fix contributed by - Aurelien Jarno in #3209. diff --git a/ChangeLog.d/fix-gcc-format-signedness-warnings.txt b/ChangeLog.d/fix-gcc-format-signedness-warnings.txt deleted file mode 100644 index 2d22b942d..000000000 --- a/ChangeLog.d/fix-gcc-format-signedness-warnings.txt +++ /dev/null @@ -1,4 +0,0 @@ -Changes - * Fix warnings about signedness issues in format strings. The build is now - clean of -Wformat-signedness warnings. Contributed by Kenneth Soerensen - in #3153. diff --git a/ChangeLog.d/fix-masked-hw-record-init-error.txt b/ChangeLog.d/fix-masked-hw-record-init-error.txt deleted file mode 100644 index 2ef80daae..000000000 --- a/ChangeLog.d/fix-masked-hw-record-init-error.txt +++ /dev/null @@ -1,3 +0,0 @@ -Bugfix - * Fix issue with a detected HW accelerated record error not being exposed - due to shadowed variable. Contributed by Sander Visser in #3310. diff --git a/ChangeLog.d/fix-null-ptr-deref-in-mbedtls_ssl_free.txt b/ChangeLog.d/fix-null-ptr-deref-in-mbedtls_ssl_free.txt deleted file mode 100644 index e631f4d02..000000000 --- a/ChangeLog.d/fix-null-ptr-deref-in-mbedtls_ssl_free.txt +++ /dev/null @@ -1,3 +0,0 @@ -Bugfix - * Avoid NULL pointer dereferencing if mbedtls_ssl_free() is called with a - NULL pointer argument. Contributed by Sander Visser in #3312. diff --git a/ChangeLog.d/fix-print-non-ascii-string-in-mbedtls_x509_dn_gets.txt b/ChangeLog.d/fix-print-non-ascii-string-in-mbedtls_x509_dn_gets.txt deleted file mode 100644 index 6be1e5b54..000000000 --- a/ChangeLog.d/fix-print-non-ascii-string-in-mbedtls_x509_dn_gets.txt +++ /dev/null @@ -1,3 +0,0 @@ -Changes - * Fix mbedtls_x509_dn_gets to escape non-ASCII characters as "?". - Contributed by Koh M. Nakagawa in #3326. diff --git a/ChangeLog.d/fix-typo-in-xts-test.txt b/ChangeLog.d/fix-typo-in-xts-test.txt deleted file mode 100644 index 4711b8cf9..000000000 --- a/ChangeLog.d/fix-typo-in-xts-test.txt +++ /dev/null @@ -1,2 +0,0 @@ -Changes - * Fix typo in XTS tests. Reported and fix submitted by Kxuan. #3319 diff --git a/ChangeLog.d/inline-mbedtls_gcc_group_to_psa.txt b/ChangeLog.d/inline-mbedtls_gcc_group_to_psa.txt deleted file mode 100644 index d0bd1dc6a..000000000 --- a/ChangeLog.d/inline-mbedtls_gcc_group_to_psa.txt +++ /dev/null @@ -1,4 +0,0 @@ -Bugfix - * Fix potential linker errors on dual world platforms by inlining - mbedtls_gcc_group_to_psa(). This allows the pk.c module to link separately - from psa_crypto.c. Fixes #3300. diff --git a/ChangeLog.d/l13-hw-accel.txt b/ChangeLog.d/l13-hw-accel.txt deleted file mode 100644 index 53c79243b..000000000 --- a/ChangeLog.d/l13-hw-accel.txt +++ /dev/null @@ -1,7 +0,0 @@ -Security - * Fix issue in Lucky 13 counter-measure that could make it ineffective when - hardware accelerators were used (using one of the MBEDTLS_SHAxxx_ALT - macros). This would cause the original Lucky 13 attack to be possible in - those configurations, allowing an active network attacker to recover - plaintext after repeated timing measurements under some conditions. - Reported and fix suggested by Luc Perneel in #3246. diff --git a/ChangeLog.d/make-cpp-check-happy.txt b/ChangeLog.d/make-cpp-check-happy.txt deleted file mode 100644 index 2ae74cb0a..000000000 --- a/ChangeLog.d/make-cpp-check-happy.txt +++ /dev/null @@ -1,3 +0,0 @@ -Bugfix - * Fix false positive uninitialised variable reported by cpp-check. - Contributed by Sander Visser in #3311. diff --git a/ChangeLog.d/max_pathlen.txt b/ChangeLog.d/max_pathlen.txt deleted file mode 100644 index 08f9c65a8..000000000 --- a/ChangeLog.d/max_pathlen.txt +++ /dev/null @@ -1,5 +0,0 @@ -Bugfix - * Fix undefined behavior in X.509 certificate parsing if the - pathLenConstraint basic constraint value is equal to INT_MAX. - The actual effect with almost every compiler is the intended - behavior, so this is unlikely to be exploitable anywhere. #3192 diff --git a/ChangeLog.d/md_switch.txt b/ChangeLog.d/md_switch.txt deleted file mode 100644 index a4d369b51..000000000 --- a/ChangeLog.d/md_switch.txt +++ /dev/null @@ -1,3 +0,0 @@ -Changes - * Combine identical cases in switch statements in md.c. Contributed - by irwir in #3208. diff --git a/ChangeLog.d/midipix-support.txt b/ChangeLog.d/midipix-support.txt deleted file mode 100644 index 53599abe4..000000000 --- a/ChangeLog.d/midipix-support.txt +++ /dev/null @@ -1,2 +0,0 @@ -Features - * Add support for midipix, a POSIX layer for Microsoft Windows. diff --git a/ChangeLog.d/montmul-cmp-branch.txt b/ChangeLog.d/montmul-cmp-branch.txt deleted file mode 100644 index 59945188a..000000000 --- a/ChangeLog.d/montmul-cmp-branch.txt +++ /dev/null @@ -1,6 +0,0 @@ -Security - * Fix a side channel vulnerability in modular exponentiation that could - reveal an RSA private key used in a secure enclave. Noticed by Sangho Lee, - Ming-Wei Shih, Prasun Gera, Taesoo Kim and Hyesoon Kim (Georgia Institute - of Technology); and Marcus Peinado (Microsoft Research). Reported by Raoul - Strackx (Fortanix) in #3394. diff --git a/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt b/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt deleted file mode 100644 index fdea746de..000000000 --- a/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt +++ /dev/null @@ -1,5 +0,0 @@ -Features - * Add new mbedtls_x509_crt_parse_der_with_ext_cb() routine which allows - parsing unsupported certificate extensions via user provided callback. - Contributed by Nicola Di Lieto in #3243 as - a solution to #3241. diff --git a/ChangeLog.d/pass-unsupported-policies-to-callback.txt b/ChangeLog.d/pass-unsupported-policies-to-callback.txt deleted file mode 100644 index d139b4c18..000000000 --- a/ChangeLog.d/pass-unsupported-policies-to-callback.txt +++ /dev/null @@ -1,4 +0,0 @@ -Features - * Pass the "certificate policies" extension to the callback supplied to - mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported - policies (#3419). diff --git a/ChangeLog.d/psa-lifetime-locations.txt b/ChangeLog.d/psa-lifetime-locations.txt deleted file mode 100644 index 6ac02bc61..000000000 --- a/ChangeLog.d/psa-lifetime-locations.txt +++ /dev/null @@ -1,8 +0,0 @@ -Default behavior changes - * In the experimental PSA secure element interface, change the encoding of - key lifetimes to encode a persistence level and the location. Although C - prototypes do not effectively change, code calling - psa_register_se_driver() must be modified to pass the driver's location - instead of the keys' lifetime. If the library is upgraded on an existing - device, keys created with the old lifetime value will not be readable or - removable through Mbed TLS after the upgrade. diff --git a/ChangeLog.d/remove-extra-assignment.txt b/ChangeLog.d/remove-extra-assignment.txt deleted file mode 100644 index a98c42347..000000000 --- a/ChangeLog.d/remove-extra-assignment.txt +++ /dev/null @@ -1,3 +0,0 @@ -Changes - * Remove superfluous assignment in mbedtls_ssl_parse_certificate(). Reported - in #3182 and fix submitted by irwir. #3217 diff --git a/ChangeLog.d/ssl_context_info.txt b/ChangeLog.d/ssl_context_info.txt deleted file mode 100644 index 6a15061fa..000000000 --- a/ChangeLog.d/ssl_context_info.txt +++ /dev/null @@ -1,3 +0,0 @@ -Features - * The new utility programs/ssl/ssl_context_info prints a human-readable - dump of an SSL context saved with mbedtls_ssl_context_save(). diff --git a/ChangeLog.d/ssl_write_certificate_request.txt b/ChangeLog.d/ssl_write_certificate_request.txt deleted file mode 100644 index 2d3067aba..000000000 --- a/ChangeLog.d/ssl_write_certificate_request.txt +++ /dev/null @@ -1,3 +0,0 @@ -Changes - * Simplify a bounds check in ssl_write_certificate_request(). Contributed - by irwir in #3150. diff --git a/ChangeLog.d/sysctl-arnd-support.txt b/ChangeLog.d/sysctl-arnd-support.txt deleted file mode 100644 index 14ad67412..000000000 --- a/ChangeLog.d/sysctl-arnd-support.txt +++ /dev/null @@ -1,2 +0,0 @@ -Features - * Added support to entropy_poll for the kern.arandom syscall supported on some BSD systems. Contributed by Nia Alarie in #3423. diff --git a/ChangeLog.d/tests-common-code.txt b/ChangeLog.d/tests-common-code.txt deleted file mode 100644 index 0af2da526..000000000 --- a/ChangeLog.d/tests-common-code.txt +++ /dev/null @@ -1,5 +0,0 @@ -Changes - * The unit tests now rely on header files in tests/include/test and source - files in tests/src. When building with make or cmake, the files in - tests/src are compiled and the resulting object linked into each test - executable. diff --git a/ChangeLog.d/unified-exit-in-examples.txt b/ChangeLog.d/unified-exit-in-examples.txt deleted file mode 100644 index 3ef9798ad..000000000 --- a/ChangeLog.d/unified-exit-in-examples.txt +++ /dev/null @@ -1,4 +0,0 @@ -Changes - * Unify the example programs termination to call mbedtls_exit() instead of - using a return command. This has been done to enable customization of the - behavior in bare metal environments. diff --git a/ChangeLog.d/uniformize_bounds_checks.txt b/ChangeLog.d/uniformize_bounds_checks.txt deleted file mode 100644 index 210ab1051..000000000 --- a/ChangeLog.d/uniformize_bounds_checks.txt +++ /dev/null @@ -1,9 +0,0 @@ -Bugfix - * Add additional bounds checks in ssl_write_client_hello() preventing - output buffer overflow if the configuration declared a buffer that was - too small. -Changes - * Abort the ClientHello writing function as soon as some extension doesn't - fit into the record buffer. Previously, such extensions were silently - dropped. As a consequence, the TLS handshake now fails when the output - buffer is not large enough to hold the ClientHello. diff --git a/ChangeLog.d/use-find-python3-cmake.txt b/ChangeLog.d/use-find-python3-cmake.txt deleted file mode 100644 index 36a5171ee..000000000 --- a/ChangeLog.d/use-find-python3-cmake.txt +++ /dev/null @@ -1,2 +0,0 @@ -Changes - * Use FindPython3 when cmake version >= 3.15.0 From 0435cd8c232b9f8399db3153eef9d71abe77b636 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 26 Jun 2020 12:26:55 +0100 Subject: [PATCH 20/21] Bump version to Mbed TLS 2.23.0 Executed "./scripts/bump_version.sh --version 2.23.0 --so-crypto 5" A symbol has been removed from the mbedcrypto library since the last release: mbedtls_ecc_group_to_psa ( enum mbedtls_ecp_group_id grpid, size_t* bits ) This is an ABI break and we need to increase the SO version. Signed-off-by: Janos Follath --- doxygen/input/doc_mainpage.h | 2 +- doxygen/mbedtls.doxyfile | 2 +- include/mbedtls/version.h | 8 ++++---- library/CMakeLists.txt | 6 +++--- library/Makefile | 2 +- tests/suites/test_suite_version.data | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/doxygen/input/doc_mainpage.h b/doxygen/input/doc_mainpage.h index 749d5c1eb..27a840a5b 100644 --- a/doxygen/input/doc_mainpage.h +++ b/doxygen/input/doc_mainpage.h @@ -24,7 +24,7 @@ */ /** - * @mainpage mbed TLS v2.22.0 source code documentation + * @mainpage mbed TLS v2.23.0 source code documentation * * This documentation describes the internal structure of mbed TLS. It was * automatically generated from specially formatted comment blocks in diff --git a/doxygen/mbedtls.doxyfile b/doxygen/mbedtls.doxyfile index 418318da5..e89021a21 100644 --- a/doxygen/mbedtls.doxyfile +++ b/doxygen/mbedtls.doxyfile @@ -28,7 +28,7 @@ DOXYFILE_ENCODING = UTF-8 # identify the project. Note that if you do not use Doxywizard you need # to put quotes around the project name if it contains spaces. -PROJECT_NAME = "mbed TLS v2.22.0" +PROJECT_NAME = "mbed TLS v2.23.0" # The PROJECT_NUMBER tag can be used to enter a project or revision number. # This could be handy for archiving the generated documentation or diff --git a/include/mbedtls/version.h b/include/mbedtls/version.h index b89e36efd..0ae4d2271 100644 --- a/include/mbedtls/version.h +++ b/include/mbedtls/version.h @@ -39,7 +39,7 @@ * Major, Minor, Patchlevel */ #define MBEDTLS_VERSION_MAJOR 2 -#define MBEDTLS_VERSION_MINOR 22 +#define MBEDTLS_VERSION_MINOR 23 #define MBEDTLS_VERSION_PATCH 0 /** @@ -47,9 +47,9 @@ * MMNNPP00 * Major version | Minor version | Patch version */ -#define MBEDTLS_VERSION_NUMBER 0x02160000 -#define MBEDTLS_VERSION_STRING "2.22.0" -#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.22.0" +#define MBEDTLS_VERSION_NUMBER 0x02170000 +#define MBEDTLS_VERSION_STRING "2.23.0" +#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.23.0" #if defined(MBEDTLS_VERSION_C) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index f6a186fae..05196e86c 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -181,19 +181,19 @@ endif(USE_STATIC_MBEDTLS_LIBRARY) if(USE_SHARED_MBEDTLS_LIBRARY) add_library(mbedcrypto SHARED ${src_crypto}) - set_target_properties(mbedcrypto PROPERTIES VERSION 2.22.0 SOVERSION 4) + set_target_properties(mbedcrypto PROPERTIES VERSION 2.23.0 SOVERSION 5) target_link_libraries(mbedcrypto ${libs}) target_include_directories(mbedcrypto PUBLIC ${MBEDTLS_DIR}/include/) add_library(mbedx509 SHARED ${src_x509}) - set_target_properties(mbedx509 PROPERTIES VERSION 2.22.0 SOVERSION 1) + set_target_properties(mbedx509 PROPERTIES VERSION 2.23.0 SOVERSION 1) target_link_libraries(mbedx509 ${libs} mbedcrypto) target_include_directories(mbedx509 PUBLIC ${MBEDTLS_DIR}/include/) add_library(mbedtls SHARED ${src_tls}) - set_target_properties(mbedtls PROPERTIES VERSION 2.22.0 SOVERSION 13) + set_target_properties(mbedtls PROPERTIES VERSION 2.23.0 SOVERSION 13) target_link_libraries(mbedtls ${libs} mbedx509) target_include_directories(mbedtls PUBLIC ${MBEDTLS_DIR}/include/) diff --git a/library/Makefile b/library/Makefile index dbdd3b679..87ea56d37 100644 --- a/library/Makefile +++ b/library/Makefile @@ -37,7 +37,7 @@ endif SOEXT_TLS=so.13 SOEXT_X509=so.1 -SOEXT_CRYPTO=so.4 +SOEXT_CRYPTO=so.5 # Set AR_DASH= (empty string) to use an ar implementation that does not accept # the - prefix for command line options (e.g. llvm-ar) diff --git a/tests/suites/test_suite_version.data b/tests/suites/test_suite_version.data index 5dc81d334..846ebb731 100644 --- a/tests/suites/test_suite_version.data +++ b/tests/suites/test_suite_version.data @@ -1,8 +1,8 @@ Check compiletime library version -check_compiletime_version:"2.22.0" +check_compiletime_version:"2.23.0" Check runtime library version -check_runtime_version:"2.22.0" +check_runtime_version:"2.23.0" Check for MBEDTLS_VERSION_C check_feature:"MBEDTLS_VERSION_C":0 From 13cba685be200410907e7652fc79fd3209a4d1ab Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 26 Jun 2020 12:40:52 +0100 Subject: [PATCH 21/21] Update ChangeLog header. Signed-off-by: Janos Follath --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index e18ef0bd5..32853ce43 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,6 @@ mbed TLS ChangeLog (Sorted per branch, date) -= mbed TLS x.x.x branch released xxxx-xx-xx += mbed TLS 2.23.0 branch released 2020-07-01 Default behavior changes * In the experimental PSA secure element interface, change the encoding of