diff --git a/ChangeLog b/ChangeLog index 17c71f78e..e3d4175b4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,39 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Security + * Fix a vulnerability in TLS ciphersuites based on CBC and using SHA-384, + in (D)TLS 1.0 to 1.2, that allowed an active network attacker to + partially recover the plaintext of messages under some conditions by + exploiting timing measurements. With DTLS, the attacker could perform + this recovery by sending many messages in the same connection. With TLS + or if mbedtls_ssl_conf_dtls_badmac_limit() was used, the attack only + worked if the same secret (for example a HTTP Cookie) has been repeatedly + sent over connections manipulated by the attacker. Connections using GCM + or CCM instead of CBC, using hash sizes other than SHA-384, or using + Encrypt-then-Mac (RFC 7366) were not affected. The vulnerability was + caused by a miscalculation (for SHA-384) in a countermeasure to the + original Lucky 13 attack. Found by Kenny Paterson, Eyal Ronen and Adi + Shamir. + * Fix a vulnerability in TLS ciphersuites based on CBC, in (D)TLS 1.0 to + 1.2, that allowed a local attacker, able to execute code on the local + machine as well as manipulate network packets, to partially recover the + plaintext of messages under some conditions by using a cache attack + targetting an internal MD/SHA buffer. With TLS or if + mbedtls_ssl_conf_dtls_badmac_limit() was used, the attack only worked if + the same secret (for example a HTTP Cookie) has been repeatedly sent over + connections manipulated by the attacker. Connections using GCM or CCM + instead of CBC or using Encrypt-then-Mac (RFC 7366) were not affected. + Found by Kenny Paterson, Eyal Ronen and Adi Shamir. + * Add a counter-measure against a vulnerability in TLS ciphersuites based + on CBC, in (D)TLS 1.0 to 1.2, that allowed a local attacker, able to + execute code on the local machine as well as manipulate network packets, + to partially recover the plaintext of messages under some conditions (see + previous entry) by using a cache attack targeting the SSL input record + buffer. Connections using GCM or CCM instead of CBC or using + Encrypt-then-Mac (RFC 7366) were not affected. Found by Kenny Paterson, + Eyal Ronen and Adi Shamir. + Bugfix * Fix compilation error on C++, because of a variable named new. Found and fixed by Hirotaka Niisato in #1783. diff --git a/library/md5.c b/library/md5.c index 5d972dc9d..a05e2f0dc 100644 --- a/library/md5.c +++ b/library/md5.c @@ -275,36 +275,51 @@ void mbedtls_md5_update( mbedtls_md5_context *ctx, const unsigned char *input, s } } -static const unsigned char md5_padding[64] = -{ - 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 -}; - /* * MD5 final digest */ void mbedtls_md5_finish( mbedtls_md5_context *ctx, unsigned char output[16] ) { - uint32_t last, padn; + uint32_t used; uint32_t high, low; - unsigned char msglen[8]; + /* + * Add padding: 0x80 then 0x00 until 8 bytes remain for the length + */ + used = ctx->total[0] & 0x3F; + + ctx->buffer[used++] = 0x80; + + if( used <= 56 ) + { + /* Enough room for padding + length in current block */ + memset( ctx->buffer + used, 0, 56 - used ); + } + else + { + /* We'll need an extra block */ + memset( ctx->buffer + used, 0, 64 - used ); + + mbedtls_md5_process( ctx, ctx->buffer ); + + memset( ctx->buffer, 0, 56 ); + } + + /* + * Add message length + */ high = ( ctx->total[0] >> 29 ) | ( ctx->total[1] << 3 ); low = ( ctx->total[0] << 3 ); - PUT_UINT32_LE( low, msglen, 0 ); - PUT_UINT32_LE( high, msglen, 4 ); + PUT_UINT32_LE( low, ctx->buffer, 56 ); + PUT_UINT32_LE( high, ctx->buffer, 60 ); - last = ctx->total[0] & 0x3F; - padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last ); - - mbedtls_md5_update( ctx, md5_padding, padn ); - mbedtls_md5_update( ctx, msglen, 8 ); + mbedtls_md5_process( ctx, ctx->buffer ); + /* + * Output final state + */ PUT_UINT32_LE( ctx->state[0], output, 0 ); PUT_UINT32_LE( ctx->state[1], output, 4 ); PUT_UINT32_LE( ctx->state[2], output, 8 ); diff --git a/library/sha1.c b/library/sha1.c index 8c77cbaa8..8caed8cde 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -307,36 +307,51 @@ void mbedtls_sha1_update( mbedtls_sha1_context *ctx, const unsigned char *input, memcpy( (void *) (ctx->buffer + left), input, ilen ); } -static const unsigned char sha1_padding[64] = -{ - 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 -}; - /* * SHA-1 final digest */ void mbedtls_sha1_finish( mbedtls_sha1_context *ctx, unsigned char output[20] ) { - uint32_t last, padn; + uint32_t used; uint32_t high, low; - unsigned char msglen[8]; + /* + * Add padding: 0x80 then 0x00 until 8 bytes remain for the length + */ + used = ctx->total[0] & 0x3F; + + ctx->buffer[used++] = 0x80; + + if( used <= 56 ) + { + /* Enough room for padding + length in current block */ + memset( ctx->buffer + used, 0, 56 - used ); + } + else + { + /* We'll need an extra block */ + memset( ctx->buffer + used, 0, 64 - used ); + + mbedtls_sha1_process( ctx, ctx->buffer ); + + memset( ctx->buffer, 0, 56 ); + } + + /* + * Add message length + */ high = ( ctx->total[0] >> 29 ) | ( ctx->total[1] << 3 ); low = ( ctx->total[0] << 3 ); - PUT_UINT32_BE( high, msglen, 0 ); - PUT_UINT32_BE( low, msglen, 4 ); + PUT_UINT32_BE( high, ctx->buffer, 56 ); + PUT_UINT32_BE( low, ctx->buffer, 60 ); - last = ctx->total[0] & 0x3F; - padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last ); - - mbedtls_sha1_update( ctx, sha1_padding, padn ); - mbedtls_sha1_update( ctx, msglen, 8 ); + mbedtls_sha1_process( ctx, ctx->buffer ); + /* + * Output final state + */ PUT_UINT32_BE( ctx->state[0], output, 0 ); PUT_UINT32_BE( ctx->state[1], output, 4 ); PUT_UINT32_BE( ctx->state[2], output, 8 ); diff --git a/library/sha256.c b/library/sha256.c index 4e82c0b79..0038cad32 100644 --- a/library/sha256.c +++ b/library/sha256.c @@ -273,36 +273,51 @@ void mbedtls_sha256_update( mbedtls_sha256_context *ctx, const unsigned char *in memcpy( (void *) (ctx->buffer + left), input, ilen ); } -static const unsigned char sha256_padding[64] = -{ - 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 -}; - /* * SHA-256 final digest */ void mbedtls_sha256_finish( mbedtls_sha256_context *ctx, unsigned char output[32] ) { - uint32_t last, padn; + uint32_t used; uint32_t high, low; - unsigned char msglen[8]; + /* + * Add padding: 0x80 then 0x00 until 8 bytes remain for the length + */ + used = ctx->total[0] & 0x3F; + + ctx->buffer[used++] = 0x80; + + if( used <= 56 ) + { + /* Enough room for padding + length in current block */ + memset( ctx->buffer + used, 0, 56 - used ); + } + else + { + /* We'll need an extra block */ + memset( ctx->buffer + used, 0, 64 - used ); + + mbedtls_sha256_process( ctx, ctx->buffer ); + + memset( ctx->buffer, 0, 56 ); + } + + /* + * Add message length + */ high = ( ctx->total[0] >> 29 ) | ( ctx->total[1] << 3 ); low = ( ctx->total[0] << 3 ); - PUT_UINT32_BE( high, msglen, 0 ); - PUT_UINT32_BE( low, msglen, 4 ); + PUT_UINT32_BE( high, ctx->buffer, 56 ); + PUT_UINT32_BE( low, ctx->buffer, 60 ); - last = ctx->total[0] & 0x3F; - padn = ( last < 56 ) ? ( 56 - last ) : ( 120 - last ); - - mbedtls_sha256_update( ctx, sha256_padding, padn ); - mbedtls_sha256_update( ctx, msglen, 8 ); + mbedtls_sha256_process( ctx, ctx->buffer ); + /* + * Output final state + */ PUT_UINT32_BE( ctx->state[0], output, 0 ); PUT_UINT32_BE( ctx->state[1], output, 4 ); PUT_UINT32_BE( ctx->state[2], output, 8 ); diff --git a/library/sha512.c b/library/sha512.c index af610bb43..f62a58dc9 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -302,40 +302,51 @@ void mbedtls_sha512_update( mbedtls_sha512_context *ctx, const unsigned char *in memcpy( (void *) (ctx->buffer + left), input, ilen ); } -static const unsigned char sha512_padding[128] = -{ - 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 -}; - /* * SHA-512 final digest */ void mbedtls_sha512_finish( mbedtls_sha512_context *ctx, unsigned char output[64] ) { - size_t last, padn; + unsigned used; uint64_t high, low; - unsigned char msglen[16]; + /* + * Add padding: 0x80 then 0x00 until 16 bytes remain for the length + */ + used = ctx->total[0] & 0x7F; + + ctx->buffer[used++] = 0x80; + + if( used <= 112 ) + { + /* Enough room for padding + length in current block */ + memset( ctx->buffer + used, 0, 112 - used ); + } + else + { + /* We'll need an extra block */ + memset( ctx->buffer + used, 0, 128 - used ); + + mbedtls_sha512_process( ctx, ctx->buffer ); + + memset( ctx->buffer, 0, 112 ); + } + + /* + * Add message length + */ high = ( ctx->total[0] >> 61 ) | ( ctx->total[1] << 3 ); low = ( ctx->total[0] << 3 ); - PUT_UINT64_BE( high, msglen, 0 ); - PUT_UINT64_BE( low, msglen, 8 ); + PUT_UINT64_BE( high, ctx->buffer, 112 ); + PUT_UINT64_BE( low, ctx->buffer, 120 ); - last = (size_t)( ctx->total[0] & 0x7F ); - padn = ( last < 112 ) ? ( 112 - last ) : ( 240 - last ); - - mbedtls_sha512_update( ctx, sha512_padding, padn ); - mbedtls_sha512_update( ctx, msglen, 16 ); + mbedtls_sha512_process( ctx, ctx->buffer ); + /* + * Output final state + */ PUT_UINT64_BE( ctx->state[0], output, 0 ); PUT_UINT64_BE( ctx->state[1], output, 8 ); PUT_UINT64_BE( ctx->state[2], output, 16 ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0f38faaec..c7ccac461 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1260,6 +1260,27 @@ static void ssl_mac( mbedtls_md_context_t *md_ctx, #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) && \ + ( defined(MBEDTLS_SSL_PROTO_TLS1) || \ + defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ + defined(MBEDTLS_SSL_PROTO_TLS1_2) ) +/* This function makes sure every byte in the memory region is accessed + * (in ascending addresses order) */ +static void ssl_read_memory( unsigned char *p, size_t len ) +{ + unsigned char acc = 0; + volatile unsigned char force; + + for( ; len != 0; p++, len-- ) + acc ^= *p; + + force = acc; + (void) force; +} +#endif /* SSL_SOME_MODES_USE_MAC && ( TLS1 || TLS1_1 || TLS1_2 ) */ + /* * Encryption/decryption functions */ @@ -1935,8 +1956,10 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } +#if defined(MBEDTLS_SSL_DEBUG_ALL) MBEDTLS_SSL_DEBUG_BUF( 4, "raw buffer after decryption", ssl->in_msg, ssl->in_msglen ); +#endif /* * Authenticate if not done yet. @@ -1969,20 +1992,69 @@ 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. + * This avoids requiring division by a variable at runtime + * (which would be marginally less efficient and would require + * linking an extra division function in some builds). */ size_t j, extra_run = 0; - extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - - ( 13 + ssl->in_msglen + 8 ) / 64; + + /* + * The next two sizes are the minimum and maximum values of + * in_msglen over all padlen values. + * + * They're independent of padlen, since we previously did + * in_msglen -= padlen. + * + * Note that max_len + maclen is never more than the buffer + * length, as we previously did in_msglen -= maclen too. + */ + const size_t max_len = ssl->in_msglen + padlen; + const size_t min_len = ( max_len > 256 ) ? max_len - 256 : 0; + + switch( ssl->transform_in->ciphersuite_info->mac ) + { +#if defined(MBEDTLS_MD5_C) || defined(MBEDTLS_SHA1_C) || \ + defined(MBEDTLS_SHA256_C) + case MBEDTLS_MD_MD5: + case MBEDTLS_MD_SHA1: + case MBEDTLS_MD_SHA256: + /* 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_SHA512_C) + case MBEDTLS_MD_SHA384: + /* 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, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } extra_run &= correct * 0xFF; @@ -1991,12 +2063,25 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_len, 2 ); mbedtls_md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg, ssl->in_msglen ); + /* Make sure we access everything even when padlen > 0. This + * makes the synchronisation requirements for just-in-time + * Prime+Probe attacks much tighter and hopefully impractical. */ + ssl_read_memory( ssl->in_msg + ssl->in_msglen, padlen ); mbedtls_md_hmac_finish( &ssl->transform_in->md_ctx_dec, mac_expect ); - /* Call mbedtls_md_process at least once due to cache attacks */ + + /* Call mbedtls_md_process at least once due to cache attacks + * that observe whether md_process() was called of not */ for( j = 0; j < extra_run + 1; j++ ) mbedtls_md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg ); mbedtls_md_hmac_reset( &ssl->transform_in->md_ctx_dec ); + + /* Make sure we access all the memory that could contain the MAC, + * before we check it in the next code block. This makes the + * synchronisation requirements for just-in-time Prime+Probe + * attacks much tighter and hopefully impractical. */ + ssl_read_memory( ssl->in_msg + min_len, + max_len - min_len + ssl->transform_in->maclen ); } else #endif /* MBEDTLS_SSL_PROTO_TLS1 || MBEDTLS_SSL_PROTO_TLS1_1 || \ @@ -2006,9 +2091,11 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } +#if defined(MBEDTLS_SSL_DEBUG_ALL) MBEDTLS_SSL_DEBUG_BUF( 4, "expected mac", mac_expect, ssl->transform_in->maclen ); MBEDTLS_SSL_DEBUG_BUF( 4, "message mac", ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen ); +#endif if( mbedtls_ssl_safer_memcmp( ssl->in_msg + ssl->in_msglen, mac_expect, ssl->transform_in->maclen ) != 0 )