Add new mbedtls_pkcs12_pbe_ext function to replace old function
Add new mbedtls_pkcs12_pbe_ext function to replace
old mbedtls_pkcs12_pbe function that have security
issues.
Signed-off-by: Waleed Elmelegy <waleed.elmelegy@arm.com>
diff --git a/include/mbedtls/pkcs12.h b/include/mbedtls/pkcs12.h
index c26e9d0..63e2e63 100644
--- a/include/mbedtls/pkcs12.h
+++ b/include/mbedtls/pkcs12.h
@@ -102,7 +102,7 @@
* \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 data the input data
* \param len data length
* \param output Output buffer.
* On success, it contains the encrypted or decrypted data,
@@ -119,9 +119,60 @@
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 *input, size_t len,
+ const unsigned char *data, size_t len,
unsigned char *output);
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+
+/**
+ * \brief PKCS12 Password Based function (encryption / decryption)
+ * for cipher-based and mbedtls_md-based PBE's
+ *
+ *
+ * \warning When decrypting:
+ * - This function validates the CBC padding and returns
+ * #MBEDTLS_ERR_PKCS12_PASSWORD_MISMATCH if the padding is
+ * invalid. Note that this can help active adversaries
+ * attempting to brute-forcing the password. Note also that
+ * there is no guarantee that an invalid password will be
+ * detected (the chances of a valid padding with a random
+ * password are about 1/255).
+ *
+ * \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 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 data the input data
+ * \param len data length
+ * \param output Output buffer.
+ * On success, it contains the encrypted or decrypted data,
+ * possibly followed by the CBC padding.
+ * On failure, the content is indeterminate.
+ * For decryption, there must be enough room for \p len
+ * bytes.
+ * For encryption, there must be enough room for
+ * \p len + 1 bytes, rounded up to the block size of
+ * the block cipher identified by \p pbe_params.
+ * \param output_size size of output buffer.
+ * This must be big enough to accommodate for output plus
+ * padding data.
+ * \param output_len On success, length of actual data written to the output buffer.
+ *
+ * \return 0 if successful, or a MBEDTLS_ERR_XXX code
+ */
+int mbedtls_pkcs12_pbe_ext(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, size_t output_size,
+ size_t *output_len);
+
+#endif /* MBEDTLS_CIPHER_PADDING_PKCS7 */
+
#endif /* MBEDTLS_ASN1_PARSE_C */
/**
diff --git a/library/pkcs12.c b/library/pkcs12.c
index 1f45f45..4670afc 100644
--- a/library/pkcs12.c
+++ b/library/pkcs12.c
@@ -49,7 +49,7 @@
mbedtls_asn1_buf *salt, int *iterations)
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
- unsigned char **p = ¶ms->p;
+ unsigned char *p = params->p;
const unsigned char *end = params->p + params->len;
/*
@@ -64,18 +64,18 @@
MBEDTLS_ERR_ASN1_UNEXPECTED_TAG);
}
- if ((ret = mbedtls_asn1_get_tag(p, end, &salt->len, MBEDTLS_ASN1_OCTET_STRING)) != 0) {
+ if ((ret = mbedtls_asn1_get_tag(&p, end, &salt->len, MBEDTLS_ASN1_OCTET_STRING)) != 0) {
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS12_PBE_INVALID_FORMAT, ret);
}
- salt->p = *p;
- *p += salt->len;
+ salt->p = p;
+ p += salt->len;
- if ((ret = mbedtls_asn1_get_int(p, end, iterations)) != 0) {
+ if ((ret = mbedtls_asn1_get_int(&p, end, iterations)) != 0) {
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS12_PBE_INVALID_FORMAT, ret);
}
- if (*p != end) {
+ if (p != end) {
return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PKCS12_PBE_INVALID_FORMAT,
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH);
}
@@ -172,18 +172,46 @@
#endif /* MBEDTLS_ARC4_C */
}
+#if !defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+int mbedtls_pkcs12_pbe_ext(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, size_t output_size,
+ size_t *output_len);
+#endif
+
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)
{
+ size_t output_len = 0;
+
+ /* We assume caller of the function is providing a big enough output buffer
+ * so we pass output_size as SIZE_MAX to pass checks, However, no guarantees
+ * for the output size actually being correct.
+ */
+ return mbedtls_pkcs12_pbe_ext(pbe_params, mode, cipher_type, md_type,
+ pwd, pwdlen, data, len, output, SIZE_MAX,
+ &output_len);
+}
+
+int mbedtls_pkcs12_pbe_ext(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, size_t output_size,
+ size_t *output_len)
+{
int ret, keylen = 0;
unsigned char key[32];
unsigned char iv[16];
const mbedtls_cipher_info_t *cipher_info;
mbedtls_cipher_context_t cipher_ctx;
- size_t olen = 0;
+ size_t finish_olen = 0;
+ unsigned int padlen = 0;
if (pwd == NULL && pwdlen != 0) {
return MBEDTLS_ERR_PKCS12_BAD_INPUT_DATA;
@@ -196,6 +224,19 @@
keylen = cipher_info->key_bitlen / 8;
+ if (mode == MBEDTLS_PKCS12_PBE_DECRYPT) {
+ if (output_size < len) {
+ return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL;
+ }
+ }
+
+ if (mode == MBEDTLS_PKCS12_PBE_ENCRYPT) {
+ padlen = cipher_info->block_size - (len % cipher_info->block_size);
+ if (output_size < (len + padlen)) {
+ return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL;
+ }
+ }
+
if ((ret = pkcs12_pbe_derive_key_iv(pbe_params, md_type, pwd, pwdlen,
key, keylen,
iv, cipher_info->iv_size)) != 0) {
@@ -242,14 +283,16 @@
}
if ((ret = mbedtls_cipher_update(&cipher_ctx, data, len,
- output, &olen)) != 0) {
+ output, output_len)) != 0) {
goto exit;
}
- if ((ret = mbedtls_cipher_finish(&cipher_ctx, output + olen, &olen)) != 0) {
+ if ((ret = mbedtls_cipher_finish(&cipher_ctx, output + (*output_len), &finish_olen)) != 0) {
ret = MBEDTLS_ERR_PKCS12_PASSWORD_MISMATCH;
}
+ *output_len += finish_olen;
+
exit:
mbedtls_platform_zeroize(key, sizeof(key));
mbedtls_platform_zeroize(iv, sizeof(iv));
diff --git a/tests/suites/test_suite_pkcs12.data b/tests/suites/test_suite_pkcs12.data
index 9787c4e..d078a68 100644
--- a/tests/suites/test_suite_pkcs12.data
+++ b/tests/suites/test_suite_pkcs12.data
@@ -36,28 +36,36 @@
PBE Encrypt, pad = 7 (OK)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_encrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAAAA":0:"5F2C15056A36F3A78856E9E662DD27CB"
+pkcs12_pbe_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAAAA":16:0:"5F2C15056A36F3A78856E9E662DD27CB"
PBE Encrypt, pad = 8 (OK)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_encrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAA":0:"5F2C15056A36F3A70F70A3D4EC4004A8"
+pkcs12_pbe_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAA":16:0:"5F2C15056A36F3A70F70A3D4EC4004A8"
+
+PBE Encrypt, pad = 8 (Invalid output size)
+depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
+pkcs12_pbe_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAA":15:MBEDTLS_ERR_ASN1_BUF_TOO_SMALL:"5F2C15056A36F3A70F70A3D4EC4004A8"
PBE Encrypt, pad = 8 (PKCS7 padding disabled)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_encrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAA":MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE:""
+pkcs12_pbe_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"AAAAAAAAAAAAAAAA":0:MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE:""
PBE Decrypt, pad = 7 (OK)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_decrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A78856E9E662DD27CB":0:"AAAAAAAAAAAAAAAAAA"
+pkcs12_pbe_decrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A78856E9E662DD27CB":16:0:"AAAAAAAAAAAAAAAAAA"
+
+PBE Decrypt, pad = 8 (Invalid output size)
+depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
+pkcs12_pbe_decrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A70F70A3D4EC4004A8":15:MBEDTLS_ERR_ASN1_BUF_TOO_SMALL:"AAAAAAAAAAAAAAAA"
PBE Decrypt, pad = 8 (OK)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_decrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A70F70A3D4EC4004A8":0:"AAAAAAAAAAAAAAAA"
+pkcs12_pbe_decrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A70F70A3D4EC4004A8":16:0:"AAAAAAAAAAAAAAAA"
PBE Decrypt, (Invalid padding & PKCS7 padding disabled)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_decrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A79F2B90F1428110E2":0:"AAAAAAAAAAAAAAAAAA07070707070708"
+pkcs12_pbe_decrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A79F2B90F1428110E2":16:0:"AAAAAAAAAAAAAAAAAA07070707070708"
PBE Decrypt, (Invalid padding & PKCS7 padding enabled)
depends_on:MBEDTLS_SHA1_C:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7
-pkcs12_pbe_decrypt:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"300E0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A79F2B90F1428110E2":MBEDTLS_ERR_PKCS12_PASSWORD_MISMATCH:"AAAAAAAAAAAAAAAAAA07070707070708"
+pkcs12_pbe_decrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:MBEDTLS_CIPHER_DES_EDE3_CBC:MBEDTLS_MD_SHA1:"0409CCCCCCCCCCCCCCCCCC02010A":"BBBBBBBBBBBBBBBBBB":"5F2C15056A36F3A79F2B90F1428110E2":16:MBEDTLS_ERR_PKCS12_PASSWORD_MISMATCH:"AAAAAAAAAAAAAAAAAA07070707070708"
diff --git a/tests/suites/test_suite_pkcs12.function b/tests/suites/test_suite_pkcs12.function
index 288188e..8014c62 100644
--- a/tests/suites/test_suite_pkcs12.function
+++ b/tests/suites/test_suite_pkcs12.function
@@ -69,62 +69,91 @@
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_ASN1_PARSE_C */
-void pkcs12_pbe_encrypt(int cipher, int md, data_t *params_hex, data_t *pw,
- data_t *data, int ref_ret, data_t *ref_out)
+void pkcs12_pbe_encrypt(int params_tag, int cipher, int md, data_t *params_hex, data_t *pw,
+ data_t *data, int outsize, int ref_ret, data_t *ref_out)
{
int my_ret;
mbedtls_asn1_buf pbe_params;
unsigned char *my_out = NULL;
mbedtls_cipher_type_t cipher_alg = (mbedtls_cipher_type_t) cipher;
mbedtls_md_type_t md_alg = (mbedtls_md_type_t) md;
- unsigned int block_size;
- const mbedtls_cipher_info_t *cipher_info;
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+ size_t my_out_len = 0;
+#endif
- cipher_info = mbedtls_cipher_info_from_type(cipher_alg);
- block_size = cipher_info->block_size;
- ASSERT_ALLOC(my_out, ((data->len/block_size) + 1) * block_size);
+ ASSERT_ALLOC(my_out, outsize);
- pbe_params.tag = params_hex->x[0];
- pbe_params.len = params_hex->x[1];
- pbe_params.p = params_hex->x + 2;
+ pbe_params.tag = params_tag;
+ pbe_params.len = params_hex->len;
+ pbe_params.p = params_hex->x;
- 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);
- TEST_EQUAL(my_ret, ref_ret);
+ 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);
+ TEST_EQUAL(my_ret, ref_ret);
+ }
if (ref_ret == 0) {
ASSERT_COMPARE(my_out, ref_out->len,
ref_out->x, ref_out->len);
}
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+ my_ret = mbedtls_pkcs12_pbe_ext(&pbe_params, MBEDTLS_PKCS12_PBE_ENCRYPT, cipher_alg,
+ md_alg, pw->x, pw->len, data->x, data->len, my_out,
+ outsize, &my_out_len);
+ TEST_EQUAL(my_ret, ref_ret);
+ if (ref_ret == 0) {
+ ASSERT_COMPARE(my_out, my_out_len,
+ ref_out->x, ref_out->len);
+ }
+#endif
+
exit:
mbedtls_free(my_out);
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_ASN1_PARSE_C */
-void pkcs12_pbe_decrypt(int cipher, int md, data_t *params_hex, data_t *pw,
- data_t *data, int ref_ret, data_t *ref_out)
+void pkcs12_pbe_decrypt(int params_tag, int cipher, int md, data_t *params_hex, data_t *pw,
+ data_t *data, int outsize, int ref_ret, data_t *ref_out)
{
int my_ret;
mbedtls_asn1_buf pbe_params;
unsigned char *my_out = NULL;
mbedtls_cipher_type_t cipher_alg = (mbedtls_cipher_type_t) cipher;
mbedtls_md_type_t md_alg = (mbedtls_md_type_t) md;
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+ size_t my_out_len = 0;
+#endif
- ASSERT_ALLOC(my_out, data->len);
+ ASSERT_ALLOC(my_out, outsize);
- pbe_params.tag = params_hex->x[0];
- pbe_params.len = params_hex->x[1];
- pbe_params.p = params_hex->x + 2;
+ pbe_params.tag = params_tag;
+ pbe_params.len = params_hex->len;
+ pbe_params.p = params_hex->x;
- 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);
- TEST_EQUAL(my_ret, ref_ret);
+ 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);
+ TEST_EQUAL(my_ret, ref_ret);
+ }
+
if (ref_ret == 0) {
ASSERT_COMPARE(my_out, ref_out->len,
ref_out->x, ref_out->len);
}
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+ my_ret = mbedtls_pkcs12_pbe_ext(&pbe_params, MBEDTLS_PKCS12_PBE_DECRYPT, cipher_alg,
+ md_alg, pw->x, pw->len, data->x, data->len, my_out,
+ outsize, &my_out_len);
+ TEST_EQUAL(my_ret, ref_ret);
+ if (ref_ret == 0) {
+ ASSERT_COMPARE(my_out, my_out_len,
+ ref_out->x, ref_out->len);
+ }
+#endif
+
exit:
mbedtls_free(my_out);
}