From fe724fe618946772c3a7906de945d46dc9354c83 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 11 Nov 2021 19:00:38 +0000 Subject: [PATCH] Fix for pkcs12 with NULL or zero length password Previously passing a NULL or zero length password into either mbedtls_pkcs12_pbe() or mbedtls_pkcs12_derive() could cause an infinate loop, and it was also possible to pass a NULL password, with a non-zero length, which would cause memory corruption. I have fixed these errors, and improved the documentation to reflect the changes and further explain what is expected of the inputs. Signed-off-by: Paul Elliott --- ChangeLog.d/fix-pkcs12-null-password.txt | 2 ++ include/mbedtls/pkcs12.h | 22 +++++++++++++--------- library/pkcs12.c | 21 ++++++++++++++++----- 3 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 ChangeLog.d/fix-pkcs12-null-password.txt diff --git a/ChangeLog.d/fix-pkcs12-null-password.txt b/ChangeLog.d/fix-pkcs12-null-password.txt new file mode 100644 index 000000000..699575f53 --- /dev/null +++ b/ChangeLog.d/fix-pkcs12-null-password.txt @@ -0,0 +1,2 @@ +Bugfix + * Fix issues in pkcs12 when a NULL and/or zero length password was supplied. diff --git a/include/mbedtls/pkcs12.h b/include/mbedtls/pkcs12.h index ba9180b3c..fbf237868 100644 --- a/include/mbedtls/pkcs12.h +++ b/include/mbedtls/pkcs12.h @@ -79,11 +79,13 @@ int mbedtls_pkcs12_pbe_sha1_rc4_128( mbedtls_asn1_buf *pbe_params, int mode, * \brief PKCS12 Password Based function (encryption / decryption) * for cipher-based and mbedtls_md-based PBE's * - * \param pbe_params an ASN1 buffer containing the pkcs-12PbeParams structure - * \param mode either MBEDTLS_PKCS12_PBE_ENCRYPT or MBEDTLS_PKCS12_PBE_DECRYPT + * \param pbe_params an ASN1 buffer containing the pkcs-12 PbeParams structure + * \param mode either MBEDTLS_PKCS12_PBE_ENCRYPT or + * MBEDTLS_PKCS12_PBE_DECRYPT * \param cipher_type the cipher used - * \param md_type the mbedtls_md used - * \param pwd the password used (may be NULL if no password is used) + * \param md_type the mbedtls_md used + * \param pwd Latin1-encoded password used (may be NULL if no password is + * used, but not if pwdlen is non-zero) * \param pwdlen length of the password (may be 0) * \param input the input data * \param len data length @@ -108,14 +110,16 @@ int mbedtls_pkcs12_pbe( mbedtls_asn1_buf *pbe_params, int mode, * integrity key. * * \param data buffer to store the derived data in - * \param datalen length to fill - * \param pwd password to use (may be NULL if no password is used) + * \param datalen length of buffer to fill + * \param pwd Null terminated BMPString password to use (may be NULL if + * no password is used, but not if pwdlen is non-zero) * \param pwdlen length of the password (may be 0) * \param salt salt buffer to use * \param saltlen length of the salt - * \param mbedtls_md mbedtls_md type to use during the derivation - * \param id id that describes the purpose (can be MBEDTLS_PKCS12_DERIVE_KEY, - * MBEDTLS_PKCS12_DERIVE_IV or MBEDTLS_PKCS12_DERIVE_MAC_KEY) + * \param mbedtls_md mbedtls_md type to use during the derivation + * \param id id that describes the purpose (can be + * MBEDTLS_PKCS12_DERIVE_KEY, MBEDTLS_PKCS12_DERIVE_IV or + * MBEDTLS_PKCS12_DERIVE_MAC_KEY) * \param iterations number of iterations * * \return 0 if successful, or a MD, BIGNUM type error. diff --git a/library/pkcs12.c b/library/pkcs12.c index 3699dd5c6..80eb9dbe8 100644 --- a/library/pkcs12.c +++ b/library/pkcs12.c @@ -179,6 +179,9 @@ int mbedtls_pkcs12_pbe( mbedtls_asn1_buf *pbe_params, int mode, mbedtls_cipher_context_t cipher_ctx; size_t olen = 0; + if( pwd == NULL && pwdlen != 0 ) + return( MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA ); + cipher_info = mbedtls_cipher_info_from_type( cipher_type ); if( cipher_info == NULL ) return( MBEDTLS_ERR_PKCS12_FEATURE_UNAVAILABLE ); @@ -231,13 +234,18 @@ static void pkcs12_fill_buffer( unsigned char *data, size_t data_len, unsigned char *p = data; size_t use_len; - while( data_len > 0 ) + if( filler != NULL && fill_len != 0 ) { - use_len = ( data_len > fill_len ) ? fill_len : data_len; - memcpy( p, filler, use_len ); - p += use_len; - data_len -= use_len; + while( data_len > 0 ) + { + use_len = ( data_len > fill_len ) ? fill_len : data_len; + memcpy( p, filler, use_len ); + p += use_len; + data_len -= use_len; + } } + else + memset( data, 0, data_len ); } int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen, @@ -263,6 +271,9 @@ int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen, if( datalen > 128 || pwdlen > 64 || saltlen > 64 ) return( MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA ); + if( pwd == NULL && pwdlen != 0 ) + return( MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA ); + md_info = mbedtls_md_info_from_type( md_type ); if( md_info == NULL ) return( MBEDTLS_ERR_PKCS12_FEATURE_UNAVAILABLE );