Merge pull request #1055 from waleed-elmelegy-arm/add-new-pkcs12-pbe2-ext-fun

Add new pkcs12 pbe2 ext fun

ChangeLog.d/add-new-pkcs5-pbe2-ext-fun.txt

@@ -1,6 +1,7 @@
 Security
-   * Developers using mbedtls_pkcs5_pbes2() should review the size of the output
+   * Developers using mbedtls_pkcs5_pbes2() or mbedtls_pkcs12_pbe() should
-     buffer passed to this function, and note that the output after decryption
+     review the size of the output buffer passed to this function, and note
-     may include CBC padding. Consider moving to the new function
+     that the output after decryption may include CBC padding. Consider moving
-     mbedtls_pkcs5_pbes2_ext() which checks for overflow of the output buffer
+     to the new functions mbedtls_pkcs5_pbes2_ext() or mbedtls_pkcs12_pbe_ext()
-     and reports the actual length of the output.
+     which checks for overflow of the output buffer and reports the actual
+     length of the output.

include/mbedtls/pkcs12.h

@@ -79,7 +79,7 @@ extern "C" {
  * \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,
@@ -96,9 +96,60 @@ extern "C" {
 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 */
 
 /**

library/pkcs12.c

@@ -129,18 +129,46 @@ static int pkcs12_pbe_derive_key_iv(mbedtls_asn1_buf *pbe_params, mbedtls_md_typ
 
 #undef PKCS12_MAX_PWDLEN
 
+#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;
@@ -153,6 +181,19 @@ int mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
 
     keylen = (int) mbedtls_cipher_info_get_key_bitlen(cipher_info) / 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, mbedtls_cipher_info_get_iv_size(cipher_info))) != 0) {
@@ -201,14 +242,16 @@ int mbedtls_pkcs12_pbe(mbedtls_asn1_buf *pbe_params, int mode,
     }
 
     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));

tests/suites/test_suite_pkcs12.data

@@ -36,28 +36,36 @@ pkcs12_derive_key:MBEDTLS_MD_MD5:48:"0123456789abcdef":USE_GIVEN_INPUT:"01234567
 
 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"
+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_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"
+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_MD_CAN_SHA1: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_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:""
+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_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"
+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_MD_CAN_SHA1: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_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"
+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_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"
+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_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"
+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"

tests/suites/test_suite_pkcs12.function

@@ -70,33 +70,52 @@ exit:
 /* 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,
+void pkcs12_pbe_encrypt(int params_tag, int cipher, int md, data_t *params_hex, data_t *pw,
-                        data_t *data, int ref_ret, data_t *ref_out)
+                        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;
-    size_t block_size;
+#if defined(MBEDTLS_CIPHER_PADDING_PKCS7)
+    size_t my_out_len = 0;
+#endif
 
     MD_PSA_INIT();
 
-    block_size = mbedtls_cipher_info_get_block_size(mbedtls_cipher_info_from_type(cipher_alg));
+    TEST_CALLOC(my_out, outsize);
-    TEST_CALLOC(my_out, ((data->len/block_size) + 1) * block_size);
 
-    pbe_params.tag = params_hex->x[0];
+    pbe_params.tag = params_tag;
-    pbe_params.len = params_hex->x[1];
+    pbe_params.len = params_hex->len;
-    pbe_params.p = params_hex->x + 2;
+    pbe_params.p = params_hex->x;
 
-    my_ret = mbedtls_pkcs12_pbe(&pbe_params, MBEDTLS_PKCS12_PBE_ENCRYPT, cipher_alg,
+    if (ref_ret != MBEDTLS_ERR_ASN1_BUF_TOO_SMALL) {
-                                md_alg, pw->x, pw->len, data->x, data->len, my_out);
+        my_ret = mbedtls_pkcs12_pbe(&pbe_params, MBEDTLS_PKCS12_PBE_ENCRYPT, cipher_alg,
-    TEST_EQUAL(my_ret, ref_ret);
+                                    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)
+
+    pbe_params.tag = params_tag;
+    pbe_params.len = params_hex->len;
+    pbe_params.p = params_hex->x;
+
+    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);
     MD_PSA_DONE();
@@ -104,31 +123,53 @@ exit:
 /* 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,
+void pkcs12_pbe_decrypt(int params_tag, int cipher, int md, data_t *params_hex, data_t *pw,
-                        data_t *data, int ref_ret, data_t *ref_out)
+                        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
 
     MD_PSA_INIT();
 
-    TEST_CALLOC(my_out, data->len);
+    TEST_CALLOC(my_out, outsize);
 
-    pbe_params.tag = params_hex->x[0];
+    pbe_params.tag = params_tag;
-    pbe_params.len = params_hex->x[1];
+    pbe_params.len = params_hex->len;
-    pbe_params.p = params_hex->x + 2;
+    pbe_params.p = params_hex->x;
+
+    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);
+    }
 
-    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)
+
+    pbe_params.tag = params_tag;
+    pbe_params.len = params_hex->len;
+    pbe_params.p = params_hex->x;
+
+    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);
     MD_PSA_DONE();