From 8e7315d2fe67b9b2b1406e62a0887aa5bde3d73b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Thu, 5 Mar 2020 16:12:23 +0100 Subject: [PATCH] mbedtls: Update to upstream version 2.16.5 Fixes https://tls.mbed.org/tech-updates/security-advisories/mbedtls-security-advisory-2020-02 Drop patch to disable VIA padlock since we no longer use libwebsockets, so there's no conflict anymore. (cherry picked from commit e435bed84708edb0c14fb94529fba7665966324f) --- thirdparty/README.md | 2 +- thirdparty/mbedtls/include/mbedtls/version.h | 8 +- thirdparty/mbedtls/library/bignum.c | 5 +- thirdparty/mbedtls/library/cipher.c | 14 +-- thirdparty/mbedtls/library/ecdsa.c | 4 +- thirdparty/mbedtls/library/entropy_poll.c | 29 ++--- thirdparty/mbedtls/library/pkparse.c | 122 ++++++++++++++----- thirdparty/mbedtls/library/rsa.c | 11 +- thirdparty/mbedtls/library/x509_crt.c | 32 +---- thirdparty/mbedtls/library/x509write_csr.c | 4 +- 10 files changed, 125 insertions(+), 106 deletions(-) diff --git a/thirdparty/README.md b/thirdparty/README.md index 207ee26811d..c001b16210a 100644 --- a/thirdparty/README.md +++ b/thirdparty/README.md @@ -285,7 +285,7 @@ Godot build configurations, check them out when updating. ## mbedtls - Upstream: https://tls.mbed.org/ -- Version: 2.16.4 +- Version: 2.16.5 - License: Apache 2.0 File extracted from upstream release tarball (`-apache.tgz` variant): diff --git a/thirdparty/mbedtls/include/mbedtls/version.h b/thirdparty/mbedtls/include/mbedtls/version.h index aeffb166991..8e2ce03c320 100644 --- a/thirdparty/mbedtls/include/mbedtls/version.h +++ b/thirdparty/mbedtls/include/mbedtls/version.h @@ -40,16 +40,16 @@ */ #define MBEDTLS_VERSION_MAJOR 2 #define MBEDTLS_VERSION_MINOR 16 -#define MBEDTLS_VERSION_PATCH 4 +#define MBEDTLS_VERSION_PATCH 5 /** * The single version number has the following structure: * MMNNPP00 * Major version | Minor version | Patch version */ -#define MBEDTLS_VERSION_NUMBER 0x02100400 -#define MBEDTLS_VERSION_STRING "2.16.4" -#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.16.4" +#define MBEDTLS_VERSION_NUMBER 0x02100500 +#define MBEDTLS_VERSION_STRING "2.16.5" +#define MBEDTLS_VERSION_STRING_FULL "mbed TLS 2.16.5" #if defined(MBEDTLS_VERSION_C) diff --git a/thirdparty/mbedtls/library/bignum.c b/thirdparty/mbedtls/library/bignum.c index 6713bcbf6f2..87ccf42fad9 100644 --- a/thirdparty/mbedtls/library/bignum.c +++ b/thirdparty/mbedtls/library/bignum.c @@ -157,9 +157,10 @@ int mbedtls_mpi_shrink( mbedtls_mpi *X, size_t nblimbs ) if( nblimbs > MBEDTLS_MPI_MAX_LIMBS ) return( MBEDTLS_ERR_MPI_ALLOC_FAILED ); - /* Actually resize up in this case */ + /* Actually resize up if there are currently fewer than nblimbs limbs. */ if( X->n <= nblimbs ) return( mbedtls_mpi_grow( X, nblimbs ) ); + /* After this point, then X->n > nblimbs and in particular X->n > 0. */ for( i = X->n - 1; i > 0; i-- ) if( X->p[i] != 0 ) @@ -198,7 +199,7 @@ int mbedtls_mpi_copy( mbedtls_mpi *X, const mbedtls_mpi *Y ) if( X == Y ) return( 0 ); - if( Y->p == NULL ) + if( Y->n == 0 ) { mbedtls_mpi_free( X ); return( 0 ); diff --git a/thirdparty/mbedtls/library/cipher.c b/thirdparty/mbedtls/library/cipher.c index 273997577b6..8d010b59ac4 100644 --- a/thirdparty/mbedtls/library/cipher.c +++ b/thirdparty/mbedtls/library/cipher.c @@ -361,6 +361,10 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i *olen = 0; block_size = mbedtls_cipher_get_block_size( ctx ); + if ( 0 == block_size ) + { + return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); + } if( ctx->cipher_info->mode == MBEDTLS_MODE_ECB ) { @@ -396,11 +400,6 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } #endif - if ( 0 == block_size ) - { - return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); - } - if( input == output && ( ctx->unprocessed_len != 0 || ilen % block_size ) ) { @@ -459,11 +458,6 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i */ if( 0 != ilen ) { - if( 0 == block_size ) - { - return( MBEDTLS_ERR_CIPHER_INVALID_CONTEXT ); - } - /* Encryption: only cache partial blocks * Decryption w/ padding: always keep at least one whole block * Decryption w/o padding: only cache partial blocks diff --git a/thirdparty/mbedtls/library/ecdsa.c b/thirdparty/mbedtls/library/ecdsa.c index 3cf3d7cc4f1..6b72e0d927e 100644 --- a/thirdparty/mbedtls/library/ecdsa.c +++ b/thirdparty/mbedtls/library/ecdsa.c @@ -297,7 +297,7 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp, *p_sign_tries = 0; do { - if( *p_sign_tries++ > 10 ) + if( (*p_sign_tries)++ > 10 ) { ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; goto cleanup; @@ -310,7 +310,7 @@ static int ecdsa_sign_restartable( mbedtls_ecp_group *grp, *p_key_tries = 0; do { - if( *p_key_tries++ > 10 ) + if( (*p_key_tries)++ > 10 ) { ret = MBEDTLS_ERR_ECP_RANDOM_FAILED; goto cleanup; diff --git a/thirdparty/mbedtls/library/entropy_poll.c b/thirdparty/mbedtls/library/entropy_poll.c index ba56b70f77e..4556f88a557 100644 --- a/thirdparty/mbedtls/library/entropy_poll.c +++ b/thirdparty/mbedtls/library/entropy_poll.c @@ -61,43 +61,28 @@ #define _WIN32_WINNT 0x0400 #endif #include -#include -#if defined(_MSC_VER) && _MSC_VER <= 1600 -/* Visual Studio 2010 and earlier issue a warning when both and - * are included, as they redefine a number of _MAX constants. - * These constants are guaranteed to be the same, though, so we suppress the - * warning when including intsafe.h. - */ -#pragma warning( push ) -#pragma warning( disable : 4005 ) -#endif -#include -#if defined(_MSC_VER) && _MSC_VER <= 1600 -#pragma warning( pop ) -#endif +#include int mbedtls_platform_entropy_poll( void *data, unsigned char *output, size_t len, size_t *olen ) { - ULONG len_as_ulong = 0; + HCRYPTPROV provider; ((void) data); *olen = 0; - /* - * BCryptGenRandom takes ULONG for size, which is smaller than size_t on - * 64-bit Windows platforms. Ensure len's value can be safely converted into - * a ULONG. - */ - if ( FAILED( SizeTToULong( len, &len_as_ulong ) ) ) + if( CryptAcquireContext( &provider, NULL, NULL, + PROV_RSA_FULL, CRYPT_VERIFYCONTEXT ) == FALSE ) { return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); } - if ( !BCRYPT_SUCCESS( BCryptGenRandom( NULL, output, len_as_ulong, BCRYPT_USE_SYSTEM_PREFERRED_RNG ) ) ) + if( CryptGenRandom( provider, (DWORD) len, output ) == FALSE ) { + CryptReleaseContext( provider, 0 ); return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); } + CryptReleaseContext( provider, 0 ); *olen = len; return( 0 ); diff --git a/thirdparty/mbedtls/library/pkparse.c b/thirdparty/mbedtls/library/pkparse.c index ae210bca6a2..d5004577a1d 100644 --- a/thirdparty/mbedtls/library/pkparse.c +++ b/thirdparty/mbedtls/library/pkparse.c @@ -677,6 +677,32 @@ int mbedtls_pk_parse_subpubkey( unsigned char **p, const unsigned char *end, } #if defined(MBEDTLS_RSA_C) +/* + * Wrapper around mbedtls_asn1_get_mpi() that rejects zero. + * + * The value zero is: + * - never a valid value for an RSA parameter + * - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete(). + * + * Since values can't be omitted in PKCS#1, passing a zero value to + * rsa_complete() would be incorrect, so reject zero values early. + */ +static int asn1_get_nonzero_mpi( unsigned char **p, + const unsigned char *end, + mbedtls_mpi *X ) +{ + int ret; + + ret = mbedtls_asn1_get_mpi( p, end, X ); + if( ret != 0 ) + return( ret ); + + if( mbedtls_mpi_cmp_int( X, 0 ) == 0 ) + return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT ); + + return( 0 ); +} + /* * Parse a PKCS#1 encoded private RSA key */ @@ -729,54 +755,84 @@ static int pk_parse_key_pkcs1_der( mbedtls_rsa_context *rsa, } /* Import N */ - if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, - MBEDTLS_ASN1_INTEGER ) ) != 0 || - ( ret = mbedtls_rsa_import_raw( rsa, p, len, NULL, 0, NULL, 0, - NULL, 0, NULL, 0 ) ) != 0 ) + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_rsa_import( rsa, &T, NULL, NULL, + NULL, NULL ) ) != 0 ) goto cleanup; - p += len; /* Import E */ - if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, - MBEDTLS_ASN1_INTEGER ) ) != 0 || - ( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, NULL, 0, - NULL, 0, p, len ) ) != 0 ) + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_rsa_import( rsa, NULL, NULL, NULL, + NULL, &T ) ) != 0 ) goto cleanup; - p += len; /* Import D */ - if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, - MBEDTLS_ASN1_INTEGER ) ) != 0 || - ( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, NULL, 0, - p, len, NULL, 0 ) ) != 0 ) + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_rsa_import( rsa, NULL, NULL, NULL, + &T, NULL ) ) != 0 ) goto cleanup; - p += len; /* Import P */ - if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, - MBEDTLS_ASN1_INTEGER ) ) != 0 || - ( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, p, len, NULL, 0, - NULL, 0, NULL, 0 ) ) != 0 ) + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_rsa_import( rsa, NULL, &T, NULL, + NULL, NULL ) ) != 0 ) goto cleanup; - p += len; /* Import Q */ - if( ( ret = mbedtls_asn1_get_tag( &p, end, &len, - MBEDTLS_ASN1_INTEGER ) ) != 0 || - ( ret = mbedtls_rsa_import_raw( rsa, NULL, 0, NULL, 0, p, len, - NULL, 0, NULL, 0 ) ) != 0 ) - goto cleanup; - p += len; - - /* Complete the RSA private key */ - if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 ) + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_rsa_import( rsa, NULL, NULL, &T, + NULL, NULL ) ) != 0 ) goto cleanup; - /* Check optional parameters */ - if( ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 || - ( ret = mbedtls_asn1_get_mpi( &p, end, &T ) ) != 0 ) +#if !defined(MBEDTLS_RSA_NO_CRT) && !defined(MBEDTLS_RSA_ALT) + /* + * The RSA CRT parameters DP, DQ and QP are nominally redundant, in + * that they can be easily recomputed from D, P and Q. However by + * parsing them from the PKCS1 structure it is possible to avoid + * recalculating them which both reduces the overhead of loading + * RSA private keys into memory and also avoids side channels which + * can arise when computing those values, since all of D, P, and Q + * are secret. See https://eprint.iacr.org/2020/055 for a + * description of one such attack. + */ + + /* Import DP */ + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_mpi_copy( &rsa->DP, &T ) ) != 0 ) + goto cleanup; + + /* Import DQ */ + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_mpi_copy( &rsa->DQ, &T ) ) != 0 ) + goto cleanup; + + /* Import QP */ + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = mbedtls_mpi_copy( &rsa->QP, &T ) ) != 0 ) + goto cleanup; + +#else + /* Verify existance of the CRT params */ + if( ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 || + ( ret = asn1_get_nonzero_mpi( &p, end, &T ) ) != 0 ) + goto cleanup; +#endif + + /* rsa_complete() doesn't complete anything with the default + * implementation but is still called: + * - for the benefit of alternative implementation that may want to + * pre-compute stuff beyond what's provided (eg Montgomery factors) + * - as is also sanity-checks the key + * + * Furthermore, we also check the public part for consistency with + * mbedtls_pk_parse_pubkey(), as it includes size minima for example. + */ + if( ( ret = mbedtls_rsa_complete( rsa ) ) != 0 || + ( ret = mbedtls_rsa_check_pubkey( rsa ) ) != 0 ) + { goto cleanup; + } if( p != end ) { diff --git a/thirdparty/mbedtls/library/rsa.c b/thirdparty/mbedtls/library/rsa.c index af1a8785991..09fd379fdb3 100644 --- a/thirdparty/mbedtls/library/rsa.c +++ b/thirdparty/mbedtls/library/rsa.c @@ -249,6 +249,9 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) { int ret = 0; int have_N, have_P, have_Q, have_D, have_E; +#if !defined(MBEDTLS_RSA_NO_CRT) + int have_DP, have_DQ, have_QP; +#endif int n_missing, pq_missing, d_missing, is_pub, is_priv; RSA_VALIDATE_RET( ctx != NULL ); @@ -259,6 +262,12 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) have_D = ( mbedtls_mpi_cmp_int( &ctx->D, 0 ) != 0 ); have_E = ( mbedtls_mpi_cmp_int( &ctx->E, 0 ) != 0 ); +#if !defined(MBEDTLS_RSA_NO_CRT) + have_DP = ( mbedtls_mpi_cmp_int( &ctx->DP, 0 ) != 0 ); + have_DQ = ( mbedtls_mpi_cmp_int( &ctx->DQ, 0 ) != 0 ); + have_QP = ( mbedtls_mpi_cmp_int( &ctx->QP, 0 ) != 0 ); +#endif + /* * Check whether provided parameters are enough * to deduce all others. The following incomplete @@ -324,7 +333,7 @@ int mbedtls_rsa_complete( mbedtls_rsa_context *ctx ) */ #if !defined(MBEDTLS_RSA_NO_CRT) - if( is_priv ) + if( is_priv && ! ( have_DP && have_DQ && have_QP ) ) { ret = mbedtls_rsa_deduce_crt( &ctx->P, &ctx->Q, &ctx->D, &ctx->DP, &ctx->DQ, &ctx->QP ); diff --git a/thirdparty/mbedtls/library/x509_crt.c b/thirdparty/mbedtls/library/x509_crt.c index a3697f13f9c..9c2e36547e3 100644 --- a/thirdparty/mbedtls/library/x509_crt.c +++ b/thirdparty/mbedtls/library/x509_crt.c @@ -65,19 +65,6 @@ #if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32) #include -#if defined(_MSC_VER) && _MSC_VER <= 1600 -/* Visual Studio 2010 and earlier issue a warning when both and - * are included, as they redefine a number of _MAX constants. - * These constants are guaranteed to be the same, though, so we suppress the - * warning when including intsafe.h. - */ -#pragma warning( push ) -#pragma warning( disable : 4005 ) -#endif -#include -#if defined(_MSC_VER) && _MSC_VER <= 1600 -#pragma warning( pop ) -#endif #else #include #endif @@ -1290,7 +1277,6 @@ int mbedtls_x509_crt_parse_path( mbedtls_x509_crt *chain, const char *path ) char filename[MAX_PATH]; char *p; size_t len = strlen( path ); - int lengthAsInt = 0; WIN32_FIND_DATAW file_data; HANDLE hFind; @@ -1305,18 +1291,7 @@ int mbedtls_x509_crt_parse_path( mbedtls_x509_crt *chain, const char *path ) p = filename + len; filename[len++] = '*'; - if ( FAILED ( SizeTToInt( len, &lengthAsInt ) ) ) - return( MBEDTLS_ERR_X509_FILE_IO_ERROR ); - - /* - * Note this function uses the code page CP_ACP, and assumes the incoming - * string is encoded in ANSI, before translating it into Unicode. If the - * incoming string were changed to be UTF-8, then the length check needs to - * change to check the number of characters, not the number of bytes, in the - * incoming string are less than MAX_PATH to avoid a buffer overrun with - * MultiByteToWideChar(). - */ - w_ret = MultiByteToWideChar( CP_ACP, 0, filename, lengthAsInt, szDir, + w_ret = MultiByteToWideChar( CP_ACP, 0, filename, (int)len, szDir, MAX_PATH - 3 ); if( w_ret == 0 ) return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); @@ -1333,11 +1308,8 @@ int mbedtls_x509_crt_parse_path( mbedtls_x509_crt *chain, const char *path ) if( file_data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY ) continue; - if ( FAILED( SizeTToInt( wcslen( file_data.cFileName ), &lengthAsInt ) ) ) - return( MBEDTLS_ERR_X509_FILE_IO_ERROR ); - w_ret = WideCharToMultiByte( CP_ACP, 0, file_data.cFileName, - lengthAsInt, + lstrlenW( file_data.cFileName ), p, (int) len - 1, NULL, NULL ); if( w_ret == 0 ) diff --git a/thirdparty/mbedtls/library/x509write_csr.c b/thirdparty/mbedtls/library/x509write_csr.c index b65a11c6aa7..7406a975423 100644 --- a/thirdparty/mbedtls/library/x509write_csr.c +++ b/thirdparty/mbedtls/library/x509write_csr.c @@ -226,7 +226,9 @@ int mbedtls_x509write_csr_der( mbedtls_x509write_csr *ctx, unsigned char *buf, s /* * Prepare signature */ - mbedtls_md( mbedtls_md_info_from_type( ctx->md_alg ), c, len, hash ); + ret = mbedtls_md( mbedtls_md_info_from_type( ctx->md_alg ), c, len, hash ); + if( ret != 0 ) + return( ret ); if( ( ret = mbedtls_pk_sign( ctx->key, ctx->md_alg, hash, 0, sig, &sig_len, f_rng, p_rng ) ) != 0 )