Correct space needed for MAC in case of NULL cipher

The macro constant `MBEDTLS_SSL_MAC_ADD` defined in `ssl_internal.h`
defines an upper bound for the amount of space needed for the record
authentication tag. Its definition distinguishes between the
presence of an ARC4 or CBC ciphersuite suite, in which case the maximum
size of an enabled SHA digest is used; otherwise, `MBEDTLS_SSL_MAC_ADD`
is set to 16 to accomodate AEAD authentication tags.

This assignment has a flaw in the situation where confidentiality is
not needed and the NULL cipher is in use. In this case, the
authentication tag also uses a SHA digest, but the definition of
`MBEDTLS_SSL_MAC_ADD` doesn't guarantee enough space.

The present commit fixes this by distinguishing between the presence
of *some* ciphersuite using a MAC, including those using a NULL cipher.
For that, the previously internal macro `SSL_SOME_MODES_USE_MAC` from
`ssl_tls.c` is renamed and moved to the public macro
`MBEDTLS_SOME_MODES_USE_MAC` defined in `ssl_internal.h`.
This commit is contained in:
Hanno Becker 2018-01-03 15:24:20 +00:00
parent e694c3ef3e
commit 52344c2972
2 changed files with 16 additions and 14 deletions

View file

@ -146,7 +146,15 @@
#define MBEDTLS_SSL_COMPRESSION_ADD 0
#endif
#if defined(MBEDTLS_ARC4_C) || defined(MBEDTLS_CIPHER_MODE_CBC)
#if defined(MBEDTLS_ARC4_C) || defined(MBEDTLS_CIPHER_NULL_CIPHER) || \
( defined(MBEDTLS_CIPHER_MODE_CBC) && \
( defined(MBEDTLS_AES_C) || \
defined(MBEDTLS_CAMELLIA_C) || \
defined(MBEDTLS_ARIA_C) ) )
#define MBEDTLS_SSL_SOME_MODES_USE_MAC
#endif
#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC)
/* Ciphersuites using HMAC */
#if defined(MBEDTLS_SHA512_C)
#define MBEDTLS_SSL_MAC_ADD 48 /* SHA-384 used for HMAC */
@ -155,7 +163,7 @@
#else
#define MBEDTLS_SSL_MAC_ADD 20 /* SHA-1 used for HMAC */
#endif
#else
#else /* MBEDTLS_SSL_SOME_MODES_USE_MAC */
/* AEAD ciphersuites: GCM and CCM use a 128 bits tag */
#define MBEDTLS_SSL_MAC_ADD 16
#endif

View file

@ -1710,15 +1710,9 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx,
}
#endif /* MBEDTLS_SSL_PROTO_SSL3 */
#if defined(MBEDTLS_ARC4_C) || defined(MBEDTLS_CIPHER_NULL_CIPHER) || \
( defined(MBEDTLS_CIPHER_MODE_CBC) && \
( defined(MBEDTLS_AES_C) || defined(MBEDTLS_CAMELLIA_C) || defined(MBEDTLS_ARIA_C)) )
#define SSL_SOME_MODES_USE_MAC
#endif
/* The function below is only used in the Lucky 13 counter-measure in
* ssl_decrypt_buf(). These are the defines that guard the call site. */
#if defined(SSL_SOME_MODES_USE_MAC) && \
#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC) && \
( defined(MBEDTLS_SSL_PROTO_TLS1) || \
defined(MBEDTLS_SSL_PROTO_TLS1_1) || \
defined(MBEDTLS_SSL_PROTO_TLS1_2) )
@ -1761,7 +1755,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl )
/*
* Add MAC before if needed
*/
#if defined(SSL_SOME_MODES_USE_MAC)
#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC)
if( mode == MBEDTLS_MODE_STREAM ||
( mode == MBEDTLS_MODE_CBC
#if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC)
@ -1814,7 +1808,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl )
ssl->out_msglen += ssl->transform_out->maclen;
auth_done++;
}
#endif /* AEAD not the only option */
#endif /* MBEDTLS_SSL_SOME_MODES_USE_MAC */
/*
* Encrypt
@ -2090,7 +2084,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
{
mbedtls_cipher_mode_t mode;
int auth_done = 0;
#if defined(SSL_SOME_MODES_USE_MAC)
#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC)
size_t padlen = 0, correct = 1;
#endif
@ -2470,7 +2464,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
* Authenticate if not done yet.
* Compute the MAC regardless of the padding result (RFC4346, CBCTIME).
*/
#if defined(SSL_SOME_MODES_USE_MAC)
#if defined(MBEDTLS_SSL_SOME_MODES_USE_MAC)
if( auth_done == 0 )
{
unsigned char mac_expect[MBEDTLS_SSL_MAC_ADD];
@ -2618,7 +2612,7 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl )
*/
if( correct == 0 )
return( MBEDTLS_ERR_SSL_INVALID_MAC );
#endif /* SSL_SOME_MODES_USE_MAC */
#endif /* MBEDTLS_SSL_SOME_MODES_USE_MAC */
/* Make extra sure authentication was performed, exactly once */
if( auth_done != 1 )