From 7109624aef4c88f4a36e22c2f666b5f598fc3c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 25 Oct 2013 19:31:25 +0200 Subject: [PATCH] Skip MAC computation/check when GCM is used --- library/ssl_tls.c | 200 +++++++++++++++++++++++++--------------------- 1 file changed, 110 insertions(+), 90 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6164a4160..b1e502151 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -973,41 +973,53 @@ static int ssl_encrypt_buf( ssl_context *ssl ) SSL_DEBUG_MSG( 2, ( "=> encrypt buf" ) ); /* - * Add MAC then encrypt + * Add MAC before encrypt, except for GCM */ -#if defined(POLARSSL_SSL_PROTO_SSL3) - if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) +#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER) || \ + ( defined(POLARSSL_CIPHER_MODE_CBC) && \ + ( defined(POLARSSL_AES_C) || defined(POLARSSL_CAMELLIA_C) ) ) + if( ssl->transform_out->cipher_ctx_enc.cipher_info->mode != + POLARSSL_MODE_GCM ) { - ssl_mac( &ssl->transform_out->md_ctx_enc, - ssl->transform_out->mac_enc, - ssl->out_msg, ssl->out_msglen, - ssl->out_ctr, ssl->out_msgtype ); - } - else +#if defined(POLARSSL_SSL_PROTO_SSL3) + if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) + { + ssl_mac( &ssl->transform_out->md_ctx_enc, + ssl->transform_out->mac_enc, + ssl->out_msg, ssl->out_msglen, + ssl->out_ctr, ssl->out_msgtype ); + } + else #endif #if defined(POLARSSL_SSL_PROTO_TLS1) || defined(POLARSSL_SSL_PROTO_TLS1_1) || \ - defined(POLARSSL_SSL_PROTO_TLS1_2) - if( ssl->minor_ver >= SSL_MINOR_VERSION_1 ) - { - md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 13 ); - md_hmac_update( &ssl->transform_out->md_ctx_enc, - ssl->out_msg, ssl->out_msglen ); - md_hmac_finish( &ssl->transform_out->md_ctx_enc, - ssl->out_msg + ssl->out_msglen ); - md_hmac_reset( &ssl->transform_out->md_ctx_enc ); - } - else + defined(POLARSSL_SSL_PROTO_TLS1_2) + if( ssl->minor_ver >= SSL_MINOR_VERSION_1 ) + { + md_hmac_update( &ssl->transform_out->md_ctx_enc, ssl->out_ctr, 13 ); + md_hmac_update( &ssl->transform_out->md_ctx_enc, + ssl->out_msg, ssl->out_msglen ); + md_hmac_finish( &ssl->transform_out->md_ctx_enc, + ssl->out_msg + ssl->out_msglen ); + md_hmac_reset( &ssl->transform_out->md_ctx_enc ); + } + else #endif - { - SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE ); + { + SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE ); + } + + SSL_DEBUG_BUF( 4, "computed mac", + ssl->out_msg + ssl->out_msglen, + ssl->transform_out->maclen ); + + ssl->out_msglen += ssl->transform_out->maclen; } +#endif /* GCM not the only option */ - SSL_DEBUG_BUF( 4, "computed mac", - ssl->out_msg + ssl->out_msglen, ssl->transform_out->maclen ); - - ssl->out_msglen += ssl->transform_out->maclen; - + /* + * Encrypt + */ #if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER) if( ssl->transform_out->cipher_ctx_enc.cipher_info->mode == POLARSSL_MODE_STREAM ) @@ -1634,84 +1646,92 @@ static int ssl_decrypt_buf( ssl_context *ssl ) ssl->in_msg, ssl->in_msglen ); /* - * Always compute the MAC (RFC4346, CBCTIME). + * Always compute the MAC (RFC4346, CBCTIME), except for GCM of course */ - ssl->in_msglen -= ( ssl->transform_in->maclen + padlen ); +#if defined(POLARSSL_ARC4_C) || defined(POLARSSL_CIPHER_NULL_CIPHER) || \ + ( defined(POLARSSL_CIPHER_MODE_CBC) && \ + ( defined(POLARSSL_AES_C) || defined(POLARSSL_CAMELLIA_C) ) ) + if( ssl->transform_in->cipher_ctx_dec.cipher_info->mode != + POLARSSL_MODE_GCM ) + { + ssl->in_msglen -= ( ssl->transform_in->maclen + padlen ); - ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 ); - ssl->in_hdr[4] = (unsigned char)( ssl->in_msglen ); + ssl->in_hdr[3] = (unsigned char)( ssl->in_msglen >> 8 ); + ssl->in_hdr[4] = (unsigned char)( ssl->in_msglen ); - memcpy( tmp, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen ); + memcpy( tmp, ssl->in_msg + ssl->in_msglen, ssl->transform_in->maclen ); #if defined(POLARSSL_SSL_PROTO_SSL3) - if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) - { - ssl_mac( &ssl->transform_in->md_ctx_dec, - ssl->transform_in->mac_dec, - ssl->in_msg, ssl->in_msglen, - ssl->in_ctr, ssl->in_msgtype ); - } - else + if( ssl->minor_ver == SSL_MINOR_VERSION_0 ) + { + ssl_mac( &ssl->transform_in->md_ctx_dec, + ssl->transform_in->mac_dec, + ssl->in_msg, ssl->in_msglen, + ssl->in_ctr, ssl->in_msgtype ); + } + else #endif /* POLARSSL_SSL_PROTO_SSL3 */ #if defined(POLARSSL_SSL_PROTO_TLS1) || defined(POLARSSL_SSL_PROTO_TLS1_1) || \ - defined(POLARSSL_SSL_PROTO_TLS1_2) - if( ssl->minor_ver > SSL_MINOR_VERSION_0 ) - { - /* - * Process MAC and always update for padlen afterwards to make - * total time independent of padlen - * - * extra_run compensates MAC check for 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) - */ - size_t j, extra_run = 0; - extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - - ( 13 + ssl->in_msglen + 8 ) / 64; + defined(POLARSSL_SSL_PROTO_TLS1_2) + if( ssl->minor_ver > SSL_MINOR_VERSION_0 ) + { + /* + * Process MAC and always update for padlen afterwards to make + * total time independent of padlen + * + * extra_run compensates MAC check for 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) + */ + size_t j, extra_run = 0; + extra_run = ( 13 + ssl->in_msglen + padlen + 8 ) / 64 - + ( 13 + ssl->in_msglen + 8 ) / 64; - extra_run &= correct * 0xFF; + extra_run &= correct * 0xFF; - md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_ctr, 13 ); - md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg, - ssl->in_msglen ); - md_hmac_finish( &ssl->transform_in->md_ctx_dec, - ssl->in_msg + ssl->in_msglen ); - for( j = 0; j < extra_run; j++ ) - md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg ); + md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_ctr, 13 ); + md_hmac_update( &ssl->transform_in->md_ctx_dec, ssl->in_msg, + ssl->in_msglen ); + md_hmac_finish( &ssl->transform_in->md_ctx_dec, + ssl->in_msg + ssl->in_msglen ); + for( j = 0; j < extra_run; j++ ) + md_process( &ssl->transform_in->md_ctx_dec, ssl->in_msg ); - md_hmac_reset( &ssl->transform_in->md_ctx_dec ); - } - else + md_hmac_reset( &ssl->transform_in->md_ctx_dec ); + } + else #endif /* POLARSSL_SSL_PROTO_TLS1 || POLARSSL_SSL_PROTO_TLS1_1 || \ - POLARSSL_SSL_PROTO_TLS1_2 */ - { - SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE ); - } + POLARSSL_SSL_PROTO_TLS1_2 */ + { + SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( POLARSSL_ERR_SSL_FEATURE_UNAVAILABLE ); + } - SSL_DEBUG_BUF( 4, "message mac", tmp, ssl->transform_in->maclen ); - SSL_DEBUG_BUF( 4, "computed mac", ssl->in_msg + ssl->in_msglen, - ssl->transform_in->maclen ); + SSL_DEBUG_BUF( 4, "message mac", tmp, ssl->transform_in->maclen ); + SSL_DEBUG_BUF( 4, "computed mac", ssl->in_msg + ssl->in_msglen, + ssl->transform_in->maclen ); - if( memcmp( tmp, ssl->in_msg + ssl->in_msglen, - ssl->transform_in->maclen ) != 0 ) - { + if( memcmp( tmp, ssl->in_msg + ssl->in_msglen, + ssl->transform_in->maclen ) != 0 ) + { #if defined(POLARSSL_SSL_DEBUG_ALL) - SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); + SSL_DEBUG_MSG( 1, ( "message mac does not match" ) ); #endif - correct = 0; - } + correct = 0; + } - /* - * Finally check the correct flag - */ - if( correct == 0 ) - return( POLARSSL_ERR_SSL_INVALID_MAC ); + /* + * Finally check the correct flag + */ + if( correct == 0 ) + return( POLARSSL_ERR_SSL_INVALID_MAC ); + } +#endif /* GCM not the only option */ if( ssl->in_msglen == 0 ) {