From 708d78f80b8b3c3cda9f46977a67625149c30591 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Wed, 19 Jul 2023 14:01:35 +0100 Subject: [PATCH] Improve & test legacy mbedtls_pkcs5_pbe2 * Prevent pkcs5_pbe2 encryption when PKCS7 padding has been disabled since this not part of the specs. * Allow decryption when PKCS7 padding is disabled for legacy reasons, However, invalid padding is not checked. * Add tests to check these scenarios. Test data has been reused but with changing padding data in last block to check for valid/invalid padding. * Document new behaviour, known limitations and possible security concerns. Signed-off-by: Waleed Elmelegy --- include/mbedtls/pkcs5.h | 25 ++++++++++++++++- library/pkcs5.c | 18 ++++++++++++ tests/suites/test_suite_pkcs5.data | 24 ++++++++++++++++ tests/suites/test_suite_pkcs5.function | 38 +++++++++++++++++++++++--- 4 files changed, 100 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/pkcs5.h b/include/mbedtls/pkcs5.h index 152b45fb1..361854116 100644 --- a/include/mbedtls/pkcs5.h +++ b/include/mbedtls/pkcs5.h @@ -53,13 +53,36 @@ extern "C" { /** * \brief PKCS#5 PBES2 function * + * \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_PKCS5_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 the ASN.1 algorithm parameters * \param mode either MBEDTLS_PKCS5_DECRYPT or MBEDTLS_PKCS5_ENCRYPT * \param pwd password to use when generating key * \param pwdlen length of password * \param data data to process * \param datalen length of data - * \param output output buffer + * \param output Output buffer. + * On success, it contains the decrypted data, possibly + * followed by the CBC padding. + * On failure, the content is indetermidate. + * For decryption, there must be enough room for \p datalen + * bytes. + * For encryption, there must be enough room for + * \p datalen + 1 bytes, rounded up to the block size of + * the block cipher identified by \p pbe_params. * * \returns 0 on success, or a MBEDTLS_ERR_XXX code if verification fails. */ diff --git a/library/pkcs5.c b/library/pkcs5.c index 0f4baf1bd..add688bfb 100644 --- a/library/pkcs5.c +++ b/library/pkcs5.c @@ -203,6 +203,24 @@ int mbedtls_pkcs5_pbes2(const mbedtls_asn1_buf *pbe_params, int mode, goto exit; } + /* PKCS5 uses CBC with PKCS7 padding (which is the same as + * "PKCS5 padding" except that it's typically only called PKCS5 + * with 64-bit-block ciphers). + */ + 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_DECRYPT) + padding = MBEDTLS_PADDING_NONE; +#endif + if ((ret = mbedtls_cipher_set_padding_mode(&cipher_ctx, padding)) != 0) { + goto exit; + } + if ((ret = mbedtls_cipher_crypt(&cipher_ctx, iv, enc_scheme_params.len, data, datalen, output, &olen)) != 0) { ret = MBEDTLS_ERR_PKCS5_PASSWORD_MISMATCH; diff --git a/tests/suites/test_suite_pkcs5.data b/tests/suites/test_suite_pkcs5.data index 06d620a12..c86f55ed8 100644 --- a/tests/suites/test_suite_pkcs5.data +++ b/tests/suites/test_suite_pkcs5.data @@ -106,10 +106,34 @@ PBKDF2 Python hashlib Test Vector #6 (SHA512) depends_on:MBEDTLS_MD_CAN_SHA512 pbkdf2_hmac:MBEDTLS_MD_SHA512:"7061737300776f7264":"7361006c74":4096:16:"9d9e9c4cd21fe4be24d5b8244c759665" +PBES2 Encrypt, pad=6 (OK) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7 +pbes2_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D55CD8A2A0EFA01C8B4EDFF":0:"1B60098D4834CA752D37B430E70B7A085CFF86E21F4849F969DD1DF623342662443F8BD1252BF83CEF6917551B08EF55A69C8F2BFFC93BCB2DFE2E354DA28F896D1BD1BFB972A1251219A6EC7183B0A4CF2C4998449ED786CAE2138437289EB2203974000C38619DA57A4E685D29649284602BD1806131772DA11A682674DC22B2CF109128DDB7FD980E1C5741FC0DB7" + +PBES2 Encrypt, pad=16 (OK) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7 +pbes2_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D5510101010101010101010101010101010":0:"1B60098D4834CA752D37B430E70B7A085CFF86E21F4849F969DD1DF623342662443F8BD1252BF83CEF6917551B08EF55A69C8F2BFFC93BCB2DFE2E354DA28F896D1BD1BFB972A1251219A6EC7183B0A4CF2C4998449ED786CAE2138437289EB2203974000C38619DA57A4E685D29649284602BD1806131772DA11A682674DC22D8D337E00CB5D1B5B76BE4AE39341405" + +PBES2 Encrypt, pad=6 (PKCS7 padding disabled) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_CIPHER_PADDING_PKCS7 +pbes2_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D55CD8A2A0EFA01C8B4EDFF":MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE:"" + +PBES2 Encrypt, pad=16 (PKCS7 padding disabled) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_CIPHER_PADDING_PKCS7 +pbes2_encrypt:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D5510101010101010101010101010101010":MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE:"" + PBES2 Decrypt (OK) depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pkcs5_pbes2:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"1B60098D4834CA752D37B430E70B7A085CFF86E21F4849F969DD1DF623342662443F8BD1252BF83CEF6917551B08EF55A69C8F2BFFC93BCB2DFE2E354DA28F896D1BD1BFB972A1251219A6EC7183B0A4CF2C4998449ED786CAE2138437289EB2203974000C38619DA57A4E685D29649284602BD1806131772DA11A682674DC22B2CF109128DDB7FD980E1C5741FC0DB7":0:"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D55CD8A2A0EFA01C8B4EDFF060606060606" +PBES2 Decrypt (Invalid padding & PKCS7 padding disabled) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_CIPHER_PADDING_PKCS7 +mbedtls_pkcs5_pbes2:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"1B60098D4834CA752D37B430E70B7A085CFF86E21F4849F969DD1DF623342662443F8BD1252BF83CEF6917551B08EF55A69C8F2BFFC93BCB2DFE2E354DA28F896D1BD1BFB972A1251219A6EC7183B0A4CF2C4998449ED786CAE2138437289EB2203974000C38619DA57A4E685D29649284602BD1806131772DA11A682674DC22B2CF109128DDB7FDA3488A7144097565":0:"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D55CD8A2A0EFA01C8B4EDFF060606060607" + +PBES2 Decrypt (Invalid padding & PKCS7 padding enabled) +depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_CIPHER_PADDING_PKCS7 +mbedtls_pkcs5_pbes2:MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE:"301B06092A864886F70D01050C300E04082ED7F24A1D516DD702020800301406082A864886F70D030704088A4FCC9DCC394910":"70617373776f7264":"1B60098D4834CA752D37B430E70B7A085CFF86E21F4849F969DD1DF623342662443F8BD1252BF83CEF6917551B08EF55A69C8F2BFFC93BCB2DFE2E354DA28F896D1BD1BFB972A1251219A6EC7183B0A4CF2C4998449ED786CAE2138437289EB2203974000C38619DA57A4E685D29649284602BD1806131772DA11A682674DC22B2CF109128DDB7FDA3488A7144097565":MBEDTLS_ERR_PKCS5_PASSWORD_MISMATCH:"308187020100301306072A8648CE3D020106082A8648CE3D030107046D306B0201010420F12A1320760270A83CBFFD53F6031EF76A5D86C8A204F2C30CA9EBF51F0F0EA7A1440342000437CC56D976091E5A723EC7592DFF206EEE7CF9069174D0AD14B5F768225962924EE500D82311FFEA2FD2345D5D16BD8A88C26B770D55CD8A2A0EFA01C8B4EDFF060606060607" + PBES2 Decrypt (bad params tag) depends_on:MBEDTLS_MD_CAN_SHA1:MBEDTLS_DES_C mbedtls_pkcs5_pbes2:MBEDTLS_ASN1_SEQUENCE:"":"":"":MBEDTLS_ERR_PKCS5_INVALID_FORMAT + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:"" diff --git a/tests/suites/test_suite_pkcs5.function b/tests/suites/test_suite_pkcs5.function index 9f9958697..1f1d052e7 100644 --- a/tests/suites/test_suite_pkcs5.function +++ b/tests/suites/test_suite_pkcs5.function @@ -26,6 +26,36 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_ASN1_PARSE_C */ +void pbes2_encrypt(int params_tag, data_t *params_hex, data_t *pw, + data_t *data, int ref_ret, data_t *ref_out) +{ + int my_ret; + mbedtls_asn1_buf params; + unsigned char *my_out = NULL; + + MD_PSA_INIT(); + + params.tag = params_tag; + params.p = params_hex->x; + params.len = params_hex->len; + + ASSERT_ALLOC(my_out, ref_out->len); + + my_ret = mbedtls_pkcs5_pbes2(¶ms, MBEDTLS_PKCS5_ENCRYPT, + 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 mbedtls_pkcs5_pbes2(int params_tag, data_t *params_hex, data_t *pw, data_t *data, int ref_ret, data_t *ref_out) @@ -40,14 +70,14 @@ void mbedtls_pkcs5_pbes2(int params_tag, data_t *params_hex, data_t *pw, params.p = params_hex->x; params.len = params_hex->len; - my_out = mbedtls_test_zero_alloc(ref_out->len); + ASSERT_ALLOC(my_out, ref_out->len); my_ret = mbedtls_pkcs5_pbes2(¶ms, MBEDTLS_PKCS5_DECRYPT, pw->x, pw->len, data->x, data->len, my_out); - TEST_ASSERT(my_ret == ref_ret); - + TEST_EQUAL(my_ret, ref_ret); if (ref_ret == 0) { - TEST_ASSERT(memcmp(my_out, ref_out->x, ref_out->len) == 0); + ASSERT_COMPARE(my_out, ref_out->len, + ref_out->x, ref_out->len); } exit: