Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls_ssl_get_record_expansion() returns wrong result for CBC suites in TLS >= 1.1 #1914

Closed
hanno-becker opened this issue Aug 3, 2018 · 2 comments

Comments

@hanno-becker
Copy link

Description


Bug

OS
All

Mbed TLS build:
Version: all versions supporting (D)TLS 1.1 or higher.
Configuration: Any configuration allowing (D)TLS 1.1 or higher.

Description
The public function mbedtls_ssl_get_record_expansion() is supposed to return the maximum difference between the length of a protected record and the length of the plaintext it encapsulates, with respect to the currently enabled outgoing record protection.

int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl )
{
    size_t transform_expansion;
    const mbedtls_ssl_transform *transform = ssl->transform_out;

#if defined(MBEDTLS_ZLIB_SUPPORT)
    if( ssl->session_out->compression != MBEDTLS_SSL_COMPRESS_NULL )
        return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
#endif

    if( transform == NULL )
        return( (int) mbedtls_ssl_hdr_len( ssl ) );

    switch( mbedtls_cipher_get_cipher_mode( &transform->cipher_ctx_enc ) )
    {
        case MBEDTLS_MODE_GCM:
        case MBEDTLS_MODE_CCM:
        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 );
            break;

        default:
            MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
            return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
    }

    return( (int)( mbedtls_ssl_hdr_len( ssl ) + transform_expansion ) );
}

In (D)TLS 1.1 or higher, CBC ciphersuites use an explicit initialization vector at the beginning of the record which is not taken into account in

        case MBEDTLS_MODE_CBC:
            transform_expansion = transform->maclen
                      + mbedtls_cipher_get_block_size( &transform->cipher_ctx_enc );

In total, there are three components contributing to the expansion: Pre-expansion due to explicit IV, post-expansion due to MAC, post-expansion due to padding.


Impact

Currently, there is no internal impact on the library as Mbed TLS does not use mbedtls_ssl_get_record_expansion(). However, PR #1879 starts to make use of mbedtls_ssl_get_record_expansion() to deduce the maximum plaintext length from the maximum MTU, and miscalculation will lead to Mbed TLS not obeying the MTU.

@ciarmcom
Copy link

ciarmcom commented Aug 3, 2018

ARM Internal Ref: IOTSSL-2462

mpg added a commit to mpg/mbedtls that referenced this issue Aug 13, 2018
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Aug 14, 2018
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Aug 17, 2018
`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 Mbed-TLS#1914.
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Aug 17, 2018
`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 Mbed-TLS#1914.
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Aug 17, 2018
`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 Mbed-TLS#1914.
hanno-becker pushed a commit to hanno-becker/mbedtls that referenced this issue Aug 17, 2018
`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 Mbed-TLS#1914.
@simonbutcher
Copy link
Contributor

This issue has been fixed in PR's #1915 and back ports #1959 and #1960, which have now been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants