From 5b559ac7ab88849a340e652022f991ebbd8f076b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 3 Aug 2018 09:40:07 +0100 Subject: [PATCH 1/3] Fix mbedtls_ssl_get_record_expansion() for ChaChaPoly and CBC `mbedtls_ssl_get_record_expansion()` is supposed to return the maximum difference between the size of a protected record and the size of the encapsulated plaintext. It had the following two bugs: (1) It did not consider the new ChaChaPoly ciphersuites, returning the error code #MBEDTLS_ERR_SSL_INTERNAL_ERROR in this case. (2) It did not correctly estimate the maximum record expansion in case of CBC ciphersuites in (D)TLS versions 1.1 and higher, in which case the ciphertext is prefixed by an explicit IV. This commit fixes both bugs. --- library/ssl_tls.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 91f96c8ab..5905a6d92 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6841,6 +6841,7 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) { size_t transform_expansion; const mbedtls_ssl_transform *transform = ssl->transform_out; + unsigned block_size; #if defined(MBEDTLS_ZLIB_SUPPORT) if( ssl->session_out->compression != MBEDTLS_SSL_COMPRESS_NULL ) @@ -6854,13 +6855,33 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) { case MBEDTLS_MODE_GCM: case MBEDTLS_MODE_CCM: + case MBEDTLS_MODE_CHACHAPOLY: case MBEDTLS_MODE_STREAM: transform_expansion = transform->minlen; break; case MBEDTLS_MODE_CBC: - transform_expansion = transform->maclen - + mbedtls_cipher_get_block_size( &transform->cipher_ctx_enc ); + + block_size = mbedtls_cipher_get_block_size( + &transform->cipher_ctx_enc ); + +#if defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) + { + /* Expansion due to addition of + * - MAC + * - CBC padding (theoretically up to 256 bytes, but + * we never use more than block_size) + * - explicit IV + */ + transform_expansion = transform->maclen + 2 * block_size; + } + else +#endif /* MBEDTLS_SSL_PROTO_TLS1_1 || MBEDTLS_SSL_PROTO_TLS1_2 */ + { + /* No explicit IV prior to TLS 1.1. */ + transform_expansion = transform->maclen + block_size; + } break; default: From 448146407f9d02aaf577700817fefea123067181 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 3 Aug 2018 09:53:48 +0100 Subject: [PATCH 2/3] Adapt ChangeLog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index bda3de8f5..d5101f409 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,6 +5,9 @@ mbed TLS ChangeLog (Sorted per branch, date) Bugfix * Fixes an issue with MBEDTLS_CHACHAPOLY_C which would not compile if MBEDTLS_ARC4_C and MBEDTLS_CIPHER_NULL_CIPHER weren't also defined. #1890 + * Fix a miscalculation of the maximum record expansion in + mbedtls_ssl_get_record_expansion() in case of ChachaPoly ciphersuites, + or CBC ciphersuites in (D)TLS versions 1.1 or higher. Fixes #1913, #1914. = mbed TLS 2.12.0 branch released 2018-07-25 From 3136ede0e85b135e0212973ef34dd2565eca6e56 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 17 Aug 2018 15:28:19 +0100 Subject: [PATCH 3/3] Compute record expansion in steps to ease readability --- library/ssl_tls.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5905a6d92..1969eaf0c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6839,7 +6839,7 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ) int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) { - size_t transform_expansion; + size_t transform_expansion = 0; const mbedtls_ssl_transform *transform = ssl->transform_out; unsigned block_size; @@ -6865,23 +6865,21 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) block_size = mbedtls_cipher_get_block_size( &transform->cipher_ctx_enc ); + /* Expansion due to the addition of the MAC. */ + transform_expansion += transform->maclen; + + /* Expansion due to the addition of CBC padding; + * Theoretically up to 256 bytes, but we never use + * more than the block size of the underlying cipher. */ + transform_expansion += block_size; + + /* For TLS 1.1 or higher, an explicit IV is added + * after the record header. */ #if defined(MBEDTLS_SSL_PROTO_TLS1_1) || defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_2 ) - { - /* Expansion due to addition of - * - MAC - * - CBC padding (theoretically up to 256 bytes, but - * we never use more than block_size) - * - explicit IV - */ - transform_expansion = transform->maclen + 2 * block_size; - } - else + transform_expansion += block_size; #endif /* MBEDTLS_SSL_PROTO_TLS1_1 || MBEDTLS_SSL_PROTO_TLS1_2 */ - { - /* No explicit IV prior to TLS 1.1. */ - transform_expansion = transform->maclen + block_size; - } + break; default: