From 20b4408fbd4c5663f73a12d05a31722a8f4a18ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 29 May 2018 14:06:49 +0200 Subject: [PATCH] Fix Lucky13 attack protection when using HMAC-SHA-384 As a protection against the Lucky Thirteen attack, the TLS code for CBC decryption in encrypt-then-MAC mode performs extra MAC calculations to compensate for variations in message size due to padding. The amount of extra MAC calculation to perform was based on the assumption that the bulk of the time is spent in processing 64-byte blocks, which is correct for most supported hashes but not for SHA-384. Correct the amount of extra work for SHA-384 (and SHA-512 which is currently not used in TLS, and MD2 although no one should care about that). --- library/ssl_tls.c | 62 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bc9dc77e1..6fdfb6349 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1985,20 +1985,66 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) { /* * Process MAC and always update for padlen afterwards to make - * total time independent of padlen - * - * extra_run compensates MAC check for padlen + * total time independent of padlen. * * Known timing attacks: * - Lucky Thirteen (http://www.isg.rhul.ac.uk/tls/TLStiming.pdf) * - * We use ( ( Lx + 8 ) / 64 ) to handle 'negative Lx' values - * correctly. (We round down instead of up, so -56 is the correct - * value for our calculations instead of -55) + * To compensate for different timings for the MAC calculation + * depending on how much padding was removed (which is determined + * by padlen), process extra_run more blocks through the hash + * function. + * + * The formula in the paper is + * extra_run = ceil( (L1-55) / 64 ) - ceil( (L2-55) / 64 ) + * where L1 is the size of the header plus the decrypted message + * plus CBC padding and L2 is the size of the header plus the + * decrypted message. This is for an underlying hash function + * with 64-byte blocks. + * We use ( (Lx+8) / 64 ) to handle 'negative Lx' values + * correctly. We round down instead of up, so -56 is the correct + * value for our calculations instead of -55. + * + * Repeat the formula rather than defining a block_size variable + * so that the code only uses division by a constant, not division + * by a variable. */ size_t j, extra_run = 0; - extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - - ( 13 + ssl->in_msglen + 8 ) / 64; + switch( ssl->transform_in->ciphersuite_info->mac ) + { +#if defined(MBEDTLS_MD2_C) + case MBEDTLS_MD_MD2: + /* no size prepended, 64-byte compression blocks */ + extra_run = ( 13 + ssl->in_msglen + padlen ) / 64 - + ( 13 + ssl->in_msglen ) / 64; + break; +#endif +#if defined(MBEDTLS_MD4_C) || defined(MBEDTLS_MD5_C) || \ + defined(MBEDTLS_SHA1_C) || defined(MBEDTLS_SHA224_C) || \ + defined(MBEDTLS_SHA256_C) || defined(MBEDTLS_RIPEMD160_C) + case MBEDTLS_MD_MD4: + case MBEDTLS_MD_MD5: + case MBEDTLS_MD_SHA1: + case MBEDTLS_MD_SHA224: + case MBEDTLS_MD_SHA256: + case MBEDTLS_MD_RIPEMD160: + /* 8 bytes of message size, 64-byte compression blocks */ + extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - + ( 13 + ssl->in_msglen + 8 ) / 64; + break; +#endif +#if defined(MBEDTLS_SHA384_C) || defined(MBEDTLS_SHA512_C) + case MBEDTLS_MD_SHA384: + case MBEDTLS_MD_SHA512: + /* 16 bytes of message size, 128-byte compression blocks */ + extra_run = ( 13 + ssl->in_msglen + padlen + 16 ) / 128 - + ( 13 + ssl->in_msglen + 16 ) / 128; + break; +#endif + default: + MBEDTLS_SSL_DEBUG_MSG( 1, ( "unsupported HMAC hash" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } extra_run &= correct * 0xFF;