From 9de84bd67775e05fabca1907cd9f2ea33078d99a Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 8 Feb 2024 17:40:27 +0100 Subject: [PATCH 01/17] rsa: reject buffers with data outside main SEQUENCE when parsing keys Signed-off-by: Valerio Setti --- library/rsa.c | 10 ++++++++-- tests/suites/test_suite_psa_crypto.data | 4 ++-- tests/suites/test_suite_rsa.data | 6 +++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index f4c08626c..2c3386912 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -108,7 +108,10 @@ int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, si return ret; } - /* mbedtls_asn1_get_tag() already ensures that len is valid (i.e. p+len <= end)*/ + if (end != p + len) { + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; + } + end = p + len; if ((ret = mbedtls_asn1_get_int(&p, end, &version)) != 0) { @@ -241,7 +244,10 @@ int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, return ret; } - /* mbedtls_asn1_get_tag() already ensures that len is valid (i.e. p+len <= end)*/ + if (end != p + len) { + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; + } + end = p + len; /* Import N */ diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 34af94a25..dc43599a7 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -106,7 +106,7 @@ import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa24 PSA import/export RSA keypair: trailing garbage ignored depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT -import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:0:1024:-1:PSA_SUCCESS:0 +import_with_data:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_ERROR_INVALID_ARGUMENT PSA import/export RSA public key: good, 1024-bit, opaque depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_PUBLIC_KEY:PSA_CRYPTO_DRIVER_TEST @@ -158,7 +158,7 @@ import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa24 PSA import/export RSA keypair: trailing garbage ignored, opaque depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT:PSA_CRYPTO_DRIVER_TEST -import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( PSA_KEY_PERSISTENCE_VOLATILE, TEST_DRIVER_LOCATION ):1024:-1:PSA_SUCCESS:0 +import_with_data:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_ERROR_INVALID_ARGUMENT PSA import RSA keypair: truncated depends_on:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT diff --git a/tests/suites/test_suite_rsa.data b/tests/suites/test_suite_rsa.data index b89d1583c..1964b3286 100644 --- a/tests/suites/test_suite_rsa.data +++ b/tests/suites/test_suite_rsa.data @@ -662,7 +662,7 @@ RSA parse private key - correct values, extra integer inside the SEQUENCE rsa_parse_pkcs1_key:0:"3066020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c020100":MBEDTLS_ERR_ASN1_LENGTH_MISMATCH RSA parse private key - correct values, extra integer outside the SEQUENCE -rsa_parse_pkcs1_key:0:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c020100":0 +rsa_parse_pkcs1_key:0:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c020100":MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA parse private key - correct values, n wrong tag rsa_parse_pkcs1_key:0:"3063020100FF1100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG @@ -707,7 +707,7 @@ RSA parse public key - public exponent 0 rsa_parse_pkcs1_key:1:"308189028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203000000":MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA parse public key - wrong sequence length -rsa_parse_pkcs1_key:1:"308188028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001":MBEDTLS_ERR_ASN1_OUT_OF_DATA +rsa_parse_pkcs1_key:1:"308188028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001":MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA parse public key - wrong modulus length rsa_parse_pkcs1_key:1:"308189028180009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG @@ -725,7 +725,7 @@ RSA parse public key - correct values, extra integer inside the SEQUENCE rsa_parse_pkcs1_key:1:"30818c028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001020100":MBEDTLS_ERR_ASN1_LENGTH_MISMATCH RSA parse public key - correct values, extra integer outside the SEQUENCE -rsa_parse_pkcs1_key:1:"308189028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001020100":0 +rsa_parse_pkcs1_key:1:"308189028181009f091e6968b474f76f0e9c237c1d895996ae704b4f6d706acec8d2daac6209bf524aa3f658d0283adba1077f6cbe92e425dcde52290b239cade91be86c88425434986806e85734e159768f3dfea932baaa9409d25bace8ee9dce0cdde0903207299de575ae60feccf0daf82334ab83638539b0da74072f253acea8afc8e66bb70203010001020100":MBEDTLS_ERR_RSA_BAD_INPUT_DATA RSA priv key write - incremental output buffer size rsa_key_write_incremental:0:"3063020100021100cc8ab070369ede72920e5a51523c857102030100010211009a6318982a7231de1894c54aa4909201020900f3058fd8dc484d61020900d7770dbd8b78a2110209009471f14c26428401020813425f060c4b72210208052b93d01747a87c" From b1f6d2ad6fa2621772d35bda1835d8b0e1fa1d02 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 8 Feb 2024 17:41:45 +0100 Subject: [PATCH 02/17] asn1: enable mbedtls_asn1_get_tag() when PEM_PARSE_C is defined Signed-off-by: Valerio Setti --- include/mbedtls/asn1.h | 5 +++-- library/asn1parse.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index ff019f432..d8ee46930 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -198,7 +198,7 @@ typedef struct mbedtls_asn1_named_data { mbedtls_asn1_named_data; #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ - defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) + defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) /** * \brief Get the length of an ASN.1 element. * Updates the pointer to immediately behind the length. @@ -245,7 +245,8 @@ int mbedtls_asn1_get_len(unsigned char **p, int mbedtls_asn1_get_tag(unsigned char **p, const unsigned char *end, size_t *len, int tag); -#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */ +#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || + MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */ #if defined(MBEDTLS_ASN1_PARSE_C) /** diff --git a/library/asn1parse.c b/library/asn1parse.c index e33fdf71d..644b43ba9 100644 --- a/library/asn1parse.c +++ b/library/asn1parse.c @@ -8,7 +8,7 @@ #include "common.h" #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ - defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) + defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) #include "mbedtls/asn1.h" #include "mbedtls/platform_util.h" @@ -74,7 +74,8 @@ int mbedtls_asn1_get_tag(unsigned char **p, return mbedtls_asn1_get_len(p, end, len); } -#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */ +#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || + MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */ #if defined(MBEDTLS_ASN1_PARSE_C) int mbedtls_asn1_get_bool(unsigned char **p, From 2653e92a578fbe027579150b8c94f2a5d36baf48 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 8 Feb 2024 17:51:00 +0100 Subject: [PATCH 03/17] pem: fix valid data length returned by mbedtls_pem_read_buffer() ctx->buflen now returns the amount of valid data in ctx->buf. Unencrypted buffers were already ok, but encrypted ones were used to return the length of the encrypted buffer, not the unencrypted one. This commit fix this behavior for encrypted buffers. Signed-off-by: Valerio Setti --- include/mbedtls/pem.h | 6 +++--- library/pem.c | 20 +++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/pem.h b/include/mbedtls/pem.h index cc617a9bc..3c6a28d98 100644 --- a/include/mbedtls/pem.h +++ b/include/mbedtls/pem.h @@ -73,11 +73,11 @@ void mbedtls_pem_init(mbedtls_pem_context *ctx); * \param data source data to look in (must be nul-terminated) * \param pwd password for decryption (can be NULL) * \param pwdlen length of password - * \param use_len destination for total length used (set after header is - * correctly read, so unless you get + * \param use_len destination for total length used from data buffer. It is + * set after header is correctly read, so unless you get * MBEDTLS_ERR_PEM_BAD_INPUT_DATA or * MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT, use_len is - * the length to skip) + * the length to skip. * * \note Attempts to check password correctness by verifying if * the decrypted text starts with an ASN.1 sequence of diff --git a/library/pem.c b/library/pem.c index 539134c29..7e7f86ff5 100644 --- a/library/pem.c +++ b/library/pem.c @@ -17,6 +17,7 @@ #include "mbedtls/cipher.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" +#include "mbedtls/asn1.h" #include @@ -431,15 +432,20 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const } /* - * The result will be ASN.1 starting with a SEQUENCE tag, with 1 to 3 - * length bytes (allow 4 to be sure) in all known use cases. - * - * Use that as a heuristic to try to detect password mismatches. + * The result will be ASN.1 starting with a SEQUENCE tag. Parse it + * with ASN.1 functions in order to: + * - Have an heuristic guess about password mismatches. + * - Update len variable to the amount of valid data inside buf. */ - if (len <= 2 || buf[0] != 0x30 || buf[1] > 0x83) { - mbedtls_zeroize_and_free(buf, len); - return MBEDTLS_ERR_PEM_PASSWORD_MISMATCH; + unsigned char *p = buf; + ret = mbedtls_asn1_get_tag(&p, buf + len, &len, + MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED); + if (ret != 0) { + mbedtls_free(buf); + return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PEM_INVALID_DATA, ret); } + /* Add also the sequence block (tag + len) to the total amount of valid data. */ + len += (p - buf); #else mbedtls_zeroize_and_free(buf, len); return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE; From 010d23f9af824f58910cf3898ce4e6da5ea0ea35 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 8 Feb 2024 17:56:03 +0100 Subject: [PATCH 04/17] test_suite_[pkparse|x509parse]: fix return values of some PEM related error tests Signed-off-by: Valerio Setti --- tests/suites/test_suite_pkparse.data | 2 +- tests/suites/test_suite_x509parse.data | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 762fd52a2..5b2dbb9cc 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -8,7 +8,7 @@ pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLTest":0 Parse RSA Key #3 (Wrong password) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_AES_C -pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PK_PASSWORD_MISMATCH +pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG Parse RSA Key #4 (DES Encrypted) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_DES_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index b9ae20c56..2b0920d80 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1774,7 +1774,7 @@ x509parse_crt:"307d3068a0030201008204deadbeef300d06092a864886f70d01010b0500300c3 X509 CRT ASN1 (TBS, inv SubPubKeyInfo, inv internal bitstring length) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 -x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400300000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY + MBEDTLS_ERR_ASN1_OUT_OF_DATA +x509parse_crt:"308180306ba0030201008204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a300806001304546573743015300d06092A864886F70D0101010500030400300000300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_PK_INVALID_PUBKEY X509 CRT ASN1 (TBS, inv SubPubKeyInfo, inv internal bitstring tag) depends_on:MBEDTLS_RSA_C:MBEDTLS_MD_CAN_SHA256 From 4ade8ee5b96a0cb0331efa2e2dd2b795fc2cb2cd Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 9 Feb 2024 17:44:07 +0100 Subject: [PATCH 05/17] test_suite_pem: more tests for ASN.1 parsing after decoding Signed-off-by: Valerio Setti --- tests/suites/test_suite_pem.data | 13 +++++++++++++ tests/suites/test_suite_pem.function | 1 + 2 files changed, 14 insertions(+) diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index a4dff45f0..32d3c279b 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -49,3 +49,16 @@ mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KE PEM read (malformed PEM AES-128-CBC) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,AA94892A169FA426AA94892A169FA426\n\nMAAA\n-----END EC PRIVATE KEY-----":"pwd":MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH:"" + +# The output sequence's length is not multiple of block size (16 bytes). This +# proves that the pem_context->len value is properly updated based on the SEQUENCE +# length read from the decoded ASN.1 data (i.e. extra padding, if any, is ignored). +PEM read (valid EC key encoded with AES-128-CBC) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,151F851B6A7F3FBDAA5B7173117D0127\n\nLw+0OM+0Bwcl+ls/vxQbLrVshGc7bsNPvvtj2sJeMFFEq3V1mj/IO++0KK/CDhMH\nh6CZPsmgVOeM5uFpqYaq0fJbUduN2eDMWszWRm0SFkY=\n-----END EC PRIVATE KEY-----":"pwdpwd":0:"3041020101040f00d8023c809afd45e426d1a4dbe0ffa00706052b81040004a1220320000400da1ecfa53d528237625e119e2e0500d2eb671724f16deb6a63749516b7" + +# The text "hello world" (which is clearly not a valid ASN.1 SEQUENCE) is encoded +# with AES-128-CBC to prove that ASN.1 parsing after decoding fails. +PEM read (Invalid SEQUENCE encoded with AES-128-CBC) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,765FCB151B573FC9E5FB3A0E5A198785\n\nU2FsdGVkX1/+Vl2WMhEy3zcdg14R+flkg/pW4ei4d0I=\n-----END EC PRIVATE KEY-----":"pwdpwd":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:"" diff --git a/tests/suites/test_suite_pem.function b/tests/suites/test_suite_pem.function index 413dc551c..2acc16e9f 100644 --- a/tests/suites/test_suite_pem.function +++ b/tests/suites/test_suite_pem.function @@ -3,6 +3,7 @@ #include "mbedtls/pem.h" #include "mbedtls/des.h" #include "mbedtls/aes.h" +#include "mbedtls/asn1.h" /* END_HEADER */ /* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */ From 095e1ac71c2703e285f199f5df8016160026449b Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 12 Feb 2024 11:01:37 +0100 Subject: [PATCH 06/17] pem: check data padding in DES/AES decrypted buffers Signed-off-by: Valerio Setti --- library/pem.c | 49 ++++++++++++++++++++++++---- tests/suites/test_suite_pkparse.data | 2 +- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/library/pem.c b/library/pem.c index 7e7f86ff5..f090f4931 100644 --- a/library/pem.c +++ b/library/pem.c @@ -241,6 +241,28 @@ exit: } #endif /* MBEDTLS_AES_C */ +#if defined(MBEDTLS_DES_C) || defined(MBEDTLS_AES_C) +static int pem_check_pkcs_padding(unsigned char *input, size_t input_len, size_t *data_len) +{ + size_t pad_len = input[input_len - 1]; + size_t i; + + if (pad_len > input_len) { + return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + } + + *data_len = input_len - pad_len; + + for (i = *data_len; i < input_len; i++) { + if (input[i] != pad_len) { + return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + } + } + + return 0; +} +#endif /* MBEDTLS_DES_C || MBEDTLS_AES_C */ + #endif /* PEM_RFC1421 */ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const char *footer, @@ -431,21 +453,36 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const return ret; } + /* Check PKCS padding and update data length based on padding info. + * This can be used to detect invalid padding data and password + * mismatches. */ + ret = pem_check_pkcs_padding(buf, len, &len); + if (ret != 0) { + mbedtls_free(buf); + return ret; + } + /* - * The result will be ASN.1 starting with a SEQUENCE tag. Parse it - * with ASN.1 functions in order to: - * - Have an heuristic guess about password mismatches. - * - Update len variable to the amount of valid data inside buf. + * In RFC1421 PEM is used as container for DER (ASN.1) content so we + * can use ASN.1 functions to parse the main SEQUENCE tag and to get its + * length. */ unsigned char *p = buf; - ret = mbedtls_asn1_get_tag(&p, buf + len, &len, + size_t sequence_len; + ret = mbedtls_asn1_get_tag(&p, buf + len, &sequence_len, MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED); if (ret != 0) { mbedtls_free(buf); return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PEM_INVALID_DATA, ret); } /* Add also the sequence block (tag + len) to the total amount of valid data. */ - len += (p - buf); + sequence_len += (p - buf); + + /* Ensure that the reported SEQUENCE length matches the data len (i.e. no + * trailing garbage data). */ + if (len != sequence_len) { + return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + } #else mbedtls_zeroize_and_free(buf, len); return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE; diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 5b2dbb9cc..7ee77da8e 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -8,7 +8,7 @@ pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLTest":0 Parse RSA Key #3 (Wrong password) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_AES_C -pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG +pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PEM_BAD_INPUT_DATA Parse RSA Key #4 (DES Encrypted) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_DES_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC From c1b93751b3091d2d0b9f8b4a197792d692375c3e Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 12 Feb 2024 11:03:16 +0100 Subject: [PATCH 07/17] test_suite_pem: add more test cases for encrypted PEM buffers Signed-off-by: Valerio Setti --- tests/suites/test_suite_pem.data | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index 32d3c279b..df9663b18 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -59,6 +59,18 @@ mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KE # The text "hello world" (which is clearly not a valid ASN.1 SEQUENCE) is encoded # with AES-128-CBC to prove that ASN.1 parsing after decoding fails. +# Since PBKDF1 isn't supported in OpenSSL, here's the steps: +# 1. generate the key (password="password"; IV=0x3132333435363738 in hex or "12345678" as string) +# echo -n "password12345678" | openssl md5 +# 2. encode data +# echo -n "hello world" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" PEM read (Invalid SEQUENCE encoded with AES-128-CBC) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC -mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,765FCB151B573FC9E5FB3A0E5A198785\n\nU2FsdGVkX1/+Vl2WMhEy3zcdg14R+flkg/pW4ei4d0I=\n-----END EC PRIVATE KEY-----":"pwdpwd":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:"" +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\nDfRGkwS+VjvR0IYsjZwW6Q==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:"" + +# Same as above, but with invalid padding data. +# Generated with: +# echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x01\x02\x03\x04\x05" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad +PEM read (Invalid padding data for AES-128-CBC) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" From 3a4f2040b3e68e6f592eb129e9f2b0deeb600d43 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 12 Feb 2024 11:05:15 +0100 Subject: [PATCH 08/17] test_suite_psa_crypto: fix some test descriptions Signed-off-by: Valerio Setti --- tests/suites/test_suite_psa_crypto.data | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index dc43599a7..38e404660 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -104,7 +104,7 @@ PSA import/export RSA keypair: export buffer too small depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:0:1024:-1:PSA_ERROR_BUFFER_TOO_SMALL:1 -PSA import/export RSA keypair: trailing garbage ignored +PSA import/export RSA keypair: trailing garbage rejected depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT import_with_data:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_ERROR_INVALID_ARGUMENT @@ -156,7 +156,7 @@ PSA import/export RSA keypair: export buffer too small, opaque depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT:PSA_CRYPTO_DRIVER_TEST import_export:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b24":PSA_KEY_TYPE_RSA_KEY_PAIR:PSA_KEY_USAGE_EXPORT:PSA_ALG_RSA_PKCS1V15_SIGN_RAW:PSA_KEY_LIFETIME_FROM_PERSISTENCE_AND_LOCATION( PSA_KEY_PERSISTENCE_VOLATILE, TEST_DRIVER_LOCATION ):1024:-1:PSA_ERROR_BUFFER_TOO_SMALL:1 -PSA import/export RSA keypair: trailing garbage ignored, opaque +PSA import/export RSA keypair: trailing garbage rejected, opaque depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR_EXPORT:PSA_CRYPTO_DRIVER_TEST import_with_data:"3082025e02010002818100af057d396ee84fb75fdbb5c2b13c7fe5a654aa8aa2470b541ee1feb0b12d25c79711531249e1129628042dbbb6c120d1443524ef4c0e6e1d8956eeb2077af12349ddeee54483bc06c2c61948cd02b202e796aebd94d3a7cbf859c2c1819c324cb82b9cd34ede263a2abffe4733f077869e8660f7d6834da53d690ef7985f6bc3020301000102818100874bf0ffc2f2a71d14671ddd0171c954d7fdbf50281e4f6d99ea0e1ebcf82faa58e7b595ffb293d1abe17f110b37c48cc0f36c37e84d876621d327f64bbe08457d3ec4098ba2fa0a319fba411c2841ed7be83196a8cdf9daa5d00694bc335fc4c32217fe0488bce9cb7202e59468b1ead119000477db2ca797fac19eda3f58c1024100e2ab760841bb9d30a81d222de1eb7381d82214407f1b975cbbfe4e1a9467fd98adbd78f607836ca5be1928b9d160d97fd45c12d6b52e2c9871a174c66b488113024100c5ab27602159ae7d6f20c3c2ee851e46dc112e689e28d5fcbbf990a99ef8a90b8bb44fd36467e7fc1789ceb663abda338652c3c73f111774902e840565927091024100b6cdbd354f7df579a63b48b3643e353b84898777b48b15f94e0bfc0567a6ae5911d57ad6409cf7647bf96264e9bd87eb95e263b7110b9a1f9f94acced0fafa4d024071195eec37e8d257decfc672b07ae639f10cbb9b0c739d0c809968d644a94e3fd6ed9287077a14583f379058f76a8aecd43c62dc8c0f41766650d725275ac4a1024100bb32d133edc2e048d463388b7be9cb4be29f4b6250be603e70e3647501c97ddde20a4e71be95fd5e71784e25aca4baf25be5738aae59bbfe1c997781447a2b2400":PSA_KEY_TYPE_RSA_KEY_PAIR:1024:PSA_ERROR_INVALID_ARGUMENT From d8840ec6e589b24b6c9b927a095ef11be460f843 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 12 Feb 2024 11:28:06 +0100 Subject: [PATCH 09/17] add changelog Signed-off-by: Valerio Setti --- ChangeLog.d/8799.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/8799.txt diff --git a/ChangeLog.d/8799.txt b/ChangeLog.d/8799.txt new file mode 100644 index 000000000..b44bb9991 --- /dev/null +++ b/ChangeLog.d/8799.txt @@ -0,0 +1,4 @@ +Bugfix + * mbedtls_pem_read_buffer() now performs a check on the padding data of + decrypted keys and it rejects invalid ones. It also parses and validates + the main ASN.1 SEQUENCE header. From 0f286d5453488acbc0ae2191ea80f1156eabbbb4 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 16 Feb 2024 14:30:58 +0100 Subject: [PATCH 10/17] pem: reject empty PEM contents Signed-off-by: Valerio Setti --- library/pem.c | 5 +++++ tests/suites/test_suite_pem.data | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/library/pem.c b/library/pem.c index f090f4931..a111970bb 100644 --- a/library/pem.c +++ b/library/pem.c @@ -244,6 +244,7 @@ exit: #if defined(MBEDTLS_DES_C) || defined(MBEDTLS_AES_C) static int pem_check_pkcs_padding(unsigned char *input, size_t input_len, size_t *data_len) { + /* input_len > 0 is guaranteed by mbedtls_pem_read_buffer(). */ size_t pad_len = input[input_len - 1]; size_t i; @@ -412,6 +413,10 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PEM_INVALID_DATA, ret); } + if (len == 0) { + return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + } + if ((buf = mbedtls_calloc(1, len)) == NULL) { return MBEDTLS_ERR_PEM_ALLOC_FAILED; } diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index df9663b18..a900a33b9 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -22,6 +22,10 @@ mbedtls_pem_write_buffer_lengths PEM read (unencrypted, valid) mbedtls_pem_read_buffer:"^":"$":"^\nTWJlZCBUTFM=\n$":"":0:"4d62656420544c53" +PEM read (unencrypted, empty content) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\n\n-----END EC PRIVATE KEY-----":"":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" + PEM read (DES-EDE3-CBC + invalid iv) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_DES_C mbedtls_pem_read_buffer:"^":"$":"^\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-EDE3-CBC,00$":"pwd":MBEDTLS_ERR_PEM_INVALID_ENC_IV:"" From 8aff4ef274688be38423820091895a2f30187f28 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 16 Feb 2024 14:31:51 +0100 Subject: [PATCH 11/17] test_suite_pem: add more test cases for invalid padding data Signed-off-by: Valerio Setti --- library/pem.c | 2 +- tests/suites/test_suite_pem.data | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/library/pem.c b/library/pem.c index a111970bb..3f01d3bdd 100644 --- a/library/pem.c +++ b/library/pem.c @@ -463,7 +463,7 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const * mismatches. */ ret = pem_check_pkcs_padding(buf, len, &len); if (ret != 0) { - mbedtls_free(buf); + mbedtls_zeroize_and_free(buf, len); return ret; } diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index a900a33b9..e3bb7e4da 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -78,3 +78,17 @@ mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KE PEM read (Invalid padding data for AES-128-CBC) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" + +# Padding data (0x11) is larger than AES block size (16). +# Generated with: +# echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x11\x11\x11\x11\x11" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad +PEM read (AES-128-CBC, padding data is larger than AES block length) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n5wA/XVXHuMsQAAOGFQmK0g==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" + +# Padding data (0x9) is larger than AES block size (8). +# Generated with: +# echo -n -e "\x68\x65\x6c\x6c\x6f\x09\x09\x09" | openssl des-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad +PEM read (DES-CBC, padding data is larger than DES block length) +depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,3132333435363738\n\n6a+B2WineBM=\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" From 4cc6522a85473e6ae49e1558988cde408739305e Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 16 Feb 2024 14:40:42 +0100 Subject: [PATCH 12/17] pem: do not parse ASN1 data after decryption (removes ASN1 dependency) Now that we have padding verification after decryption and since this can be used to validate the password as well there is no need to parse ASN1 content any more, so we can simplify/remove that dependency. Signed-off-by: Valerio Setti --- ChangeLog.d/8799.txt | 3 +-- include/mbedtls/asn1.h | 5 ++--- library/asn1parse.c | 5 ++--- library/pem.c | 23 ----------------------- tests/suites/test_suite_pem.data | 13 +++---------- tests/suites/test_suite_pem.function | 1 - 6 files changed, 8 insertions(+), 42 deletions(-) diff --git a/ChangeLog.d/8799.txt b/ChangeLog.d/8799.txt index b44bb9991..50e7c118c 100644 --- a/ChangeLog.d/8799.txt +++ b/ChangeLog.d/8799.txt @@ -1,4 +1,3 @@ Bugfix * mbedtls_pem_read_buffer() now performs a check on the padding data of - decrypted keys and it rejects invalid ones. It also parses and validates - the main ASN.1 SEQUENCE header. + decrypted keys and it rejects invalid ones. diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index d8ee46930..ff019f432 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -198,7 +198,7 @@ typedef struct mbedtls_asn1_named_data { mbedtls_asn1_named_data; #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ - defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) + defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) /** * \brief Get the length of an ASN.1 element. * Updates the pointer to immediately behind the length. @@ -245,8 +245,7 @@ int mbedtls_asn1_get_len(unsigned char **p, int mbedtls_asn1_get_tag(unsigned char **p, const unsigned char *end, size_t *len, int tag); -#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || - MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */ +#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */ #if defined(MBEDTLS_ASN1_PARSE_C) /** diff --git a/library/asn1parse.c b/library/asn1parse.c index 644b43ba9..e33fdf71d 100644 --- a/library/asn1parse.c +++ b/library/asn1parse.c @@ -8,7 +8,7 @@ #include "common.h" #if defined(MBEDTLS_ASN1_PARSE_C) || defined(MBEDTLS_X509_CREATE_C) || \ - defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) || defined(MBEDTLS_PEM_PARSE_C) + defined(MBEDTLS_PSA_UTIL_HAVE_ECDSA) #include "mbedtls/asn1.h" #include "mbedtls/platform_util.h" @@ -74,8 +74,7 @@ int mbedtls_asn1_get_tag(unsigned char **p, return mbedtls_asn1_get_len(p, end, len); } -#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || - MBEDTLS_PSA_UTIL_HAVE_ECDSA || MBEDTLS_PEM_PARSE_C */ +#endif /* MBEDTLS_ASN1_PARSE_C || MBEDTLS_X509_CREATE_C || MBEDTLS_PSA_UTIL_HAVE_ECDSA */ #if defined(MBEDTLS_ASN1_PARSE_C) int mbedtls_asn1_get_bool(unsigned char **p, diff --git a/library/pem.c b/library/pem.c index 3f01d3bdd..48180eed4 100644 --- a/library/pem.c +++ b/library/pem.c @@ -17,7 +17,6 @@ #include "mbedtls/cipher.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" -#include "mbedtls/asn1.h" #include @@ -466,28 +465,6 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const mbedtls_zeroize_and_free(buf, len); return ret; } - - /* - * In RFC1421 PEM is used as container for DER (ASN.1) content so we - * can use ASN.1 functions to parse the main SEQUENCE tag and to get its - * length. - */ - unsigned char *p = buf; - size_t sequence_len; - ret = mbedtls_asn1_get_tag(&p, buf + len, &sequence_len, - MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED); - if (ret != 0) { - mbedtls_free(buf); - return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_PEM_INVALID_DATA, ret); - } - /* Add also the sequence block (tag + len) to the total amount of valid data. */ - sequence_len += (p - buf); - - /* Ensure that the reported SEQUENCE length matches the data len (i.e. no - * trailing garbage data). */ - if (len != sequence_len) { - return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; - } #else mbedtls_zeroize_and_free(buf, len); return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE; diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index e3bb7e4da..6f8af6c31 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -61,21 +61,14 @@ PEM read (valid EC key encoded with AES-128-CBC) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,151F851B6A7F3FBDAA5B7173117D0127\n\nLw+0OM+0Bwcl+ls/vxQbLrVshGc7bsNPvvtj2sJeMFFEq3V1mj/IO++0KK/CDhMH\nh6CZPsmgVOeM5uFpqYaq0fJbUduN2eDMWszWRm0SFkY=\n-----END EC PRIVATE KEY-----":"pwdpwd":0:"3041020101040f00d8023c809afd45e426d1a4dbe0ffa00706052b81040004a1220320000400da1ecfa53d528237625e119e2e0500d2eb671724f16deb6a63749516b7" -# The text "hello world" (which is clearly not a valid ASN.1 SEQUENCE) is encoded -# with AES-128-CBC to prove that ASN.1 parsing after decoding fails. +# The text "hello world" together with some invalid padding data is encoded +# with AES-128-CBC in order to test padding validation. # Since PBKDF1 isn't supported in OpenSSL, here's the steps: # 1. generate the key (password="password"; IV=0x3132333435363738 in hex or "12345678" as string) # echo -n "password12345678" | openssl md5 # 2. encode data -# echo -n "hello world" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -PEM read (Invalid SEQUENCE encoded with AES-128-CBC) -depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC -mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\nDfRGkwS+VjvR0IYsjZwW6Q==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_INVALID_DATA + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG:"" - -# Same as above, but with invalid padding data. -# Generated with: # echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x01\x02\x03\x04\x05" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad -PEM read (Invalid padding data for AES-128-CBC) +PEM read (AES-128-CBC, invalid padding data) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" diff --git a/tests/suites/test_suite_pem.function b/tests/suites/test_suite_pem.function index 2acc16e9f..413dc551c 100644 --- a/tests/suites/test_suite_pem.function +++ b/tests/suites/test_suite_pem.function @@ -3,7 +3,6 @@ #include "mbedtls/pem.h" #include "mbedtls/des.h" #include "mbedtls/aes.h" -#include "mbedtls/asn1.h" /* END_HEADER */ /* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */ From eba4ca19c6494c2ca81e577696b64e93c158b80a Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 19 Feb 2024 07:42:18 +0100 Subject: [PATCH 13/17] test_suite_pem: solve driver test disparities Signed-off-by: Valerio Setti --- tests/scripts/analyze_outcomes.py | 3 +-- tests/suites/test_suite_pem.data | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 6503f9a27..2da16b9cf 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -571,8 +571,7 @@ KNOWN_TASKS = { 'test_suite_pem': [ # Following tests require AES_C, but this is diabled in the # accelerated component. - 'PEM read (AES-128-CBC + invalid iv)', - 'PEM read (malformed PEM AES-128-CBC)', + re.compile('PEM read .*AES.*'), 'PEM read (unknown encryption algorithm)', ], 'test_suite_error': [ diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index 6f8af6c31..53b8494e3 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -23,7 +23,6 @@ PEM read (unencrypted, valid) mbedtls_pem_read_buffer:"^":"$":"^\nTWJlZCBUTFM=\n$":"":0:"4d62656420544c53" PEM read (unencrypted, empty content) -depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\n\n-----END EC PRIVATE KEY-----":"":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" PEM read (DES-EDE3-CBC + invalid iv) From e10674d54758008fa20c19019511ac04da3abb06 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 19 Feb 2024 14:52:24 +0100 Subject: [PATCH 14/17] test_suite_pem: fix comment in test case Signed-off-by: Valerio Setti --- tests/suites/test_suite_pem.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index 53b8494e3..b5bcb406e 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -78,7 +78,7 @@ PEM read (AES-128-CBC, padding data is larger than AES block length) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n5wA/XVXHuMsQAAOGFQmK0g==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" -# Padding data (0x9) is larger than AES block size (8). +# Padding data (0x9) is larger than DES block size (8). # Generated with: # echo -n -e "\x68\x65\x6c\x6c\x6f\x09\x09\x09" | openssl des-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad PEM read (DES-CBC, padding data is larger than DES block length) From e88a1c5b85163f21d12bda2a6ed64e56bc4cf87c Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 19 Feb 2024 15:08:49 +0100 Subject: [PATCH 15/17] pem: fix return values in pem_check_pkcs_padding() Return MBEDTLS_ERR_PEM_PASSWORD_MISMATCH instead of MBEDTLS_ERR_PEM_BAD_INPUT_DATA in case of errors. This commit also fix related failures in test pkparse and pem test suites. Signed-off-by: Valerio Setti --- library/pem.c | 4 ++-- tests/suites/test_suite_pem.data | 6 +++--- tests/suites/test_suite_pkparse.data | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/pem.c b/library/pem.c index 48180eed4..1b1edc06b 100644 --- a/library/pem.c +++ b/library/pem.c @@ -248,14 +248,14 @@ static int pem_check_pkcs_padding(unsigned char *input, size_t input_len, size_t size_t i; if (pad_len > input_len) { - return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + return MBEDTLS_ERR_PEM_PASSWORD_MISMATCH; } *data_len = input_len - pad_len; for (i = *data_len; i < input_len; i++) { if (input[i] != pad_len) { - return MBEDTLS_ERR_PEM_BAD_INPUT_DATA; + return MBEDTLS_ERR_PEM_PASSWORD_MISMATCH; } } diff --git a/tests/suites/test_suite_pem.data b/tests/suites/test_suite_pem.data index b5bcb406e..007ba104a 100644 --- a/tests/suites/test_suite_pem.data +++ b/tests/suites/test_suite_pem.data @@ -69,18 +69,18 @@ mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KE # echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x01\x02\x03\x04\x05" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad PEM read (AES-128-CBC, invalid padding data) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC -mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n333hxynfxEdXrSHQfIabxQ==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_PASSWORD_MISMATCH:"" # Padding data (0x11) is larger than AES block size (16). # Generated with: # echo -n -e "\x68\x65\x6c\x6c\x6f\x20\x77\x6f\x72\x6c\x64\x11\x11\x11\x11\x11" | openssl aes-128-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad PEM read (AES-128-CBC, padding data is larger than AES block length) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC -mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n5wA/XVXHuMsQAAOGFQmK0g==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: AES-128-CBC,31323334353637380000000000000000\n\n5wA/XVXHuMsQAAOGFQmK0g==\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_PASSWORD_MISMATCH:"" # Padding data (0x9) is larger than DES block size (8). # Generated with: # echo -n -e "\x68\x65\x6c\x6c\x6f\x09\x09\x09" | openssl des-cbc -e -base64 -p -K "bbb0ddff1b944b3cc68eaaeb7ac20099" -iv "3132333435363738" -nopad PEM read (DES-CBC, padding data is larger than DES block length) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC -mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,3132333435363738\n\n6a+B2WineBM=\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_BAD_INPUT_DATA:"" +mbedtls_pem_read_buffer:"-----BEGIN EC PRIVATE KEY-----":"-----END EC PRIVATE KEY-----":"-----BEGIN EC PRIVATE KEY-----\nProc-Type\: 4,ENCRYPTED\nDEK-Info\: DES-CBC,3132333435363738\n\n6a+B2WineBM=\n-----END EC PRIVATE KEY-----":"password":MBEDTLS_ERR_PEM_PASSWORD_MISMATCH:"" diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 7ee77da8e..762fd52a2 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -8,7 +8,7 @@ pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLTest":0 Parse RSA Key #3 (Wrong password) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC:MBEDTLS_AES_C -pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PEM_BAD_INPUT_DATA +pk_parse_keyfile_rsa:"data_files/test-ca.key":"PolarSSLWRONG":MBEDTLS_ERR_PK_PASSWORD_MISMATCH Parse RSA Key #4 (DES Encrypted) depends_on:MBEDTLS_MD_CAN_MD5:MBEDTLS_DES_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_CIPHER_MODE_CBC From 02f30230c4fb8fe8bf4d75e4146620a5bea914a9 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 20 Feb 2024 10:22:36 +0100 Subject: [PATCH 16/17] pem: zeroize the entire buffer in case of errors in mbedtls_pem_read_buffer() Signed-off-by: Valerio Setti --- library/pem.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/pem.c b/library/pem.c index 1b1edc06b..0fee5df43 100644 --- a/library/pem.c +++ b/library/pem.c @@ -453,18 +453,20 @@ int mbedtls_pem_read_buffer(mbedtls_pem_context *ctx, const char *header, const #endif /* MBEDTLS_AES_C */ if (ret != 0) { - mbedtls_free(buf); + mbedtls_zeroize_and_free(buf, len); return ret; } /* Check PKCS padding and update data length based on padding info. * This can be used to detect invalid padding data and password * mismatches. */ - ret = pem_check_pkcs_padding(buf, len, &len); + size_t unpadded_len; + ret = pem_check_pkcs_padding(buf, len, &unpadded_len); if (ret != 0) { mbedtls_zeroize_and_free(buf, len); return ret; } + len = unpadded_len; #else mbedtls_zeroize_and_free(buf, len); return MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE; From 7e1596d24cffb8f0675e6be5cf6d6510b09142dd Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 20 Feb 2024 10:23:49 +0100 Subject: [PATCH 17/17] rsa: remove leftovers from mbedtls_rsa_parse_[pub]key() Signed-off-by: Valerio Setti --- library/rsa.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 2c3386912..da32659b7 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -112,8 +112,6 @@ int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, si return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } - end = p + len; - if ((ret = mbedtls_asn1_get_int(&p, end, &version)) != 0) { return ret; } @@ -248,8 +246,6 @@ int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } - end = p + len; - /* Import N */ if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { return ret;