From e75b88db492a6ca8bf5138667f514f3b2b93ecb0 Mon Sep 17 00:00:00 2001 From: Joris Aerts Date: Fri, 4 Nov 2016 23:05:56 +0100 Subject: [PATCH 01/84] Fix missing void argument declarations #678 --- library/memory_buffer_alloc.c | 12 ++++++------ library/version.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/library/memory_buffer_alloc.c b/library/memory_buffer_alloc.c index 545d5a2c32..c0a72c2ad1 100644 --- a/library/memory_buffer_alloc.c +++ b/library/memory_buffer_alloc.c @@ -113,7 +113,7 @@ static void debug_header( memory_header *hdr ) #endif } -static void debug_chain() +static void debug_chain( void ) { memory_header *cur = heap.first; @@ -180,7 +180,7 @@ static int verify_header( memory_header *hdr ) return( 0 ); } -static int verify_chain() +static int verify_chain( void ) { memory_header *prv = heap.first, *cur = heap.first->next; @@ -500,13 +500,13 @@ void mbedtls_memory_buffer_set_verify( int verify ) heap.verify = verify; } -int mbedtls_memory_buffer_alloc_verify() +int mbedtls_memory_buffer_alloc_verify( void ) { return verify_chain(); } #if defined(MBEDTLS_MEMORY_DEBUG) -void mbedtls_memory_buffer_alloc_status() +void mbedtls_memory_buffer_alloc_status( void ) { mbedtls_fprintf( stderr, "Current use: %zu blocks / %zu bytes, max: %zu blocks / " @@ -600,7 +600,7 @@ void mbedtls_memory_buffer_alloc_init( unsigned char *buf, size_t len ) heap.first_free = heap.first; } -void mbedtls_memory_buffer_alloc_free() +void mbedtls_memory_buffer_alloc_free( void ) { #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_free( &heap.mutex ); @@ -620,7 +620,7 @@ static int check_pointer( void *p ) return( 0 ); } -static int check_all_free( ) +static int check_all_free( void ) { if( #if defined(MBEDTLS_MEMORY_DEBUG) diff --git a/library/version.c b/library/version.c index 6ca80d4695..fd96750885 100644 --- a/library/version.c +++ b/library/version.c @@ -30,7 +30,7 @@ #include "mbedtls/version.h" #include -unsigned int mbedtls_version_get_number() +unsigned int mbedtls_version_get_number( void ) { return( MBEDTLS_VERSION_NUMBER ); } From a7f51f6e1ff8da886ed26dc8408e1b01f2869440 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 15 May 2017 11:23:55 +0300 Subject: [PATCH 02/84] Remove Yotta module from footprint.sh script Remove Yotta module configuration usd in footprint.sh script --- scripts/footprint.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/scripts/footprint.sh b/scripts/footprint.sh index d38e50af27..c08ef1c902 100755 --- a/scripts/footprint.sh +++ b/scripts/footprint.sh @@ -11,7 +11,6 @@ # # Configurations included: # default include/mbedtls/config.h -# yotta yotta/module/mbedtls/config.h # thread configs/config-thread.h # suite-b configs/config-suite-b.h # psk configs/config-ccm-psk-tls1_2.h @@ -102,11 +101,7 @@ log "mbed TLS $MBEDTLS_VERSION$GIT_VERSION" log "$( arm-none-eabi-gcc --version | head -n1 )" log "CFLAGS=$ARMGCC_FLAGS" -# creates the yotta config -yotta/create-module.sh >/dev/null - doit default include/mbedtls/config.h -doit yotta yotta/module/mbedtls/config.h doit thread configs/config-thread.h doit suite-b configs/config-suite-b.h doit psk configs/config-ccm-psk-tls1_2.h From 4ae7d5df96b29ab275b9040796775aa337116cc3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 26 May 2017 11:59:29 +0200 Subject: [PATCH 03/84] Clarified documentation of mbedtls_ssl_setup Note that the configuration structure must remain accessible. The previous wording could have been taken as implying that it's ok to change the structure but changes wouldn't be taken into account. Also note that calling this function twice is not supported (it would at least be a memory leak). --- include/mbedtls/ssl.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 495e02cb0e..e3fd890cf2 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -960,8 +960,13 @@ void mbedtls_ssl_init( mbedtls_ssl_context *ssl ); * \note No copy of the configuration context is made, it can be * shared by many mbedtls_ssl_context structures. * - * \warning Modifying the conf structure after it has been used in this - * function is unsupported! + * \warning The conf structure will be accessed during the session. + * It must not be modified or freed as long as the session + * is active. + * + * \warning This function must be called exactly once per context. + * Calling mbedtls_ssl_setup again is not supported, even + * if no session is active. * * \param ssl SSL context * \param conf SSL configuration to use From 88ec2381d6af23935c491272375cea85d942d894 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 13:51:16 +0100 Subject: [PATCH 04/84] Add configuration options for verification and blinding This commit defines some configuration options to control the mandatory use of blinding and verification in RSA private key operations. --- include/mbedtls/config.h | 72 +++++++++++++++++++++++++++++++++++++++- include/mbedtls/rsa.h | 35 ++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index c4b8995c14..1ce92c5a1e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -970,16 +970,86 @@ */ #define MBEDTLS_PKCS1_V21 +/** + * \def MBEDTLS_RSA_FORCE_BLINDING + * + * Force the use of blinding in RSA private key operations. + * This makes these operations fail when the caller doesn't + * provide a PRNG. + * + * Comment this macro to allow RSA private key operations + * without blinding. + * + * \warning Disabling this can be a security risk! + * Blinding RSA private key operations is a way + * to prevent statistical timing attacks as in + * [P. Kocher ', Timing Attacks on Implementations + * of Diffie-Hellman, RSA, DSS, and Other Systems] + * + * \note Disabling this does not mean that blinding + * will never be used, but instead makes private + * key operations fail if, perhaps unintentionally, + * the user failed to call them with a PRNG. + * + * \note For more on the use of blinding in RSA + * private key operations, see the documentation + * of \c mbedtls_rsa_private. + */ +#define MBEDTLS_RSA_FORCE_BLINDING + /** * \def MBEDTLS_RSA_NO_CRT * - * Do not use the Chinese Remainder Theorem for the RSA private operation. + * Do not use the Chinese Remainder Theorem + * for the RSA private operation. * * Uncomment this macro to disable the use of CRT in RSA. * */ //#define MBEDTLS_RSA_NO_CRT +/** + * \def MBEDTLS_RSA_FORCE_CRT_VERIFICATION + * + * Force verification of results of RSA private key operations + * when RSA-CRT is used. + * + * Comment this macro to disable RSA-CRT verification. + * + * \warning Disabling this can be a security risk! + * Omitting verification makes the RSA-CRT + * signing vulnerable to the Bellcore + * glitch attack leading to private key + * compromise if an attacker can cause a + * glitch in a certain timeframe during + * the signing operation. Uncomment only + * if you're sure that glitches are out of + * your attack model. + */ +#define MBEDTLS_RSA_FORCE_CRT_VERIFICATION + +/** + * \def MBEDTLS_RSA_FORCE_VERIFICATION + * + * Force verification of results of any RSA private key + * operation regardless of the algorithm used. + * + * Uncomment this to enable unconditional RSA verification. + * + * \note This is to prevent the RSA signing operation + * (regardless of the particular algorithm chosen) + * from potential future glitch attacks. We are + * currently not aware of any such for our default + * implementation, therefore disabling the option + * by default. + * + * \note Enabling it comes at the cost of roughly an + * additional public key operation at the end of + * signing (low compared to private key operations), + * as well as minor memory consumption. + */ +//#define MBEDTLS_RSA_FORCE_VERIFICATION + /** * \def MBEDTLS_SELF_TEST * diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index 54653dfdcd..e34fea0f28 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -63,6 +63,15 @@ #define MBEDTLS_RSA_SALT_LEN_ANY -1 +/* + * RSA configuration + */ +#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) || \ + ( ! defined(MBEDTLS_RSA_NO_CRT) && \ + defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION ) ) +#define MBEDTLS_RSA_REQUIRE_VERIFICATION +#endif + /* * The above constants may be used even if the RSA module is compile out, * eg for alternative (PKCS#11) RSA implemenations in the PK layers. @@ -220,7 +229,7 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx, * \brief Do an RSA private key operation * * \param ctx RSA context - * \param f_rng RNG function (Needed for blinding) + * \param f_rng RNG function (used for blinding) * \param p_rng RNG parameter * \param input input buffer * \param output output buffer @@ -229,6 +238,30 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx, * * \note The input and output buffers must be large * enough (eg. 128 bytes if RSA-1024 is used). + * + * \note Enabling and disabling of blinding: + * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING + * is disabled, blinding is disabled. + * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING + * is enabled, the function fails. + * + * \note If blinding is used, both the base of exponentation + * and the exponent are blinded, preventing both statistical + * timing and power analysis attacks. + * + * \note Depending on the way RSA is implemented, a failure + * in the computation can lead to disclosure of the private + * key if the wrong result is passed to attacker - e.g., + * implementing RSA through CRT is vulnerable to the + * Bellcore glitch attack. + * + * As a remedy, the user can force double checking the + * result of the private key operation through the option + * MBEDTLS_RSA_FORCE_VERIFICATION. If verification is + * to be enabled only when RSA-CRT is used (as controlled + * by the configuration option MBEDTLS_RSA_NO_CRT), the + * option MBEDTLS_RSA_FORCE_CRT_VERIFICATION can be used. + * */ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, int (*f_rng)(void *, unsigned char *, size_t), From 5bc8729b9e7738d8f9a32e96b8e1fb2f597e3609 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 15:09:31 +0100 Subject: [PATCH 05/84] Correct memory leak in RSA self test The RSA self test didn't free the RSA context on failure. --- library/rsa.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 122bc13605..c8090044a7 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1772,7 +1772,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1786,7 +1787,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1799,7 +1801,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( memcmp( rsa_decrypted, rsa_plaintext, len ) != 0 ) @@ -1807,7 +1810,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1825,7 +1829,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) @@ -1837,7 +1842,8 @@ int mbedtls_rsa_self_test( int verbose ) if( verbose != 0 ) mbedtls_printf( "failed\n" ); - return( 1 ); + ret = 1; + goto cleanup; } if( verbose != 0 ) From a540068a56efcadb6cf05b7a197021aa7c4788b5 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 16:43:15 +0100 Subject: [PATCH 06/84] Modify PK test suite to provide PRNG to RSA signature function To prepare for the option of mandatory blinding, this commit changes the PK test suite to always call signature functions with a PRNG. --- tests/suites/test_suite_pk.function | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 5fa8a693aa..33453ac6f8 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -43,7 +43,7 @@ int mbedtls_rsa_decrypt_func( void *ctx, int mode, size_t *olen, const unsigned char *input, unsigned char *output, size_t output_max_len ) { - return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, NULL, NULL, mode, olen, + return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, rnd_std_rand, NULL, mode, olen, input, output, output_max_len ) ); } int mbedtls_rsa_sign_func( void *ctx, @@ -51,7 +51,9 @@ int mbedtls_rsa_sign_func( void *ctx, int mode, mbedtls_md_type_t md_alg, unsigned int hashlen, const unsigned char *hash, unsigned char *sig ) { - return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, f_rng, p_rng, mode, + ((void) f_rng); + ((void) p_rng); + return( mbedtls_rsa_pkcs1_sign( (mbedtls_rsa_context *) ctx, rnd_std_rand, NULL, mode, md_alg, hashlen, hash, sig ) ); } size_t mbedtls_rsa_key_len_func( void *ctx ) From 06811ced27d809610cfde1db85dd138452f40436 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 3 May 2017 15:10:34 +0100 Subject: [PATCH 07/84] Put configuration options for RSA blinding and verification to work. --- library/rsa.c | 132 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 24 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index c8090044a7..d3feeba88b 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -398,24 +398,68 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, { int ret; size_t olen; - mbedtls_mpi T, T1, T2; + + /* Temporary holding the result */ + mbedtls_mpi T; + + /* Temporaries holding P-1, Q-1 and the + * exponent blinding factor, respectively. */ mbedtls_mpi P1, Q1, R; -#if defined(MBEDTLS_RSA_NO_CRT) - mbedtls_mpi D_blind; - mbedtls_mpi *D = &ctx->D; -#else + +#if !defined(MBEDTLS_RSA_NO_CRT) + /* Temporaries holding the results mod p resp. mod q. */ + mbedtls_mpi TP, TQ; + + /* Temporaries holding the blinded exponents for + * the mod p resp. mod q computation (if used). */ mbedtls_mpi DP_blind, DQ_blind; + + /* Pointers to actual exponents to be used - either the unblinded + * or the blinded ones, depending on the presence of a PRNG. */ mbedtls_mpi *DP = &ctx->DP; mbedtls_mpi *DQ = &ctx->DQ; +#else + /* Temporary holding the blinded exponent (if used). */ + mbedtls_mpi D_blind; + + /* Pointer to actual exponent to be used - either the unblinded + * or the blinded one, depending on the presence of a PRNG. */ + mbedtls_mpi *D = &ctx->D; +#endif + +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + /* Temporaries holding input mod p resp. mod q. */ + mbedtls_mpi IP, IQ; + + /* Temporaries holding double check results mod p resp. mod q; + * should in the end have the same values as IP and IQ. */ + mbedtls_mpi CP, CQ; + + /* Comparison results */ + int check = 0; +#endif + +#if defined(MBEDTLS_RSA_FORCE_BLINDING) + if( f_rng == NULL ) + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); #endif /* Make sure we have private key info, prevent possible misuse */ if( ctx->P.p == NULL || ctx->Q.p == NULL || ctx->D.p == NULL ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); - mbedtls_mpi_init( &T ); mbedtls_mpi_init( &T1 ); mbedtls_mpi_init( &T2 ); - mbedtls_mpi_init( &P1 ); mbedtls_mpi_init( &Q1 ); mbedtls_mpi_init( &R ); +#if defined(MBEDTLS_THREADING_C) + if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) + return( ret ); +#endif + /* MPI Initialization */ + + mbedtls_mpi_init( &T ); + + mbedtls_mpi_init( &P1 ); + mbedtls_mpi_init( &Q1 ); + mbedtls_mpi_init( &R ); if( f_rng != NULL ) { @@ -427,12 +471,17 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif } - -#if defined(MBEDTLS_THREADING_C) - if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) - return( ret ); +#if !defined(MBEDTLS_RSA_NO_CRT) + mbedtls_mpi_init( &TP ); mbedtls_mpi_init( &TQ ); #endif +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + mbedtls_mpi_init( &IP ); mbedtls_mpi_init( &IQ ); + mbedtls_mpi_init( &CP ); mbedtls_mpi_init( &CQ ); +#endif + + /* End of MPI initialization */ + MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( &T, input, ctx->len ) ); if( mbedtls_mpi_cmp_mpi( &T, &ctx->N ) >= 0 ) { @@ -440,6 +489,11 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, goto cleanup; } +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IP, &T, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IQ, &T, &ctx->Q ) ); +#endif + if( f_rng != NULL ) { /* @@ -498,24 +552,25 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* * Faster decryption using the CRT * - * T1 = input ^ dP mod P - * T2 = input ^ dQ mod Q + * TP = input ^ dP mod P + * TQ = input ^ dQ mod Q */ - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &T1, &T, DP, &ctx->P, &ctx->RP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &T2, &T, DQ, &ctx->Q, &ctx->RQ ) ); + + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &TP, &T, DP, &ctx->P, &ctx->RP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &TQ, &T, DQ, &ctx->Q, &ctx->RQ ) ); /* - * T = (T1 - T2) * (Q^-1 mod P) mod P + * T = (TP - TQ) * (Q^-1 mod P) mod P */ - MBEDTLS_MPI_CHK( mbedtls_mpi_sub_mpi( &T, &T1, &T2 ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T1, &T, &ctx->QP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T1, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_sub_mpi( &T, &TP, &TQ ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &TP, &T, &ctx->QP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &TP, &ctx->P ) ); /* - * T = T2 + T * Q + * T = TQ + T * Q */ - MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &T1, &T, &ctx->Q ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &T, &T2, &T1 ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &TP, &T, &ctx->Q ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_add_mpi( &T, &TQ, &TP ) ); #endif /* MBEDTLS_RSA_NO_CRT */ if( f_rng != NULL ) @@ -528,6 +583,23 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } + /* If requested by the config, verify the result to prevent glitching attacks. + * For that, check the two prime moduli separately. */ +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CP, &T, &ctx->E, &ctx->P, &ctx->RP ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CQ, &T, &ctx->E, &ctx->Q, &ctx->RQ ) ); + + check |= mbedtls_mpi_cmp_mpi( &CP, &IP ); + check |= mbedtls_mpi_cmp_mpi( &CQ, &IQ ); + + if( check != 0 ) + { + /* Verification failed */ + ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; + goto cleanup; + } +#endif /* MBEDTLS_RSA_REQUIRE_VERIFICATION */ + olen = ctx->len; MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &T, output, olen ) ); @@ -537,8 +609,9 @@ cleanup: return( MBEDTLS_ERR_THREADING_MUTEX_ERROR ); #endif - mbedtls_mpi_free( &T ); mbedtls_mpi_free( &T1 ); mbedtls_mpi_free( &T2 ); - mbedtls_mpi_free( &P1 ); mbedtls_mpi_free( &Q1 ); mbedtls_mpi_free( &R ); + mbedtls_mpi_free( &P1 ); + mbedtls_mpi_free( &Q1 ); + mbedtls_mpi_free( &R ); if( f_rng != NULL ) { @@ -550,6 +623,17 @@ cleanup: #endif } + mbedtls_mpi_free( &T ); + +#if !defined(MBEDTLS_RSA_NO_CRT) + mbedtls_mpi_free( &TP ); mbedtls_mpi_free( &TQ ); +#endif + +#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) + mbedtls_mpi_free( &IP ); mbedtls_mpi_free( &IQ ); + mbedtls_mpi_free( &CP ); mbedtls_mpi_free( &CQ ); +#endif + if( ret != 0 ) return( MBEDTLS_ERR_RSA_PRIVATE_FAILED + ret ); From b624b85b04e3b335ba6e03f1d06d7c5167bf7843 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 May 2017 09:00:08 +0100 Subject: [PATCH 08/84] Adapt ChangeLog --- ChangeLog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ChangeLog b/ChangeLog index 08edd77969..b6ab9665a8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Fix memory leak in RSA self test. + +Security + * Add option for mandatory use of blinding in RSA private key operations. + * Add options for verification of RSA private key operations to defend + against Bellcore glitch attack. + = mbed TLS 2.x.x branch released xxxx-xx-xx Security From 9f4e670b14b41ac2978469852acae943f8a2b19c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 12 Jun 2017 10:23:19 +0100 Subject: [PATCH 09/84] Correct documentation for RSA_FORCE_BLINDING option --- include/mbedtls/config.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 1ce92c5a1e..d54f0c3824 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -987,9 +987,12 @@ * of Diffie-Hellman, RSA, DSS, and Other Systems] * * \note Disabling this does not mean that blinding - * will never be used, but instead makes private - * key operations fail if, perhaps unintentionally, - * the user failed to call them with a PRNG. + * will never be used: if a PRNG is provided, + * blinding will be in place. Instead, disabling this + * option may result in private key operations being + * performed in a way potentially leaking sensitive + * information through side-channels when no PRNG + * is supplied by the user. * * \note For more on the use of blinding in RSA * private key operations, see the documentation From e507c82084a31a674d70e3b2337cf65e54b55c2c Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 12 Jul 2017 14:04:40 +0100 Subject: [PATCH 10/84] Fix typo and bracketing in macro args --- library/net_sockets.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/net_sockets.c b/library/net_sockets.c index 80be6ec6a4..31c42db05a 100644 --- a/library/net_sockets.c +++ b/library/net_sockets.c @@ -63,8 +63,8 @@ #endif #endif /* _MSC_VER */ -#define read(fd,buf,len) recv(fd,(char*)buf,(int) len,0) -#define write(fd,buf,len) send(fd,(char*)buf,(int) len,0) +#define read(fd,buf,len) recv( fd, (char*)( buf ), (int)( len ), 0 ) +#define write(fd,buf,len) send( fd, (char*)( buf ), (int)( len ), 0 ) #define close(fd) closesocket(fd) static int wsa_init_done = 0; @@ -85,7 +85,7 @@ static int wsa_init_done = 0; #endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */ /* Some MS functions want int and MSVC warns if we pass size_t, - * but the standard fucntions use socklen_t, so cast only for MSVC */ + * but the standard functions use socklen_t, so cast only for MSVC */ #if defined(_MSC_VER) #define MSVC_INT_CAST (int) #else From b5e6a77010a859e13bd177f96d786de91c6c2212 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 11:23:31 +0300 Subject: [PATCH 11/84] Add Contribution guidelines to github Add Contribution Guidelines that will be shown in github, when PRs are made. --- CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..55ebf15b1c --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,40 @@ +We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: + +- All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. +- To accept the Contributor’s Licence Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions should be fully tested before submission. +As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. + +### Making a Contribution + +1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. +2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. +3. Write a test which shows that the bug was fixed or that the feature works as expected. +4. Send a pull request and bug us until it gets merged and published. Contributions may need some modifications, so work with us to get your change accepted. We will include your name in the ChangeLog :) + +### Backports + +mbed TLS maintains some legacy branches, which are release as LTS versions. As such, backporting to these branches should be handled according to the following rules: + +1. If the contribution is a new feature\enhancement, no backporting is needed +2. Bug fixes should be backported, as long as the legacy branches have these bugs reproduced +3. Changes in the API, do not require backporting. If a bug fix introduced new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. + +It would be highly appreciated if a contribution would be backported to a legacy branch as well. +At the moment, the legacy branches are: + +1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) +2. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) +3. [development](https://github.com/ARMmbed/mbedtls/tree/development) + +### Tests + +As mentioned, tests that show the correctness of the feature\bug fix should be added to the Pull Request, if not such test exist. +mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. + +### Continuous Integration Tests + +Once a PR has been made, the Continuous Integration tests ( CI ) are triggered and run. You should follow the result of the CI tests, and fix failures. + + + \ No newline at end of file From 7f888982fd3a2d924b890ca7c8c0d23faf7d79a1 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 16:05:52 +0300 Subject: [PATCH 12/84] Modify Contribution Guidelines after comments Modify the Contribution guidelines after comments from Gilles, Andres and Jaeden --- CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 55ebf15b1c..bfd6cb3d70 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,24 +1,32 @@ We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: + - As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. + - The contribution should not break API or ABI, unless there is a real justification for that. If there is an API change, the contribution, if accepted, will be merged only when there will be a major release. + +### Contributor License Agreement ( CLA ) - All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. -- To accept the Contributor’s Licence Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. -- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions should be fully tested before submission. -As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. +- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. + +### Coding Standards +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. +- The code should be written in a clean and readable style. +- The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. +- The code should be secure, and will be reviewed in a security point of view as well. ### Making a Contribution 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. 2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. 3. Write a test which shows that the bug was fixed or that the feature works as expected. -4. Send a pull request and bug us until it gets merged and published. Contributions may need some modifications, so work with us to get your change accepted. We will include your name in the ChangeLog :) +4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) ### Backports -mbed TLS maintains some legacy branches, which are release as LTS versions. As such, backporting to these branches should be handled according to the following rules: +mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: -1. If the contribution is a new feature\enhancement, no backporting is needed -2. Bug fixes should be backported, as long as the legacy branches have these bugs reproduced -3. Changes in the API, do not require backporting. If a bug fix introduced new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. +1. If the contribution is a new feature or enhancement, no backporting is needed. +2. Bug fixes should be backported to the legacy branches containing these bugs. +3. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. It would be highly appreciated if a contribution would be backported to a legacy branch as well. At the moment, the legacy branches are: @@ -29,12 +37,24 @@ At the moment, the legacy branches are: ### Tests -As mentioned, tests that show the correctness of the feature\bug fix should be added to the Pull Request, if not such test exist. +As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. +Sample applications, if needed, should be modified as well. + ### Continuous Integration Tests -Once a PR has been made, the Continuous Integration tests ( CI ) are triggered and run. You should follow the result of the CI tests, and fix failures. +Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. + +### Documentation + +mbed TLS should be well documented. If documentation is needed, speak out! + +1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. +2. Complex parts in the code should include comments. +3. If needed, a Readme file is advised +4. If a KB article should be added, write this as a comment in the PR description. +5. A Changelog entry should be added for this contribution. \ No newline at end of file From 1680d3dc1929f325f80530b8eb97a11fc96296bf Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 16 Aug 2017 17:28:21 +0300 Subject: [PATCH 13/84] Add a couple of statements to the contribution section Add a notice for short contributions, and for Apache license header that should be added. Added an adivce to enable the git hooks scripts as well. --- CONTRIBUTING.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bfd6cb3d70..95219e5444 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,8 @@ We gratefully accept bug reports and contributions from the community. There are 2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. 3. Write a test which shows that the bug was fixed or that the feature works as expected. 4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +5. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. +6. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. ### Backports @@ -45,6 +47,7 @@ Sample applications, if needed, should be modified as well. ### Continuous Integration Tests Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. +It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtls/tree/development/tests/git-scripts) prior to pushing your changes, for catching some of the issues as early as possible. ### Documentation From c6075cc5acccf5bdd105a31300da7957a16e7ce3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:45:35 +0100 Subject: [PATCH 14/84] Don't use CRT for signature verification If CRT is not used, the helper fields CRT are not assumed to be present in the RSA context structure, so do the verification directly in this case. If CRT is used, verification could be done using CRT, but we're sticking to ordinary verification for uniformity. --- library/rsa.c | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index d3feeba88b..0c5bc4fb5d 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -428,15 +428,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - /* Temporaries holding input mod p resp. mod q. */ - mbedtls_mpi IP, IQ; - - /* Temporaries holding double check results mod p resp. mod q; - * should in the end have the same values as IP and IQ. */ - mbedtls_mpi CP, CQ; - - /* Comparison results */ - int check = 0; + /* Temporaries holding the initial input and the double + * checked result; should be the same in the end. */ + mbedtls_mpi I, C; #endif #if defined(MBEDTLS_RSA_FORCE_BLINDING) @@ -476,8 +470,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - mbedtls_mpi_init( &IP ); mbedtls_mpi_init( &IQ ); - mbedtls_mpi_init( &CP ); mbedtls_mpi_init( &CQ ); + mbedtls_mpi_init( &I ); + mbedtls_mpi_init( &C ); #endif /* End of MPI initialization */ @@ -490,8 +484,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, } #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IP, &T, &ctx->P ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &IQ, &T, &ctx->Q ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) ); #endif if( f_rng != NULL ) @@ -583,18 +576,11 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } - /* If requested by the config, verify the result to prevent glitching attacks. - * For that, check the two prime moduli separately. */ + /* If requested by the config, verify the result to prevent glitching attacks. */ #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CP, &T, &ctx->E, &ctx->P, &ctx->RP ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &CQ, &T, &ctx->E, &ctx->Q, &ctx->RQ ) ); - - check |= mbedtls_mpi_cmp_mpi( &CP, &IP ); - check |= mbedtls_mpi_cmp_mpi( &CQ, &IQ ); - - if( check != 0 ) + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, &ctx->N, &ctx->RN ) ); + if( mbedtls_mpi_cmp_mpi( &C, &I ) != 0 ) { - /* Verification failed */ ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; goto cleanup; } @@ -630,8 +616,8 @@ cleanup: #endif #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) - mbedtls_mpi_free( &IP ); mbedtls_mpi_free( &IQ ); - mbedtls_mpi_free( &CP ); mbedtls_mpi_free( &CQ ); + mbedtls_mpi_free( &C ); + mbedtls_mpi_free( &I ); #endif if( ret != 0 ) @@ -1245,11 +1231,6 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, size_t nb_pad, olen, oid_size = 0; unsigned char *p = sig; const char *oid = NULL; - unsigned char *sig_try = NULL, *verif = NULL; - size_t i; - unsigned char diff; - volatile unsigned char diff_no_optimize; - int ret; if( mode == MBEDTLS_RSA_PRIVATE && ctx->padding != MBEDTLS_RSA_PKCS_V15 ) return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); From 43f94721ab4e331517b71e678d9c5a72b6834958 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:50:00 +0100 Subject: [PATCH 15/84] Add quick-check for presence of relevant parameters in rsa_private --- library/rsa.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 0c5bc4fb5d..9b7d346c24 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -425,7 +425,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Pointer to actual exponent to be used - either the unblinded * or the blinded one, depending on the presence of a PRNG. */ mbedtls_mpi *D = &ctx->D; -#endif +#endif /* MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) /* Temporaries holding the initial input and the double @@ -438,9 +438,24 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); #endif - /* Make sure we have private key info, prevent possible misuse */ - if( ctx->P.p == NULL || ctx->Q.p == NULL || ctx->D.p == NULL ) + /* Sanity-check that all relevant fields are at least set, + * but don't perform a full keycheck. */ + if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ) + { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + } +#if !defined(MBEDTLS_RSA_NO_CRT) + if( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->QP, 0 ) == 0 ) + { + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + } +#endif /* MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_THREADING_C) if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) @@ -1294,7 +1309,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, } if( mode == MBEDTLS_RSA_PUBLIC ) - return( mbedtls_rsa_public( ctx, sig, sig ) ); + return( mbedtls_rsa_public( ctx, sig, sig ) ); /* * In order to prevent Lenstra's attack, make the signature in a From cc209ca56d0592404f5019a03f4887e383f956d0 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 25 Aug 2017 11:51:03 +0100 Subject: [PATCH 16/84] Remove signature verification from rsa_rsassa_pkcs1_v15_sign This verification path is redundant now that verification is uniformly done in rsa_private. --- library/rsa.c | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 9b7d346c24..680df0d8e5 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1311,42 +1311,7 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, if( mode == MBEDTLS_RSA_PUBLIC ) return( mbedtls_rsa_public( ctx, sig, sig ) ); - /* - * In order to prevent Lenstra's attack, make the signature in a - * temporary buffer and check it before returning it. - */ - sig_try = mbedtls_calloc( 1, ctx->len ); - if( sig_try == NULL ) - return( MBEDTLS_ERR_MPI_ALLOC_FAILED ); - - verif = mbedtls_calloc( 1, ctx->len ); - if( verif == NULL ) - { - mbedtls_free( sig_try ); - return( MBEDTLS_ERR_MPI_ALLOC_FAILED ); - } - - MBEDTLS_MPI_CHK( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig_try ) ); - MBEDTLS_MPI_CHK( mbedtls_rsa_public( ctx, sig_try, verif ) ); - - /* Compare in constant time just in case */ - for( diff = 0, i = 0; i < ctx->len; i++ ) - diff |= verif[i] ^ sig[i]; - diff_no_optimize = diff; - - if( diff_no_optimize != 0 ) - { - ret = MBEDTLS_ERR_RSA_PRIVATE_FAILED; - goto cleanup; - } - - memcpy( sig, sig_try, ctx->len ); - -cleanup: - mbedtls_free( sig_try ); - mbedtls_free( verif ); - - return( ret ); + return( mbedtls_rsa_private( ctx, f_rng, p_rng, sig, sig ) ); } #endif /* MBEDTLS_PKCS1_V15 */ From ea24d75c67d79b31d50499affa66ec88d3756e59 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Thu, 31 Aug 2017 17:02:01 +0300 Subject: [PATCH 17/84] Addres Andres' comment Update the document after Andres review comments --- CONTRIBUTING.md | 61 +++++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95219e5444..f7bf5f8dbf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,63 +1,64 @@ +Contributing +============ We gratefully accept bug reports and contributions from the community. There are some requirements we need to fulfill in order to be able to integrate contributions: - As with any open source project, contributions will be reviewed by the project team and community and may need some modifications to be accepted. - The contribution should not break API or ABI, unless there is a real justification for that. If there is an API change, the contribution, if accepted, will be merged only when there will be a major release. -### Contributor License Agreement ( CLA ) +Contributor License Agreement (CLA) +----------------------------------- - All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. - To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. -### Coding Standards +Coding Standards +---------------- - We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. - The code should be written in a clean and readable style. - The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. -- The code should be secure, and will be reviewed in a security point of view as well. - -### Making a Contribution +- The code should be secure, and will be reviewed from a security point of view as well. +Making a Contribution +--------------------- 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. -2. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the "development" branch as a basis. -3. Write a test which shows that the bug was fixed or that the feature works as expected. -4. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) -5. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. -6. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. - -### Backports +1. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. +1. Write a test which shows that the bug was fixed or that the feature works as expected. +1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +1. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. +1. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. +Backports +--------- mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: 1. If the contribution is a new feature or enhancement, no backporting is needed. -2. Bug fixes should be backported to the legacy branches containing these bugs. -3. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. +1. Bug fixes should be backported to the legacy branches containing these bugs. +1. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. It would be highly appreciated if a contribution would be backported to a legacy branch as well. At the moment, the legacy branches are: -1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) -2. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) -3. [development](https://github.com/ARMmbed/mbedtls/tree/development) - -### Tests +1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) +1. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) +1. [development](https://github.com/ARMmbed/mbedtls/tree/development) +Tests +----- As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. Sample applications, if needed, should be modified as well. -### Continuous Integration Tests - +Continuous Integration Tests +---------------------------- Once a PR has been made, the Continuous Integration (CI) tests are triggered and run. You should follow the result of the CI tests, and fix failures. It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtls/tree/development/tests/git-scripts) prior to pushing your changes, for catching some of the issues as early as possible. -### Documentation - +Documentation +------------- mbed TLS should be well documented. If documentation is needed, speak out! 1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. -2. Complex parts in the code should include comments. -3. If needed, a Readme file is advised -4. If a KB article should be added, write this as a comment in the PR description. -5. A Changelog entry should be added for this contribution. - - - \ No newline at end of file +1. Complex parts in the code should include comments. +1. If needed, a Readme file is advised. +1. If a [Knowledge Base (KB)](https://tls.mbed.org/kb) article should be added, write this as a comment in the PR description. +1. A [ChangeLog](https://github.com/ARMmbed/mbedtls/blob/development/ChangeLog) entry should be added for this contribution. From 0a47d127170a94c76932a9b1dcc4525fd8521435 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 3 Sep 2017 10:20:25 +0300 Subject: [PATCH 18/84] Rephrase the backport sectio Rephrase the backport sectoin, since development branch is not a legacy branch --- CONTRIBUTING.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7bf5f8dbf..c1870547ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,12 +34,11 @@ mbed TLS maintains some legacy branches, which are released as LTS versions. mbe 1. Bug fixes should be backported to the legacy branches containing these bugs. 1. Changes in the API do not require backporting. If a bug fix introduced a new API, such as new error codes, the bug fix should be implemented differently in the legacy branch. -It would be highly appreciated if a contribution would be backported to a legacy branch as well. +It would be highly appreciated if a contribution would be backported to a legacy branch in addition to the [development branch](https://github.com/ARMmbed/mbedtls/tree/development). At the moment, the legacy branches are: 1. [mbedtls-1.3](https://github.com/ARMmbed/mbedtls/tree/mbedtls-1.3) 1. [mbedtls-2.1](https://github.com/ARMmbed/mbedtls/tree/mbedtls-2.1) -1. [development](https://github.com/ARMmbed/mbedtls/tree/development) Tests ----- From bc18eb3b928e861d0b71f7792cafbf2ad4c38972 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 6 Sep 2017 17:49:10 +0300 Subject: [PATCH 19/84] Fix compilation error with Mingw32 Fix compilation error on Mingw32 when `_TRUNCATE` is defined. Use `_TRUNCATE` only if `__MINGW32__` not defined. Fix suggested by Thomas Glanzmann and Nick Wilson on issue #355 --- ChangeLog | 7 +++++++ library/debug.c | 2 +- library/platform.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 227faed6b6..f8dcae521c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Fix compilation error on Mingw32 when `_TRUNCATE` is defined. Use `_TRUNCATE` + only if `__MINGW32__` not defined. Fix suggested by Thomas Glanzmann and + Nick Wilson on issue #355 + = mbed TLS 2.6.0 branch released 2017-08-10 Security diff --git a/library/debug.c b/library/debug.c index f9229b3606..db3924ac54 100644 --- a/library/debug.c +++ b/library/debug.c @@ -91,7 +91,7 @@ void mbedtls_debug_print_msg( const mbedtls_ssl_context *ssl, int level, va_start( argp, format ); #if defined(_WIN32) -#if defined(_TRUNCATE) +#if defined(_TRUNCATE) && !defined(__MINGW32__) ret = _vsnprintf_s( str, DEBUG_BUF_SIZE, _TRUNCATE, format, argp ); #else ret = _vsnprintf( str, DEBUG_BUF_SIZE, format, argp ); diff --git a/library/platform.c b/library/platform.c index af3b2f15ec..68506f544f 100644 --- a/library/platform.c +++ b/library/platform.c @@ -74,7 +74,7 @@ int mbedtls_platform_win32_snprintf( char *s, size_t n, const char *fmt, ... ) return( -1 ); va_start( argp, fmt ); -#if defined(_TRUNCATE) +#if defined(_TRUNCATE) && !defined(__MINGW32__) ret = _vsnprintf_s( s, n, _TRUNCATE, fmt, argp ); #else ret = _vsnprintf( s, n, fmt, argp ); From 936f72c641c0953cc288d01de30a2dd811b5f8ac Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 10:56:10 +0100 Subject: [PATCH 20/84] Disable MBEDTLS_RSA_FORCE_BLINDING by default This commit disables the new MBEDTLS_RSA_FORCE_BLINDING option by default to preserve backwards compatibility. Further, it deprecates disabling to prepare for a future release in which blinding will be unconditionally enforced. --- include/mbedtls/config.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index d54f0c3824..741ce416ae 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -980,6 +980,11 @@ * Comment this macro to allow RSA private key operations * without blinding. * + * \deprecated Disabling this option is deprecated and only + * disabled by default for backwards compatibility. + * Future versions of Mbed TLS will remove this + * option and enforce blinding unconditionally. + * * \warning Disabling this can be a security risk! * Blinding RSA private key operations is a way * to prevent statistical timing attacks as in @@ -998,7 +1003,7 @@ * private key operations, see the documentation * of \c mbedtls_rsa_private. */ -#define MBEDTLS_RSA_FORCE_BLINDING +//#define MBEDTLS_RSA_FORCE_BLINDING /** * \def MBEDTLS_RSA_NO_CRT From 6ac972d815107812be6df8ab591e475208709720 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 10:57:48 +0100 Subject: [PATCH 21/84] Style correction in test_suite_pk.function --- tests/suites/test_suite_pk.function | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 33453ac6f8..a6372c52a7 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -43,8 +43,9 @@ int mbedtls_rsa_decrypt_func( void *ctx, int mode, size_t *olen, const unsigned char *input, unsigned char *output, size_t output_max_len ) { - return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, rnd_std_rand, NULL, mode, olen, - input, output, output_max_len ) ); + return( mbedtls_rsa_pkcs1_decrypt( (mbedtls_rsa_context *) ctx, + rnd_std_rand, NULL, mode, olen, + input, output, output_max_len ) ); } int mbedtls_rsa_sign_func( void *ctx, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng, @@ -107,7 +108,8 @@ void mbedtls_pk_check_pair( char *pub_file, char *prv_file, int ret ) if( mbedtls_pk_get_type( &prv ) == MBEDTLS_PK_RSA ) { TEST_ASSERT( mbedtls_pk_setup_rsa_alt( &alt, mbedtls_pk_rsa( prv ), - mbedtls_rsa_decrypt_func, mbedtls_rsa_sign_func, mbedtls_rsa_key_len_func ) == 0 ); + mbedtls_rsa_decrypt_func, mbedtls_rsa_sign_func, + mbedtls_rsa_key_len_func ) == 0 ); TEST_ASSERT( mbedtls_pk_check_pair( &pub, &alt ) == ret ); } #endif From a988a2702ab402e119502f9759347b12d91c0ee4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 7 Sep 2017 11:32:04 +0100 Subject: [PATCH 22/84] Emit deprecation warning if MBEDTLS_RSA_FORCE_BLINDING is not set --- library/rsa.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/library/rsa.c b/library/rsa.c index 680df0d8e5..88257aa578 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -66,6 +66,13 @@ #define mbedtls_free free #endif +#if !defined(MBEDTLS_RSA_FORCE_BLINDING) && \ + defined(MBEDTLS_DEPRECATED_WARNING) +#warning Not enforcing blinding checks for RSA private key operations\ + is deprecated. Please uncomment MBEDTLS_RSA_FORCE_BLINDING\ + in config.h to enforce blinding checks. +#endif + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0; From b2231fc31a8e7840734b5fd6d9b64d30635ac3d4 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 10 Sep 2017 17:32:05 +0300 Subject: [PATCH 23/84] Address review comments Addres review comments done by Hanno --- CONTRIBUTING.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c1870547ba..3c6dc74c81 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,12 +7,12 @@ We gratefully accept bug reports and contributions from the community. There are Contributor License Agreement (CLA) ----------------------------------- -- All contributions, whether large or small require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. -- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to ARM as described in the instructions given. +- All contributions, whether large or small, require a Contributor's License Agreement (CLA) to be accepted. This is because source code can possibly fall under copyright law and we need your consent to share in the ownership of the copyright. +- To accept the Contributor’s License Agreement (CLA), individual contributors can do this by creating an Mbed account and [accepting the online agreement here with a click through](https://developer.mbed.org/contributor_agreement/). Alternatively, for contributions from corporations, or those that do not wish to create an Mbed account, a slightly different agreement can be found [here](https://www.mbed.com/en/about-mbed/contributor-license-agreements/). This agreement should be signed and returned to Arm as described in the instructions given. Coding Standards ---------------- -- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission. +- We would ask that contributions conform to [our coding standards](https://tls.mbed.org/kb/development/mbedtls-coding-standards), and that contributions are fully tested before submission, as mentioned in the [Tests](#tests) and [Continuous Integration](#continuous-integration-tests) sections. - The code should be written in a clean and readable style. - The code should be written in a portable generic way, that will benefit the whole community, and not only your own needs. - The code should be secure, and will be reviewed from a security point of view as well. @@ -20,15 +20,15 @@ Coding Standards Making a Contribution --------------------- 1. [Check for open issues](https://github.com/ARMmbed/mbedtls/issues) or [start a discussion](https://tls.mbed.org/discussions) around a feature idea or a bug. -1. Fork the [mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. +1. Fork the [Mbed TLS repository on GitHub](https://github.com/ARMmbed/mbedtls) to start making your changes. As a general rule, you should use the ["development" branch](https://github.com/ARMmbed/mbedtls/tree/development) as a basis. 1. Write a test which shows that the bug was fixed or that the feature works as expected. -1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) +1. Send a pull request (PR) and work with us until it gets merged and published. Contributions may need some modifications, so a few rounds of review and fixing may be necessary. We will include your name in the ChangeLog :) 1. For quick merging, the contribution should be short, and concentrated on a single feature or topic. The larger the contribution is, the longer it would take to review it and merge it. -1. mbed TLS is release with Apache license, and as such, all the added files should include the Apache license header. +1. Mbed TLS is released under the Apache license, and as such, all the added files should include the Apache license header. Backports --------- -mbed TLS maintains some legacy branches, which are released as LTS versions. mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: +Mbed TLS maintains some legacy branches, which are released as LTS versions. Mbed TLS should follow backwards compatibility rules, to fit with existing users. As such, backporting to these branches should be handled according to the following rules: 1. If the contribution is a new feature or enhancement, no backporting is needed. 1. Bug fixes should be backported to the legacy branches containing these bugs. @@ -42,8 +42,8 @@ At the moment, the legacy branches are: Tests ----- -As mentioned, tests that show the correctness of the feature or bug fix should be added to the Pull Request, if no such tests exist. -mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. +As mentioned, tests that show the correctness of the feature or bug fix should be added to the pull request, if no such tests exist. +Mbed TLS includes an elaborate test suite in `tests/` that initially requires Perl to generate the tests files (e.g. `test_suite_mpi.c`). These files are generated from a `function file` (e.g. `suites/test_suite_mpi.function`) and a `data file` (e.g. `suites/test_suite_mpi.data`). The function file contains the test functions. The data file contains the test cases, specified as parameters that will be passed to the test function. Sample applications, if needed, should be modified as well. @@ -54,7 +54,7 @@ It is advised to enable the [githooks scripts](https://github.com/ARMmbed/mbedtl Documentation ------------- -mbed TLS should be well documented. If documentation is needed, speak out! +Mbed TLS should be well documented. If documentation is needed, speak out! 1. All interfaces should be documented through Doxygen. New APIs should introduce Doxygen documentation. 1. Complex parts in the code should include comments. From 714785dcc217a769ce5e46f3dcb915291ed944c7 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Mon, 28 Aug 2017 13:55:55 +0300 Subject: [PATCH 24/84] Write correct number of ciphersuites in log Change location of log, to fit the correct number of used ciphersuites --- ChangeLog | 7 ++++++- library/ssl_cli.c | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 227faed6b6..000084b775 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,9 @@ -mbed TLS ChangeLog (Sorted per branch, date) +mbed TLS ChangeLog (Sorted per branch, date) + += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Log correct number of ciphersuites used in Client Hello message. Fix for #918. = mbed TLS 2.6.0 branch released 2017-08-10 diff --git a/library/ssl_cli.c b/library/ssl_cli.c index a2b9f8cfe1..9babb695c9 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -891,6 +891,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) *p++ = (unsigned char)( ciphersuites[i] ); } + MBEDTLS_SSL_DEBUG_MSG(3, ("client hello, got %d ciphersuites", n)); + /* * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV */ @@ -917,8 +919,6 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) *q++ = (unsigned char)( n >> 7 ); *q++ = (unsigned char)( n << 1 ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, got %d ciphersuites", n ) ); - #if defined(MBEDTLS_ZLIB_SUPPORT) offer_compress = 1; #else From 147d1429481135cc989fea47dabbca905717e978 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 5 Sep 2017 16:09:53 +0300 Subject: [PATCH 25/84] Add log and fix stle issues Address Andres comments of PR --- library/ssl_cli.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9babb695c9..8a48f7b000 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -891,7 +891,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) *p++ = (unsigned char)( ciphersuites[i] ); } - MBEDTLS_SSL_DEBUG_MSG(3, ("client hello, got %d ciphersuites", n)); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, got %d ciphersuites (excluding SCSVs)", n ) ); /* * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV @@ -900,6 +900,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) #endif { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO ); n++; From 4a2fb4c6be50134cbf6198ed6cc904f7d71af77b Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 10 Sep 2017 17:03:50 +0300 Subject: [PATCH 26/84] Addres review comments Resolves comments raised in the review --- library/ssl_cli.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 8a48f7b000..36086e9d98 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -891,7 +891,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) *p++ = (unsigned char)( ciphersuites[i] ); } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, got %d ciphersuites (excluding SCSVs)", n ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, got %d ciphersuites (excluding SCSVs)", n ) ); /* * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV @@ -900,7 +900,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( ssl->renego_status == MBEDTLS_SSL_INITIAL_HANDSHAKE ) #endif { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO ); n++; From 2f73c9342fd9d31728c0bf4fb34266fdcc489a88 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 26 Sep 2017 15:06:56 +0300 Subject: [PATCH 27/84] Fix Changelog notation Remove backticks, since ChangeLog is not in MarkDown --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f8dcae521c..4eb52fb8fc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,8 +3,8 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Bugfix - * Fix compilation error on Mingw32 when `_TRUNCATE` is defined. Use `_TRUNCATE` - only if `__MINGW32__` not defined. Fix suggested by Thomas Glanzmann and + * Fix compilation error on Mingw32 when _TRUNCATE is defined. Use _TRUNCATE + only if __MINGW32__ not defined. Fix suggested by Thomas Glanzmann and Nick Wilson on issue #355 = mbed TLS 2.6.0 branch released 2017-08-10 From 8d1dd1b5b9ffd1e615d1dea6524c8ea53a13216a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:02:24 +0100 Subject: [PATCH 28/84] Fix bug in mbedtls_mpi_exp_mod Calling `mbedtls_mpi_exp_mod` with a freshly initialized exponent MPI `N`, i.e. `N.p == NULL`, would lead to a null-pointer dereference. --- library/bignum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/bignum.c b/library/bignum.c index 8b9082cdcb..e9ac565052 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1614,7 +1614,7 @@ int mbedtls_mpi_exp_mod( mbedtls_mpi *X, const mbedtls_mpi *A, const mbedtls_mpi mbedtls_mpi RR, T, W[ 2 << MBEDTLS_MPI_WINDOW_SIZE ], Apos; int neg; - if( mbedtls_mpi_cmp_int( N, 0 ) < 0 || ( N->p[0] & 1 ) == 0 ) + if( mbedtls_mpi_cmp_int( N, 0 ) <= 0 || ( N->p[0] & 1 ) == 0 ) return( MBEDTLS_ERR_MPI_BAD_INPUT_DATA ); if( mbedtls_mpi_cmp_int( E, 0 ) < 0 ) From 2c9f027e32f3fc83ccb3d24d132a77a711bd141b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:04:13 +0100 Subject: [PATCH 29/84] Don't require P,Q if CRT is not used Previously, verification used P,Q regardless of whether CRT was used in the computation, but this has changed in the meantime. --- library/rsa.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 88257aa578..11ba2019ab 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -448,15 +448,15 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } #if !defined(MBEDTLS_RSA_NO_CRT) - if( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 || + if( mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->QP, 0 ) == 0 ) { From 7c0f17d1155d8a3e0fd52f831ecc84ce11673f2e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 11:49:46 +0100 Subject: [PATCH 30/84] Add `MBEDTLS_RSA_NO_CRT` to options unaffected by `config.pl full` The effect of `config.pl full` on 'negative' options such as `NO_PLATFORM_ENTROPY` is usually inverted, but `MBEDTLS_RSA_NO_CRT` was not included in the list of such options. This commit adds it. --- scripts/config.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/config.pl b/scripts/config.pl index 2757f17fe3..e2760b15cf 100755 --- a/scripts/config.pl +++ b/scripts/config.pl @@ -17,7 +17,7 @@ # # Full usage description provided below. # -# Things that shouldn't be enabled with "full". +# The following options are disabled instead of enabled with "full". # # MBEDTLS_TEST_NULL_ENTROPY # MBEDTLS_DEPRECATED_REMOVED @@ -30,6 +30,7 @@ # MBEDTLS_NO_PLATFORM_ENTROPY # MBEDTLS_REMOVE_ARC4_CIPHERSUITES # MBEDTLS_SSL_HW_RECORD_ACCEL +# MBEDTLS_RSA_NO_CRT # MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 # MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION # - this could be enabled if the respective tests were adapted @@ -85,6 +86,7 @@ MBEDTLS_ECP_DP_M383_ENABLED MBEDTLS_ECP_DP_M511_ENABLED MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES MBEDTLS_NO_PLATFORM_ENTROPY +MBEDTLS_RSA_NO_CRT MBEDTLS_REMOVE_ARC4_CIPHERSUITES MBEDTLS_SSL_HW_RECORD_ACCEL MBEDTLS_X509_ALLOW_EXTENSIONS_NON_V3 From d5ba5effaa30addc721f27f65b15a97af3f33248 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 12:53:51 +0100 Subject: [PATCH 31/84] Add ASan build-and-test run for MBEDTLS_RSA_NO_CRT in all.sh --- tests/scripts/all.sh | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7c33c5c2cc..5fe9191cc1 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -326,6 +326,22 @@ OPENSSL_CMD="$OPENSSL_LEGACY" tests/compat.sh -m 'ssl3' msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min tests/ssl-opt.sh +msg "build: Default + RSA_NO_CRT (ASan build)" # ~ 6 min +cleanup +cp "$CONFIG_H" "$CONFIG_BAK" +scripts/config.pl set MBEDTLS_RSA_NO_CRT +CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . +make + +msg "test: RSA_NO_CRT - main suites (inc. selftests) (ASan build)" # ~ 50s +make test + +msg "test: RSA_NO_CRT - RSA-related part of ssl-opt.sh (ASan build)" # ~ 5s +tests/ssl-opt.sh -f RSA + +msg "test: RSA_NO_CRT - RSA-related part of compat.sh (ASan build)" # ~ 3 min +tests/compat.sh -t RSA + msg "build: cmake, full config, clang, C99" # ~ 50s cleanup cp "$CONFIG_H" "$CONFIG_BAK" @@ -572,4 +588,3 @@ rm -rf "$OUT_OF_SOURCE_DIR" msg "Done, cleaning up" cleanup - From a6f55394137487b7298ab929202d70b5f210c7c2 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 28 Sep 2017 12:56:28 +0100 Subject: [PATCH 32/84] Adapt version_features.c to new config options --- library/version_features.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/version_features.c b/library/version_features.c index 9f97c7bc3e..f7fa041c42 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -345,9 +345,18 @@ static const char *features[] = { #if defined(MBEDTLS_PKCS1_V21) "MBEDTLS_PKCS1_V21", #endif /* MBEDTLS_PKCS1_V21 */ +#if defined(MBEDTLS_RSA_FORCE_BLINDING) + "MBEDTLS_RSA_FORCE_BLINDING", +#endif /* MBEDTLS_RSA_FORCE_BLINDING */ #if defined(MBEDTLS_RSA_NO_CRT) "MBEDTLS_RSA_NO_CRT", #endif /* MBEDTLS_RSA_NO_CRT */ +#if defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION) + "MBEDTLS_RSA_FORCE_CRT_VERIFICATION", +#endif /* MBEDTLS_RSA_FORCE_CRT_VERIFICATION */ +#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) + "MBEDTLS_RSA_FORCE_VERIFICATION", +#endif /* MBEDTLS_RSA_FORCE_VERIFICATION */ #if defined(MBEDTLS_SELF_TEST) "MBEDTLS_SELF_TEST", #endif /* MBEDTLS_SELF_TEST */ From 2fdffe0da0bf74cb94682730fe2db6b0ba8472fa Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 29 Sep 2017 15:19:28 +0100 Subject: [PATCH 33/84] Check exactly for the RSA context fields required in rsa_private Previously, the code was also checking for the presence of D for RSA-CRT, which is not needed in this case. --- library/rsa.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 11ba2019ab..d866c7aa3c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -447,14 +447,19 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ +#if defined(MBEDTLS_RSA_NO_CRT) if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 ) + mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } -#if !defined(MBEDTLS_RSA_NO_CRT) - if( mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || +#else /* ! MBEDTLS_RSA_NO_CRT */ + if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 || + mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->DP, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) == 0 || @@ -462,7 +467,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } -#endif /* MBEDTLS_RSA_NO_CRT */ +#endif /* ! MBEDTLS_RSA_NO_CRT */ #if defined(MBEDTLS_THREADING_C) if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) From 4e1be398f64170a10495561e91ccc27aa31f94a3 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 2 Oct 2017 15:56:48 +0100 Subject: [PATCH 34/84] Remove FORCE_VERIFICATION and FORCE_BLINDING --- include/mbedtls/config.h | 77 -------------------------------------- include/mbedtls/rsa.h | 31 +++------------ library/rsa.c | 22 ----------- library/version_features.c | 9 ----- 4 files changed, 5 insertions(+), 134 deletions(-) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 741ce416ae..52556262a9 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -970,41 +970,6 @@ */ #define MBEDTLS_PKCS1_V21 -/** - * \def MBEDTLS_RSA_FORCE_BLINDING - * - * Force the use of blinding in RSA private key operations. - * This makes these operations fail when the caller doesn't - * provide a PRNG. - * - * Comment this macro to allow RSA private key operations - * without blinding. - * - * \deprecated Disabling this option is deprecated and only - * disabled by default for backwards compatibility. - * Future versions of Mbed TLS will remove this - * option and enforce blinding unconditionally. - * - * \warning Disabling this can be a security risk! - * Blinding RSA private key operations is a way - * to prevent statistical timing attacks as in - * [P. Kocher ', Timing Attacks on Implementations - * of Diffie-Hellman, RSA, DSS, and Other Systems] - * - * \note Disabling this does not mean that blinding - * will never be used: if a PRNG is provided, - * blinding will be in place. Instead, disabling this - * option may result in private key operations being - * performed in a way potentially leaking sensitive - * information through side-channels when no PRNG - * is supplied by the user. - * - * \note For more on the use of blinding in RSA - * private key operations, see the documentation - * of \c mbedtls_rsa_private. - */ -//#define MBEDTLS_RSA_FORCE_BLINDING - /** * \def MBEDTLS_RSA_NO_CRT * @@ -1016,48 +981,6 @@ */ //#define MBEDTLS_RSA_NO_CRT -/** - * \def MBEDTLS_RSA_FORCE_CRT_VERIFICATION - * - * Force verification of results of RSA private key operations - * when RSA-CRT is used. - * - * Comment this macro to disable RSA-CRT verification. - * - * \warning Disabling this can be a security risk! - * Omitting verification makes the RSA-CRT - * signing vulnerable to the Bellcore - * glitch attack leading to private key - * compromise if an attacker can cause a - * glitch in a certain timeframe during - * the signing operation. Uncomment only - * if you're sure that glitches are out of - * your attack model. - */ -#define MBEDTLS_RSA_FORCE_CRT_VERIFICATION - -/** - * \def MBEDTLS_RSA_FORCE_VERIFICATION - * - * Force verification of results of any RSA private key - * operation regardless of the algorithm used. - * - * Uncomment this to enable unconditional RSA verification. - * - * \note This is to prevent the RSA signing operation - * (regardless of the particular algorithm chosen) - * from potential future glitch attacks. We are - * currently not aware of any such for our default - * implementation, therefore disabling the option - * by default. - * - * \note Enabling it comes at the cost of roughly an - * additional public key operation at the end of - * signing (low compared to private key operations), - * as well as minor memory consumption. - */ -//#define MBEDTLS_RSA_FORCE_VERIFICATION - /** * \def MBEDTLS_SELF_TEST * diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index e34fea0f28..bc2f810ae8 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -63,15 +63,6 @@ #define MBEDTLS_RSA_SALT_LEN_ANY -1 -/* - * RSA configuration - */ -#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) || \ - ( ! defined(MBEDTLS_RSA_NO_CRT) && \ - defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION ) ) -#define MBEDTLS_RSA_REQUIRE_VERIFICATION -#endif - /* * The above constants may be used even if the RSA module is compile out, * eg for alternative (PKCS#11) RSA implemenations in the PK layers. @@ -239,28 +230,16 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx, * \note The input and output buffers must be large * enough (eg. 128 bytes if RSA-1024 is used). * - * \note Enabling and disabling of blinding: - * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING - * is disabled, blinding is disabled. - * - If f_rng is NULL and MBEDTLS_RSA_FORCE_BLINDING - * is enabled, the function fails. + * \note Blinding is used if and onlf if a PRNG is provided. * * \note If blinding is used, both the base of exponentation * and the exponent are blinded, preventing both statistical * timing and power analysis attacks. * - * \note Depending on the way RSA is implemented, a failure - * in the computation can lead to disclosure of the private - * key if the wrong result is passed to attacker - e.g., - * implementing RSA through CRT is vulnerable to the - * Bellcore glitch attack. - * - * As a remedy, the user can force double checking the - * result of the private key operation through the option - * MBEDTLS_RSA_FORCE_VERIFICATION. If verification is - * to be enabled only when RSA-CRT is used (as controlled - * by the configuration option MBEDTLS_RSA_NO_CRT), the - * option MBEDTLS_RSA_FORCE_CRT_VERIFICATION can be used. + * \warning It is deprecated and a security risk to not provide + * a PRNG here and thereby prevent the use of blinding. + * Future versions of the library may enforce the presence + * of a PRNG. * */ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, diff --git a/library/rsa.c b/library/rsa.c index d866c7aa3c..de684b39c7 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -66,13 +66,6 @@ #define mbedtls_free free #endif -#if !defined(MBEDTLS_RSA_FORCE_BLINDING) && \ - defined(MBEDTLS_DEPRECATED_WARNING) -#warning Not enforcing blinding checks for RSA private key operations\ - is deprecated. Please uncomment MBEDTLS_RSA_FORCE_BLINDING\ - in config.h to enforce blinding checks. -#endif - /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0; @@ -434,16 +427,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, mbedtls_mpi *D = &ctx->D; #endif /* MBEDTLS_RSA_NO_CRT */ -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) /* Temporaries holding the initial input and the double * checked result; should be the same in the end. */ mbedtls_mpi I, C; -#endif - -#if defined(MBEDTLS_RSA_FORCE_BLINDING) - if( f_rng == NULL ) - return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); -#endif /* Sanity-check that all relevant fields are at least set, * but don't perform a full keycheck. */ @@ -496,10 +482,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, mbedtls_mpi_init( &TP ); mbedtls_mpi_init( &TQ ); #endif -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) mbedtls_mpi_init( &I ); mbedtls_mpi_init( &C ); -#endif /* End of MPI initialization */ @@ -510,9 +494,7 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, goto cleanup; } -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) MBEDTLS_MPI_CHK( mbedtls_mpi_copy( &I, &T ) ); -#endif if( f_rng != NULL ) { @@ -604,14 +586,12 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, } /* If requested by the config, verify the result to prevent glitching attacks. */ -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, &ctx->N, &ctx->RN ) ); if( mbedtls_mpi_cmp_mpi( &C, &I ) != 0 ) { ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; goto cleanup; } -#endif /* MBEDTLS_RSA_REQUIRE_VERIFICATION */ olen = ctx->len; MBEDTLS_MPI_CHK( mbedtls_mpi_write_binary( &T, output, olen ) ); @@ -642,10 +622,8 @@ cleanup: mbedtls_mpi_free( &TP ); mbedtls_mpi_free( &TQ ); #endif -#if defined(MBEDTLS_RSA_REQUIRE_VERIFICATION) mbedtls_mpi_free( &C ); mbedtls_mpi_free( &I ); -#endif if( ret != 0 ) return( MBEDTLS_ERR_RSA_PRIVATE_FAILED + ret ); diff --git a/library/version_features.c b/library/version_features.c index f7fa041c42..9f97c7bc3e 100644 --- a/library/version_features.c +++ b/library/version_features.c @@ -345,18 +345,9 @@ static const char *features[] = { #if defined(MBEDTLS_PKCS1_V21) "MBEDTLS_PKCS1_V21", #endif /* MBEDTLS_PKCS1_V21 */ -#if defined(MBEDTLS_RSA_FORCE_BLINDING) - "MBEDTLS_RSA_FORCE_BLINDING", -#endif /* MBEDTLS_RSA_FORCE_BLINDING */ #if defined(MBEDTLS_RSA_NO_CRT) "MBEDTLS_RSA_NO_CRT", #endif /* MBEDTLS_RSA_NO_CRT */ -#if defined(MBEDTLS_RSA_FORCE_CRT_VERIFICATION) - "MBEDTLS_RSA_FORCE_CRT_VERIFICATION", -#endif /* MBEDTLS_RSA_FORCE_CRT_VERIFICATION */ -#if defined(MBEDTLS_RSA_FORCE_VERIFICATION) - "MBEDTLS_RSA_FORCE_VERIFICATION", -#endif /* MBEDTLS_RSA_FORCE_VERIFICATION */ #if defined(MBEDTLS_SELF_TEST) "MBEDTLS_SELF_TEST", #endif /* MBEDTLS_SELF_TEST */ From 2dec5e8b00d25f2fd6946172eb3b30177a4b124e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 3 Oct 2017 07:49:52 +0100 Subject: [PATCH 35/84] Correct outdated comment --- library/rsa.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index de684b39c7..56f434563a 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -585,8 +585,9 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &T, &T, &ctx->N ) ); } - /* If requested by the config, verify the result to prevent glitching attacks. */ - MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, &ctx->N, &ctx->RN ) ); + /* Verify the result to prevent glitching attacks. */ + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &C, &T, &ctx->E, + &ctx->N, &ctx->RN ) ); if( mbedtls_mpi_cmp_mpi( &C, &I ) != 0 ) { ret = MBEDTLS_ERR_RSA_VERIFY_FAILED; From 7bba968afcb9d2a352d2e39cc9eae5a338d94c53 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 26 Oct 2017 11:53:26 +0100 Subject: [PATCH 36/84] Adapt ChangeLog --- ChangeLog | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index b6ab9665a8..2f1f0557c5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,12 +2,8 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx -Bugfix - * Fix memory leak in RSA self test. - Security - * Add option for mandatory use of blinding in RSA private key operations. - * Add options for verification of RSA private key operations to defend + * Verify results of RSA private key operations to defend against Bellcore glitch attack. = mbed TLS 2.x.x branch released xxxx-xx-xx From 2412061a5a55410e8fffc583b3ce3a2f0dfc067d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 26 Oct 2017 11:53:35 +0100 Subject: [PATCH 37/84] Correct typo and improve documentation --- include/mbedtls/rsa.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index bc2f810ae8..54a1f25207 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -230,11 +230,11 @@ int mbedtls_rsa_public( mbedtls_rsa_context *ctx, * \note The input and output buffers must be large * enough (eg. 128 bytes if RSA-1024 is used). * - * \note Blinding is used if and onlf if a PRNG is provided. + * \note Blinding is used if and only if a PRNG is provided. * * \note If blinding is used, both the base of exponentation - * and the exponent are blinded, preventing both statistical - * timing and power analysis attacks. + * and the exponent are blinded, providing protection + * against some side-channel attacks. * * \warning It is deprecated and a security risk to not provide * a PRNG here and thereby prevent the use of blinding. From 7512bf7d6398525b74e248c9a19bd17b1b9c600a Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:12:11 +0100 Subject: [PATCH 38/84] Add macros to ASN.1 module to parse ASN.1 tags The macros simply extract the component bits of an ASN.1 tag value --- include/mbedtls/asn1.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index e159e57ea0..8d35c42451 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -87,6 +87,22 @@ #define MBEDTLS_ASN1_PRIMITIVE 0x00 #define MBEDTLS_ASN1_CONSTRUCTED 0x20 #define MBEDTLS_ASN1_CONTEXT_SPECIFIC 0x80 + +/* + * Bit masks for each of the components of an ASN.1 tag as specified in + * Information technnology - ASN.1 encoding rules: Specification of Basic + * Encoding Rules (BER), Canonical Encoding Rules (CER), and Distinguished + * encoding rules (DER) Section 8.1.2.2: + * + * Bit 8 7 6 5 1 + * +-------+-----+------------+ + * | Class | P/C | Tag number | + * +-------+-----+------------+ + */ +#define MBEDTLS_ASN1_TAG_CLASS_MASK ( 0x03 << 6 ) +#define MBEDTLS_ASN1_TAG_PC_MASK ( 0x01 << 5 ) +#define MBEDTLS_ASN1_TAG_VALUE_MASK ( 0x1F << 0 ) + /* \} name */ /* \} addtogroup asn1_module */ From 849bc65bbfcc8a06b17169274dbfb163de0d59f1 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:13:12 +0100 Subject: [PATCH 39/84] Fix x509_get_subject_alt_name to drop invalid tag Fix the x509_get_subject_alt_name() function to not accept invalid tags. The problem was that the ASN.1 class for tags consists of two bits. Simply doing bit-wise and of the CONTEXT_SPECIFIC macro with the input tag has the potential of accepting tag values 0x10 (private) which would indicate that the certificate has an incorrect format. --- library/x509_crt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index c6209fb40d..6d08d77955 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -472,9 +472,12 @@ static int x509_get_subject_alt_name( unsigned char **p, if( ( ret = mbedtls_asn1_get_len( p, end, &tag_len ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); - if( ( tag & MBEDTLS_ASN1_CONTEXT_SPECIFIC ) != MBEDTLS_ASN1_CONTEXT_SPECIFIC ) + if( ( tag & MBEDTLS_ASN1_TAG_CLASS_MASK ) != + MBEDTLS_ASN1_CONTEXT_SPECIFIC ) + { return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); + } /* Skip everything but DNS name */ if( tag != ( MBEDTLS_ASN1_CONTEXT_SPECIFIC | 2 ) ) From ceae42659b9d172df9c28a2643a7e6fe7b620cc9 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:17:34 +0100 Subject: [PATCH 40/84] Add ChangeLog entry --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index ded60d39f3..c81c5d6f5c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Bugfix * Fix leap year calculation in x509_date_is_valid() to ensure that invalid dates on leap years with 100 and 400 intervals are handled correctly. Found by Nicholas Wilson. #694 + * Fix X509 CRT parsing that would potentially accept an invalid tag when + parsing the subject alternative names. = mbed TLS 2.6.0 branch released 2017-08-10 From 72705c906c73cc15512c04d317ff0fba6bc4c8e0 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 7 Nov 2017 20:16:19 +0000 Subject: [PATCH 41/84] Add regression test for parsing subjectAltNames --- tests/suites/test_suite_x509parse.data | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index d4cc11a08f..374f1c2ccc 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1124,6 +1124,10 @@ x509parse_crt:"30173015a0030201038204deadbeef30080604cafed00d0500":"":MBEDTLS_ER X509 Certificate ASN1 (invalid version overflow) x509parse_crt:"301A3018a00602047FFFFFFF8204deadbeef30080604cafed00d0500":"":MBEDTLS_ERR_X509_UNKNOWN_VERSION +X509 Certificate ASN1 (invalid SubjectAltNames tag) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +x509parse_crt:"308203723082025AA003020102020111300D06092A864886F70D0101050500303B310B3009060355040613024E4C3111300F060355040A1308506F6C617253534C3119301706035504031310506F6C617253534C2054657374204341301E170D3132303531303133323334315A170D3232303531313133323334315A303A310B3009060355040613024E4C3111300F060355040A1308506F6C617253534C311830160603550403130F7777772E6578616D706C652E636F6D30820122300D06092A864886F70D01010105000382010F003082010A0282010100B93C4AC5C8A38E9017A49E52AA7175266180E7C7B56D8CFFAAB64126B7BE11AD5C73160C64114804FFD6E13B05DB89BBB39709D51C14DD688739B03D71CBE276D01AD8182D801B54F6E5449AF1CBAF612EDF490D9D09B7EDB1FD3CFD3CFA24CF5DBF7CE453E725B5EA4422E926D3EA20949EE66167BA2E07670B032FA209EDF0338F0BCE10EF67A4C608DAC1EDC23FD74ADD153DF95E1C8160463EB5B33D2FA6DE471CBC92AEEBDF276B1656B7DCECD15557A56EEC7525F5B77BDFABD23A5A91987D97170B130AA76B4A8BC14730FB3AF84104D5C1DFB81DBF7B01A565A2E01E36B7A65CCC305AF8CD6FCDF1196225CA01E3357FFA20F5DCFD69B26A007D17F70203010001A38181307F30090603551D1304023000301D0603551D0E041604147DE49C6BE6F9717D46D2123DAD6B1DFDC2AA784C301F0603551D23041830168014B45AE4A5B3DED252F6B9D5A6950FEB3EBCC7FDFF30320603551D11042B3029C20B6578616D706C652E636F6D820B6578616D706C652E6E6574820D2A2E6578616D706C652E6F7267300D06092A864886F70D010105050003820101004F09CB7AD5EEF5EF620DDC7BA285D68CCA95B46BDA115B92007513B9CA0BCEEAFBC31FE23F7F217479E2E6BCDA06E52F6FF655C67339CF48BC0D2F0CD27A06C34A4CD9485DA0D07389E4D4851D969A0E5799C66F1D21271F8D0529E840AE823968C39707CF3C934C1ADF2FA6A455487F7C8C1AC922DA24CD9239C68AECB08DF5698267CB04EEDE534196C127DC2FFE33FAD30EB8D432A9842853A5F0D189D5A298E71691BB9CC0418E8C58ACFFE3DD2E7AABB0B97176AD0F2733F7A929D3C076C0BF06407C0ED5A47C8AE2326E16AEDA641FB0557CDBDDF1A4BA447CB39958D2346E00EA976C143AF2101E0AA249107601F4F2C818FDCC6346128B091BF194E6":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + X509 CRL ASN1 (Incorrect first tag) x509parse_crl:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT From 7786abc16b91897ff185b8141426393f23a5ff7f Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 7 Nov 2017 20:21:56 +0000 Subject: [PATCH 42/84] Define ASN1 bitmask macros in more direct way --- include/mbedtls/asn1.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 8d35c42451..75b7b3dfbc 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -99,9 +99,9 @@ * | Class | P/C | Tag number | * +-------+-----+------------+ */ -#define MBEDTLS_ASN1_TAG_CLASS_MASK ( 0x03 << 6 ) -#define MBEDTLS_ASN1_TAG_PC_MASK ( 0x01 << 5 ) -#define MBEDTLS_ASN1_TAG_VALUE_MASK ( 0x1F << 0 ) +#define MBEDTLS_ASN1_TAG_CLASS_MASK 0xC0 +#define MBEDTLS_ASN1_TAG_PC_MASK 0x20 +#define MBEDTLS_ASN1_TAG_VALUE_MASK 0x1F /* \} name */ /* \} addtogroup asn1_module */ From 63073aa3d389500251fcda9bcb0eb3e9d4774f3d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 27 Nov 2017 15:33:18 +0000 Subject: [PATCH 43/84] Don't require P,Q in rsa_private in case of non-blinded non-CRT For non-CRT, P and Q are only used for the purpose of blinding the exponent. --- library/rsa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 56f434563a..35ace85c5f 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -437,8 +437,8 @@ int mbedtls_rsa_private( mbedtls_rsa_context *ctx, if( mbedtls_mpi_cmp_int( &ctx->N, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->D, 0 ) == 0 || mbedtls_mpi_cmp_int( &ctx->E, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 || - mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 ) + ( f_rng != NULL && mbedtls_mpi_cmp_int( &ctx->P, 0 ) == 0 ) || + ( f_rng != NULL && mbedtls_mpi_cmp_int( &ctx->Q, 0 ) == 0 ) ) { return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); } From b89c472ad5994f3e2e30f51192fe04d49a3da964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 26 Dec 2017 12:52:53 +0100 Subject: [PATCH 44/84] Improve cmake usage notes in Readme --- README.md | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 4270e80693..b9aadc7c02 100644 --- a/README.md +++ b/README.md @@ -84,9 +84,10 @@ In case you find that you need to do something else as well, please let us know ### CMake -In order to build the source using CMake, just enter at the command line: +In order to build the source using CMake in a separate directory (recommended), just enter at the command line: - cmake . + mkdir /path/to/build_dir && cd /path/to/build_dir + cmake /path/to/mbedtls_source make In order to run the tests, enter: @@ -95,7 +96,7 @@ In order to run the tests, enter: The test suites need Perl to be built. If you don't have Perl installed, you'll want to disable the test suites with: - cmake -DENABLE_TESTING=Off . + cmake -DENABLE_TESTING=Off /path/to/mbedtls_source If you disabled the test suites, but kept the programs enabled, you can still run a much smaller set of tests with: @@ -103,7 +104,7 @@ If you disabled the test suites, but kept the programs enabled, you can still ru To configure CMake for building shared libraries, use: - cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On . + cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On /path/to/mbedtls_source There are many different build modes available within the CMake buildsystem. Most of them are available for gcc and clang, though some are compiler-specific: @@ -118,16 +119,40 @@ There are many different build modes available within the CMake buildsystem. Mos Switching build modes in CMake is simple. For debug mode, enter at the command line: - cmake -D CMAKE_BUILD_TYPE=Debug . + cmake -D CMAKE_BUILD_TYPE=Debug /path/to/mbedtls_source To list other available CMake options, use: cmake -LH -Note that, with CMake, if you want to change the compiler or its options after you already ran CMake, you need to clear its cache first, e.g. (using GNU find): +Note that, with CMake, you can't adjust the compiler of compiler after the +initial invocation of cmake. This means that `CC=your_cc make` and `make +CC=your_cc` will *not* work (similarly with `CFLAGS` and other variables). +These variables need to be adjusted when invoking cmake for the first time, +for example: + + CC=your_cc cmake /path/to/mbedtls_source + +If you already invoked cmake and want to change those settings, you need to +remove the build directory and create it again. + +Note that it is possible to build in-place; this will however overwrite the +provided Makefiles (see `scripts/tmp_ignore_makefiles.sh` if you want to +prevent `git status` from showing them as modified). In order to do so, from +the Mbed TLS source directory, use: + + cmake . + make + +If you want to change `CC` or `CFLAGS` afterwards, you will need to remove the +CMake cache. This can be done with the following command using GNU find: find . -iname '*cmake*' -not -name CMakeLists.txt -exec rm -rf {} + - CC=gcc CFLAGS='-fstack-protector-strong -Wa,--noexecstack' cmake . + +You can not make the desired change: + + CC=your_cc cmake . + make ### Microsoft Visual Studio From 05c92715be05c310bf6e046f2cafcd2569800e58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 28 Dec 2017 09:14:47 +0100 Subject: [PATCH 45/84] readme: clarify CFLAGS prepending/overriding --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b9aadc7c02..d0d81fc765 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,9 @@ You'll still be able to run a much smaller set of tests with: In order to build for a Windows platform, you should use `WINDOWS_BUILD=1` if the target is Windows but the build environment is Unix-like (for instance when cross-compiling, or compiling from an MSYS shell), and `WINDOWS=1` if the build environment is a Windows shell (for instance using mingw32-make) (in that case some targets will not be available). -Setting the variable `SHARED` in your environment will build shared libraries in addition to the static libraries. Setting `DEBUG` gives you a debug build. You can override `CFLAGS` and `LDFLAGS` by setting them in your environment or on the make command line; if you do so, essential parts such as `-I` will still be preserved. Warning options may be overridden separately using `WARNING_CFLAGS`. +Setting the variable `SHARED` in your environment will build shared libraries in addition to the static libraries. Setting `DEBUG` gives you a debug build. You can override `CFLAGS` and `LDFLAGS` by setting them in your environment or on the make command line; compiler warning options may be overridden separately using `WARNING_CFLAGS`. Some directory-specific options (for example, `-I` directives) are still preserved. + +Please note that setting `CFLAGS` overrides its default value of `-O2` and setting `WARNING_CFLAGS` overrides its default value (starting with `-Wall -W`), so it you just want to add some warning options to the default ones, you can do so by setting `CFLAGS=-O2 -Werror` for example. Setting `WARNING_CFLAGS` is useful when you want to get rid of its default content (for example because your compiler doesn't accept `-Wall` as an option). Directory-specific options cannot be overriden from the command line. Depending on your platform, you might run into some issues. Please check the Makefiles in `library/`, `programs/` and `tests/` for options to manually add or remove for specific platforms. You can also check [the Mbed TLS Knowledge Base](https://tls.mbed.org/kb) for articles on your platform or issue. @@ -154,6 +156,10 @@ You can not make the desired change: CC=your_cc cmake . make +Regarding variables, also note that if you set CFLAGS when invoking cmake, +your value of CFLAGS doesn't override the content provided by cmake (depending +on the build mode as seen above), it's merely prepended to it. + ### Microsoft Visual Studio The build files for Microsoft Visual Studio are generated for Visual Studio 2010. From 976dd1674a25c242d423503032d446c53f40b4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 2 Jan 2018 10:49:46 +0100 Subject: [PATCH 46/84] Fix typos in previous commits --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d0d81fc765..2c6cc62a05 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ To list other available CMake options, use: cmake -LH -Note that, with CMake, you can't adjust the compiler of compiler after the +Note that, with CMake, you can't adjust the compiler or its flags after the initial invocation of cmake. This means that `CC=your_cc make` and `make CC=your_cc` will *not* work (similarly with `CFLAGS` and other variables). These variables need to be adjusted when invoking cmake for the first time, @@ -151,7 +151,7 @@ CMake cache. This can be done with the following command using GNU find: find . -iname '*cmake*' -not -name CMakeLists.txt -exec rm -rf {} + -You can not make the desired change: +You can now make the desired change: CC=your_cc cmake . make From 9736b9d59ab86bd9d7ab00fde866c27fa677f1da Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 2 Jan 2018 21:54:17 +0100 Subject: [PATCH 47/84] all.sh --keep-going: work if TERM is unset --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2ea31dbc29..945d404851 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -328,7 +328,7 @@ if [ $KEEP_GOING -eq 1 ]; then start_red= end_color= if [ -t 1 ]; then - case "$TERM" in + case "${TERM:-}" in *color*|cygwin|linux|rxvt*|screen|[Eex]term*) start_red=$(printf '\033[31m') end_color=$(printf '\033[0m') From 5e9f14d4d988222aacb49b8eb15eedd266e9f147 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Sun, 28 May 2017 10:46:38 +0300 Subject: [PATCH 48/84] Set correct minimal versions in default conf Set `MBEDTLS_SSL_MIN_MAJOR_VERSION` and `MBEDTLS_SSL_MIN_MINOR_VERSION` instead of `MBEDTLS_SSL_MAJOR_VERSION_3` and `MBEDTLS_SSL_MINOR_VERSION_1` --- ChangeLog | 26 +++++++++++++++----------- include/mbedtls/ssl_internal.h | 3 +++ library/ssl_tls.c | 10 ++++++++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index a6fa6bbda7..6dab645dc3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -41,17 +41,6 @@ New deprecations * Direct manipulation of structure fields of RSA contexts is deprecated. Users are advised to use the extended RSA API instead. -API Changes - * Extend RSA interface by multiple functions allowing structure- - independent setup and export of RSA contexts. Most notably, - mbedtls_rsa_import and mbedtls_rsa_complete are introduced for setting - up RSA contexts from partial key material and having them completed to the - needs of the implementation automatically. This allows to setup private RSA - contexts from keys consisting of N,D,E only, even if P,Q are needed for the - purpose or CRT and/or blinding. - * The configuration option MBEDTLS_RSA_ALT can be used to define alternative - implementations of the RSA interface declared in rsa.h. - Bugfix * Fix ssl_parse_record_header() to silently discard invalid DTLS records as recommended in RFC 6347 Section 4.1.2.7. @@ -101,6 +90,10 @@ Bugfix RSA test suite where the failure of CTR DRBG initialization lead to freeing an RSA context and several MPI's without proper initialization beforehand. + * Fix setting version TLSv1 as minimal version, even if TLS 1 + is not enabled. Set `MBEDTLS_SSL_MIN_MAJOR_VERSION` + and `MBEDTLS_SSL_MIN_MINOR_VERSION` instead + of `MBEDTLS_SSL_MAJOR_VERSION_3` and `MBEDTLS_SSL_MINOR_VERSION_1` Changes * Extend cert_write example program by options to set the CRT version @@ -114,6 +107,17 @@ Changes * Tighten the RSA PKCS#1 v1.5 signature verification code and remove the undeclared dependency of the RSA module on the ASN.1 module. +API Changes + * Extend RSA interface by multiple functions allowing structure- + independent setup and export of RSA contexts. Most notably, + mbedtls_rsa_import and mbedtls_rsa_complete are introduced for setting + up RSA contexts from partial key material and having them completed to the + needs of the implementation automatically. This allows to setup private RSA + contexts from keys consisting of N,D,E only, even if P,Q are needed for the + purpose or CRT and/or blinding. + * The configuration option MBEDTLS_RSA_ALT can be used to define alternative + implementations of the RSA interface declared in rsa.h. + = mbed TLS 2.6.0 branch released 2017-08-10 Security diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 756360b181..56e376b881 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -69,6 +69,9 @@ #endif /* MBEDTLS_SSL_PROTO_TLS1 */ #endif /* MBEDTLS_SSL_PROTO_SSL3 */ +#define MBEDTLS_SSL_MIN_VALID_MINOR_VERSION MBEDTLS_SSL_MINOR_VERSION_1 +#define MBEDTLS_SSL_MIN_VALID_MAJOR_VERSION MBEDTLS_SSL_MAJOR_VERSION_3 + /* Determine maximum supported version */ #define MBEDTLS_SSL_MAX_MAJOR_VERSION MBEDTLS_SSL_MAJOR_VERSION_3 diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 2690e46730..bc98708f66 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7602,8 +7602,14 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, * Default */ default: - conf->min_major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; - conf->min_minor_ver = MBEDTLS_SSL_MINOR_VERSION_1; /* TLS 1.0 */ + conf->min_major_ver = ( MBEDTLS_SSL_MIN_MAJOR_VERSION > + MBEDTLS_SSL_MIN_VALID_MAJOR_VERSION ) ? + MBEDTLS_SSL_MIN_MAJOR_VERSION : + MBEDTLS_SSL_MIN_VALID_MAJOR_VERSION; + conf->min_minor_ver = ( MBEDTLS_SSL_MIN_MINOR_VERSION > + MBEDTLS_SSL_MIN_VALID_MINOR_VERSION ) ? + MBEDTLS_SSL_MIN_MINOR_VERSION : + MBEDTLS_SSL_MIN_VALID_MINOR_VERSION; conf->max_major_ver = MBEDTLS_SSL_MAX_MAJOR_VERSION; conf->max_minor_ver = MBEDTLS_SSL_MAX_MINOR_VERSION; From 9cf1f96a7b4d6dc5e9bb38cb41b407aa65cfeca2 Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 30 Jan 2017 14:34:25 +0000 Subject: [PATCH 49/84] Fix corner case uses of memory_buffer_alloc.c The corner cases fixed include: * Allocating a buffer of size 0. With this change, the allocator now returns a NULL pointer in this case. Note that changes in pem.c and x509_crl.c were required to fix tests that did not work under this assumption. * Initialising the allocator with less memory than required for headers. * Fix header chain checks for uninitialised allocator. --- ChangeLog | 2 ++ library/memory_buffer_alloc.c | 29 ++++++++++++++++++----------- library/pem.c | 4 ++-- library/x509_crl.c | 4 ++-- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index a200d51fb7..4aa66fd822 100644 --- a/ChangeLog +++ b/ChangeLog @@ -104,6 +104,8 @@ Bugfix * Fix error message in programs/pkey/gen_key.c. Found and fixed by Chris Xue. * Fix programs/pkey/dh_server.c so that it actually works with dh_client.c. Found and fixed by Martijn de Milliano. + * Fix memory allocation corner cases in memory_buffer_alloc.c module. Found + by Guido Vranken. #639 Changes * Extend cert_write example program by options to set the CRT version diff --git a/library/memory_buffer_alloc.c b/library/memory_buffer_alloc.c index 545d5a2c32..0d3342dea7 100644 --- a/library/memory_buffer_alloc.c +++ b/library/memory_buffer_alloc.c @@ -182,9 +182,9 @@ static int verify_header( memory_header *hdr ) static int verify_chain() { - memory_header *prv = heap.first, *cur = heap.first->next; + memory_header *prv = heap.first, *cur; - if( verify_header( heap.first ) != 0 ) + if( heap.first == NULL || verify_header( heap.first ) != 0 ) { #if defined(MBEDTLS_MEMORY_DEBUG) mbedtls_fprintf( stderr, "FATAL: verification of first header " @@ -202,6 +202,8 @@ static int verify_chain() return( 1 ); } + cur = heap.first->next; + while( cur != NULL ) { if( verify_header( cur ) != 0 ) @@ -245,7 +247,9 @@ static void *buffer_alloc_calloc( size_t n, size_t size ) original_len = len = n * size; - if( n != 0 && len / n != size ) + if( n == 0 || size == 0 || len / n != size ) + return( NULL ); + else if( len > (size_t)-MBEDTLS_MEMORY_ALIGN_MULTIPLE ) return( NULL ); if( len % MBEDTLS_MEMORY_ALIGN_MULTIPLE ) @@ -386,7 +390,7 @@ static void buffer_alloc_free( void *ptr ) if( ptr == NULL || heap.buf == NULL || heap.first == NULL ) return; - if( p < heap.buf || p > heap.buf + heap.len ) + if( p < heap.buf || p >= heap.buf + heap.len ) { #if defined(MBEDTLS_MEMORY_DEBUG) mbedtls_fprintf( stderr, "FATAL: mbedtls_free() outside of managed " @@ -570,8 +574,7 @@ static void buffer_alloc_free_mutexed( void *ptr ) void mbedtls_memory_buffer_alloc_init( unsigned char *buf, size_t len ) { - memset( &heap, 0, sizeof(buffer_alloc_ctx) ); - memset( buf, 0, len ); + memset( &heap, 0, sizeof( buffer_alloc_ctx ) ); #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_init( &heap.mutex ); @@ -581,20 +584,24 @@ void mbedtls_memory_buffer_alloc_init( unsigned char *buf, size_t len ) mbedtls_platform_set_calloc_free( buffer_alloc_calloc, buffer_alloc_free ); #endif - if( (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE ) + if( len < sizeof( memory_header ) + MBEDTLS_MEMORY_ALIGN_MULTIPLE ) + return; + else if( (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE ) { /* Adjust len first since buf is used in the computation */ len -= MBEDTLS_MEMORY_ALIGN_MULTIPLE - - (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE; + - (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE; buf += MBEDTLS_MEMORY_ALIGN_MULTIPLE - - (size_t) buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE; + - (size_t)buf % MBEDTLS_MEMORY_ALIGN_MULTIPLE; } + memset( buf, 0, len ); + heap.buf = buf; heap.len = len; - heap.first = (memory_header *) buf; - heap.first->size = len - sizeof(memory_header); + heap.first = (memory_header *)buf; + heap.first->size = len - sizeof( memory_header ); heap.first->magic1 = MAGIC1; heap.first->magic2 = MAGIC2; heap.first_free = heap.first; diff --git a/library/pem.c b/library/pem.c index 87401ba55f..d726bd61be 100644 --- a/library/pem.c +++ b/library/pem.c @@ -423,7 +423,7 @@ int mbedtls_pem_write_buffer( const char *header, const char *footer, unsigned char *buf, size_t buf_len, size_t *olen ) { int ret; - unsigned char *encode_buf, *c, *p = buf; + unsigned char *encode_buf = NULL, *c, *p = buf; size_t len = 0, use_len, add_len = 0; mbedtls_base64_encode( NULL, 0, &use_len, der_data, der_len ); @@ -435,7 +435,7 @@ int mbedtls_pem_write_buffer( const char *header, const char *footer, return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL ); } - if( ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL ) + if( use_len != 0 && ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL ) return( MBEDTLS_ERR_PEM_ALLOC_FAILED ); if( ( ret = mbedtls_base64_encode( encode_buf, use_len, &use_len, der_data, diff --git a/library/x509_crl.c b/library/x509_crl.c index 55d12acd03..c302bb2fb7 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -257,7 +257,7 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, { int ret; size_t len; - unsigned char *p, *end; + unsigned char *p = NULL, *end; mbedtls_x509_buf sig_params1, sig_params2, sig_oid2; mbedtls_x509_crl *crl = chain; @@ -294,7 +294,7 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, /* * Copy raw DER-encoded CRL */ - if( ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) + if( buflen != 0 && ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) return( MBEDTLS_ERR_X509_ALLOC_FAILED ); memcpy( p, buf, buflen ); From 8ec3bfe1800d7b18e24d9439b641e91c4dd408ed Mon Sep 17 00:00:00 2001 From: Andres AG Date: Mon, 30 Jan 2017 14:35:08 +0000 Subject: [PATCH 50/84] Test corner case uses of memory_buffer_alloc.c --- .../test_suite_memory_buffer_alloc.data | 5 ++++ .../test_suite_memory_buffer_alloc.function | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tests/suites/test_suite_memory_buffer_alloc.data b/tests/suites/test_suite_memory_buffer_alloc.data index 8d3813a7ba..d59f1135ae 100644 --- a/tests/suites/test_suite_memory_buffer_alloc.data +++ b/tests/suites/test_suite_memory_buffer_alloc.data @@ -16,3 +16,8 @@ memory_buffer_alloc_free_alloc:100:64:100:100:0:0:0:1:200:0 Memory buffer alloc - Out of Memory test memory_buffer_alloc_oom_test: +Memory buffer small buffer +memory_buffer_small_buffer: + +Memory buffer underalloc +memory_buffer_underalloc: diff --git a/tests/suites/test_suite_memory_buffer_alloc.function b/tests/suites/test_suite_memory_buffer_alloc.function index a0c70d8a20..09684c1d41 100644 --- a/tests/suites/test_suite_memory_buffer_alloc.function +++ b/tests/suites/test_suite_memory_buffer_alloc.function @@ -232,3 +232,31 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_MEMORY_DEBUG */ +void memory_buffer_small_buffer( ) +{ + unsigned char buf[1]; + + mbedtls_memory_buffer_alloc_init( buf, sizeof( buf ) ); + TEST_ASSERT( mbedtls_memory_buffer_alloc_verify() != 0 ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_MEMORY_DEBUG */ +void memory_buffer_underalloc( ) +{ + unsigned char buf[100]; + size_t i; + + mbedtls_memory_buffer_alloc_init( buf, sizeof( buf ) ); + for( i = 1; i < MBEDTLS_MEMORY_ALIGN_MULTIPLE; i++ ) + { + TEST_ASSERT( mbedtls_calloc( 1, + (size_t)-( MBEDTLS_MEMORY_ALIGN_MULTIPLE - i ) ) == NULL ); + TEST_ASSERT( mbedtls_memory_buffer_alloc_verify() == 0 ); + } + +exit: + mbedtls_memory_buffer_alloc_free(); +} +/* END_CASE */ From f1ee63562aa6bd42603adc61cda158b9fe109360 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Thu, 6 Jul 2017 10:06:58 +0100 Subject: [PATCH 51/84] Style fixes in pem, x509_crl and buf_alloc --- library/memory_buffer_alloc.c | 2 +- library/pem.c | 3 ++- library/x509_crl.c | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/library/memory_buffer_alloc.c b/library/memory_buffer_alloc.c index 0d3342dea7..1cfc27ca61 100644 --- a/library/memory_buffer_alloc.c +++ b/library/memory_buffer_alloc.c @@ -184,7 +184,7 @@ static int verify_chain() { memory_header *prv = heap.first, *cur; - if( heap.first == NULL || verify_header( heap.first ) != 0 ) + if( prv == NULL || verify_header( prv ) != 0 ) { #if defined(MBEDTLS_MEMORY_DEBUG) mbedtls_fprintf( stderr, "FATAL: verification of first header " diff --git a/library/pem.c b/library/pem.c index d726bd61be..7b3ae8d3df 100644 --- a/library/pem.c +++ b/library/pem.c @@ -435,7 +435,8 @@ int mbedtls_pem_write_buffer( const char *header, const char *footer, return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL ); } - if( use_len != 0 && ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL ) + if( use_len != 0 && + ( ( encode_buf = mbedtls_calloc( 1, use_len ) ) == NULL ) ) return( MBEDTLS_ERR_PEM_ALLOC_FAILED ); if( ( ret = mbedtls_base64_encode( encode_buf, use_len, &use_len, der_data, diff --git a/library/x509_crl.c b/library/x509_crl.c index c302bb2fb7..8f98d8c928 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -257,7 +257,7 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, { int ret; size_t len; - unsigned char *p = NULL, *end; + unsigned char *p = NULL, *end = NULL; mbedtls_x509_buf sig_params1, sig_params2, sig_oid2; mbedtls_x509_crl *crl = chain; @@ -294,7 +294,7 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, /* * Copy raw DER-encoded CRL */ - if( buflen != 0 && ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) + if( buflen != 0 && ( ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) ) return( MBEDTLS_ERR_X509_ALLOC_FAILED ); memcpy( p, buf, buflen ); From cb5123fa86982c75f2c8061b58ac51c9e9938fdb Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 6 Dec 2017 09:39:23 +0000 Subject: [PATCH 52/84] Ensure memcpy is not called with NULL and 0 args in x509 module --- library/x509_crl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/x509_crl.c b/library/x509_crl.c index 8f98d8c928..9422457b50 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -294,7 +294,9 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, /* * Copy raw DER-encoded CRL */ - if( buflen != 0 && ( ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) ) + if( buflen == 0 ) + return( MBEDTLS_ERR_X509_INVALID_FORMAT ); + else if( ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) return( MBEDTLS_ERR_X509_ALLOC_FAILED ); memcpy( p, buf, buflen ); From c9d6226d2c7b7a99be6694c6014e4e4be1cc69d7 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 12 Dec 2017 20:15:03 +0000 Subject: [PATCH 53/84] Change formatting of allocation check in x509_crl --- library/x509_crl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/x509_crl.c b/library/x509_crl.c index 9422457b50..0bb7236bd1 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -296,7 +296,9 @@ int mbedtls_x509_crl_parse_der( mbedtls_x509_crl *chain, */ if( buflen == 0 ) return( MBEDTLS_ERR_X509_INVALID_FORMAT ); - else if( ( p = mbedtls_calloc( 1, buflen ) ) == NULL ) + + p = mbedtls_calloc( 1, buflen ); + if( p == NULL ) return( MBEDTLS_ERR_X509_ALLOC_FAILED ); memcpy( p, buf, buflen ); From e9124b943da5c30899cc75294f390d46ea23c995 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 23 Jan 2018 20:03:52 +0000 Subject: [PATCH 54/84] Ensure that mbedtls_pk_parse_key() does not allocate 0 bytes --- library/pkparse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pkparse.c b/library/pkparse.c index f97d89ea14..491cecf50f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1274,6 +1274,9 @@ int mbedtls_pk_parse_key( mbedtls_pk_context *pk, { unsigned char *key_copy; + if( keylen == 0 ) + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); + if( ( key_copy = mbedtls_calloc( 1, keylen ) ) == NULL ) return( MBEDTLS_ERR_PK_ALLOC_FAILED ); From 8db3efbc76243971adcae0d5abe439bc3af931f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 21 Feb 2018 19:16:20 +0100 Subject: [PATCH 55/84] Add missing MBEDTLS_DEPRECATED_REMOVED guards Add missing MBEDTLS_DEPRECATED_REMOVED guards around the definitions of mbedtls_aes_decrypt and mbedtls_aes_encrypt. This fixes the build under -Wmissing-prototypes -Werror. Fixes #1388 --- ChangeLog | 2 ++ library/aes.c | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 5f49c0beb7..9a61ec31dd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,8 @@ Bugfix * Fix the name of a DHE parameter that was accidentally changed in 2.7.0. Fixes #1358. * Fix test_suite_pk to work on 64-bit ILP32 systems. #849 + * Don't define mbedtls_aes_decrypt and mbedtls_aes_encrypt under + MBEDTLS_DEPRECATED_REMOVED. #1388 Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. diff --git a/library/aes.c b/library/aes.c index dba4a5f578..3d2eac82dd 100644 --- a/library/aes.c +++ b/library/aes.c @@ -765,12 +765,14 @@ int mbedtls_internal_aes_encrypt( mbedtls_aes_context *ctx, } #endif /* !MBEDTLS_AES_ENCRYPT_ALT */ +#if !defined(MBEDTLS_DEPRECATED_REMOVED) void mbedtls_aes_encrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { mbedtls_internal_aes_encrypt( ctx, input, output ); } +#endif /* !MBEDTLS_DEPRECATED_REMOVED */ /* * AES-ECB block decryption @@ -831,12 +833,14 @@ int mbedtls_internal_aes_decrypt( mbedtls_aes_context *ctx, } #endif /* !MBEDTLS_AES_DECRYPT_ALT */ +#if !defined(MBEDTLS_DEPRECATED_REMOVED) void mbedtls_aes_decrypt( mbedtls_aes_context *ctx, const unsigned char input[16], unsigned char output[16] ) { mbedtls_internal_aes_decrypt( ctx, input, output ); } +#endif /* !MBEDTLS_DEPRECATED_REMOVED */ /* * AES-ECB block encryption/decryption From 693a1d9ca703c902058ad00f6ecedf39f329c855 Mon Sep 17 00:00:00 2001 From: itayzafrir Date: Mon, 26 Feb 2018 12:02:10 +0200 Subject: [PATCH 56/84] Test suite test_suite_pk test pk_rsa_overflow passes valid parameters for hash and sig. Test suite test_suite_pk test pk_rsa_overflow passes valid parameters for hash and sig. --- ChangeLog | 2 ++ tests/suites/test_suite_pk.function | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 13203a5cf8..716567b04d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -34,6 +34,8 @@ Bugfix with flag MBEDTLS_X509_BADCERT_BAD_PK even when the key type was correct. In the context of SSL, this resulted in handshake failure. #1351 * Fix Windows x64 builds with the included mbedTLS.sln file. #1347 + * In test_suite_pk pass valid parameters when testing for hash length + overflow. #1179 Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 2180f5c8e8..421227f5e8 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -417,11 +417,15 @@ exit: void pk_rsa_overflow( ) { mbedtls_pk_context pk; - size_t hash_len = SIZE_MAX; + size_t hash_len = SIZE_MAX, sig_len = SIZE_MAX; + unsigned char hash[50], sig[100]; if( SIZE_MAX <= UINT_MAX ) return; + memset( hash, 0x2a, sizeof hash ); + memset( sig, 0, sizeof sig ); + mbedtls_pk_init( &pk ); TEST_ASSERT( mbedtls_pk_setup( &pk, @@ -429,14 +433,14 @@ void pk_rsa_overflow( ) #if defined(MBEDTLS_PKCS1_V21) TEST_ASSERT( mbedtls_pk_verify_ext( MBEDTLS_PK_RSASSA_PSS, NULL, &pk, - MBEDTLS_MD_NONE, NULL, hash_len, NULL, 0 ) == + MBEDTLS_MD_NONE, hash, hash_len, sig, sig_len ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); #endif /* MBEDTLS_PKCS1_V21 */ - TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_NONE, NULL, hash_len, - NULL, 0 ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); + TEST_ASSERT( mbedtls_pk_verify( &pk, MBEDTLS_MD_NONE, hash, hash_len, + sig, sig_len ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_NONE, NULL, hash_len, NULL, 0, + TEST_ASSERT( mbedtls_pk_sign( &pk, MBEDTLS_MD_NONE, hash, hash_len, sig, &sig_len, rnd_std_rand, NULL ) == MBEDTLS_ERR_PK_BAD_INPUT_DATA ); exit: From cf092b2ccf6fe88ec7b6e075aa89d93cadaa059a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 6 Mar 2018 14:23:38 +0000 Subject: [PATCH 57/84] Deprecate support for record compression --- ChangeLog | 4 ++++ include/mbedtls/check_config.h | 8 ++++++++ include/mbedtls/config.h | 3 +++ 3 files changed, 15 insertions(+) diff --git a/ChangeLog b/ChangeLog index 68fb6f5e96..75a8f1186a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -26,6 +26,10 @@ Features OpenVPN Inc. Fixes #1339 * Add support for public keys encoded in PKCS#1 format. #1122 +New deprecations + * Deprecate support for record compression (configuration option + MBEDTLS_ZLIB_SUPPORT). + Bugfix * Fix the name of a DHE parameter that was accidentally changed in 2.7.0. Fixes #1358. diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index be80332963..655612e201 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -66,6 +66,14 @@ #error "MBEDTLS_HAVE_TIME_DATE without MBEDTLS_HAVE_TIME does not make sense" #endif +#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_WARNING) +#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will likely be removed in a future version of the library" +#endif + +#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_REMOVED) +#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" +#endif + #if defined(MBEDTLS_AESNI_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_AESNI_C defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 1c98558ebc..05f67fa3c0 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1541,6 +1541,9 @@ * * \note Currently compression can't be used with DTLS. * + * \deprecated This feature is deprecated and will likely be removed + * in a future version of the library. + * * Used in: library/ssl_tls.c * library/ssl_cli.c * library/ssl_srv.c From b2b29d52592b1c632f0b4b79f7c11c74f0798459 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Mon, 21 Aug 2017 15:58:12 +0100 Subject: [PATCH 58/84] Add end-of-buffer check to prevent heap-buffer-overflow Dereference of *p should not happen when it points past the end of the buffer. Internal reference: IOTSSL-1663 --- library/pkparse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pkparse.c b/library/pkparse.c index b4def4f914..89a0c5dbfd 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -181,6 +181,9 @@ static int pk_get_ecparams( unsigned char **p, const unsigned char *end, { int ret; + if ( end - *p < 1 ) + return MBEDTLS_ERR_ASN1_OUT_OF_DATA; + /* Tag may be either OID or SEQUENCE */ params->tag = **p; if( params->tag != MBEDTLS_ASN1_OID From 7b2e85dd7ccb4c253df76ac0517841874bf72e17 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Wed, 30 Aug 2017 21:10:42 +0100 Subject: [PATCH 59/84] Use both applicable error codes and a proper coding style --- library/pkparse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/pkparse.c b/library/pkparse.c index 89a0c5dbfd..6e22ce4f7d 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -182,7 +182,8 @@ static int pk_get_ecparams( unsigned char **p, const unsigned char *end, int ret; if ( end - *p < 1 ) - return MBEDTLS_ERR_ASN1_OUT_OF_DATA; + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + + MBEDTLS_ERR_ASN1_OUT_OF_DATA ); /* Tag may be either OID or SEQUENCE */ params->tag = **p; From 90da97d587b1eef67a6742605c891a086f7fb710 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Thu, 31 Aug 2017 12:57:35 +0100 Subject: [PATCH 60/84] Add test case found through fuzzing to pkparse test suite --- tests/suites/test_suite_pkparse.data | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 416f9dfe44..e420fb04e2 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -486,3 +486,6 @@ pk_parse_key_rsa:"301c02010002010102010102010102010102010102010102010102010100": Key ASN1 (RSAPrivateKey, values present, check_privkey fails) pk_parse_key_rsa:"301b020100020102020101020101020101020101020101020101020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + +Key ASN1 (heap-buffer-overflow, unchecked access of tag) +pk_parse_key_rsa:"30070201010400a000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT From 52895b2b2e2328d0cebdba102ea2f57136ec7175 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Tue, 5 Sep 2017 17:00:54 +0100 Subject: [PATCH 61/84] Add Changelog entry --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 8db0215914..f835e4aa71 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Fix a heap-buffer-overflow during private key parsing. Found through + fuzzing. + = mbed TLS 2.7.0 branch released 2018-02-03 Security From cf79312a6d3688637b6e2dbaf35b7c319fa8e02c Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Thu, 7 Sep 2017 16:33:44 +0100 Subject: [PATCH 62/84] Update changelog entry --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index f835e4aa71..64361bed9b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,8 +3,8 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Bugfix - * Fix a heap-buffer-overflow during private key parsing. Found through - fuzzing. + * Fix a 1-byte heap buffer overflow (read-only) during private key parsing. + Found through fuzz testing. = mbed TLS 2.7.0 branch released 2018-02-03 From bb50113123df6b4d53f28cee14df23ff04a4a710 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Thu, 7 Sep 2017 16:44:06 +0100 Subject: [PATCH 63/84] Rename test and update dependencies --- tests/suites/test_suite_pkparse.data | 25 +++++++++++++++--------- tests/suites/test_suite_pkparse.function | 4 ++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index e420fb04e2..932d8907b6 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -467,25 +467,32 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256K1_ENABLED:MB pk_parse_keyfile_ec:"data_files/ec_prv.specdom.der":"NULL":0 Key ASN1 (Incorrect first tag) -pk_parse_key_rsa:"":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +pk_parse_key:"":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, incorrect version tag) -pk_parse_key_rsa:"300100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"300100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, version tag missing) -pk_parse_key_rsa:"3000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"3000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, invalid version) -pk_parse_key_rsa:"3003020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"3003020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, correct version, incorrect tag) -pk_parse_key_rsa:"300402010000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"300402010000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, values present, length mismatch) -pk_parse_key_rsa:"301c02010002010102010102010102010102010102010102010102010100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"301c02010002010102010102010102010102010102010102010102010100":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT Key ASN1 (RSAPrivateKey, values present, check_privkey fails) -pk_parse_key_rsa:"301b020100020102020101020101020101020101020101020101020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +depends_on:MBEDTLS_RSA_C +pk_parse_key:"301b020100020102020101020101020101020101020101020101020101":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT -Key ASN1 (heap-buffer-overflow, unchecked access of tag) -pk_parse_key_rsa:"30070201010400a000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +Key ASN1 (ECPrivateKey, empty parameters) +depends_on:MBEDTLS_ECP_C +pk_parse_key:"30070201010400a000":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 4f1a616061..59f7877fc6 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -113,8 +113,8 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_RSA_C */ -void pk_parse_key_rsa( char *key_data, char *result_str, int result ) +/* BEGIN_CASE depends_on:MBEDTLS_PK_PARSE_C */ +void pk_parse_key( char *key_data, char *result_str, int result ) { mbedtls_pk_context pk; unsigned char buf[2000]; From 22797fcc57a59321d2cc18d37bac43c44474edb2 Mon Sep 17 00:00:00 2001 From: Sanne Wouda Date: Fri, 8 Sep 2017 10:58:37 +0100 Subject: [PATCH 64/84] Remove redundant dependency --- tests/suites/test_suite_pkparse.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index 59f7877fc6..94d25e7eb0 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -113,7 +113,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_PK_PARSE_C */ +/* BEGIN_CASE */ void pk_parse_key( char *key_data, char *result_str, int result ) { mbedtls_pk_context pk; From e494e20f0c39499badb1a52eaafea23d2f7b02db Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Mar 2018 13:26:12 +0000 Subject: [PATCH 65/84] Move and reword deprecation warning/error on compression support --- include/mbedtls/check_config.h | 8 -------- include/mbedtls/config.h | 4 ++-- include/mbedtls/ssl.h | 9 +++++++++ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 655612e201..be80332963 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -66,14 +66,6 @@ #error "MBEDTLS_HAVE_TIME_DATE without MBEDTLS_HAVE_TIME does not make sense" #endif -#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_WARNING) -#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will likely be removed in a future version of the library" -#endif - -#if defined(MBEDTLS_ZLIB_SUPPORT) && defined(MBEDTLS_DEPRECATED_REMOVED) -#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" -#endif - #if defined(MBEDTLS_AESNI_C) && !defined(MBEDTLS_HAVE_ASM) #error "MBEDTLS_AESNI_C defined, but not all prerequisites" #endif diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 05f67fa3c0..d47e9c7aff 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1541,8 +1541,8 @@ * * \note Currently compression can't be used with DTLS. * - * \deprecated This feature is deprecated and will likely be removed - * in a future version of the library. + * \deprecated This feature is deprecated and will be removed + * in the next major revision of the library. * * Used in: library/ssl_tls.c * library/ssl_cli.c diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 51e843ae24..a679717225 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -49,6 +49,15 @@ #endif #if defined(MBEDTLS_ZLIB_SUPPORT) + +#if defined(MBEDTLS_DEPRECATED_WARNING) +#warning "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and will be removed in the next major revision of the library" +#endif + +#if defined(MBEDTLS_DEPRECATED_REMOVED) +#error "Record compression support via MBEDTLS_ZLIB_SUPPORT is deprecated and cannot be used if MBEDTLS_DEPRECATED_REMOVED is set" +#endif + #include "zlib.h" #endif From 6f486a6fb5c7311a8d07913778b53f128ec37cd8 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 8 Mar 2018 13:31:44 +0000 Subject: [PATCH 66/84] Fix merge error --- tests/scripts/all.sh | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 140a90f096..e60530fd78 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -881,8 +881,5 @@ rm -rf "$OUT_OF_SOURCE_DIR" msg "Done, cleaning up" cleanup -<<<<<<< HEAD -======= final_report ->>>>>>> development-restricted From 1ed45ea36b345327d4f6af0344fc0518b22872fb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Mar 2018 18:16:45 +0100 Subject: [PATCH 67/84] Refer to X.690 by number It's easier to identify and find by number than by its very wordy title, especially as there was a typo in the title. --- include/mbedtls/asn1.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 75b7b3dfbc..86b50e6c84 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -90,9 +90,8 @@ /* * Bit masks for each of the components of an ASN.1 tag as specified in - * Information technnology - ASN.1 encoding rules: Specification of Basic - * Encoding Rules (BER), Canonical Encoding Rules (CER), and Distinguished - * encoding rules (DER) Section 8.1.2.2: + * ITU X.690 (08/2015), section 8.1 "General rules for encoding", + * paragraph 8.1.2.2: * * Bit 8 7 6 5 1 * +-------+-----+------------+ From 9c4f4038ddbf0b3999649385846a0b66623b6cbc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 29 May 2017 14:46:36 +0200 Subject: [PATCH 68/84] Add changelog entry --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 13de8672c7..b729d6c7c8 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.x.x branch released 2017-xx-xx + +Changes + * Clarify the documentation of mbedtls_ssl_setup. + = mbed TLS 2.4.2 branch released 2017-03-08 Security From 08af538ec90af91d530d34ddc129d386f68ebe8e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 11 Mar 2018 00:15:56 +0100 Subject: [PATCH 69/84] Fix grammar in ChangeLog entry --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 716567b04d..13adfb590d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -34,7 +34,7 @@ Bugfix with flag MBEDTLS_X509_BADCERT_BAD_PK even when the key type was correct. In the context of SSL, this resulted in handshake failure. #1351 * Fix Windows x64 builds with the included mbedTLS.sln file. #1347 - * In test_suite_pk pass valid parameters when testing for hash length + * In test_suite_pk, pass valid parameters when testing for hash length overflow. #1179 Changes From 3f1b89d251bd654c77cd61ddf3aac64ebee9be21 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 11 Mar 2018 00:35:39 +0100 Subject: [PATCH 70/84] This fixes #664 --- ChangeLog | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6dab645dc3..40aa075b6a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -91,9 +91,9 @@ Bugfix freeing an RSA context and several MPI's without proper initialization beforehand. * Fix setting version TLSv1 as minimal version, even if TLS 1 - is not enabled. Set `MBEDTLS_SSL_MIN_MAJOR_VERSION` - and `MBEDTLS_SSL_MIN_MINOR_VERSION` instead - of `MBEDTLS_SSL_MAJOR_VERSION_3` and `MBEDTLS_SSL_MINOR_VERSION_1` + is not enabled. Set MBEDTLS_SSL_MIN_MAJOR_VERSION + and MBEDTLS_SSL_MIN_MINOR_VERSION instead of + MBEDTLS_SSL_MAJOR_VERSION_3 and MBEDTLS_SSL_MINOR_VERSION_1. #664 Changes * Extend cert_write example program by options to set the CRT version From b21a085baeaec8c3c3288b98096c24b279231b01 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Mar 2018 14:24:36 +0100 Subject: [PATCH 71/84] Show build modes in code font This clarifies that it's the string to type and not just some description of it. --- README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 2c6cc62a05..a2c3c6f21e 100644 --- a/README.md +++ b/README.md @@ -110,14 +110,14 @@ To configure CMake for building shared libraries, use: There are many different build modes available within the CMake buildsystem. Most of them are available for gcc and clang, though some are compiler-specific: -- Release. This generates the default code without any unnecessary information in the binary files. -- Debug. This generates debug information and disables optimization of the code. -- Coverage. This generates code coverage information in addition to debug information. -- ASan. This instruments the code with AddressSanitizer to check for memory errors. (This includes LeakSanitizer, with recent version of gcc and clang.) (With recent version of clang, this mode also instruments the code with UndefinedSanitizer to check for undefined behaviour.) -- ASanDbg. Same as ASan but slower, with debug information and better stack traces. -- MemSan. This instruments the code with MemorySanitizer to check for uninitialised memory reads. Experimental, needs recent clang on Linux/x86\_64. -- MemSanDbg. Same as MemSan but slower, with debug information, better stack traces and origin tracking. -- Check. This activates the compiler warnings that depend on optimization and treats all warnings as errors. +- `Release`. This generates the default code without any unnecessary information in the binary files. +- `Debug`. This generates debug information and disables optimization of the code. +- `Coverage`. This generates code coverage information in addition to debug information. +- `ASan`. This instruments the code with AddressSanitizer to check for memory errors. (This includes LeakSanitizer, with recent version of gcc and clang.) (With recent version of clang, this mode also instruments the code with UndefinedSanitizer to check for undefined behaviour.) +- `ASanDbg`. Same as ASan but slower, with debug information and better stack traces. +- `MemSan`. This instruments the code with MemorySanitizer to check for uninitialised memory reads. Experimental, needs recent clang on Linux/x86\_64. +- `MemSanDbg`. Same as MemSan but slower, with debug information, better stack traces and origin tracking. +- `Check`. This activates the compiler warnings that depend on optimization and treats all warnings as errors. Switching build modes in CMake is simple. For debug mode, enter at the command line: From 3ff4a074af5f188c4d7c8ab1ae57bd1d50dd11d6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 12 Mar 2018 23:54:20 +0100 Subject: [PATCH 72/84] Fix ChangeLog style. Fix #918 --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 000084b775..edf3eb39af 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,9 +1,9 @@ -mbed TLS ChangeLog (Sorted per branch, date) +mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Bugfix - * Log correct number of ciphersuites used in Client Hello message. Fix for #918. + * Log correct number of ciphersuites used in Client Hello message. #918 = mbed TLS 2.6.0 branch released 2017-08-10 From 6dc4a319884d03d967bb00eac3b0d81e7a3d66e1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 13 Mar 2018 00:13:06 +0100 Subject: [PATCH 73/84] Add ChangeLog entry. Fixes #678 --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index f96786d72a..984ab030e6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Changes + * Use (void) when defining functions with no parameters. Contributed by + Joris Aerts. #678 + = mbed TLS 2.4.0 branch released 2016-10-17 Security From a1098f81c252b317ad34ea978aea2bc47760b215 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:28:49 +0100 Subject: [PATCH 74/84] Add bounds check before signature length read --- library/ssl_cli.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 2534346a49..279a127ba2 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2478,6 +2478,14 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) /* * Read signature */ + + if( p > end - 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } sig_len = ( p[0] << 8 ) | p[1]; p += 2; From 027f84c69f4ef30c0693832a6c396ef19e563ca1 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:29:24 +0100 Subject: [PATCH 75/84] Prevent arithmetic overflow on bounds check --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 279a127ba2..df6abc389e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2489,7 +2489,7 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) sig_len = ( p[0] << 8 ) | p[1]; p += 2; - if( end != p + sig_len ) + if( p != end - sig_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, From 740b218386083dc708ce98ccc94a63a95cd5629e Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:31:14 +0100 Subject: [PATCH 76/84] Add bounds check before length read --- library/ssl_cli.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 2534346a49..585750ef2e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2057,6 +2057,12 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, * * opaque psk_identity_hint<0..2^16-1>; */ + if( (*p) > end - 2 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " + "(psk_identity_hint length)" ) ); + return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); + } len = (*p)[0] << 8 | (*p)[1]; *p += 2; From 5224a7544c95552553e2e6be0b4a789956a6464e Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 11:31:38 +0100 Subject: [PATCH 77/84] Prevent arithmetic overflow on bounds check --- library/ssl_cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 585750ef2e..759a4562a1 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -2066,7 +2066,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, len = (*p)[0] << 8 | (*p)[1]; *p += 2; - if( (*p) + len > end ) + if( (*p) > end - len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " "(psk_identity_hint length)" ) ); From fd3e4fbae75049810379e0845580402502629d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Mar 2018 11:53:30 +0100 Subject: [PATCH 78/84] x509: CRL: reject unsupported critical extensions --- ChangeLog | 2 + library/x509_crl.c | 57 ++++++++++++++++++++++++-- tests/data_files/Makefile | 3 ++ tests/data_files/crl-idp.pem | 12 ++++++ tests/data_files/test-ca.opensslconf | 9 ++++ tests/suites/test_suite_x509parse.data | 4 ++ 6 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 tests/data_files/crl-idp.pem diff --git a/ChangeLog b/ChangeLog index cfe27f3eb9..0b3dacd1fd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Security implementation allowed an offline 2^80 brute force attack on the HMAC key of a single, uninterrupted connection (with no resumption of the session). + * Fix CRL parsing to reject CRLs containing unsupported critical + extensions. Found by Falko Strenzke and Evangelos Karatsiolis. Features * Extend PKCS#8 interface by introducing support for the entire SHA diff --git a/library/x509_crl.c b/library/x509_crl.c index 0bb7236bd1..b0f39d428b 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -95,17 +95,23 @@ static int x509_crl_get_version( unsigned char **p, } /* - * X.509 CRL v2 extensions (no extensions parsed yet.) + * X.509 CRL v2 extensions + * + * We currently don't parse any extension's content, but we do check that the + * list of extensions is well-formed and abort on critical extensions (that + * are unsupported as we don't support any extension so far) */ static int x509_get_crl_ext( unsigned char **p, const unsigned char *end, mbedtls_x509_buf *ext ) { int ret; - size_t len = 0; - /* Get explicit tag */ - if( ( ret = mbedtls_x509_get_ext( p, end, ext, 0) ) != 0 ) + /* + * crlExtensions [0] EXPLICIT Extensions OPTIONAL + * -- if present, version MUST be v2 + */ + if( ( ret = mbedtls_x509_get_ext( p, end, ext, 0 ) ) != 0 ) { if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) return( 0 ); @@ -115,11 +121,54 @@ static int x509_get_crl_ext( unsigned char **p, while( *p < end ) { + /* + * Extension ::= SEQUENCE { + * extnID OBJECT IDENTIFIER, + * critical BOOLEAN DEFAULT FALSE, + * extnValue OCTET STRING } + */ + int is_critical = 0; + const unsigned char *end_ext_data; + size_t len; + + /* Get enclosing sequence tag */ if( ( ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + end_ext_data = *p + len; + + /* Get OID (currently ignored) */ + if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &len, + MBEDTLS_ASN1_OID ) ) != 0 ) + { + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + } *p += len; + + /* Get optional critical */ + if( ( ret = mbedtls_asn1_get_bool( p, end_ext_data, + &is_critical ) ) != 0 && + ( ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) ) + { + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + } + + /* Data should be octet string type */ + if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &len, + MBEDTLS_ASN1_OCTET_STRING ) ) != 0 ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + + /* Ignore data so far and just check its length */ + *p += len; + if( *p != end_ext_data ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH ); + + /* Abort on (unsupported) critical extensions */ + if( is_critical ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); } if( *p != end ) diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 0380633df1..46d134f951 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -46,6 +46,9 @@ test-ca-sha256.crt: $(test_ca_key_file_rsa) $(test_ca_config_file) test-ca.csr $(OPENSSL) req -x509 -config $(test_ca_config_file) -key $(test_ca_key_file_rsa) -passin "pass:$(test_ca_pwd_rsa)" -set_serial 0 -days 3653 -sha256 -in test-ca.csr -out $@ all_final += test-ca-sha256.crt +crl-idp.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file) + $(OPENSSL) ca -gencrl -batch -cert $(test_ca_crt) -keyfile $(test_ca_key_file_rsa) -key $(test_ca_pwd_rsa) -config $(test_ca_config_file) -name test_ca -md sha256 -crldays 3653 -crlexts crl_ext_idp -out $@ + cli_crt_key_file_rsa = cli-rsa.key cli_crt_extensions_file = cli.opensslconf diff --git a/tests/data_files/crl-idp.pem b/tests/data_files/crl-idp.pem new file mode 100644 index 0000000000..a229e7d6d9 --- /dev/null +++ b/tests/data_files/crl-idp.pem @@ -0,0 +1,12 @@ +-----BEGIN X509 CRL----- +MIIBszCBnAIBATANBgkqhkiG9w0BAQsFADA7MQswCQYDVQQGEwJOTDERMA8GA1UE +ChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EXDTE4MDMxNDA3 +MzE0OFoXDTI4MDMxNDA3MzE0OFqgLTArMCkGA1UdHAEB/wQfMB2gG6AZhhdodHRw +Oi8vcGtpLmV4YW1wbGUuY29tLzANBgkqhkiG9w0BAQsFAAOCAQEAs/vp1Ybq9Lj/ +YM+O2uBqhRNdt494GYSYcZcltbQDwLgDwsFQ9S+q5zBtanhxiF3C6dyDoWS6xyY3 +dkdO9kK2YAQLNaFBCsKRrI9vGKuF5/1uIr0a8cQcqVzyRI9uK0KgGEk9/APGtqob +nj/nt2ryGC+yEh20FmvwFn1vN5xaWK3uUIJCNDTZe+KQn150iAU/mWZG2xDdSXgm +JtpTrY6toBgTwDGyus2wIDvAF6rBc1lRoR0BPuTR1fcUPMvr8jceZqG+xuH+vmkU +j1B4Tu+K27ZmZMlhltfgwLzcgH9Ee1TgWPN2QqMzeZW/vNMyIIvWAWk2cFyCJj6r +16/9upL64w== +-----END X509 CRL----- diff --git a/tests/data_files/test-ca.opensslconf b/tests/data_files/test-ca.opensslconf index 12835dfa54..6ca6613303 100644 --- a/tests/data_files/test-ca.opensslconf +++ b/tests/data_files/test-ca.opensslconf @@ -11,3 +11,12 @@ commonName = PolarSSL Test CA subjectKeyIdentifier=hash authorityKeyIdentifier=keyid:always,issuer:always basicConstraints = CA:true + +[test_ca] +database = /dev/null + +[crl_ext_idp] +issuingDistributionPoint=critical, @idpdata + +[idpdata] +fullname=URI:http://pki.example.com/ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 73ccead253..71049c751a 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -202,6 +202,10 @@ X509 CRL Malformed Input (trailing spaces at end of file) depends_on:MBEDTLS_PEM_PARSE_C mbedtls_x509_crl_parse:"data_files/crl-malformed-trailing-spaces.pem":MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT +X509 CRL Unsupported critical extension (issuingDistributionPoint) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +mbedtls_x509_crl_parse:"data_files/crl-idp.pem":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + X509 CSR Information RSA with MD4 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_MD4_C mbedtls_x509_csr_info:"data_files/server1.req.md4":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL, CN=PolarSSL Server 1\nsigned using \: RSA with MD4\nRSA key size \: 2048 bits\n" From 00bbf572afc5558026a65ccb1000023bd1ce872d Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Wed, 14 Mar 2018 11:14:13 +0100 Subject: [PATCH 79/84] Update change log --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index dfd34bf69a..6e497bc1da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Security implementation allowed an offline 2^80 brute force attack on the HMAC key of a single, uninterrupted connection (with no resumption of the session). + * Fix a buffer overread in ssl_parse_server_key_exchange() that could cause + a crash on invalid input. Features * Extend PKCS#8 interface by introducing support for the entire SHA @@ -44,6 +46,8 @@ Bugfix Nick Wilson on issue #355 * In test_suite_pk, pass valid parameters when testing for hash length overflow. #1179 + * Fix a possible arithmetic overflow in ssl_parse_server_key_exchange() + that could cause a key exchange to fail on valid data. Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. From 7fa1ae70c85e847fcd5e434b1417c8dc4cc62c72 Mon Sep 17 00:00:00 2001 From: Krzysztof Stachowiak Date: Tue, 13 Mar 2018 17:17:38 +0100 Subject: [PATCH 80/84] Add Changelog entry --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index dfd34bf69a..585c81a1e6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Security implementation allowed an offline 2^80 brute force attack on the HMAC key of a single, uninterrupted connection (with no resumption of the session). + * Fix a buffer overread in ssl_parse_server_psk_hint() that could cause a + crash on invalid input. Features * Extend PKCS#8 interface by introducing support for the entire SHA @@ -44,6 +46,8 @@ Bugfix Nick Wilson on issue #355 * In test_suite_pk, pass valid parameters when testing for hash length overflow. #1179 + * Fix a possible arithmetic overflow in ssl_parse_server_psk_hint() that + could cause a key exchange to fail on valid data. Changes * Fix tag lengths and value ranges in the documentation of CCM encryption. From 0bdb050b2deffce65f728a7622a388c00a474e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Mar 2018 11:34:29 +0100 Subject: [PATCH 81/84] x509: CRL: add tests for malformed extensions This covers all lines added in the previous commit. Coverage was tested using: make CFLAGS='--coverage -g3 -O0' (cd tests && ./test_suite_x509parse) make lcov firefox Coverage/index.html # then visual check Test data was generated by taking a copy of tests/data_files/crl-idp.pem, encoding it as hex, and then manually changing the values of some bytes to achieve the desired errors, using https://lapo.it/asn1js/ for help in locating the desired bytes. --- tests/suites/test_suite_x509parse.data | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 71049c751a..755c91dd85 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1218,6 +1218,21 @@ x509parse_crl:"30463031020102300d06092a864886f70d01010e0500300f310d300b060355040 X509 CRL ASN1 (invalid version overflow) x509parse_crl:"3049303102047FFFFFFF300d06092a864886f70d01010e0500300f310d300b0603550403130441424344170c303930313031303030303030300d06092a864886f70d01010e050003020001":"":MBEDTLS_ERR_X509_UNKNOWN_VERSION +X509 CRL ASN1 (extension seq too long, crl-idp.pem byte 121) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30300603551d1c0101ff041f301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_OUT_OF_DATA + +X509 CRL ASN1 (extension oid too long, crl-idp.pem byte 123) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290628551d1c0101ff041f301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_OUT_OF_DATA + +X509 CRL ASN1 (extension critical invalid length, crl-idp.pem byte 128) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290603551d1c0102ff041f301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_INVALID_LENGTH + +X509 CRL ASN1 (extension data too long, crl-idp.pem byte 131) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290603551d1c0101ff0420301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_OUT_OF_DATA + +X509 CRL ASN1 (extension data too short, crl-idp.pem byte 131) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290603551d1c0101ff041e301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH + X509 CRT parse path #2 (one cert) depends_on:MBEDTLS_SHA1_C:MBEDTLS_RSA_C mbedtls_x509_crt_parse_path:"data_files/dir1":0:1 From a63305d134a2223477b4bfc46774518467b687d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Mar 2018 12:23:56 +0100 Subject: [PATCH 82/84] x509: CRL: add tests for non-critical extension The 'critical' boolean can be set to false in two ways: - by leaving it implicit (test data generated by openssl) - by explicitly setting it to false (generated by hand) --- tests/data_files/Makefile | 4 ++++ tests/data_files/test-ca.opensslconf | 3 +++ tests/suites/test_suite_x509parse.data | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index 46d134f951..59516bab86 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -48,6 +48,10 @@ all_final += test-ca-sha256.crt crl-idp.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file) $(OPENSSL) ca -gencrl -batch -cert $(test_ca_crt) -keyfile $(test_ca_key_file_rsa) -key $(test_ca_pwd_rsa) -config $(test_ca_config_file) -name test_ca -md sha256 -crldays 3653 -crlexts crl_ext_idp -out $@ +all_final += crl-idp.pem +crl-idpnc.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file) + $(OPENSSL) ca -gencrl -batch -cert $(test_ca_crt) -keyfile $(test_ca_key_file_rsa) -key $(test_ca_pwd_rsa) -config $(test_ca_config_file) -name test_ca -md sha256 -crldays 3653 -crlexts crl_ext_idp_nc -out $@ +all_final += crl-idpnc.pem cli_crt_key_file_rsa = cli-rsa.key cli_crt_extensions_file = cli.opensslconf diff --git a/tests/data_files/test-ca.opensslconf b/tests/data_files/test-ca.opensslconf index 6ca6613303..571d96ee4e 100644 --- a/tests/data_files/test-ca.opensslconf +++ b/tests/data_files/test-ca.opensslconf @@ -18,5 +18,8 @@ database = /dev/null [crl_ext_idp] issuingDistributionPoint=critical, @idpdata +[crl_ext_idp_nc] +issuingDistributionPoint=@idpdata + [idpdata] fullname=URI:http://pki.example.com/ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 755c91dd85..57d2448b65 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -206,6 +206,10 @@ X509 CRL Unsupported critical extension (issuingDistributionPoint) depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA256_C mbedtls_x509_crl_parse:"data_files/crl-idp.pem":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +X509 CRL Unsupported non-critical extension (issuingDistributionPoint) +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +mbedtls_x509_crl_parse:"data_files/crl-idpnc.pem":0 + X509 CSR Information RSA with MD4 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_MD4_C mbedtls_x509_csr_info:"data_files/server1.req.md4":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL, CN=PolarSSL Server 1\nsigned using \: RSA with MD4\nRSA key size \: 2048 bits\n" @@ -1233,6 +1237,9 @@ x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060 X509 CRL ASN1 (extension data too short, crl-idp.pem byte 131) x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290603551d1c0101ff041e301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH +X509 CRL ASN1 (extension not critical explicit, crl-idp.pem byte 129) +x509parse_crl:"308201b330819c020101300d06092a864886f70d01010b0500303b310b3009060355040613024e4c3111300f060355040a1308506f6c617253534c3119301706035504031310506f6c617253534c2054657374204341170d3138303331343037333134385a170d3238303331343037333134385aa02d302b30290603551d1c010100041f301da01ba0198617687474703a2f2f706b692e6578616d706c652e636f6d2f300d06092a864886f70d01010b05000382010100b3fbe9d586eaf4b8ff60cf8edae06a85135db78f78198498719725b5b403c0b803c2c150f52faae7306d6a7871885dc2e9dc83a164bac7263776474ef642b660040b35a1410ac291ac8f6f18ab85e7fd6e22bd1af1c41ca95cf2448f6e2b42a018493dfc03c6b6aa1b9e3fe7b76af2182fb2121db4166bf0167d6f379c5a58adee5082423434d97be2909f5e7488053f996646db10dd49782626da53ad8eada01813c031b2bacdb0203bc017aac1735951a11d013ee4d1d5f7143ccbebf2371e66a1bec6e1febe69148f50784eef8adbb66664c96196d7e0c0bcdc807f447b54e058f37642a3337995bfbcd332208bd6016936705c82263eabd7affdba92fae3":"CRL version \: 2\nissuer name \: C=NL, O=PolarSSL, CN=PolarSSL Test CA\nthis update \: 2018-03-14 07\:31\:48\nnext update \: 2028-03-14 07\:31\:48\nRevoked certificates\:\nsigned using \: RSA with SHA-256\n":0 + X509 CRT parse path #2 (one cert) depends_on:MBEDTLS_SHA1_C:MBEDTLS_RSA_C mbedtls_x509_crt_parse_path:"data_files/dir1":0:1 From 47a98d4e2c198e68538192ab72cbd0a2850dbb54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Mar 2018 14:08:57 +0100 Subject: [PATCH 83/84] fixup previous commit: add forgotten file --- tests/data_files/crl-idpnc.pem | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/data_files/crl-idpnc.pem diff --git a/tests/data_files/crl-idpnc.pem b/tests/data_files/crl-idpnc.pem new file mode 100644 index 0000000000..0ebe480ee6 --- /dev/null +++ b/tests/data_files/crl-idpnc.pem @@ -0,0 +1,12 @@ +-----BEGIN X509 CRL----- +MIIBsDCBmQIBATANBgkqhkiG9w0BAQsFADA7MQswCQYDVQQGEwJOTDERMA8GA1UE +ChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EXDTE4MDMxNDEx +MTQzNloXDTI4MDMxNDExMTQzNlqgKjAoMCYGA1UdHAQfMB2gG6AZhhdodHRwOi8v +cGtpLmV4YW1wbGUuY29tLzANBgkqhkiG9w0BAQsFAAOCAQEACsszsNwAMkmUrbti +H1wpWN3LIb32MTZkBWZeFWWQ1MyzSFslgnOcu6tesJuTQJVJMGCSXZv7jkVHeeiK +x+BAoHCrR2aRVPbmiaP43Qp/dFOOfHVMM/VVWmuEYuCQaCAeVLQgGbgAYHE9aHQN +vBg8m7NJ95av2svLHMFIhirZlKWsAXM+aCyzoudEIhrP4Ppwt01SCtDl5gyg1Gkd +B3wuOckjTk0xwXdlOSMH9o0SD2fkc41AFDqOZTK2NTQzNChDNFbKXl8sr9SavJCm +k72l7wNJs6UOEhQMygyXEvqp8JbIi9JI+3TD4z4wUt0EnPkw0U48grLXFhjwBLWi +cxyjQQ== +-----END X509 CRL----- From 8be0e6db41b4a085e90cb03983f99d3a5158d450 Mon Sep 17 00:00:00 2001 From: Jaeden Amero Date: Fri, 16 Mar 2018 16:25:12 +0000 Subject: [PATCH 84/84] Update version to 2.8.0 --- ChangeLog | 2 +- doxygen/input/doc_mainpage.h | 2 +- doxygen/mbedtls.doxyfile | 2 +- include/mbedtls/version.h | 8 ++++---- library/CMakeLists.txt | 6 +++--- tests/suites/test_suite_version.data | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 882dbb5771..e0b016dfb5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,6 @@ mbed TLS ChangeLog (Sorted per branch, date) -= mbed TLS 2.7.x branch released 2018-xx-xx += mbed TLS 2.8.0 branch released 2018-03-16 Default behavior changes * The truncated HMAC extension now conforms to RFC 6066. This means diff --git a/doxygen/input/doc_mainpage.h b/doxygen/input/doc_mainpage.h index 641d5c4fb3..7952cbcbdb 100644 --- a/doxygen/input/doc_mainpage.h +++ b/doxygen/input/doc_mainpage.h @@ -24,7 +24,7 @@ */ /** - * @mainpage mbed TLS v2.7.0 source code documentation + * @mainpage mbed TLS v2.8.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 cbe0db4f2c..3592af2dae 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.7.0" +PROJECT_NAME = "mbed TLS v2.8.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 961be59c35..c3ee649f5c 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 7 +#define MBEDTLS_VERSION_MINOR 8 #define MBEDTLS_VERSION_PATCH 0 /** @@ -47,9 +47,9 @@ * MMNNPP00 * Major version | Minor version | Patch version */ -#define MBEDTLS_VERSION_NUMBER 0x02070000 -#define MBEDTLS_VERSION_STRING "2.7.0" -#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.7.0" +#define MBEDTLS_VERSION_NUMBER 0x02080000 +#define MBEDTLS_VERSION_STRING "2.8.0" +#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.8.0" #if defined(MBEDTLS_VERSION_C) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 02ccea8bc9..7742c22d25 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -141,15 +141,15 @@ endif(USE_STATIC_MBEDTLS_LIBRARY) if(USE_SHARED_MBEDTLS_LIBRARY) add_library(mbedcrypto SHARED ${src_crypto}) - set_target_properties(mbedcrypto PROPERTIES VERSION 2.7.0 SOVERSION 1) + set_target_properties(mbedcrypto PROPERTIES VERSION 2.8.0 SOVERSION 1) target_link_libraries(mbedcrypto ${libs}) add_library(mbedx509 SHARED ${src_x509}) - set_target_properties(mbedx509 PROPERTIES VERSION 2.7.0 SOVERSION 0) + set_target_properties(mbedx509 PROPERTIES VERSION 2.8.0 SOVERSION 0) target_link_libraries(mbedx509 ${libs} mbedcrypto) add_library(mbedtls SHARED ${src_tls}) - set_target_properties(mbedtls PROPERTIES VERSION 2.7.0 SOVERSION 10) + set_target_properties(mbedtls PROPERTIES VERSION 2.8.0 SOVERSION 10) target_link_libraries(mbedtls ${libs} mbedx509) install(TARGETS mbedtls mbedx509 mbedcrypto diff --git a/tests/suites/test_suite_version.data b/tests/suites/test_suite_version.data index 1aa4ffa754..79cc751ec9 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.7.0" +check_compiletime_version:"2.8.0" Check runtime library version -check_runtime_version:"2.7.0" +check_runtime_version:"2.8.0" Check for MBEDTLS_VERSION_C check_feature:"MBEDTLS_VERSION_C":0