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() does not consider ChaChaPoly suites #1913

Closed
hanno-arm opened this Issue Aug 3, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@hanno-arm
Contributor

hanno-arm commented Aug 3, 2018

Description

  • Type: Bug
  • Priority: Major

Bug

OS
All

Mbed TLS build:
Version: 2.12
Configuration: ChaChaPoly ciphersuites enabled (e.g., default configuration)

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 ) );
}

PR #1752 introduced ChaChaPoly ciphersuites, the cipher mode for which is MBEDTLS_MODE_CHACHAPOLY, but this mode is not yet considered in mbedtls_ssl_get_record_expansion().


Impact

Internal

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.

Users

Users relying on mbedtls_ssl_get_record_expansion() in any way will observe a failure with error MBEDTLS_ERR_SSL_INTERNAL_ERROR when enabling to ChaChaPoly ciphersuites (default).

@hanno-arm hanno-arm changed the title from `mbedtls_ssl_get_record_expansion()` does not consider ChaChaPoly suites to mbedtls_ssl_get_record_expansion() does not consider ChaChaPoly suites Aug 3, 2018

@ciarmcom

This comment has been minimized.

Member

ciarmcom commented Aug 3, 2018

ARM Internal Ref: IOTSSL-2463

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