From bb564e0fb498c9c2feb5bdde0cd1d4d4ef65e72d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 3 Sep 2015 10:44:32 +0200 Subject: [PATCH 1/2] Fix possible client crash on API misuse --- ChangeLog | 5 +++++ library/ssl_cli.c | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/ChangeLog b/ChangeLog index 2030ceb57..8c602fab5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,11 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 1.3.13 reladsed 2015-??-?? +Security + * Fix possible client-side NULL pointer dereference (read) when the client + tries to continue the handshake after it failed (a misuse of the API). + (Found by GDS Labs using afl-fuzz, patch provided by GDS Labs.) + Bugfix * Setting SSL_MIN_DHM_BYTES in config.h had no effect (overriden in ssl.h) (found by Fabio Solari) (#256) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 7f46cbb33..f603cffc1 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1602,6 +1602,12 @@ static int ssl_write_encrypted_pms( ssl_context *ssl, ssl->handshake->pmslen = 48; + if( ssl->session_negotiate->peer_cert == NULL ) + { + SSL_DEBUG_MSG( 2, ( "certificate required" ) ); + return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE ); + } + /* * Now write it out, encrypted */ @@ -1699,6 +1705,12 @@ static int ssl_get_ecdh_params_from_cert( ssl_context *ssl ) int ret; const ecp_keypair *peer_key; + if( ssl->session_negotiate->peer_cert == NULL ) + { + SSL_DEBUG_MSG( 2, ( "certificate required" ) ); + return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE ); + } + if( ! pk_can_do( &ssl->session_negotiate->peer_cert->pk, POLARSSL_PK_ECKEY ) ) { @@ -2012,6 +2024,12 @@ static int ssl_parse_server_key_exchange( ssl_context *ssl ) SSL_DEBUG_BUF( 3, "parameters hash", hash, hashlen != 0 ? hashlen : (unsigned int) ( md_info_from_type( md_alg ) )->size ); + if( ssl->session_negotiate->peer_cert == NULL ) + { + SSL_DEBUG_MSG( 2, ( "certificate required" ) ); + return( POLARSSL_ERR_SSL_UNEXPECTED_MESSAGE ); + } + /* * Verify signature */ From a1cdcd2364668c948afba4ecf0d7f62ebb3a1a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 3 Sep 2015 20:03:15 +0200 Subject: [PATCH 2/2] Add counter-measure against RSA-CRT attack https://securityblog.redhat.com/2015/09/02/factoring-rsa-keys-with-tls-perfect-forward-secrecy/ backport of 5f50104 --- ChangeLog | 3 +++ library/rsa.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8c602fab5..18486b382 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,9 @@ Security * Fix possible client-side NULL pointer dereference (read) when the client tries to continue the handshake after it failed (a misuse of the API). (Found by GDS Labs using afl-fuzz, patch provided by GDS Labs.) + * Add countermeasure against Lenstra's RSA-CRT attack for PKCS#1 v1.5 + signatures. (Found by Florian Weimer, Red Hat.) + https://securityblog.redhat.com/2015/09/02/factoring-rsa-keys-with-tls-perfect-forward-secrecy/ Bugfix * Setting SSL_MIN_DHM_BYTES in config.h had no effect (overriden in ssl.h) diff --git a/library/rsa.c b/library/rsa.c index c4e83c0e8..59ec35f9c 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -52,6 +52,8 @@ #else #include #define polarssl_printf printf +#define polarssl_malloc malloc +#define polarssl_free free #endif /* @@ -1005,6 +1007,11 @@ int rsa_rsassa_pkcs1_v15_sign( 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 == RSA_PRIVATE && ctx->padding != RSA_PKCS_V15 ) return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); @@ -1067,9 +1074,39 @@ int rsa_rsassa_pkcs1_v15_sign( rsa_context *ctx, memcpy( p, hash, hashlen ); } - return( ( mode == RSA_PUBLIC ) - ? rsa_public( ctx, sig, sig ) - : rsa_private( ctx, f_rng, p_rng, sig, sig ) ); + if( mode == RSA_PUBLIC ) + return( 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 = polarssl_malloc( ctx->len ); + verif = polarssl_malloc( ctx->len ); + if( sig_try == NULL || verif == NULL ) + return( POLARSSL_ERR_MPI_MALLOC_FAILED ); + + MPI_CHK( rsa_private( ctx, f_rng, p_rng, sig, sig_try ) ); + MPI_CHK( 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 = POLARSSL_ERR_RSA_PRIVATE_FAILED; + goto cleanup; + } + + memcpy( sig, sig_try, ctx->len ); + +cleanup: + polarssl_free( sig_try ); + polarssl_free( verif ); + + return( ret ); } #endif /* POLARSSL_PKCS1_V15 */