diff --git a/ChangeLog b/ChangeLog index c6d71c94e..ebf4ab587 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,17 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS 2.x.x branch released xxxx-xx-xx Bugfix + * Fix the redefinition of macro ssl_set_bio to an undefined symbol + mbedtls_ssl_set_bio_timeout in compat-1.3.h, by removing it. + Found by omlib-lin. #673 + * Fix unused variable/function compilation warnings in pem.c, x509_crt.c and + x509_csr.c that are reported when building mbed TLS with a config.h that + does not define MBEDTLS_PEM_PARSE_C. Found by omnium21. #562 + * Fix incorrect renegotiation condition in ssl_check_ctr_renegotiate() that + would compare 64 bits of the record counter instead of 48 bits as indicated + in RFC 6347 Section 4.3.1. This could cause the execution of the + renegotiation routines at unexpected times when the protocol is DTLS. Found + by wariua. #687 * Fixed potential arithmetic overflow in mbedtls_ctr_drbg_reseed() that could cause buffer bound checks to be bypassed. Found by Eyal Itkin. * Fixed potential arithmetic overflows in mbedtls_cipher_update() that could diff --git a/include/mbedtls/compat-1.3.h b/include/mbedtls/compat-1.3.h index 27abbd972..af51b5f82 100644 --- a/include/mbedtls/compat-1.3.h +++ b/include/mbedtls/compat-1.3.h @@ -2453,7 +2453,6 @@ #define ssl_set_arc4_support mbedtls_ssl_conf_arc4_support #define ssl_set_authmode mbedtls_ssl_conf_authmode #define ssl_set_bio mbedtls_ssl_set_bio -#define ssl_set_bio mbedtls_ssl_set_bio_timeout #define ssl_set_ca_chain mbedtls_ssl_conf_ca_chain #define ssl_set_cbc_record_splitting mbedtls_ssl_conf_cbc_record_splitting #define ssl_set_ciphersuites mbedtls_ssl_conf_ciphersuites diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 2c021900b..42c9779c6 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1146,7 +1146,7 @@ void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) * * \note See the documentation of \c mbedtls_ssl_set_timer_t and * \c mbedtls_ssl_get_timer_t for the conventions this pair of - * callbacks must fallow. + * callbacks must follow. * * \note On some platforms, timing.c provides * \c mbedtls_timing_set_delay() and @@ -2183,7 +2183,7 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_ /** * \brief Set record counter threshold for periodic renegotiation. - * (Default: 2^64 - 256.) + * (Default: 2^48 - 1) * * Renegotiation is automatically triggered when a record * counter (outgoing or ingoing) crosses the defined @@ -2194,9 +2194,17 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_ * Lower values can be used to enforce policies such as "keys * must be refreshed every N packets with cipher X". * + * The renegotiation period can be disabled by setting + * conf->disable_renegotiation to + * MBEDTLS_SSL_RENEGOTIATION_DISABLED. + * + * \note When the configured transport is + * MBEDTLS_SSL_TRANSPORT_DATAGRAM the maximum renegotiation + * period is 2^48 - 1, and for MBEDTLS_SSL_TRANSPORT_STREAM, + * the maximum renegotiation period is 2^64 - 1. + * * \param conf SSL configuration * \param period The threshold value: a big-endian 64-bit number. - * Set to 2^64 - 1 to disable periodic renegotiation */ void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf, const unsigned char period[8] ); diff --git a/library/pem.c b/library/pem.c index d1c660412..8dd86a4ac 100644 --- a/library/pem.c +++ b/library/pem.c @@ -44,12 +44,12 @@ #define mbedtls_free free #endif +#if defined(MBEDTLS_PEM_PARSE_C) /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if defined(MBEDTLS_PEM_PARSE_C) void mbedtls_pem_init( mbedtls_pem_context *ctx ) { memset( ctx, 0, sizeof( mbedtls_pem_context ) ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 121c13526..abad0b385 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6482,6 +6482,10 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl ) */ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) { + size_t ep_len = ssl_ep_len( ssl ); + int in_ctr_cmp; + int out_ctr_cmp; + if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER || ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING || ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED ) @@ -6489,8 +6493,12 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) return( 0 ); } - if( memcmp( ssl->in_ctr, ssl->conf->renego_period, 8 ) <= 0 && - memcmp( ssl->out_ctr, ssl->conf->renego_period, 8 ) <= 0 ) + in_ctr_cmp = memcmp( ssl->in_ctr + ep_len, + ssl->conf->renego_period + ep_len, 8 - ep_len ); + out_ctr_cmp = memcmp( ssl->out_ctr + ep_len, + ssl->conf->renego_period + ep_len, 8 - ep_len ); + + if( in_ctr_cmp <= 0 && out_ctr_cmp <= 0 ) { return( 0 ); } @@ -7231,8 +7239,8 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_RENEGOTIATION) conf->renego_max_records = MBEDTLS_SSL_RENEGO_MAX_RECORDS_DEFAULT; - memset( conf->renego_period, 0xFF, 7 ); - conf->renego_period[7] = 0x00; + memset( conf->renego_period, 0x00, 2 ); + memset( conf->renego_period + 2, 0xFF, 6 ); #endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_SRV_C) diff --git a/library/x509_crt.c b/library/x509_crt.c index 80af7259c..056dc16fe 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -969,8 +969,8 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu */ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { - int success = 0, first_error = 0, total_failed = 0; #if defined(MBEDTLS_PEM_PARSE_C) + int success = 0, first_error = 0, total_failed = 0; int buf_format = MBEDTLS_X509_FORMAT_DER; #endif diff --git a/library/x509_csr.c b/library/x509_csr.c index 603d06b64..f92b66c58 100644 --- a/library/x509_csr.c +++ b/library/x509_csr.c @@ -265,8 +265,8 @@ int mbedtls_x509_csr_parse_der( mbedtls_x509_csr *csr, */ int mbedtls_x509_csr_parse( mbedtls_x509_csr *csr, const unsigned char *buf, size_t buflen ) { - int ret; #if defined(MBEDTLS_PEM_PARSE_C) + int ret; size_t use_len; mbedtls_pem_context pem; #endif diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 18bda599f..d98b669b5 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -63,6 +63,8 @@ int main( void ) #include #include #include +#include +#include #if !defined(_WIN32) #include @@ -113,7 +115,7 @@ int main( void ) #define DFL_ALLOW_LEGACY -2 #define DFL_RENEGOTIATE 0 #define DFL_RENEGO_DELAY -2 -#define DFL_RENEGO_PERIOD -1 +#define DFL_RENEGO_PERIOD ( (uint64_t)-1 ) #define DFL_EXCHANGES 1 #define DFL_MIN_VERSION -1 #define DFL_MAX_VERSION -1 @@ -292,7 +294,7 @@ int main( void ) " renegotiation=%%d default: 0 (disabled)\n" \ " renegotiate=%%d default: 0 (disabled)\n" \ " renego_delay=%%d default: -2 (library default)\n" \ - " renego_period=%%d default: (library default)\n" + " renego_period=%%d default: (2^64 - 1 for TLS, 2^48 - 1 for DTLS)\n" #else #define USAGE_RENEGO "" #endif @@ -351,6 +353,19 @@ int main( void ) " force_ciphersuite= default: all enabled\n" \ " acceptable ciphersuite names:\n" + +#define PUT_UINT64_BE(out_be,in_le,i) \ +{ \ + (out_be)[(i) + 0] = (unsigned char)( ( (in_le) >> 56 ) & 0xFF ); \ + (out_be)[(i) + 1] = (unsigned char)( ( (in_le) >> 48 ) & 0xFF ); \ + (out_be)[(i) + 2] = (unsigned char)( ( (in_le) >> 40 ) & 0xFF ); \ + (out_be)[(i) + 3] = (unsigned char)( ( (in_le) >> 32 ) & 0xFF ); \ + (out_be)[(i) + 4] = (unsigned char)( ( (in_le) >> 24 ) & 0xFF ); \ + (out_be)[(i) + 5] = (unsigned char)( ( (in_le) >> 16 ) & 0xFF ); \ + (out_be)[(i) + 6] = (unsigned char)( ( (in_le) >> 8 ) & 0xFF ); \ + (out_be)[(i) + 7] = (unsigned char)( ( (in_le) >> 0 ) & 0xFF ); \ +} + /* * global options */ @@ -377,7 +392,7 @@ struct options int allow_legacy; /* allow legacy renegotiation */ int renegotiate; /* attempt renegotiation? */ int renego_delay; /* delay before enforcing renegotiation */ - int renego_period; /* period for automatic renegotiation */ + uint64_t renego_period; /* period for automatic renegotiation */ int exchanges; /* number of data exchanges */ int min_version; /* minimum protocol version accepted */ int max_version; /* maximum protocol version accepted */ @@ -1041,8 +1056,8 @@ int main( int argc, char *argv[] ) } else if( strcmp( p, "renego_period" ) == 0 ) { - opt.renego_period = atoi( q ); - if( opt.renego_period < 2 || opt.renego_period > 255 ) + if( sscanf( q, "%" SCNu64, &opt.renego_period ) != 1 || + opt.renego_period < 2 ) goto usage; } else if( strcmp( p, "exchanges" ) == 0 ) @@ -1757,7 +1772,7 @@ int main( int argc, char *argv[] ) if( opt.renego_period != DFL_RENEGO_PERIOD ) { - renego_period[7] = opt.renego_period; + PUT_UINT64_BE( renego_period, opt.renego_period, 0 ); mbedtls_ssl_conf_renegotiation_period( &conf, renego_period ); } #endif diff --git a/scripts/data_files/rename-1.3-2.0.txt b/scripts/data_files/rename-1.3-2.0.txt index 397f6beae..cb3381ab8 100644 --- a/scripts/data_files/rename-1.3-2.0.txt +++ b/scripts/data_files/rename-1.3-2.0.txt @@ -1996,7 +1996,6 @@ ssl_set_alpn_protocols mbedtls_ssl_conf_alpn_protocols ssl_set_arc4_support mbedtls_ssl_conf_arc4_support ssl_set_authmode mbedtls_ssl_conf_authmode ssl_set_bio mbedtls_ssl_set_bio -ssl_set_bio_timeout mbedtls_ssl_set_bio_timeout ssl_set_ca_chain mbedtls_ssl_conf_ca_chain ssl_set_cbc_record_splitting mbedtls_ssl_conf_cbc_record_splitting ssl_set_ciphersuites mbedtls_ssl_conf_ciphersuites diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 57155b89d..41fbc3d29 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1601,6 +1601,19 @@ run_test "Renegotiation: DTLS, server-initiated" \ -s "=> renegotiate" \ -s "write hello request" +run_test "Renegotiation: DTLS, renego_period overflow" \ + "$P_SRV debug_level=3 dtls=1 exchanges=4 renegotiation=1 renego_period=18446462598732840962 auth_mode=optional" \ + "$P_CLI debug_level=3 dtls=1 exchanges=4 renegotiation=1" \ + 0 \ + -c "client hello, adding renegotiation extension" \ + -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ + -s "found renegotiation extension" \ + -s "server hello, secure renegotiation extension" \ + -s "record counter limit reached: renegotiate" \ + -c "=> renegotiate" \ + -s "=> renegotiate" \ + -s "write hello request" \ + requires_gnutls run_test "Renegotiation: DTLS, gnutls server, client-initiated" \ "$G_SRV -u --mtu 4096" \