From dd3ab13da3fd3dc9ec2d3d247c25ac954ca66f5e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 17 Oct 2018 14:43:14 +0100 Subject: [PATCH 1/2] Fail when encountering invalid CBC padding in EtM records This commit changes the behavior of the record decryption routine `ssl_decrypt_buf()` in the following situation: 1. A CBC ciphersuite with Encrypt-then-MAC is used. 2. A record with valid MAC but invalid CBC padding is received. In this situation, the previous code would not raise and error but instead forward the decrypted packet, including the wrong padding, to the user. This commit changes this behavior to return the error MBEDTLS_ERR_SSL_INVALID_MAC instead. While erroneous, the previous behavior does not constitute a security flaw since it can only happen for properly authenticated records, that is, if the peer makes a mistake while preparing the padded plaintext. --- library/ssl_tls.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8bd74db8d7..6afb624c5d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2304,13 +2304,13 @@ static int ssl_decrypt_buf( mbedtls_ssl_context *ssl ) correct = 0; } auth_done++; - - /* - * Finally check the correct flag - */ - if( correct == 0 ) - return( MBEDTLS_ERR_SSL_INVALID_MAC ); } + + /* + * Finally check the correct flag + */ + if( correct == 0 ) + return( MBEDTLS_ERR_SSL_INVALID_MAC ); #endif /* SSL_SOME_MODES_USE_MAC */ /* Make extra sure authentication was performed, exactly once */ From f24c3360fc5550dd748e6c4bf0b04a05ae07c233 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 17 Oct 2018 14:53:05 +0100 Subject: [PATCH 2/2] Adapt ChangeLog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 513f24f3ab..acf1bd7ac4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,9 @@ Bugfix invalidated keys of a lifetime of less than a 1s. Fixes #1968. * Fix failure in hmac_drbg in the benchmark sample application, when MBEDTLS_THREADING_C is defined. Found by TrinityTonic, #1095 + * Fix a bug in the record decryption routine ssl_decrypt_buf() + which lead to accepting properly authenticated but improperly + padded records in case of CBC ciphersuites using Encrypt-then-MAC. Changes * Add tests for session resumption in DTLS.