From f5e753a942053df4be097b19242e33d9d7870ddb Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 30 May 2017 17:18:06 +0100 Subject: [PATCH] Add missing ret code checks in PEM module Add missing return code checks in the functions pem_des_decrypt(), pem_3des_decrypt() and pem_aes_decrypt() so that the calling function pem_read_buffer() is notified of errors reported by the crypto primitives AES, DES and 3DES. --- ChangeLog | 9 +++++++ library/pem.c | 66 +++++++++++++++++++++++++++++++++------------------ 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index b46c72879..0638b6959 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.3.x branch released xxxx-xx-xx + +Bugfix + * Fix unchecked return codes from AES, DES and 3DES functions in + pem_aes_decrypt(), pem_des_decrypt() and pem_des3_decrypt() respectively. + If a call to one of the functions of the cryptographic primitive modules + failed, the error may not be noticed by the function pem_read_buffer() + causing it to return invalid values. Found by Guido Vranken. #756 + = mbed TLS 1.3.19 branch released 2017-03-08 Security diff --git a/library/pem.c b/library/pem.c index b2c16c292..08c182f18 100644 --- a/library/pem.c +++ b/library/pem.c @@ -135,45 +135,53 @@ static void pem_pbkdf1( unsigned char *key, size_t keylen, /* * Decrypt with DES-CBC, using PBKDF1 for key derivation */ -static void pem_des_decrypt( unsigned char des_iv[8], - unsigned char *buf, size_t buflen, - const unsigned char *pwd, size_t pwdlen ) +static int pem_des_decrypt( unsigned char des_iv[8], + unsigned char *buf, size_t buflen, + const unsigned char *pwd, size_t pwdlen ) { des_context des_ctx; unsigned char des_key[8]; + int ret; des_init( &des_ctx ); pem_pbkdf1( des_key, 8, des_iv, pwd, pwdlen ); - des_setkey_dec( &des_ctx, des_key ); - des_crypt_cbc( &des_ctx, DES_DECRYPT, buflen, - des_iv, buf, buf ); + if( ( ret = des_setkey_dec( &des_ctx, des_key ) ) != 0 ) + goto exit; + ret = des_crypt_cbc( &des_ctx, DES_DECRYPT, buflen, des_iv, buf, buf ); +exit: des_free( &des_ctx ); polarssl_zeroize( des_key, 8 ); + + return( ret ); } /* * Decrypt with 3DES-CBC, using PBKDF1 for key derivation */ -static void pem_des3_decrypt( unsigned char des3_iv[8], - unsigned char *buf, size_t buflen, - const unsigned char *pwd, size_t pwdlen ) +static int pem_des3_decrypt( unsigned char des3_iv[8], + unsigned char *buf, size_t buflen, + const unsigned char *pwd, size_t pwdlen ) { des3_context des3_ctx; unsigned char des3_key[24]; + int ret; des3_init( &des3_ctx ); pem_pbkdf1( des3_key, 24, des3_iv, pwd, pwdlen ); - des3_set3key_dec( &des3_ctx, des3_key ); - des3_crypt_cbc( &des3_ctx, DES_DECRYPT, buflen, - des3_iv, buf, buf ); + if( ( ret = des3_set3key_dec( &des3_ctx, des3_key ) ) != 0 ) + goto exit; + ret = des3_crypt_cbc( &des3_ctx, DES_DECRYPT, buflen, des3_iv, buf, buf ); +exit: des3_free( &des3_ctx ); polarssl_zeroize( des3_key, 24 ); + + return( ret ); } #endif /* POLARSSL_DES_C */ @@ -181,23 +189,27 @@ static void pem_des3_decrypt( unsigned char des3_iv[8], /* * Decrypt with AES-XXX-CBC, using PBKDF1 for key derivation */ -static void pem_aes_decrypt( unsigned char aes_iv[16], unsigned int keylen, - unsigned char *buf, size_t buflen, - const unsigned char *pwd, size_t pwdlen ) +static int pem_aes_decrypt( unsigned char aes_iv[16], unsigned int keylen, + unsigned char *buf, size_t buflen, + const unsigned char *pwd, size_t pwdlen ) { aes_context aes_ctx; unsigned char aes_key[32]; + int ret; aes_init( &aes_ctx ); pem_pbkdf1( aes_key, keylen, aes_iv, pwd, pwdlen ); - aes_setkey_dec( &aes_ctx, aes_key, keylen * 8 ); - aes_crypt_cbc( &aes_ctx, AES_DECRYPT, buflen, - aes_iv, buf, buf ); + if( ( ret = aes_setkey_dec( &aes_ctx, aes_key, keylen * 8 ) ) != 0 ) + goto exit; + ret = aes_crypt_cbc( &aes_ctx, AES_DECRYPT, buflen, aes_iv, buf, buf ); +exit: aes_free( &aes_ctx ); polarssl_zeroize( aes_key, keylen ); + + return( ret ); } #endif /* POLARSSL_AES_C */ @@ -347,22 +359,30 @@ int pem_read_buffer( pem_context *ctx, const char *header, const char *footer, return( POLARSSL_ERR_PEM_PASSWORD_REQUIRED ); } + ret = 0; + #if defined(POLARSSL_DES_C) if( enc_alg == POLARSSL_CIPHER_DES_EDE3_CBC ) - pem_des3_decrypt( pem_iv, buf, len, pwd, pwdlen ); + ret = pem_des3_decrypt( pem_iv, buf, len, pwd, pwdlen ); else if( enc_alg == POLARSSL_CIPHER_DES_CBC ) - pem_des_decrypt( pem_iv, buf, len, pwd, pwdlen ); + ret = pem_des_decrypt( pem_iv, buf, len, pwd, pwdlen ); #endif /* POLARSSL_DES_C */ #if defined(POLARSSL_AES_C) if( enc_alg == POLARSSL_CIPHER_AES_128_CBC ) - pem_aes_decrypt( pem_iv, 16, buf, len, pwd, pwdlen ); + ret = pem_aes_decrypt( pem_iv, 16, buf, len, pwd, pwdlen ); else if( enc_alg == POLARSSL_CIPHER_AES_192_CBC ) - pem_aes_decrypt( pem_iv, 24, buf, len, pwd, pwdlen ); + ret = pem_aes_decrypt( pem_iv, 24, buf, len, pwd, pwdlen ); else if( enc_alg == POLARSSL_CIPHER_AES_256_CBC ) - pem_aes_decrypt( pem_iv, 32, buf, len, pwd, pwdlen ); + ret = pem_aes_decrypt( pem_iv, 32, buf, len, pwd, pwdlen ); #endif /* POLARSSL_AES_C */ + if( ret != 0 ) + { + polarssl_free( buf ); + return( ret ); + } + /* * The result will be ASN.1 starting with a SEQUENCE tag, with 1 to 3 * length bytes (allow 4 to be sure) in all known use cases.