From 07eb7ca17c8d8a2095bdb46ce7c244baec6b2fd5 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 CBC modes `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. Previously, 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 this bug. Fixes #1914. --- library/ssl_tls.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c7ccac461..609ff3058 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6450,6 +6450,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 ) @@ -6468,8 +6469,27 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) 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 d3475498e557c1a481924a12ed9488bf5fb1e0a2 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 3e144a7af..3b8ab592c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,9 @@ Bugfix * Add ecc extensions only if an ecc based ciphersuite is used. This improves compliance to RFC 4492, and as a result, solves interoperability issues with BouncyCastle. Raised by milenamil in #1157. + * Fix a miscalculation of the maximum record expansion in + mbedtls_ssl_get_record_expansion() in case of CBC ciphersuites + in (D)TLS versions 1.1 or higher. Fixes #1914. = mbed TLS 2.1.14 branch released 2018-07-25 From 42d267bbe4ff520c9b5dc9a887daee1920427c9c 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 609ff3058..af0799334 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6448,7 +6448,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; @@ -6473,23 +6473,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: