From 637376c2fe882e8904627ff7271bdeb97b14a5c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 24 Apr 2015 14:07:07 +0200 Subject: [PATCH 1/8] Fix bugs in programs displaying verify flags --- programs/test/ssl_cert_test.c | 2 +- programs/x509/cert_app.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/test/ssl_cert_test.c b/programs/test/ssl_cert_test.c index a2127306f..aade251bf 100644 --- a/programs/test/ssl_cert_test.c +++ b/programs/test/ssl_cert_test.c @@ -176,7 +176,7 @@ int main( void ) char vrfy_buf[512]; polarssl_printf( " failed\n" ); - x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", ret ); + x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", flags ); polarssl_printf( "%s\n", vrfy_buf ); } else diff --git a/programs/x509/cert_app.c b/programs/x509/cert_app.c index 3a6ae87e4..51d71aef2 100644 --- a/programs/x509/cert_app.c +++ b/programs/x509/cert_app.c @@ -346,7 +346,7 @@ int main( int argc, char *argv[] ) polarssl_printf( " failed\n" ); - x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", ret ); + x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", flags ); polarssl_printf( "%s\n", vrfy_buf ); } From f5203e0bb5a33b65aafdeb35fb6082ea69d700ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 29 Apr 2015 09:58:00 +0200 Subject: [PATCH 2/8] Fix "make install" handling of symlinks --- ChangeLog | 1 + Makefile | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 8d7a8108e..b0841ba6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,7 @@ Features errors on use of deprecated functions. Bugfix + * Fix handling of symlinks by "make install" (found by Gaël PORTAY). * Fix potential NULL pointer dereference (not trigerrable remotely) when ssl_write() is called before the handshake is finished (introduced in 1.3.10) (first reported by Martin Blumenstingl). diff --git a/Makefile b/Makefile index f9a4ce24c..441d1f9f2 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ install: cp -r include/polarssl $(DESTDIR)/include mkdir -p $(DESTDIR)/lib - cp library/libpolarssl.* library/libmbedtls.* $(DESTDIR)/lib + cp -RP library/libpolarssl.* library/libmbedtls.* $(DESTDIR)/lib mkdir -p $(DESTDIR)/bin for p in programs/*/* ; do \ From d97828e7afbc7c3bc5824083f79a2a6a73bc2420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 29 Apr 2015 14:03:28 +0200 Subject: [PATCH 3/8] Fix detection of getrandom() --- ChangeLog | 2 ++ library/entropy_poll.c | 66 ++++++++++++++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index b0841ba6e..19382bc87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,8 @@ Features errors on use of deprecated functions. Bugfix + * Fix detection of support for getrandom() on Linux (reported by syzzer) by + doing it at runtime (using uname) rather that compile time. * Fix handling of symlinks by "make install" (found by Gaël PORTAY). * Fix potential NULL pointer dereference (not trigerrable remotely) when ssl_write() is called before the handshake is finished (introduced in diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 61eb3e78a..e0f9ae286 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -86,27 +86,46 @@ static int getrandom_wrapper( void *buf, size_t buflen, unsigned int flags ) { return( syscall( SYS_getrandom, buf, buflen, flags ) ); } -#endif /* SYS_getrandom */ -#endif /* __linux__ */ -#if defined(HAVE_GETRANDOM) - -#include - -int platform_entropy_poll( void *data, - unsigned char *output, size_t len, size_t *olen ) +#include +/* Check if version is at least 3.17.0 */ +static int check_version_3_17_plus( void ) { - int ret; - ((void) data); + int minor; + struct utsname un; + const char *ver; - if( ( ret = getrandom_wrapper( output, len, 0 ) ) < 0 ) - return( POLARSSL_ERR_ENTROPY_SOURCE_FAILED ); + /* Get version information */ + uname(&un); + ver = un.release; + + /* Check major version; assume a single digit */ + if( ver[0] < '3' || ver[0] > '9' || ver [1] != '.' ) + return( -1 ); + + if( ver[0] - '0' > 3 ) + return( 0 ); + + /* Ok, so now we know major == 3, check minor. + * Assume 1 or 2 digits. */ + if( ver[2] < '0' || ver[2] > '9' ) + return( -1 ); + + minor = ver[2] - '0'; + + if( ver[3] >= '0' && ver[3] <= '9' ) + minor = 10 * minor + ver[3] - '0'; + else if( ver [3] != '.' ) + return( -1 ); + + if( minor < 17 ) + return( -1 ); - *olen = ret; return( 0 ); } - -#else /* HAVE_GETRANDOM */ +static int has_getrandom = -1; +#endif /* SYS_getrandom */ +#endif /* __linux__ */ #include @@ -117,6 +136,22 @@ int platform_entropy_poll( void *data, size_t ret; ((void) data); +#if defined(HAVE_GETRANDOM) + if( has_getrandom == -1 ) + has_getrandom = ( check_version_3_17_plus() == 0 ); + + if( has_getrandom ) + { + int ret; + + if( ( ret = getrandom_wrapper( output, len, 0 ) ) < 0 ) + return( POLARSSL_ERR_ENTROPY_SOURCE_FAILED ); + + *olen = ret; + return( 0 ); + } +#endif /* HAVE_GETRANDOM */ + *olen = 0; file = fopen( "/dev/urandom", "rb" ); @@ -135,7 +170,6 @@ int platform_entropy_poll( void *data, return( 0 ); } -#endif /* HAVE_GETRANDOM */ #endif /* _WIN32 && !EFIX64 && !EFI32 */ #endif /* !POLARSSL_NO_PLATFORM_ENTROPY */ From 770b5e1e9e043a00f79276d588c8c7437c0b774c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 29 Apr 2015 17:02:01 +0200 Subject: [PATCH 4/8] Fix missing NULL check in MPI --- ChangeLog | 2 ++ include/polarssl/bignum.h | 4 +++- library/bignum.c | 3 +++ tests/suites/test_suite_mpi.data | 3 +++ tests/suites/test_suite_mpi.function | 19 +++++++++++++++++++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 19382bc87..75ddfdb15 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,8 @@ Features errors on use of deprecated functions. Bugfix + * mpi_size() and mpi_msb() would segfault when called on an mpi that is + initialized but not set (found by pravic). * Fix detection of support for getrandom() on Linux (reported by syzzer) by doing it at runtime (using uname) rather that compile time. * Fix handling of symlinks by "make install" (found by Gaël PORTAY). diff --git a/include/polarssl/bignum.h b/include/polarssl/bignum.h index df25bd1f1..8e1687b97 100644 --- a/include/polarssl/bignum.h +++ b/include/polarssl/bignum.h @@ -188,7 +188,9 @@ typedef struct mpi; /** - * \brief Initialize one MPI + * \brief Initialize one MPI (make internal references valid) + * This just makes it ready to be set or freed, + * but does not define a value for the MPI. * * \param X One MPI to initialize. */ diff --git a/library/bignum.c b/library/bignum.c index 12c72af3a..f479bc9ed 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -356,6 +356,9 @@ size_t mpi_msb( const mpi *X ) { size_t i, j; + if( X->n == 0 ) + return( 0 ); + for( i = X->n - 1; i > 0; i-- ) if( X->p[i] != 0 ) break; diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data index 7908f9144..56817ccbe 100644 --- a/tests/suites/test_suite_mpi.data +++ b/tests/suites/test_suite_mpi.data @@ -1,3 +1,6 @@ +Arguments with no value +mpi_null: + Base test mpi_read_write_string #1 mpi_read_write_string:10:"128":10:"128":100:0:0 diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index ce1a07205..023cab412 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -7,6 +7,25 @@ * END_DEPENDENCIES */ +/* BEGIN_CASE */ +void mpi_null( ) +{ + mpi X, Y, Z; + + mpi_init( &X ); + mpi_init( &Y ); + mpi_init( &Z ); + + TEST_ASSERT( mpi_get_bit( &X, 42 ) == 0 ); + TEST_ASSERT( mpi_lsb( &X ) == 0 ); + TEST_ASSERT( mpi_msb( &X ) == 0 ); + TEST_ASSERT( mpi_size( &X ) == 0 ); + +exit: + mpi_free( &X ); +} +/* END_CASE */ + /* BEGIN_CASE */ void mpi_read_write_string( int radix_X, char *input_X, int radix_A, char *input_A, int output_size, int result_read, From e16b62c3a9533dfe4d896e2c373c4484b1326d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Apr 2015 16:55:53 +0200 Subject: [PATCH 5/8] Make results of (ext)KeyUsage accessible --- ChangeLog | 3 +++ include/polarssl/ssl.h | 3 ++- library/ssl_srv.c | 3 ++- library/ssl_tls.c | 20 +++++++++++++++----- tests/ssl-opt.sh | 22 ++++++++++++++++++++++ 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 75ddfdb15..7186addd0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,9 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 1.3 branch Security + * With authmode set to SSL_VERIFY_OPTIONAL, verification of keyUsage and + extendedKeyUsage on the leaf certificate was lost (results not accessible + via ssl_get_verify_results()). Features * Add x509_crt_verify_info() to display certificate verification results. diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index cd9f770e9..54382e5a0 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -1980,7 +1980,8 @@ static inline x509_crt *ssl_own_cert( ssl_context *ssl ) */ int ssl_check_cert_usage( const x509_crt *cert, const ssl_ciphersuite_t *ciphersuite, - int cert_endpoint ); + int cert_endpoint, + int *flags ); #endif /* POLARSSL_X509_CRT_PARSE_C */ /* constant-time buffer comparison */ diff --git a/library/ssl_srv.c b/library/ssl_srv.c index dad6872ea..5f01a01bc 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -829,6 +829,7 @@ static int ssl_pick_cert( ssl_context *ssl, { ssl_key_cert *cur, *list, *fallback = NULL; pk_type_t pk_alg = ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ); + int flags; #if defined(POLARSSL_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_key_cert != NULL ) @@ -862,7 +863,7 @@ static int ssl_pick_cert( ssl_context *ssl, * and decrypting with the same RSA key. */ if( ssl_check_cert_usage( cur->cert, ciphersuite_info, - SSL_IS_SERVER ) != 0 ) + SSL_IS_SERVER, &flags ) != 0 ) { SSL_DEBUG_MSG( 3, ( "certificate mismatch: " "(extended) key usage extension" ) ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d2e0c52bc..72cd6d21a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2859,7 +2859,8 @@ int ssl_parse_certificate( ssl_context *ssl ) if( ssl_check_cert_usage( ssl->session_negotiate->peer_cert, ciphersuite_info, - ! ssl->endpoint ) != 0 ) + ! ssl->endpoint, + &ssl->session_negotiate->verify_result ) != 0 ) { SSL_DEBUG_MSG( 1, ( "bad certificate (usage extensions)" ) ); if( ret == 0 ) @@ -5199,8 +5200,10 @@ int ssl_curve_is_acceptable( const ssl_context *ssl, ecp_group_id grp_id ) #if defined(POLARSSL_X509_CRT_PARSE_C) int ssl_check_cert_usage( const x509_crt *cert, const ssl_ciphersuite_t *ciphersuite, - int cert_endpoint ) + int cert_endpoint, + int *flags ) { + int ret = 0; #if defined(POLARSSL_X509_CHECK_KEY_USAGE) int usage = 0; #endif @@ -5213,6 +5216,7 @@ int ssl_check_cert_usage( const x509_crt *cert, !defined(POLARSSL_X509_CHECK_EXTENDED_KEY_USAGE) ((void) cert); ((void) cert_endpoint); + ((void) flags); #endif #if defined(POLARSSL_X509_CHECK_KEY_USAGE) @@ -5252,7 +5256,10 @@ int ssl_check_cert_usage( const x509_crt *cert, } if( x509_crt_check_key_usage( cert, usage ) != 0 ) - return( -1 ); + { + *flags |= BADCERT_KEY_USAGE; + ret = -1; + } #else ((void) ciphersuite); #endif /* POLARSSL_X509_CHECK_KEY_USAGE */ @@ -5270,10 +5277,13 @@ int ssl_check_cert_usage( const x509_crt *cert, } if( x509_crt_check_extended_key_usage( cert, ext_oid, ext_len ) != 0 ) - return( -1 ); + { + *flags |= BADCERT_EXT_KEY_USAGE; + ret = -1; + } #endif /* POLARSSL_X509_CHECK_EXTENDED_KEY_USAGE */ - return( 0 ); + return( ret ); } #endif /* POLARSSL_X509_CRT_PARSE_C */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 75c59423a..5cf4ff608 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1878,6 +1878,17 @@ run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" +run_test "keyUsage cli: KeyEncipherment, DHE-RSA: fail, soft" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ke.crt" \ + "$P_CLI debug_level=1 auth_mode=optional \ + force_ciphersuite=TLS-DHE-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -c "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" \ + -c "! Usage does not match the keyUsage extension" + run_test "keyUsage cli: DigitalSignature, DHE-RSA: OK" \ "$O_SRV -key data_files/server2.key \ -cert data_files/server2.ku-ds.crt" \ @@ -1898,6 +1909,17 @@ run_test "keyUsage cli: DigitalSignature, RSA: fail" \ -c "Processing of the Certificate handshake message failed" \ -C "Ciphersuite is TLS-" +run_test "keyUsage cli: DigitalSignature, RSA: fail, soft" \ + "$O_SRV -key data_files/server2.key \ + -cert data_files/server2.ku-ds.crt" \ + "$P_CLI debug_level=1 auth_mode=optional \ + force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA" \ + 0 \ + -c "bad certificate (usage extensions)" \ + -C "Processing of the Certificate handshake message failed" \ + -c "Ciphersuite is TLS-" \ + -c "! Usage does not match the keyUsage extension" + # Tests for keyUsage in leaf certificates, part 3: # server-side checking of client cert From 7d1e95c991a508cfa4912c7b93dfab358fc40374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 29 Apr 2015 01:35:48 +0200 Subject: [PATCH 6/8] Add countermeasure against cache-based lucky 13 --- ChangeLog | 2 ++ library/ssl_tls.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 7186addd0..bbaddbcc1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,8 @@ Security * With authmode set to SSL_VERIFY_OPTIONAL, verification of keyUsage and extendedKeyUsage on the leaf certificate was lost (results not accessible via ssl_get_verify_results()). + * Add countermeasure against "Lucky 13 strikes back" cache-based attack, + https://dl.acm.org/citation.cfm?id=2714625 Features * Add x509_crt_verify_info() to display certificate verification results. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 72cd6d21a..f079adc45 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1825,7 +1825,8 @@ static int ssl_decrypt_buf( ssl_context *ssl ) ssl->in_msglen ); md_hmac_finish( &ssl->transform_in->md_ctx_dec, ssl->in_msg + ssl->in_msglen ); - for( j = 0; j < extra_run; j++ ) + /* Call md_process at least once due to cache attacks */ + for( j = 0; j < extra_run + 1; j++ ) md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg ); md_hmac_reset( &ssl->transform_in->md_ctx_dec ); From ac906733458633c8c0e430d041306830a5c26aeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 30 Apr 2015 10:09:50 +0200 Subject: [PATCH 7/8] Remove unused headers in o_p_test --- programs/test/o_p_test.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/programs/test/o_p_test.c b/programs/test/o_p_test.c index d949d51de..f0ade14ec 100644 --- a/programs/test/o_p_test.c +++ b/programs/test/o_p_test.c @@ -52,8 +52,6 @@ #include #include #include -#include -#include #endif #if !defined(POLARSSL_BIGNUM_C) || !defined(POLARSSL_RSA_C) || \ From 7b12492c7704e3688c2f39ccd443ae2443a1264c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 30 Apr 2015 10:16:19 +0200 Subject: [PATCH 8/8] Include changes from the 1.2 branch --- ChangeLog | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/ChangeLog b/ChangeLog index bbaddbcc1..c4203930d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -584,6 +584,67 @@ Security * RSA blinding on CRT operations to counter timing attacks (found by Cyril Arnaud and Pierre-Alain Fouque) + += Version 1.2.14 released 2015-05-?? + +Security + * Fix potential invalid memory read in the server, that allows a client to + crash it remotely (found by Caj Larsson). + * Fix potential invalid memory read in certificate parsing, that allows a + client to crash the server remotely if client authentication is enabled + (found using Codenomicon Defensics). + * Add countermeasure against "Lucky 13 strikes back" cache-based attack, + https://dl.acm.org/citation.cfm?id=2714625 + +Bugfix + * Fix bug in Via Padlock support (found by Nikos Mavrogiannopoulos). + * Fix hardclock() (only used in the benchmarking program) with some + versions of mingw64 (found by kxjhlele). + * Fix warnings from mingw64 in timing.c (found by kxjklele). + * Fix potential unintended sign extension in asn1_get_len() on 64-bit + platforms (found with Coverity Scan). + += Version 1.2.13 released 2015-02-16 +Note: Although PolarSSL has been renamed to mbed TLS, no changes reflecting + this will be made in the 1.2 branch at this point. + +Security + * Fix remotely-triggerable uninitialised pointer dereference caused by + crafted X.509 certificate (TLS server is not affected if it doesn't ask + for a client certificate) (found using Codenomicon Defensics). + * Fix remotely-triggerable memory leak caused by crafted X.509 certificates + (TLS server is not affected if it doesn't ask for a client certificate) + (found using Codenomicon Defensics). + * Fix potential stack overflow while parsing crafted X.509 certificates + (TLS server is not affected if it doesn't ask for a client certificate) + found using Codenomicon Defensics). + * Fix buffer overread of size 1 when parsing crafted X.509 certificates + (TLS server is not affected if it doesn't ask for a client certificate). + +Bugfix + * Fix potential undefined behaviour in Camellia. + * Fix memory leaks in PKCS#5 and PKCS#12. + * Stack buffer overflow if ctr_drbg_update() is called with too large + add_len (found by Jean-Philippe Aumasson) (not triggerable remotely). + * Fix bug in MPI/bignum on s390/s390x (reported by Dan Horák) (introduced + in 1.2.12). + * Fix unchecked return code in x509_crt_parse_path() on Windows (found by + Peter Vaskovic). + * Fix assembly selection for MIPS64 (thanks to James Cowgill). + * ssl_get_verify_result() now works even if the handshake was aborted due + to a failed verification (found by Fredrik Axelsson). + * Skip writing and parsing signature_algorithm extension if none of the + key exchanges enabled needs certificates. This fixes a possible interop + issue with some servers when a zero-length extension was sent. (Reported + by Peter Dettman.) + * On a 0-length input, base64_encode() did not correctly set output length + (found by Hendrik van den Boogaard). + +Changes + * Blind RSA private operations even when POLARSSL_RSA_NO_CRT is defined. + * Forbid repeated extensions in X.509 certificates. + * Add compile-time option POLARSSL_X509_MAX_INTERMEDIATE_CA to limit the + length of an X.509 verification chain (default = 8). = Version 1.2.12 released 2014-10-24 Security