mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2025-03-28 19:21:08 +00:00
Fix AEAD additional data computation for TLS 1.3
The AEAD additional data (AAD) is computed differently in TLS 1.3 compared to TLS 1.2, but this change hasn't yet been reflected in the codee, rendering the current implementation of ``` mbedtls_ssl_{encrypt,decrypt}_buf() ``` not standard compliant. This commit fixes this by adjusting the AAD extraction function ssl_extract_add_data_from_record() and its call-sites. Please see the documentation of the code for an explanation of how the AAD has changed from TLS 1.2 to TLS 1.3. Signed-off-by: Hanno Becker <hanno.becker@arm.com>
This commit is contained in:
parent
c94060c641
commit
79e2d1b6f6
@ -384,7 +384,8 @@ static int ssl_parse_inner_plaintext( unsigned char const *content,
|
||||
static void ssl_extract_add_data_from_record( unsigned char* add_data,
|
||||
size_t *add_data_len,
|
||||
mbedtls_record *rec,
|
||||
unsigned minor_ver )
|
||||
unsigned minor_ver,
|
||||
size_t taglen )
|
||||
{
|
||||
/* Quoting RFC 5246 (TLS 1.2):
|
||||
*
|
||||
@ -403,15 +404,37 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data,
|
||||
*
|
||||
* For TLS 1.3, the record sequence number is dropped from the AAD
|
||||
* and encoded within the nonce of the AEAD operation instead.
|
||||
* Moreover, the additional data involves the length of the TLS
|
||||
* ciphertext, not the TLS plaintext as in earlier versions.
|
||||
* Quoting RFC 8446 (TLS 1.3):
|
||||
*
|
||||
* additional_data = TLSCiphertext.opaque_type ||
|
||||
* TLSCiphertext.legacy_record_version ||
|
||||
* TLSCiphertext.length
|
||||
*
|
||||
* We pass the tag length to this function in order to compute the
|
||||
* ciphertext length from the inner plaintext length rec->data_len via
|
||||
*
|
||||
* TLSCiphertext.length = TLSInnerPlaintext.length + taglen.
|
||||
*
|
||||
*/
|
||||
|
||||
unsigned char *cur = add_data;
|
||||
size_t ad_len_field = rec->data_len;
|
||||
|
||||
#if defined(MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL)
|
||||
if( minor_ver != MBEDTLS_SSL_MINOR_VERSION_4 )
|
||||
if( minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 )
|
||||
{
|
||||
/* In TLS 1.3, the AAD contains the length of the TLSCiphertext,
|
||||
* which differs from the length of the TLSInnerPlaintext
|
||||
* by the length of the authentication tag. */
|
||||
ad_len_field += taglen;
|
||||
}
|
||||
else
|
||||
#endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */
|
||||
{
|
||||
((void) minor_ver);
|
||||
((void) taglen);
|
||||
memcpy( cur, rec->ctr, sizeof( rec->ctr ) );
|
||||
cur += sizeof( rec->ctr );
|
||||
}
|
||||
@ -431,15 +454,15 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data,
|
||||
*cur = rec->cid_len;
|
||||
cur++;
|
||||
|
||||
cur[0] = ( rec->data_len >> 8 ) & 0xFF;
|
||||
cur[1] = ( rec->data_len >> 0 ) & 0xFF;
|
||||
cur[0] = ( ad_len_field >> 8 ) & 0xFF;
|
||||
cur[1] = ( ad_len_field >> 0 ) & 0xFF;
|
||||
cur += 2;
|
||||
}
|
||||
else
|
||||
#endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */
|
||||
{
|
||||
cur[0] = ( rec->data_len >> 8 ) & 0xFF;
|
||||
cur[1] = ( rec->data_len >> 0 ) & 0xFF;
|
||||
cur[0] = ( ad_len_field >> 8 ) & 0xFF;
|
||||
cur[1] = ( ad_len_field >> 0 ) & 0xFF;
|
||||
cur += 2;
|
||||
}
|
||||
|
||||
@ -646,7 +669,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl,
|
||||
unsigned char mac[MBEDTLS_SSL_MAC_ADD];
|
||||
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len, rec,
|
||||
transform->minor_ver );
|
||||
transform->minor_ver,
|
||||
transform->taglen );
|
||||
|
||||
mbedtls_md_hmac_update( &transform->md_ctx_enc, add_data,
|
||||
add_data_len );
|
||||
@ -743,7 +767,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl,
|
||||
* This depends on the TLS version.
|
||||
*/
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len, rec,
|
||||
transform->minor_ver );
|
||||
transform->minor_ver,
|
||||
transform->taglen );
|
||||
|
||||
MBEDTLS_SSL_DEBUG_BUF( 4, "IV used (internal)",
|
||||
iv, transform->ivlen );
|
||||
@ -897,7 +922,8 @@ int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl,
|
||||
}
|
||||
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len,
|
||||
rec, transform->minor_ver );
|
||||
rec, transform->minor_ver,
|
||||
transform->taglen );
|
||||
|
||||
MBEDTLS_SSL_DEBUG_MSG( 3, ( "using encrypt then mac" ) );
|
||||
MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", add_data,
|
||||
@ -1304,7 +1330,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
|
||||
* This depends on the TLS version.
|
||||
*/
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len, rec,
|
||||
transform->minor_ver );
|
||||
transform->minor_ver,
|
||||
transform->taglen );
|
||||
MBEDTLS_SSL_DEBUG_BUF( 4, "additional data used for AEAD",
|
||||
add_data, add_data_len );
|
||||
|
||||
@ -1414,7 +1441,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
|
||||
* Further, we still know that data_len > minlen */
|
||||
rec->data_len -= transform->maclen;
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len, rec,
|
||||
transform->minor_ver );
|
||||
transform->minor_ver,
|
||||
transform->taglen );
|
||||
|
||||
/* Calculate expected MAC. */
|
||||
MBEDTLS_SSL_DEBUG_BUF( 4, "MAC'd meta-data", add_data,
|
||||
@ -1606,7 +1634,8 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl,
|
||||
*/
|
||||
rec->data_len -= transform->maclen;
|
||||
ssl_extract_add_data_from_record( add_data, &add_data_len, rec,
|
||||
transform->minor_ver );
|
||||
transform->minor_ver,
|
||||
transform->taglen );
|
||||
|
||||
#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
|
||||
/*
|
||||
|
Loading…
x
Reference in New Issue
Block a user