Merge pull request #1053 from waleed-elmelegy-arm/Improve-and-test-mbedtls_pkcs12_pbe
Improve & test legacy mbedtls_pkcs12_pbe
diff --git a/include/mbedtls/pkcs12.h b/include/mbedtls/pkcs12.h
index eb9e2d9..8d003b5 100644
--- a/include/mbedtls/pkcs12.h
+++ b/include/mbedtls/pkcs12.h
@@ -56,6 +56,21 @@
* \brief PKCS12 Password Based function (encryption / decryption)
* for cipher-based and mbedtls_md-based PBE's
*
+ * \note When encrypting, #MBEDTLS_CIPHER_PADDING_PKCS7 must
+ * be enabled at compile time.
+ *
+ * \warning When decrypting:
+ * - if #MBEDTLS_CIPHER_PADDING_PKCS7 is enabled at compile
+ * time, 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).
+ * - if #MBEDTLS_CIPHER_PADDING_PKCS7 is disabled at compile
+ * time, this function does not validate the CBC padding.
+ *
* \param pbe_params an ASN1 buffer containing the pkcs-12 PbeParams structure
* \param mode either #MBEDTLS_PKCS12_PBE_ENCRYPT or
* #MBEDTLS_PKCS12_PBE_DECRYPT
@@ -66,7 +81,15 @@
* \param pwdlen length of the password (may be 0)
* \param input the input data
* \param len data length
- * \param output the output buffer
+ * \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.
*
* \return 0 if successful, or a MBEDTLS_ERR_XXX code
*/
diff --git a/library/pkcs12.c b/library/pkcs12.c
index db31722..6d52305 100644
--- a/library/pkcs12.c
+++ b/library/pkcs12.c
@@ -171,6 +171,25 @@
goto exit;
}
+#if defined(MBEDTLS_CIPHER_MODE_WITH_PADDING)
+ /* PKCS12 uses CBC with PKCS7 padding */
+
+ mbedtls_cipher_padding_t padding = MBEDTLS_PADDING_PKCS7;
+#if !defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+ /* For historical reasons, when decrypting, this function works when
+ * decrypting even when support for PKCS7 padding is disabled. In this
+ * case, it ignores the padding, and so will never report a
+ * password mismatch.
+ */
+ if (mode == MBEDTLS_PKCS12_PBE_DECRYPT) {
+ padding = MBEDTLS_PADDING_NONE;
+ }
+#endif
+ if ((ret = mbedtls_cipher_set_padding_mode(&cipher_ctx, padding)) != 0) {
+ goto exit;
+ }
+#endif /* MBEDTLS_CIPHER_MODE_WITH_PADDING */
+
if ((ret =
mbedtls_cipher_set_iv(&cipher_ctx, iv,
mbedtls_cipher_info_get_iv_size(cipher_info))) != 0) {
diff --git a/tests/suites/test_suite_pkcs12.data b/tests/suites/test_suite_pkcs12.data
index d8e41fe..074c6c8 100644
--- a/tests/suites/test_suite_pkcs12.data
+++ b/tests/suites/test_suite_pkcs12.data
@@ -33,3 +33,31 @@
PKCS#12 derive key: MD5: Valid password and salt
depends_on:MBEDTLS_MD_CAN_MD5
pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"0123456789abcdef":USE_GIVEN_INPUT:3:"46559deeee036836ab1b633ec620178d4c70eacf42f72a2ad7360c812efa09ca3d7567b489a109050345c2dc6a262995":0
+
+PBE Encrypt, pad = 7 (OK)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
+
+PBE Encrypt, pad = 8 (OK)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
+
+PBE Encrypt, pad = 8 (PKCS7 padding disabled)
+depends_on:MBEDTLS_MD_CAN_SHA1: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:""
+
+PBE Decrypt, pad = 7 (OK)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
+
+PBE Decrypt, pad = 8 (OK)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
+
+PBE Decrypt, (Invalid padding & PKCS7 padding disabled)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
+
+PBE Decrypt, (Invalid padding & PKCS7 padding enabled)
+depends_on:MBEDTLS_MD_CAN_SHA1: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"
diff --git a/tests/suites/test_suite_pkcs12.function b/tests/suites/test_suite_pkcs12.function
index 2c93c13..b557098 100644
--- a/tests/suites/test_suite_pkcs12.function
+++ b/tests/suites/test_suite_pkcs12.function
@@ -68,3 +68,69 @@
MD_PSA_DONE();
}
/* 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)
+{
+ 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;
+ size_t block_size;
+
+ MD_PSA_INIT();
+
+ block_size = mbedtls_cipher_info_get_block_size(mbedtls_cipher_info_from_type(cipher_alg));
+ TEST_CALLOC(my_out, ((data->len/block_size) + 1) * block_size);
+
+ pbe_params.tag = params_hex->x[0];
+ pbe_params.len = params_hex->x[1];
+ pbe_params.p = params_hex->x + 2;
+
+ 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);
+ }
+
+exit:
+ mbedtls_free(my_out);
+ MD_PSA_DONE();
+}
+/* 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)
+{
+ 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;
+
+ MD_PSA_INIT();
+
+ TEST_CALLOC(my_out, data->len);
+
+ pbe_params.tag = params_hex->x[0];
+ pbe_params.len = params_hex->x[1];
+ pbe_params.p = params_hex->x + 2;
+
+ 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);
+ }
+
+exit:
+ mbedtls_free(my_out);
+ MD_PSA_DONE();
+}
+/* END_CASE */