yuzu-emu/mbedtls
1
0
Fork 0
mirror of https://github.com/yuzu-emu/mbedtls.git synced 2024-11-22 17:05:36 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix renegotiation at incorrect times in DTLS

Browse Source 
Fix an incorrect condition in ssl_check_ctr_renegotiate() that compared
64 bits of record counter instead of 48 bits as described in RFC 6347
Section 4.3.1. This would cause the function's return value to be
occasionally incorrect and the renegotiation routines to be triggered
at unexpected times.
This commit is contained in:
Andres AG 2016-12-15 17:01:16 +00:00 committed by Simon Butcher
parent 747fceb785
commit 2196c7f81c
3 changed files with 21 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ChangeLog
View File

@ -8,6 +8,11 @@ Bugfix
* Fix unused variable/function compilation warnings in pem.c, x509_crt.c and * 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 x509_csr.c that are reported when building mbed TLS with a config.h that
does not define MBEDTLS_PEM_PARSE_C. #562 does not define MBEDTLS_PEM_PARSE_C. #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 multiple buffer overreads in mbedtls_pem_read_buffer() when parsing * Fixed multiple buffer overreads in mbedtls_pem_read_buffer() when parsing
the input string in PEM format to extract the different components. Found the input string in PEM format to extract the different components. Found
by Eyal Itkin. by Eyal Itkin.

6
include/mbedtls/ssl.h
View File

@ -2183,7 +2183,7 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_
/** /**
* \brief Set record counter threshold for periodic renegotiation. * \brief Set record counter threshold for periodic renegotiation.
* (Default: 2^64 - 256.) * (Default: 2^48 - 1)
* *
* Renegotiation is automatically triggered when a record * Renegotiation is automatically triggered when a record
* counter (outgoing or ingoing) crosses the defined * counter (outgoing or ingoing) crosses the defined
@ -2194,9 +2194,11 @@ void mbedtls_ssl_conf_renegotiation_enforced( mbedtls_ssl_config *conf, int max_
* Lower values can be used to enforce policies such as "keys * Lower values can be used to enforce policies such as "keys
* must be refreshed every N packets with cipher X". * must be refreshed every N packets with cipher X".
* *
* \note When the transport is set to MBEDTLS_SSL_TRANSPORT_DATAGRAM,
* the maximum renegotiation period is 2^48 - 1.
*
* \param conf SSL configuration * \param conf SSL configuration
* \param period The threshold value: a big-endian 64-bit number. * \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, void mbedtls_ssl_conf_renegotiation_period( mbedtls_ssl_config *conf,
const unsigned char period[8] ); const unsigned char period[8] );

16
library/ssl_tls.c
View File

@ -6482,6 +6482,10 @@ int mbedtls_ssl_renegotiate( mbedtls_ssl_context *ssl )
*/ */
static int ssl_check_ctr_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 || if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER ||
ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING || ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING ||
ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED ) ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED )
@ -6489,8 +6493,12 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl )
return( 0 ); return( 0 );
} }
if( memcmp( ssl->in_ctr, ssl->conf->renego_period, 8 ) <= 0 && in_ctr_cmp = memcmp( ssl->in_ctr + ep_len,
memcmp( ssl->out_ctr, ssl->conf->renego_period, 8 ) <= 0 ) 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 ); return( 0 );
} }
@ -7231,8 +7239,8 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf,
#if defined(MBEDTLS_SSL_RENEGOTIATION) #if defined(MBEDTLS_SSL_RENEGOTIATION)
conf->renego_max_records = MBEDTLS_SSL_RENEGO_MAX_RECORDS_DEFAULT; conf->renego_max_records = MBEDTLS_SSL_RENEGO_MAX_RECORDS_DEFAULT;
memset( conf->renego_period, 0xFF, 7 ); memset( conf->renego_period, 0x00, 2 );
conf->renego_period[7] = 0x00; memset( conf->renego_period + 2, 0xFF, 6 );
#endif #endif
#if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_SRV_C) #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_SRV_C)