Merge pull request #1049 from waleed-elmelegy-arm/Switch-pkparse-to-mbedtls_pkcs5_pbe2_ext

Switch pkparse to use new pkcs5/12 pbe functions
This commit is contained in:
Gilles Peskine 2023-09-22 18:06:50 +02:00 committed by GitHub
commit 18e1d11cfe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 111 additions and 21 deletions

View File

@ -0,0 +1,9 @@
New deprecations
* mbedtls_pkcs5_pbes2() and mbedtls_pkcs12_pbe() functions are now
deprecated in favor of mbedtls_pkcs5_pbes2_ext() and
mbedtls_pkcs12_pbe_ext() as they offer more security by checking
for overflow of the output buffer and reporting the actual length
of the output.
Bugfix
* mbedtls_pk_parse_key() now rejects trailing garbage in encrypted keys.

View File

@ -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. */

View File

@ -52,6 +52,7 @@ extern "C" {
#if defined(MBEDTLS_ASN1_PARSE_C)
#if !defined(MBEDTLS_DEPRECATED_REMOVED)
/**
* \brief PKCS12 Password Based function (encryption / decryption)
* for cipher-based and mbedtls_md-based PBE's
@ -59,6 +60,10 @@ extern "C" {
* \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_pkcs12_pbe_ext() instead.
*
* \warning When decrypting:
* - if #MBEDTLS_CIPHER_PADDING_PKCS7 is enabled at compile
* time, this function validates the CBC padding and returns
@ -93,11 +98,13 @@ extern "C" {
*
* \return 0 if successful, or a MBEDTLS_ERR_XXX code
*/
int mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
mbedtls_cipher_type_t cipher_type, mbedtls_md_type_t md_type,
const unsigned char *pwd, size_t pwdlen,
const unsigned char *data, size_t len,
unsigned char *output);
int MBEDTLS_DEPRECATED mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
mbedtls_cipher_type_t cipher_type,
mbedtls_md_type_t md_type,
const unsigned char *pwd, size_t pwdlen,
const unsigned char *data, size_t len,
unsigned char *output);
#endif /* MBEDTLS_DEPRECATED_REMOVED */
#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)

View File

@ -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)

View File

@ -117,5 +117,14 @@ static inline mbedtls_ecp_group_id mbedtls_pk_get_group_id(const mbedtls_pk_cont
#endif /* MBEDTLS_ECP_DP_CURVE25519_ENABLED || MBEDTLS_ECP_DP_CURVE448_ENABLED */
#endif /* MBEDTLS_PK_HAVE_ECC_KEYS */
#if defined(MBEDTLS_TEST_HOOKS)
MBEDTLS_STATIC_TESTABLE int mbedtls_pk_parse_key_pkcs8_encrypted_der(
mbedtls_pk_context *pk,
unsigned char *key, size_t keylen,
const unsigned char *pwd, size_t pwdlen,
int (*f_rng)(void *, unsigned char *, size_t), void *p_rng);
#endif
#endif /* MBEDTLS_PK_INTERNAL_H */

View File

@ -138,6 +138,7 @@ int mbedtls_pkcs12_pbe_ext(mbedtls_asn1_buf *pbe_params, int mode,
size_t *output_len);
#endif
#if !defined(MBEDTLS_DEPRECATED_REMOVED)
int mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
mbedtls_cipher_type_t cipher_type, mbedtls_md_type_t md_type,
const unsigned char *pwd, size_t pwdlen,
@ -154,6 +155,7 @@ int mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
pwd, pwdlen, data, len, output, SIZE_MAX,
&output_len);
}
#endif
int mbedtls_pkcs12_pbe_ext(mbedtls_asn1_buf *pbe_params, int mode,
mbedtls_cipher_type_t cipher_type, mbedtls_md_type_t md_type,

View File

@ -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,

View File

@ -1417,6 +1417,12 @@ static int pk_parse_key_pkcs8_unencrypted_der(
#endif /* MBEDTLS_PK_HAVE_ECC_KEYS */
return MBEDTLS_ERR_PK_UNKNOWN_PK_ALG;
end = p + len;
if (end != (key + keylen)) {
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PK_KEY_INVALID_FORMAT,
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH);
}
return 0;
}
@ -1430,7 +1436,7 @@ static int pk_parse_key_pkcs8_unencrypted_der(
*
*/
#if defined(MBEDTLS_PKCS12_C) || defined(MBEDTLS_PKCS5_C)
static int pk_parse_key_pkcs8_encrypted_der(
MBEDTLS_STATIC_TESTABLE int mbedtls_pk_parse_key_pkcs8_encrypted_der(
mbedtls_pk_context *pk,
unsigned char *key, size_t keylen,
const unsigned char *pwd, size_t pwdlen,
@ -1445,6 +1451,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;
@ -1490,9 +1497,9 @@ static int pk_parse_key_pkcs8_encrypted_der(
*/
#if defined(MBEDTLS_PKCS12_C)
if (mbedtls_oid_get_pkcs12_pbe_alg(&pbe_alg_oid, &md_alg, &cipher_alg) == 0) {
if ((ret = mbedtls_pkcs12_pbe(&pbe_params, MBEDTLS_PKCS12_PBE_DECRYPT,
cipher_alg, md_alg,
pwd, pwdlen, p, len, buf)) != 0) {
if ((ret = mbedtls_pkcs12_pbe_ext(&pbe_params, MBEDTLS_PKCS12_PBE_DECRYPT,
cipher_alg, md_alg,
pwd, pwdlen, p, len, buf, len, &outlen)) != 0) {
if (ret == MBEDTLS_ERR_PKCS12_PASSWORD_MISMATCH) {
return MBEDTLS_ERR_PK_PASSWORD_MISMATCH;
}
@ -1505,8 +1512,8 @@ static int pk_parse_key_pkcs8_encrypted_der(
#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 +1531,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 */
@ -1644,8 +1650,8 @@ int mbedtls_pk_parse_key(mbedtls_pk_context *pk,
key, NULL, 0, &len);
}
if (ret == 0) {
if ((ret = pk_parse_key_pkcs8_encrypted_der(pk, pem.buf, pem.buflen,
pwd, pwdlen, f_rng, p_rng)) != 0) {
if ((ret = mbedtls_pk_parse_key_pkcs8_encrypted_der(pk, pem.buf, pem.buflen,
pwd, pwdlen, f_rng, p_rng)) != 0) {
mbedtls_pk_free(pk);
}
@ -1677,8 +1683,8 @@ int mbedtls_pk_parse_key(mbedtls_pk_context *pk,
memcpy(key_copy, key, keylen);
ret = pk_parse_key_pkcs8_encrypted_der(pk, key_copy, keylen,
pwd, pwdlen, f_rng, p_rng);
ret = mbedtls_pk_parse_key_pkcs8_encrypted_der(pk, key_copy, keylen,
pwd, pwdlen, f_rng, p_rng);
mbedtls_zeroize_and_free(key_copy, keylen);
}

View File

@ -90,6 +90,7 @@ void pkcs12_pbe_encrypt(int params_tag, int cipher, int md, data_t *params_hex,
pbe_params.len = params_hex->len;
pbe_params.p = params_hex->x;
#if defined(MBEDTLS_TEST_DEPRECATED)
if (ref_ret != MBEDTLS_ERR_ASN1_BUF_TOO_SMALL) {
my_ret = mbedtls_pkcs12_pbe(&pbe_params, MBEDTLS_PKCS12_PBE_ENCRYPT, cipher_alg,
md_alg, pw->x, pw->len, data->x, data->len, my_out);
@ -99,6 +100,7 @@ void pkcs12_pbe_encrypt(int params_tag, int cipher, int md, data_t *params_hex,
ASSERT_COMPARE(my_out, ref_out->len,
ref_out->x, ref_out->len);
}
#endif
#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
@ -143,6 +145,7 @@ void pkcs12_pbe_decrypt(int params_tag, int cipher, int md, data_t *params_hex,
pbe_params.len = params_hex->len;
pbe_params.p = params_hex->x;
#if defined(MBEDTLS_TEST_DEPRECATED)
if (ref_ret != MBEDTLS_ERR_ASN1_BUF_TOO_SMALL) {
my_ret = mbedtls_pkcs12_pbe(&pbe_params, MBEDTLS_PKCS12_PBE_DECRYPT, cipher_alg,
md_alg, pw->x, pw->len, data->x, data->len, my_out);
@ -153,6 +156,7 @@ void pkcs12_pbe_decrypt(int params_tag, int cipher, int md, data_t *params_hex,
ASSERT_COMPARE(my_out, ref_out->len,
ref_out->x, ref_out->len);
}
#endif
#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)

View File

@ -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(&params, 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(&params, 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(&params, 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(&params, MBEDTLS_PKCS5_DECRYPT,
pw->x, pw->len, data->x, data->len, my_out,

View File

@ -1219,6 +1219,14 @@ Key ASN1 (OneAsymmetricKey X25519, unsupported version 2 with public key and uns
depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED
pk_parse_key:"3072020101300506032b656e04220420b06d829655543a51cba36e53522bc0acfd60af59466555fb3e1e796872ab1a59a01f301d060a2a864886f70d01090914310f0c0d437572646c65204368616972738121009bc3b0e93d8233fe6a8ba6138948cc12a91362d5c2ed81584db05ab5419c9d11":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
Key ASN1 (Encrypted key PKCS5, trailing garbage data)
depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7:MBEDTLS_PKCS5_C
pk_parse_key_encrypted:"307C304006092A864886F70D01050D3033301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC3949100438AD100BAC552FD0AE70BECAFA60F5E519B6180C77E8DB0B9ECC6F23FEDD30AB9BDCA2AF9F97BC470FC3A82DCA2364E22642DE0AF9275A82CB":"AAAAAAAAAAAAAAAAAA":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH
Key ASN1 (Encrypted key PKCS12, trailing garbage data)
depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_DP_CURVE25519_ENABLED:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7:MBEDTLS_PKCS12_C
pk_parse_key_encrypted:"3058301C060A2A864886F70D010C0103300E0409CCCCCCCCCCCCCCCCCC02010A04380A8CAF39C4FA001884D0583B323C5E70942444FBE1F650B92F8ADF4AD7BD5049B4748F53A2531139EBF253FE01E8FC925C82C759C944B4D0":"AAAAAAAAAAAAAAAAAA":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH
# From RFC8410 Appendix A but made into version 0
OneAsymmetricKey X25519, doesn't match masking requirements #1
depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED

View File

@ -5,6 +5,11 @@
#include "mbedtls/ecp.h"
#include "mbedtls/psa_util.h"
#include "pk_internal.h"
#if defined(MBEDTLS_PKCS12_C) || defined(MBEDTLS_PKCS5_C)
#define HAVE_mbedtls_pk_parse_key_pkcs8_encrypted_der
#endif
/* END_HEADER */
/* BEGIN_DEPENDENCIES
@ -150,6 +155,24 @@ exit:
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:HAVE_mbedtls_pk_parse_key_pkcs8_encrypted_der */
void pk_parse_key_encrypted(data_t *buf, data_t *pass, int result)
{
mbedtls_pk_context pk;
mbedtls_pk_init(&pk);
USE_PSA_INIT();
TEST_EQUAL(mbedtls_pk_parse_key_pkcs8_encrypted_der(&pk, buf->x, buf->len,
pass->x, pass->len,
mbedtls_test_rnd_std_rand,
NULL), result);
exit:
mbedtls_pk_free(&pk);
USE_PSA_DONE();
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_PK_WRITE_C */
void pk_parse_fix_montgomery(data_t *input_key, data_t *exp_output)
{