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>
diff --git a/library/ssl_msg.c b/library/ssl_msg.c
index 76cc2b1..cf2eab5 100644
--- a/library/ssl_msg.c
+++ b/library/ssl_msg.c
@@ -384,7 +384,8 @@
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 @@
*
* 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 @@
*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 @@
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 @@
* 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 @@
}
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 @@
* 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 @@
* 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 @@
*/
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)
/*
