From 8316209c02d66134eda881e7e9b96c0ca6f4dd28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Mar 2023 23:58:50 +0100 Subject: [PATCH 1/4] Use MD_LIGHT rather than md5.h in pem.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit But, for now, still guard things with MBEDTLS_MD5_C, as md.c can only compute MD5 hashes when MBEDTLS_MD5_C is defined. We'll change the guards once that has changed. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/build_info.h | 6 ++++++ library/pem.c | 33 ++++++++++++++++++++------------- tests/scripts/all.sh | 5 +++-- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index 4835beb00c..86a3e161d4 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -87,6 +87,12 @@ #define MBEDTLS_MD_LIGHT #endif +/* Auto-enable MBEDTLS_MD_LIGHT it one module needs it. + */ +#if defined(MBEDTLS_PEM_PARSE_C) +#define MBEDTLS_MD_LIGHT +#endif + /* If MBEDTLS_PSA_CRYPTO_C is defined, make sure MBEDTLS_PSA_CRYPTO_CLIENT * is defined as well to include all PSA code. */ diff --git a/library/pem.c b/library/pem.c index 9f14052e59..84bbb3df10 100644 --- a/library/pem.c +++ b/library/pem.c @@ -25,7 +25,7 @@ #include "mbedtls/base64.h" #include "mbedtls/des.h" #include "mbedtls/aes.h" -#include "mbedtls/md5.h" +#include "mbedtls/md.h" #include "mbedtls/cipher.h" #include "mbedtls/platform_util.h" #include "mbedtls/error.h" @@ -99,26 +99,33 @@ static int pem_pbkdf1(unsigned char *key, size_t keylen, unsigned char *iv, const unsigned char *pwd, size_t pwdlen) { - mbedtls_md5_context md5_ctx; + mbedtls_md_context_t md5_ctx; + const mbedtls_md_info_t *md5_info; unsigned char md5sum[16]; size_t use_len; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - mbedtls_md5_init(&md5_ctx); + mbedtls_md_init(&md5_ctx); + + /* Prepare the context. (setup() errors gracefully on NULL info.) */ + md5_info = mbedtls_md_info_from_type(MBEDTLS_MD_MD5); + if ((ret = mbedtls_md_setup(&md5_ctx, md5_info, 0)) != 0) { + goto exit; + } /* * key[ 0..15] = MD5(pwd || IV) */ - if ((ret = mbedtls_md5_starts(&md5_ctx)) != 0) { + if ((ret = mbedtls_md_starts(&md5_ctx)) != 0) { goto exit; } - if ((ret = mbedtls_md5_update(&md5_ctx, pwd, pwdlen)) != 0) { + if ((ret = mbedtls_md_update(&md5_ctx, pwd, pwdlen)) != 0) { goto exit; } - if ((ret = mbedtls_md5_update(&md5_ctx, iv, 8)) != 0) { + if ((ret = mbedtls_md_update(&md5_ctx, iv, 8)) != 0) { goto exit; } - if ((ret = mbedtls_md5_finish(&md5_ctx, md5sum)) != 0) { + if ((ret = mbedtls_md_finish(&md5_ctx, md5sum)) != 0) { goto exit; } @@ -132,19 +139,19 @@ static int pem_pbkdf1(unsigned char *key, size_t keylen, /* * key[16..23] = MD5(key[ 0..15] || pwd || IV]) */ - if ((ret = mbedtls_md5_starts(&md5_ctx)) != 0) { + if ((ret = mbedtls_md_starts(&md5_ctx)) != 0) { goto exit; } - if ((ret = mbedtls_md5_update(&md5_ctx, md5sum, 16)) != 0) { + if ((ret = mbedtls_md_update(&md5_ctx, md5sum, 16)) != 0) { goto exit; } - if ((ret = mbedtls_md5_update(&md5_ctx, pwd, pwdlen)) != 0) { + if ((ret = mbedtls_md_update(&md5_ctx, pwd, pwdlen)) != 0) { goto exit; } - if ((ret = mbedtls_md5_update(&md5_ctx, iv, 8)) != 0) { + if ((ret = mbedtls_md_update(&md5_ctx, iv, 8)) != 0) { goto exit; } - if ((ret = mbedtls_md5_finish(&md5_ctx, md5sum)) != 0) { + if ((ret = mbedtls_md_finish(&md5_ctx, md5sum)) != 0) { goto exit; } @@ -156,7 +163,7 @@ static int pem_pbkdf1(unsigned char *key, size_t keylen, memcpy(key + 16, md5sum, use_len); exit: - mbedtls_md5_free(&md5_ctx); + mbedtls_md_free(&md5_ctx); mbedtls_platform_zeroize(md5sum, 16); return ret; diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d444cbaf68..2b1a3e99cb 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2529,8 +2529,9 @@ component_test_psa_crypto_config_accel_hash_use_psa () { make CFLAGS="$ASAN_CFLAGS -Werror -I../tests/include -I../tests -I../../tests -DPSA_CRYPTO_DRIVER_TEST -DMBEDTLS_TEST_LIBTESTDRIVER1 $loc_accel_flags" LDFLAGS="-ltestdriver1 $ASAN_CFLAGS" all # There's a risk of something getting re-enabled via config_psa.h; - # make sure it did not happen. - not grep mbedtls_md library/md.o + # make sure it did not happen. Note: it's OK for MD_LIGHT to be enabled, + # but not the full MD_C (for now), so check mbedtls_md_hmac for that. + not grep mbedtls_md_hmac library/md.o not grep mbedtls_md5 library/md5.o not grep mbedtls_sha1 library/sha1.o not grep mbedtls_sha256 library/sha256.o From b33ef74d442f81f1169b05c7793c0ca42e69c26c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 7 Mar 2023 00:04:16 +0100 Subject: [PATCH 2/4] Use MD_LIGHT, not sha1.h, in RSA selftest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same note as previous commit regarding guards. Note that we could auto-enable MD_LIGHT only when SELF_TEST is defined, and even only when SHA1_C is defined too, but somewhere down the line we'll want to auto-enable it for the sake of other RSA function (not in selftest and could use any hash), so there's little point in optimizing the temporary condition, let's use the simple one upfront. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/build_info.h | 5 +++-- library/rsa.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h index 86a3e161d4..bfbf9de57e 100644 --- a/include/mbedtls/build_info.h +++ b/include/mbedtls/build_info.h @@ -87,9 +87,10 @@ #define MBEDTLS_MD_LIGHT #endif -/* Auto-enable MBEDTLS_MD_LIGHT it one module needs it. +/* Auto-enable MBEDTLS_MD_LIGHT if some module needs it. */ -#if defined(MBEDTLS_PEM_PARSE_C) +#if defined(MBEDTLS_PEM_PARSE_C) || \ + defined(MBEDTLS_RSA_C) #define MBEDTLS_MD_LIGHT #endif diff --git a/library/rsa.c b/library/rsa.c index 7159588e76..584b363cb0 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -2344,7 +2344,7 @@ void mbedtls_rsa_free(mbedtls_rsa_context *ctx) #if defined(MBEDTLS_SELF_TEST) -#include "mbedtls/sha1.h" +#include "mbedtls/md.h" /* * Example RSA-1024 keypair, for test purposes @@ -2508,7 +2508,8 @@ int mbedtls_rsa_self_test(int verbose) mbedtls_printf(" PKCS#1 data sign : "); } - if (mbedtls_sha1(rsa_plaintext, PT_LEN, sha1sum) != 0) { + if (mbedtls_md(mbedtls_md_info_from_type(MBEDTLS_MD_SHA1), + rsa_plaintext, PT_LEN, sha1sum) != 0) { if (verbose != 0) { mbedtls_printf("failed\n"); } From 1b5ffc63ccac0223c969a3080837fd6588cd7e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 14 Mar 2023 10:11:20 +0100 Subject: [PATCH 3/4] Avoid double definition of MD_LIGHT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/scripts/all.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2b1a3e99cb..2d65760fd1 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1230,8 +1230,9 @@ component_test_crypto_full_md_light_only () { scripts/config.py unset MBEDTLS_PKCS7_C # Disable indirect dependencies of MD scripts/config.py unset MBEDTLS_ECDSA_DETERMINISTIC # needs HMAC_DRBG - # Enable "light" subset of MD - make CFLAGS="$ASAN_CFLAGS -DMBEDTLS_MD_LIGHT" LDFLAGS="$ASAN_CFLAGS" + # Note: MD-light is auto-enabled in build_info.h by modules that need it, + # which we haven't disabled, so no need to explicitly enable it. + make CFLAGS="$ASAN_CFLAGS" LDFLAGS="$ASAN_CFLAGS" # Make sure we don't have the HMAC functions, but the hashing functions not grep mbedtls_md_hmac library/md.o From 6ea8d3414fc835b5490ef048a419ac2c5454dc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Mar 2023 09:43:50 +0100 Subject: [PATCH 4/4] Fix a comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- tests/suites/test_suite_md.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_md.function b/tests/suites/test_suite_md.function index 64a417147e..32c9c8ca0a 100644 --- a/tests/suites/test_suite_md.function +++ b/tests/suites/test_suite_md.function @@ -128,7 +128,7 @@ void md_info(int md_type, char *md_name, int md_size) (void) md_name; #endif - /* Note: PSA Crypto init not needed to info functions */ + /* Note: PSA Crypto init not needed for info functions */ md_info = mbedtls_md_info_from_type(md_type); TEST_ASSERT(md_info != NULL);