packages/dsoftbus
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
dsoftbus/0005-fix-CVE-2021-43666.patch
liheavy 92830da5f5 init dsoftbus
2022-11-30 17:59:29 +08:00

410 lines
16 KiB
Diff
Raw Blame History

From: maoyufeng <maoyufeng3@huawei.com>
Date: Mon, 23 May 2022 14:57:48 +0800
Subject: [PATCH] Fix issue CVE-2021-43666
Signed-off-by: maoyufeng <maoyufeng3@huawei.com>
---
ChangeLog.d/fix-pkcs12-null-password.txt | 5 ++
include/mbedtls/pkcs12.h | 34 ++++++----
library/pkcs12.c | 82 ++++++++++++++++++------
tests/CMakeLists.txt | 1 +
tests/scripts/all.sh | 30 +++++++++
tests/suites/test_suite_pkcs12.data | 35 ++++++++++
tests/suites/test_suite_pkcs12.function | 68 ++++++++++++++++++++
7 files changed, 223 insertions(+), 32 deletions(-)
create mode 100644 ChangeLog.d/fix-pkcs12-null-password.txt
mode change 100755 => 100644 library/pkcs12.c
create mode 100644 tests/suites/test_suite_pkcs12.data
create mode 100644 tests/suites/test_suite_pkcs12.function
diff --git a/ChangeLog.d/fix-pkcs12-null-password.txt b/ChangeLog.d/fix-pkcs12-null-password.txt
new file mode 100644
index 0000000000000000000000000000000000000000..a6ce140fdc3479cfac671542692ab646f7c8b5f3
--- /dev/null
+++ b/ChangeLog.d/fix-pkcs12-null-password.txt
@@ -0,0 +1,5 @@
+Bugfix
+ * Fix a potential invalid pointer dereference and infinite loop bugs in
+ pkcs12 functions when the password is empty. Fix the documentation to
+ better describe the inputs to these functions and their possible values.
+ Fixes #5136.
\ No newline at end of file
diff --git a/include/mbedtls/pkcs12.h b/include/mbedtls/pkcs12.h
index 9cbcb1730559bb7d3a22a378467b9f6aa1b1c3fa..9e11e24a8d07b9e377fd49d9206fd300330bcdbf 100755
--- a/include/mbedtls/pkcs12.h
+++ b/include/mbedtls/pkcs12.h
@@ -83,8 +83,9 @@ extern "C" {
* \brief PKCS12 Password Based function (encryption / decryption)
* for pbeWithSHAAnd128BitRC4
*
- * \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 pwd the password used (may be NULL if no password is used)
* \param pwdlen length of the password (may be 0)
* \param input the input data
@@ -105,8 +106,9 @@ int mbedtls_pkcs12_pbe_sha1_rc4_128( mbedtls_asn1_buf *pbe_params, int mode,
* \param pbe_params an ASN1 buffer containing the pkcs-12PbeParams 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. This may only be \c NULL when
+ * \p pwdlen is 0. No null terminator should be used.
* \param pwdlen length of the password (may be 0)
* \param input the input data
* \param len data length
@@ -127,18 +129,24 @@ int mbedtls_pkcs12_pbe( mbedtls_asn1_buf *pbe_params, int mode,
* to produce pseudo-random bits for a particular "purpose".
*
* Depending on the given id, this function can produce an
- * encryption/decryption key, an nitialization vector or an
+ * encryption/decryption key, an initialization vector or an
* 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 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 datalen length of buffer to fill
+ * \param pwd The password to use. For compliance with PKCS#12 §B.1, this
+ * should be a BMPString, i.e. a Unicode string where each
+ * character is encoded as 2 bytes in big-endian order, with
+ * no byte order mark and with a null terminator (i.e. the
+ * last two bytes should be 0x00 0x00).
+ * \param pwdlen length of the password (may be 0).
+ * \param salt Salt buffer to use This may only be \c NULL when
+ * \p saltlen is 0.
+ * \param saltlen length of the salt (may be zero)
+ * \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
old mode 100755
new mode 100644
index 3d23d5e354923cd01d69a479fcf572d80af540a6..05ade49e93b3d2cb8e03f7915f0ead4b79e919c4
--- a/library/pkcs12.c
+++ b/library/pkcs12.c
@@ -209,6 +209,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 );
@@ -261,12 +264,23 @@ 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
+ {
+ /* If either of the above are not true then clearly there is nothing
+ * that this function can do. The function should *not* be called
+ * under either of those circumstances, as you could end up with an
+ * incorrect output but for safety's sake, leaving the check in as
+ * otherwise we could end up with memory corruption.*/
}
}
@@ -283,6 +297,8 @@ int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen,
unsigned char hash_output[MBEDTLS_MD_MAX_SIZE];
unsigned char *p;
unsigned char c;
+ int use_password = 0;
+ int use_salt = 0;
size_t hlen, use_len, v, i;
@@ -293,6 +309,15 @@ 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 );
+
+ if( salt == NULL && saltlen != 0 )
+ return( MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA );
+
+ use_password = ( pwd && pwdlen != 0 );
+ use_salt = ( salt && saltlen != 0 );
+
md_info = mbedtls_md_info_from_type( md_type );
if( md_info == NULL )
return( MBEDTLS_ERR_PKCS12_FEATURE_UNAVAILABLE );
@@ -310,8 +335,15 @@ int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen,
memset( diversifier, (unsigned char) id, v );
- pkcs12_fill_buffer( salt_block, v, salt, saltlen );
- pkcs12_fill_buffer( pwd_block, v, pwd, pwdlen );
+ if( use_salt != 0 )
+ {
+ pkcs12_fill_buffer( salt_block, v, salt, saltlen );
+ }
+
+ if( use_password != 0 )
+ {
+ pkcs12_fill_buffer( pwd_block, v, pwd, pwdlen );
+ }
p = data;
while( datalen > 0 )
@@ -323,11 +355,17 @@ int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen,
if( ( ret = mbedtls_md_update( &md_ctx, diversifier, v ) ) != 0 )
goto exit;
- if( ( ret = mbedtls_md_update( &md_ctx, salt_block, v ) ) != 0 )
- goto exit;
+ if( use_salt != 0 )
+ {
+ if( ( ret = mbedtls_md_update( &md_ctx, salt_block, v )) != 0 )
+ goto exit;
+ }
- if( ( ret = mbedtls_md_update( &md_ctx, pwd_block, v ) ) != 0 )
- goto exit;
+ if( use_password != 0)
+ {
+ if( ( ret = mbedtls_md_update( &md_ctx, pwd_block, v )) != 0 )
+ goto exit;
+ }
if( ( ret = mbedtls_md_finish( &md_ctx, hash_output ) ) != 0 )
goto exit;
@@ -355,22 +393,28 @@ int mbedtls_pkcs12_derivation( unsigned char *data, size_t datalen,
if( ++hash_block[i - 1] != 0 )
break;
- // salt_block += B
- c = 0;
- for( i = v; i > 0; i-- )
+ if( use_salt != 0 )
{
- j = salt_block[i - 1] + hash_block[i - 1] + c;
+ // salt_block += B
+ c = 0;
+ for( i = v; i > 0; i-- )
+ {
+ j = salt_block[i - 1] + hash_block[i - 1] + c;
c = (unsigned char) (j >> 8);
salt_block[i - 1] = j & 0xFF;
+ }
}
- // pwd_block += B
- c = 0;
- for( i = v; i > 0; i-- )
+ if( use_password != 0 )
{
- j = pwd_block[i - 1] + hash_block[i - 1] + c;
+ // pwd_block += B
+ c = 0;
+ for( i = v; i > 0; i-- )
+ {
+ j = pwd_block[i - 1] + hash_block[i - 1] + c;
c = (unsigned char) (j >> 8);
pwd_block[i - 1] = j & 0xFF;
+ }
}
}
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index a8e7523e504bc5cff9925648dd1b3bd7fb55f0cf..c5d484f924328bde2d2525b6f89e125974c3b770 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -120,6 +120,7 @@ add_test_suite(pem)
add_test_suite(pkcs1_v15)
add_test_suite(pkcs1_v21)
add_test_suite(pkcs5)
+add_test_suite(pkcs12)
add_test_suite(pk)
add_test_suite(pkparse)
add_test_suite(pkwrite)
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 1a4de44b665f8df822adbd83bece912119bca98f..00222f379e808d8662967fdf200e18739cbabe61 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -1683,6 +1683,36 @@ component_test_valgrind () {
fi
}
+support_test_cmake_out_of_source () {
+ distrib_id=""
+ distrib_ver=""
+ distrib_ver_minor=""
+ distrib_ver_major=""
+
+ # Attempt to parse lsb-release to find out distribution and version. If not
+ # found this should fail safe (test is supported).
+ if [[ -f /etc/lsb-release ]]; then
+
+ while read -r lsb_line; do
+ case "$lsb_line" in
+ "DISTRIB_ID"*) distrib_id=${lsb_line/#DISTRIB_ID=};;
+ "DISTRIB_RELEASE"*) distrib_ver=${lsb_line/#DISTRIB_RELEASE=};;
+ esac
+ done < /etc/lsb-release
+
+ distrib_ver_major="${distrib_ver%%.*}"
+ distrib_ver="${distrib_ver#*.}"
+ distrib_ver_minor="${distrib_ver%%.*}"
+ fi
+
+ # Running the out of source CMake test on Ubuntu 16.04 using more than one
+ # processor (as the CI does) can create a race condition whereby the build
+ # fails to see a generated file, despite that file actually having been
+ # generated. This problem appears to go away with 18.04 or newer, so make
+ # the out of source tests unsupported on Ubuntu 16.04.
+ [ "$distrib_id" != "Ubuntu" ] || [ "$distrib_ver_major" -gt 16 ]
+}
+
component_test_cmake_out_of_source () {
msg "build: cmake 'out-of-source' build"
MBEDTLS_ROOT_DIR="$PWD"
diff --git a/tests/suites/test_suite_pkcs12.data b/tests/suites/test_suite_pkcs12.data
new file mode 100644
index 0000000000000000000000000000000000000000..bda7d9921caad35c1835e0093df8a9644d4bd9cb
--- /dev/null
+++ b/tests/suites/test_suite_pkcs12.data
@@ -0,0 +1,35 @@
+PKCS#12 derive key : MD5: Zero length password and hash
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"":USE_GIVEN_INPUT:"":USE_GIVEN_INPUT:3:"6afdcbd5ebf943272134f1c3de2dc11b6afdcbd5ebf943272134f1c3de2dc11b6afdcbd5ebf943272134f1c3de2dc11b":0
+
+PKCS#12 derive key: MD5: NULL password and hash
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"":USE_NULL_INPUT:"":USE_NULL_INPUT:3:"6afdcbd5ebf943272134f1c3de2dc11b6afdcbd5ebf943272134f1c3de2dc11b6afdcbd5ebf943272134f1c3de2dc11b":0
+
+PKCS#12 derive key: MD5: Zero length password
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"":USE_GIVEN_INPUT:"0123456789abcdef":USE_GIVEN_INPUT:3:"832d8502114fcccfd3de0c2b2863b1c45fb92a8db2ed1e704727b324adc267bdd66ae4918a81fa2d1ba15febfb9e6c4e":0
+
+PKCS#12 derive key: MD5: NULL password
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"":USE_NULL_INPUT:"0123456789abcdef":USE_GIVEN_INPUT:3:"832d8502114fcccfd3de0c2b2863b1c45fb92a8db2ed1e704727b324adc267bdd66ae4918a81fa2d1ba15febfb9e6c4e":0
+
+PKCS#12 derive key: MD5: Invalid length NULL password
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_NULL_INPUT:"0123456789abcdef":USE_GIVEN_INPUT:3:"":MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA
+
+PKCS#12 derive key: MD5: Zero length salt
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"":USE_GIVEN_INPUT:3:"832d8502114fcccfd3de0c2b2863b1c45fb92a8db2ed1e704727b324adc267bdd66ae4918a81fa2d1ba15febfb9e6c4e":0
+
+PKCS#12 derive key: MD5: NULL salt
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"":USE_NULL_INPUT:3:"832d8502114fcccfd3de0c2b2863b1c45fb92a8db2ed1e704727b324adc267bdd66ae4918a81fa2d1ba15febfb9e6c4e":0
+
+PKCS#12 derive key: MD5: Invalid length NULL salt
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"0123456789abcdef":USE_NULL_INPUT:3:"":MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA
+
+PKCS#12 derive key: MD5: Valid password and salt
+depends_on:MBEDTLS_MD5_C
+pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"0123456789abcdef":USE_GIVEN_INPUT:3:"46559deeee036836ab1b633ec620178d4c70eacf42f72a2ad7360c812efa09ca3d7567b489a109050345c2dc6a262995":0
diff --git a/tests/suites/test_suite_pkcs12.function b/tests/suites/test_suite_pkcs12.function
new file mode 100644
index 0000000000000000000000000000000000000000..56b896c8253b5d9734d27b0bd7cd315b3cbc7ee7
--- /dev/null
+++ b/tests/suites/test_suite_pkcs12.function
@@ -0,0 +1,68 @@
+/* BEGIN_HEADER */
+#include "mbedtls/pkcs12.h"
+
+typedef enum
+{
+ USE_NULL_INPUT = 0,
+ USE_GIVEN_INPUT = 1,
+} input_usage_method_t;
+
+/* END_HEADER */
+
+/* BEGIN_DEPENDENCIES
+ * depends_on:MBEDTLS_PKCS12_C
+ * END_DEPENDENCIES
+ */
+
+/* BEGIN_CASE */
+void pkcs12_derive_key( int md_type, int key_size_arg,
+ data_t *password_arg, int password_usage,
+ data_t *salt_arg, int salt_usage,
+ int iterations,
+ data_t* expected_output, int expected_status )
+
+{
+ int ret = 0;
+ unsigned char *output_data = NULL;
+
+ unsigned char *password = NULL;
+ size_t password_len = 0;
+ unsigned char *salt = NULL;
+ size_t salt_len = 0;
+ size_t key_size = key_size_arg;
+
+ if( password_usage == USE_GIVEN_INPUT )
+ password = password_arg->x;
+
+ password_len = password_arg->len;
+
+ if( salt_usage == USE_GIVEN_INPUT )
+ salt = salt_arg->x;
+
+ salt_len = salt_arg->len;
+
+ ASSERT_ALLOC( output_data, key_size );
+
+ ret = mbedtls_pkcs12_derivation( output_data,
+ key_size,
+ password,
+ password_len,
+ salt,
+ salt_len,
+ md_type,
+ MBEDTLS_PKCS12_DERIVE_KEY,
+ iterations );
+
+ TEST_EQUAL( ret, expected_status );
+
+ if( expected_status == 0 )
+ {
+ ASSERT_COMPARE( expected_output->x, expected_output->len,
+ output_data, key_size );
+ }
+
+exit:
+ mbedtls_free( output_data );
+
+}
+/* END_CASE */
\ No newline at end of file
Reference in New Issue View Git Blame Copy Permalink