diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index c0424da82f..1786ddaeac 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -229,6 +229,14 @@ #define MBEDTLS_PK_HAVE_ECC_KEYS #endif /* MBEDTLS_PK_USE_PSA_EC_DATA || MBEDTLS_ECP_C */ +/* Historically pkparse did not check the CBC padding when decrypting + * a key. This was a bug, which is now fixed. As a consequence, pkparse + * now needs PKCS7 padding support, but existing configurations might not + * enable it, so we enable it here. */ +#if defined(MBEDTLS_PK_PARSE_C) && defined(MBEDTLS_PKCS5_C) && defined(MBEDTLS_CIPHER_MODE_CBC) +#define MBEDTLS_CIPHER_PADDING_PKCS7 +#endif + /* The following blocks make it easier to disable all of TLS, * or of TLS 1.2 or 1.3 or DTLS, without having to manually disable all * key exchanges, options and extensions related to them. */ diff --git a/include/mbedtls/pkcs5.h b/include/mbedtls/pkcs5.h index be54b25f0e..8b086aa2e2 100644 --- a/include/mbedtls/pkcs5.h +++ b/include/mbedtls/pkcs5.h @@ -25,6 +25,7 @@ #define MBEDTLS_PKCS5_H #include "mbedtls/build_info.h" +#include "mbedtls/platform_util.h" #include "mbedtls/asn1.h" #include "mbedtls/md.h" @@ -50,12 +51,17 @@ extern "C" { #if defined(MBEDTLS_ASN1_PARSE_C) +#if !defined(MBEDTLS_DEPRECATED_REMOVED) /** * \brief PKCS#5 PBES2 function * * \note When encrypting, #MBEDTLS_CIPHER_PADDING_PKCS7 must * be enabled at compile time. * + * \deprecated This function is deprecated and will be removed in a + * future version of the library. + * Please use mbedtls_pkcs5_pbes2_ext() instead. + * * \warning When decrypting: * - if #MBEDTLS_CIPHER_PADDING_PKCS7 is enabled at compile * time, this function validates the CBC padding and returns @@ -86,10 +92,11 @@ extern "C" { * * \returns 0 on success, or a MBEDTLS_ERR_XXX code if verification fails. */ -int mbedtls_pkcs5_pbes2(const mbedtls_asn1_buf *pbe_params, int mode, - const unsigned char *pwd, size_t pwdlen, - const unsigned char *data, size_t datalen, - unsigned char *output); +int MBEDTLS_DEPRECATED mbedtls_pkcs5_pbes2(const mbedtls_asn1_buf *pbe_params, int mode, + const unsigned char *pwd, size_t pwdlen, + const unsigned char *data, size_t datalen, + unsigned char *output); +#endif /* MBEDTLS_DEPRECATED_REMOVED */ #if defined(MBEDTLS_CIPHER_PADDING_PKCS7) diff --git a/library/pkcs5.c b/library/pkcs5.c index 7a209ddd7b..2756d058e0 100644 --- a/library/pkcs5.c +++ b/library/pkcs5.c @@ -119,6 +119,7 @@ int mbedtls_pkcs5_pbes2_ext(const mbedtls_asn1_buf *pbe_params, int mode, size_t *output_len); #endif +#if !defined(MBEDTLS_DEPRECATED_REMOVED) int mbedtls_pkcs5_pbes2(const mbedtls_asn1_buf *pbe_params, int mode, const unsigned char *pwd, size_t pwdlen, const unsigned char *data, size_t datalen, @@ -133,6 +134,7 @@ int mbedtls_pkcs5_pbes2(const mbedtls_asn1_buf *pbe_params, int mode, return mbedtls_pkcs5_pbes2_ext(pbe_params, mode, pwd, pwdlen, data, datalen, output, SIZE_MAX, &output_len); } +#endif int mbedtls_pkcs5_pbes2_ext(const mbedtls_asn1_buf *pbe_params, int mode, const unsigned char *pwd, size_t pwdlen, diff --git a/library/pkparse.c b/library/pkparse.c index fe01a1149b..188cc286ec 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -1417,6 +1417,13 @@ static int pk_parse_key_pkcs8_unencrypted_der( #endif /* MBEDTLS_PK_HAVE_ECC_KEYS */ return MBEDTLS_ERR_PK_UNKNOWN_PK_ALG; +#if !defined(MBEDTLS_PKCS12_C) + end = p + len; + if (end != (key + keylen)) { + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT, + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH); + } +#endif return 0; } @@ -1445,6 +1452,7 @@ static int pk_parse_key_pkcs8_encrypted_der( mbedtls_cipher_type_t cipher_alg; mbedtls_md_type_t md_alg; #endif + size_t outlen = 0; p = key; end = p + keylen; @@ -1499,14 +1507,14 @@ static int pk_parse_key_pkcs8_encrypted_der( return ret; } - + outlen = len; decrypted = 1; } else #endif /* MBEDTLS_PKCS12_C */ #if defined(MBEDTLS_PKCS5_C) if (MBEDTLS_OID_CMP(MBEDTLS_OID_PKCS5_PBES2, &pbe_alg_oid) == 0) { - if ((ret = mbedtls_pkcs5_pbes2(&pbe_params, MBEDTLS_PKCS5_DECRYPT, pwd, pwdlen, - p, len, buf)) != 0) { + if ((ret = mbedtls_pkcs5_pbes2_ext(&pbe_params, MBEDTLS_PKCS5_DECRYPT, pwd, pwdlen, + p, len, buf, len, &outlen)) != 0) { if (ret == MBEDTLS_ERR_PKCS5_PASSWORD_MISMATCH) { return MBEDTLS_ERR_PK_PASSWORD_MISMATCH; } @@ -1524,8 +1532,7 @@ static int pk_parse_key_pkcs8_encrypted_der( if (decrypted == 0) { return MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE; } - - return pk_parse_key_pkcs8_unencrypted_der(pk, buf, len, f_rng, p_rng); + return pk_parse_key_pkcs8_unencrypted_der(pk, buf, outlen, f_rng, p_rng); } #endif /* MBEDTLS_PKCS12_C || MBEDTLS_PKCS5_C */ diff --git a/tests/suites/test_suite_pkcs5.function b/tests/suites/test_suite_pkcs5.function index fd4fde1170..2b0b0c1e00 100644 --- a/tests/suites/test_suite_pkcs5.function +++ b/tests/suites/test_suite_pkcs5.function @@ -46,6 +46,7 @@ void pbes2_encrypt(int params_tag, data_t *params_hex, data_t *pw, ASSERT_ALLOC(my_out, outsize); +#if defined(MBEDTLS_TEST_DEPRECATED) if (ref_ret != MBEDTLS_ERR_ASN1_BUF_TOO_SMALL) { my_ret = mbedtls_pkcs5_pbes2(¶ms, MBEDTLS_PKCS5_ENCRYPT, pw->x, pw->len, data->x, data->len, my_out); @@ -55,6 +56,7 @@ void pbes2_encrypt(int params_tag, data_t *params_hex, data_t *pw, ASSERT_COMPARE(my_out, ref_out->len, ref_out->x, ref_out->len); } +#endif #if defined(MBEDTLS_CIPHER_PADDING_PKCS7) my_ret = mbedtls_pkcs5_pbes2_ext(¶ms, MBEDTLS_PKCS5_ENCRYPT, @@ -93,6 +95,7 @@ void pbes2_decrypt(int params_tag, data_t *params_hex, data_t *pw, ASSERT_ALLOC(my_out, outsize); +#if defined(MBEDTLS_TEST_DEPRECATED) if (ref_ret != MBEDTLS_ERR_ASN1_BUF_TOO_SMALL) { my_ret = mbedtls_pkcs5_pbes2(¶ms, MBEDTLS_PKCS5_DECRYPT, pw->x, pw->len, data->x, data->len, my_out); @@ -102,6 +105,8 @@ void pbes2_decrypt(int params_tag, data_t *params_hex, data_t *pw, ASSERT_COMPARE(my_out, ref_out->len, ref_out->x, ref_out->len); } +#endif + #if defined(MBEDTLS_CIPHER_PADDING_PKCS7) my_ret = mbedtls_pkcs5_pbes2_ext(¶ms, MBEDTLS_PKCS5_DECRYPT, pw->x, pw->len, data->x, data->len, my_out,