From 575938e806572e47e8959eaf2af87cd1bf265240 Mon Sep 17 00:00:00 2001 From: Chien Wong Date: Wed, 27 Dec 2023 21:22:31 +0800 Subject: [PATCH 01/53] Fix a comment in ecp Fixes: 5521b4ce37b2b7768a66c3a2b5a2fcff200ca11a Signed-off-by: Chien Wong --- library/ecp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ecp.c b/library/ecp.c index 31a6b9e305..f67b4d057c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2614,8 +2614,8 @@ static int ecp_mul_mxz(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, /* RP.X might be slightly larger than P, so reduce it */ MOD_ADD(RP.X); + /* Randomize coordinates of the starting point */ #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) - /* Derandomize coordinates of the starting point */ if (f_rng == NULL) { have_rng = 0; } From 670100f4757a5d2267686f58635c88b32bcd26fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=98rjan=20Malde?= Date: Wed, 31 Jan 2024 14:14:27 +0100 Subject: [PATCH 02/53] fix build for midipix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ørjan Malde --- library/entropy_poll.c | 2 +- library/platform_util.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/entropy_poll.c b/library/entropy_poll.c index cde49e66a0..f007f2d8d4 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -5,7 +5,7 @@ * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later */ -#if defined(__linux__) && !defined(_GNU_SOURCE) +#if defined(__linux__) || defined(__midipix__) && !defined(_GNU_SOURCE) /* Ensure that syscall() is available even when compiling with -std=c99 */ #define _GNU_SOURCE #endif diff --git a/library/platform_util.c b/library/platform_util.c index a86b07fa3f..df34167a8f 100644 --- a/library/platform_util.c +++ b/library/platform_util.c @@ -66,10 +66,10 @@ void mbedtls_platform_zeroize(void *buf, size_t len) #include #if !defined(_WIN32) && (defined(unix) || \ defined(__unix) || defined(__unix__) || (defined(__APPLE__) && \ - defined(__MACH__))) + defined(__MACH__)) || defined(__midipix__)) #include #endif /* !_WIN32 && (unix || __unix || __unix__ || - * (__APPLE__ && __MACH__)) */ + * (__APPLE__ && __MACH__)) || __midipix__ */ #if !((defined(_POSIX_VERSION) && _POSIX_VERSION >= 200809L) || \ (defined(_POSIX_THREAD_SAFE_FUNCTIONS) && \ From bfa27e33fff13c850bec6559c8e690f64bf24b7e Mon Sep 17 00:00:00 2001 From: Kusumit Ghoderao Date: Fri, 2 Feb 2024 16:32:55 +0530 Subject: [PATCH 03/53] Fix kdf incorrect initial capacity Signed-off-by: Kusumit Ghoderao --- .../fix_kdf_incorrect_initial_capacity.txt | 3 + library/psa_crypto.c | 57 +++++++++++++------ tests/suites/test_suite_psa_crypto.data | 47 +++++++++++++-- tests/suites/test_suite_psa_crypto.function | 4 +- 4 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 ChangeLog.d/fix_kdf_incorrect_initial_capacity.txt diff --git a/ChangeLog.d/fix_kdf_incorrect_initial_capacity.txt b/ChangeLog.d/fix_kdf_incorrect_initial_capacity.txt new file mode 100644 index 0000000000..11b82782fb --- /dev/null +++ b/ChangeLog.d/fix_kdf_incorrect_initial_capacity.txt @@ -0,0 +1,3 @@ +Bugfix + * Correct initial capacities for key derivation algorithms:TLS12_PRF, + TLS12_PSK_TO_MS diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 533ded6ff9..ec5934e0e0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4375,21 +4375,10 @@ static psa_status_t psa_hash_try_support(psa_algorithm_t alg) return status; } -static psa_status_t psa_key_derivation_setup_kdf( +static psa_status_t psa_key_derivation_set_maximum_capacity( psa_key_derivation_operation_t *operation, psa_algorithm_t kdf_alg) { - /* Make sure that operation->ctx is properly zero-initialised. (Macro - * initialisers for this union leave some bytes unspecified.) */ - memset(&operation->ctx, 0, sizeof(operation->ctx)); - - /* Make sure that kdf_alg is a supported key derivation algorithm. */ - if (!is_kdf_alg_supported(kdf_alg)) { - return PSA_ERROR_NOT_SUPPORTED; - } - - /* All currently supported key derivation algorithms are based on a - * hash algorithm. */ psa_algorithm_t hash_alg = PSA_ALG_HKDF_GET_HASH(kdf_alg); size_t hash_size = PSA_HASH_LENGTH(hash_alg); if (hash_size == 0) { @@ -4404,14 +4393,48 @@ static psa_status_t psa_key_derivation_setup_kdf( return status; } - if ((PSA_ALG_IS_TLS12_PRF(kdf_alg) || - PSA_ALG_IS_TLS12_PSK_TO_MS(kdf_alg)) && - !(hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384)) { +#if defined(PSA_WANT_ALG_HKDF) + if (PSA_ALG_IS_HKDF(kdf_alg)) { + operation->capacity = 255 * hash_size; + } else +#endif +#if defined(PSA_WANT_ALG_TLS12_PRF) + if (PSA_ALG_IS_TLS12_PRF(kdf_alg) && + (hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384)) { + operation->capacity = SIZE_MAX; + } else +#endif +#if defined(PSA_WANT_ALG_TLS12_PSK_TO_MS) + if (PSA_ALG_IS_TLS12_PSK_TO_MS(kdf_alg) && + (hash_alg == PSA_ALG_SHA_256 || hash_alg == PSA_ALG_SHA_384)) { + /* Master Secret is always 48 bytes + * https://datatracker.ietf.org/doc/html/rfc5246.html#section-8.1 */ + operation->capacity = 48U; + } else +#endif + { + (void) hash_size; + status = PSA_ERROR_NOT_SUPPORTED; + } + return status; +} + + +static psa_status_t psa_key_derivation_setup_kdf( + psa_key_derivation_operation_t *operation, + psa_algorithm_t kdf_alg) +{ + /* Make sure that operation->ctx is properly zero-initialised. (Macro + * initialisers for this union leave some bytes unspecified.) */ + memset(&operation->ctx, 0, sizeof(operation->ctx)); + /* Make sure that kdf_alg is a supported key derivation algorithm. */ + if (!is_kdf_alg_supported(kdf_alg)) { return PSA_ERROR_NOT_SUPPORTED; } - operation->capacity = 255 * hash_size; - return PSA_SUCCESS; + psa_status_t status = psa_key_derivation_set_maximum_capacity(operation, + kdf_alg); + return status; } static psa_status_t psa_key_agreement_try_support(psa_algorithm_t alg) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 03cc2ffe67..718b10c5dc 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -3597,6 +3597,25 @@ PSA key derivation: HKDF SHA-1, request too much capacity depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_1 derive_set_capacity:PSA_ALG_HKDF(PSA_ALG_SHA_1):255 * PSA_HASH_LENGTH(PSA_ALG_SHA_1) + 1:PSA_ERROR_INVALID_ARGUMENT +# TLS 1.2 PRF does not have a maximum capacity therefore +# derive_set_capacity negative test case is not added + +PSA key derivation: TLS 1.2 PSK-to-MS SHA-256, request too much capacity +depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_set_capacity:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):48U + 1U:PSA_ERROR_INVALID_ARGUMENT + +PSA key derivation: TLS 1.2 PSK-to-MS SHA-384, request too much capacity +depends_on:PSA_WANT_ALG_SHA_384:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_set_capacity:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_384):48U + 1U:PSA_ERROR_INVALID_ARGUMENT + +PSA key derivation: TLS 1.2 PRF SHA-256, request maximum capacity +depends_on:PSA_WANT_ALG_TLS12_PRF:PSA_WANT_ALG_SHA_256 +derive_set_capacity:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_256):SIZE_MAX:PSA_SUCCESS + +PSA key derivation: TLS 1.2 PRF SHA-384, request maximum capacity +depends_on:PSA_WANT_ALG_TLS12_PRF:PSA_WANT_ALG_SHA_384 +derive_set_capacity:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_384):SIZE_MAX:PSA_SUCCESS + PSA key derivation: over capacity 42: output 42+1 depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 derive_output:PSA_ALG_HKDF(PSA_ALG_SHA_256):PSA_KEY_DERIVATION_INPUT_SALT:"000102030405060708090a0b0c":PSA_KEY_DERIVATION_INPUT_SECRET:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_KEY_DERIVATION_INPUT_INFO:"f0f1f2f3f4f5f6f7f8f9":42:"3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf34007208d5b887185865":"ff" @@ -3617,17 +3636,33 @@ PSA key derivation: HKDF SHA-256, read maximum capacity minus 1 depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 derive_full:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_256) - 1 +PSA key derivation: HKDF SHA-512, read maximum capacity minus 1 +depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_512 +derive_full:PSA_ALG_HKDF(PSA_ALG_SHA_512):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_512) - 1 + PSA key derivation: HKDF SHA-256, read maximum capacity depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256 derive_full:PSA_ALG_HKDF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_256) -PSA key derivation: TLS 1.2 PRF SHA-256, read maximum capacity minus 1 -depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PRF -derive_full:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_256) - 1 +PSA key derivation: HKDF SHA-512, read maximum capacity +depends_on:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_512 +derive_full:PSA_ALG_HKDF(PSA_ALG_SHA_512):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_512) -PSA key derivation: TLS 1.2 PRF SHA-256, read maximum capacity -depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PRF -derive_full:PSA_ALG_TLS12_PRF(PSA_ALG_SHA_256):"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":"000102030405060708090a0b0c":"f0f1f2f3f4f5f6f7f8f9":255 * PSA_HASH_LENGTH(PSA_ALG_SHA_256) +PSA key derivation: TLS 1.2 PSK-to-MS SHA-256, read maximum capacity minus 1 +depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_full:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"01020304":"5bc0b19b4a8b24b07afe7ec65c471e94a7d518fcef06c3574315255c52afe21b5bc0b19b872b9b26508458f03603744d575f463a11ae7f1b090c012606fd3e9f":"6d617374657220736563726574":47 + +PSA key derivation: TLS 1.2 PSK-to-MS SHA-384, read maximum capacity minus 1 +depends_on:PSA_WANT_ALG_SHA_384:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_full:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_384):"01020304":"5bc0b19b4a8b24b07afe7ec65c471e94a7d518fcef06c3574315255c52afe21b5bc0b19b872b9b26508458f03603744d575f463a11ae7f1b090c012606fd3e9f":"6d617374657220736563726574":47 + +PSA key derivation: TLS 1.2 PSK-to-MS SHA-256, read maximum capacity +depends_on:PSA_WANT_ALG_SHA_256:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_full:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256):"01020304":"5bc0b19b4a8b24b07afe7ec65c471e94a7d518fcef06c3574315255c52afe21b5bc0b19b872b9b26508458f03603744d575f463a11ae7f1b090c012606fd3e9f":"6d617374657220736563726574":48 + +PSA key derivation: TLS 1.2 PSK-to-MS SHA-384, read maximum capacity +depends_on:PSA_WANT_ALG_SHA_384:PSA_WANT_ALG_TLS12_PSK_TO_MS +derive_full:PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_384):"01020304":"5bc0b19b4a8b24b07afe7ec65c471e94a7d518fcef06c3574315255c52afe21b5bc0b19b872b9b26508458f03603744d575f463a11ae7f1b090c012606fd3e9f":"6d617374657220736563726574":48 PSA key derivation: HKDF SHA-256, exercise AES128-CTR depends_on:PSA_WANT_ALG_CTR:PSA_WANT_ALG_HKDF:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_AES diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 0db5bff79a..21b768bd3a 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -4466,7 +4466,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ -void derive_set_capacity(int alg_arg, int capacity_arg, +void derive_set_capacity(int alg_arg, int64_t capacity_arg, int expected_status_arg) { psa_algorithm_t alg = alg_arg; @@ -4788,7 +4788,7 @@ void derive_full(int alg_arg, psa_algorithm_t alg = alg_arg; size_t requested_capacity = requested_capacity_arg; psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; - unsigned char output_buffer[16]; + unsigned char output_buffer[32]; size_t expected_capacity = requested_capacity; size_t current_capacity; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; From b6d57934bca5aeac5ddc8936b13e370b067972a2 Mon Sep 17 00:00:00 2001 From: Chien Wong Date: Wed, 7 Feb 2024 21:48:12 +0800 Subject: [PATCH 04/53] Reduce many unnecessary static memory consumption .data section of ssl_client1 becomes 128 bytes smaller on AMD64. Signed-off-by: Chien Wong --- library/ecp_curves.c | 14 +++++++------- library/ssl_tls.c | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/library/ecp_curves.c b/library/ecp_curves.c index c7565cce5d..61a1046f3a 100644 --- a/library/ecp_curves.c +++ b/library/ecp_curves.c @@ -535,10 +535,10 @@ static inline void ecp_mpi_load(mbedtls_mpi *X, const mbedtls_mpi_uint *p, size_ */ static inline void ecp_mpi_set1(mbedtls_mpi *X) { - static mbedtls_mpi_uint one[] = { 1 }; + static const mbedtls_mpi_uint one[] = { 1 }; X->s = 1; X->n = 1; - X->p = one; + X->p = (mbedtls_mpi_uint *) one; /* X->p will not be modified so the cast is safe */ } /* @@ -1348,7 +1348,7 @@ cleanup: */ #define P_KOBLITZ_MAX (256 / 8 / sizeof(mbedtls_mpi_uint)) // Max limbs in P #define P_KOBLITZ_R (8 / sizeof(mbedtls_mpi_uint)) // Limbs in R -static inline int ecp_mod_koblitz(mbedtls_mpi *N, mbedtls_mpi_uint *Rp, size_t p_limbs, +static inline int ecp_mod_koblitz(mbedtls_mpi *N, const mbedtls_mpi_uint *Rp, size_t p_limbs, size_t adjust, size_t shift, mbedtls_mpi_uint mask) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1362,7 +1362,7 @@ static inline int ecp_mod_koblitz(mbedtls_mpi *N, mbedtls_mpi_uint *Rp, size_t p /* Init R */ R.s = 1; - R.p = Rp; + R.p = (mbedtls_mpi_uint *) Rp; /* R.p will not be modified so the cast is safe */ R.n = P_KOBLITZ_R; /* Common setup for M */ @@ -1433,7 +1433,7 @@ cleanup: */ static int ecp_mod_p192k1(mbedtls_mpi *N) { - static mbedtls_mpi_uint Rp[] = { + static const mbedtls_mpi_uint Rp[] = { MBEDTLS_BYTES_TO_T_UINT_8(0xC9, 0x11, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00) }; @@ -1450,7 +1450,7 @@ static int ecp_mod_p192k1(mbedtls_mpi *N) */ static int ecp_mod_p224k1(mbedtls_mpi *N) { - static mbedtls_mpi_uint Rp[] = { + static const mbedtls_mpi_uint Rp[] = { MBEDTLS_BYTES_TO_T_UINT_8(0x93, 0x1A, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00) }; @@ -1472,7 +1472,7 @@ static int ecp_mod_p224k1(mbedtls_mpi *N) */ static int ecp_mod_p256k1(mbedtls_mpi *N) { - static mbedtls_mpi_uint Rp[] = { + static const mbedtls_mpi_uint Rp[] = { MBEDTLS_BYTES_TO_T_UINT_8(0xD1, 0x03, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00) }; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6ba3e6964e..fc8e8c6237 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5254,7 +5254,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con (SSL_SERIALIZED_SESSION_CONFIG_ETM << SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT))) -static unsigned char ssl_serialized_session_header[] = { +static const unsigned char ssl_serialized_session_header[] = { MBEDTLS_VERSION_MAJOR, MBEDTLS_VERSION_MINOR, MBEDTLS_VERSION_PATCH, @@ -6123,7 +6123,7 @@ void mbedtls_ssl_session_free(mbedtls_ssl_session *session) (SSL_SERIALIZED_CONTEXT_CONFIG_ALPN << SSL_SERIALIZED_CONTEXT_CONFIG_ALPN_BIT) | \ 0u)) -static unsigned char ssl_serialized_context_header[] = { +static const unsigned char ssl_serialized_context_header[] = { MBEDTLS_VERSION_MAJOR, MBEDTLS_VERSION_MINOR, MBEDTLS_VERSION_PATCH, @@ -6821,7 +6821,7 @@ void mbedtls_ssl_config_init(mbedtls_ssl_config *conf) } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -static int ssl_preset_default_hashes[] = { +static const int ssl_preset_default_hashes[] = { #if defined(MBEDTLS_SHA512_C) MBEDTLS_MD_SHA512, #endif @@ -6839,14 +6839,14 @@ static int ssl_preset_default_hashes[] = { }; #endif -static int ssl_preset_suiteb_ciphersuites[] = { +static const int ssl_preset_suiteb_ciphersuites[] = { MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, MBEDTLS_TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, 0 }; #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -static int ssl_preset_suiteb_hashes[] = { +static const int ssl_preset_suiteb_hashes[] = { MBEDTLS_MD_SHA256, MBEDTLS_MD_SHA384, MBEDTLS_MD_NONE @@ -6854,7 +6854,7 @@ static int ssl_preset_suiteb_hashes[] = { #endif #if defined(MBEDTLS_ECP_C) -static mbedtls_ecp_group_id ssl_preset_suiteb_curves[] = { +static const mbedtls_ecp_group_id ssl_preset_suiteb_curves[] = { #if defined(MBEDTLS_ECP_DP_SECP256R1_ENABLED) MBEDTLS_ECP_DP_SECP256R1, #endif From 0af7a9032926b8719817d56ae7d6142f58e8c237 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 12 Feb 2024 14:16:05 +0100 Subject: [PATCH 05/53] depends.py: set unique configuration names in outcome file Set unique configuration names in the outcome file. This was lost in the rewrite from depends-*.pl to depends.py. Fix #7290 Signed-off-by: Gilles Peskine --- tests/scripts/depends.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/scripts/depends.py b/tests/scripts/depends.py index c6a438ee36..346f6ff710 100755 --- a/tests/scripts/depends.py +++ b/tests/scripts/depends.py @@ -192,7 +192,10 @@ and subsequent commands are tests that cannot run if the build failed).''' success = True for command in self.commands: log_command(command) - ret = subprocess.call(command) + env = os.environ.copy() + if 'MBEDTLS_TEST_CONFIGURATION' in env: + env['MBEDTLS_TEST_CONFIGURATION'] += '-' + self.name + ret = subprocess.call(command, env=env) if ret != 0: if command[0] not in ['make', options.make_command]: log_line('*** [{}] Error {}'.format(' '.join(command), ret)) From c89f9ceb41d64b209774f6f34ba168a60638958f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 12 Feb 2024 14:25:18 +0100 Subject: [PATCH 06/53] Don't define pk_sign_verify in configurations where it's unused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some configurations (e.g. ECDH but no ECDSA or RSA), the PK module is useful but cannot perform any signatures. Then modern GCC complains: ``` ../source/tests/suites/test_suite_pk.function: In function ‘test_pk_sign_verify’: ../source/tests/suites/test_suite_pk.function:1136:12: error: array subscript 0 is outside array bounds of ‘unsigned char[0]’ [-Werror=array-bounds] ../source/tests/suites/test_suite_pk.function:1094:19: note: while referencing sig’ … ``` This fixes test-ref-configs.pl with a modern GCC (specifically with config-thread.h). Signed-off-by: Gilles Peskine --- tests/suites/test_suite_pk.function | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index c17037f079..0e69d806be 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -18,6 +18,13 @@ #define RSA_KEY_SIZE 512 #define RSA_KEY_LEN 64 +#if defined(MBEDTLS_RSA_C) || \ + defined(MBEDTLS_PK_RSA_ALT_SUPPORT) || \ + defined(MBEDTLS_ECDSA_C) || \ + defined(MBEDTLS_USE_PSA_CRYPTO) +#define PK_CAN_SIGN_SOME +#endif + /** Generate a key of the desired type. * * \param pk The PK object to fill. It must have been initialized @@ -894,7 +901,7 @@ exit: } /* END_CASE */ -/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */ +/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C:PK_CAN_SIGN_SOME */ void pk_sign_verify(int type, int parameter, int sign_ret, int verify_ret) { mbedtls_pk_context pk; From 0196f4886a9aa44ecb3d3a19465cca422ff2c0f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 1 Feb 2024 22:33:06 +0100 Subject: [PATCH 07/53] Fix mbedtls_pk_get_bitlen() for RSA with non-byte-aligned sizes Add non-regression tests. Update some test functions to not assume that byte_length == bit_length / 8. Signed-off-by: Gilles Peskine --- ChangeLog.d/rsa-bitlen.txt | 3 +++ library/pk_wrap.c | 18 +++++++++++++++++- tests/suites/test_suite_pk.data | 13 +++++++++++++ tests/suites/test_suite_pk.function | 13 ++++++++----- tests/suites/test_suite_pkparse.data | 16 ++++++++++++++++ tests/suites/test_suite_pkparse.function | 8 ++++++++ 6 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 ChangeLog.d/rsa-bitlen.txt diff --git a/ChangeLog.d/rsa-bitlen.txt b/ChangeLog.d/rsa-bitlen.txt new file mode 100644 index 0000000000..9cb868947a --- /dev/null +++ b/ChangeLog.d/rsa-bitlen.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix mbedtls_pk_get_bitlen() for RSA keys whose size is not a + multiple of 8. Fixes #868. diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 14c6d3f99c..dd460a6a0c 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -53,7 +53,23 @@ static int rsa_can_do(mbedtls_pk_type_t type) static size_t rsa_get_bitlen(const void *ctx) { const mbedtls_rsa_context *rsa = (const mbedtls_rsa_context *) ctx; - return 8 * mbedtls_rsa_get_len(rsa); + /* Unfortunately, the rsa.h interface does not have a direct way + * to access the bit-length that works with MBEDTLS_RSA_ALT. + * So we have to do a little work here. + */ + mbedtls_mpi N; + mbedtls_mpi_init(&N); + int ret = mbedtls_rsa_export(rsa, &N, NULL, NULL, NULL, NULL); + /* If the export fails for some reason (e.g. the RSA_ALT implementation + * does not support export, or there is not enough memory), + * we have no way of returning an error from this function. + * As a fallback, return the byte-length converted in bits, which is + * the correct value if the modulus size is a multiple of 8 bits, which + * is very often the case in practice. */ + size_t bitlen = (ret == 0 ? mbedtls_mpi_bitlen(&N) : + 8 * mbedtls_rsa_get_len(rsa)); + mbedtls_mpi_free(&N); + return bitlen; } static int rsa_verify_wrap(void *ctx, mbedtls_md_type_t md_alg, diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 52480741a8..ee545293e7 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -12,6 +12,19 @@ PK utils: RSA 512-bit depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME pk_utils:MBEDTLS_PK_RSA:512:512:64:"RSA" +# mbedtls_rsa_gen_key() only supports even sizes, so we don't test 513 etc. +PK utils: RSA 514-bit +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:514:514:65:"RSA" + +PK utils: RSA 516-bit +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:516:516:65:"RSA" + +PK utils: RSA 518-bit +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:518:518:65:"RSA" + PK utils: ECKEY SECP192R1 depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED pk_utils:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:192:24:"EC" diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index c17037f079..2111926629 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -155,7 +155,7 @@ void pk_psa_utils() TEST_ASSERT(strcmp(mbedtls_pk_get_name(&pk), name) == 0); TEST_ASSERT(mbedtls_pk_get_bitlen(&pk) == bitlen); - TEST_ASSERT(mbedtls_pk_get_len(&pk) == bitlen / 8); + TEST_ASSERT(mbedtls_pk_get_len(&pk) == (bitlen + 7) / 8); TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECKEY) == 1); TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECDSA) == 1); @@ -683,7 +683,7 @@ void pk_rsa_verify_test_vec(data_t *message_str, int digest, int mod, TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0); rsa = mbedtls_pk_rsa(pk); - rsa->len = mod / 8; + rsa->len = (mod + 7) / 8; TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0); TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0); @@ -731,7 +731,7 @@ void pk_rsa_verify_ext_test_vec(data_t *message_str, int digest, TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0); rsa = mbedtls_pk_rsa(pk); - rsa->len = mod / 8; + rsa->len = (mod + 7) / 8; TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0); TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0); @@ -1004,7 +1004,7 @@ void pk_rsa_encrypt_test_vec(data_t *message, int mod, TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0); rsa = mbedtls_pk_rsa(pk); - rsa->len = mod / 8; + rsa->len = (mod + 7) / 8; TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0); TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0); @@ -1053,9 +1053,12 @@ void pk_rsa_decrypt_test_vec(data_t *cipher, int mod, TEST_ASSERT(mbedtls_test_read_mpi(&P, input_P) == 0); TEST_ASSERT(mbedtls_test_read_mpi(&Q, input_Q) == 0); TEST_ASSERT(mbedtls_rsa_import(rsa, &N, &P, &Q, NULL, &E) == 0); - TEST_ASSERT(mbedtls_rsa_get_len(rsa) == (size_t) (mod / 8)); + TEST_EQUAL(mbedtls_rsa_get_len(rsa), (mod + 7) / 8); TEST_ASSERT(mbedtls_rsa_complete(rsa) == 0); + TEST_EQUAL(mbedtls_pk_get_bitlen(&pk), mod); + TEST_EQUAL(mbedtls_pk_get_len(&pk), (mod + 7) / 8); + /* decryption test */ memset(output, 0, sizeof(output)); olen = 0; diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index 9108a215b6..0837636122 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -938,6 +938,22 @@ Parse RSA Key #99.2 (PKCS#8 encrypted v2 PBKDF2 DES hmacWithSHA512 DER, 4096-bit depends_on:MBEDTLS_DES_C:MBEDTLS_SHA512_C:MBEDTLS_PKCS5_C pk_parse_keyfile_rsa:"data_files/rsa_pkcs8_pbes2_pbkdf2_4096_des_sha512.der":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT +Parse RSA Key #100.1 (512-bit) +depends_on:MBEDTLS_PEM_C +pk_parse_keyfile_rsa:"data_files/rsa512.key":"":0 + +Parse RSA Key #100.1 (521-bit) +depends_on:MBEDTLS_PEM_C +pk_parse_keyfile_rsa:"data_files/rsa521.key":"":0 + +Parse RSA Key #100.1 (522-bit) +depends_on:MBEDTLS_PEM_C +pk_parse_keyfile_rsa:"data_files/rsa522.key":"":0 + +Parse RSA Key #100.1 (528-bit) +depends_on:MBEDTLS_PEM_C +pk_parse_keyfile_rsa:"data_files/rsa528.key":"":0 + Parse Public RSA Key #1 (PKCS#8 wrapped) depends_on:MBEDTLS_PEM_PARSE_C pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_2048_public.pem":0 diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function index d6b698463f..08f27637d5 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -32,6 +32,10 @@ void pk_parse_keyfile_rsa(char *key_file, char *password, int result) TEST_ASSERT(mbedtls_pk_can_do(&ctx, MBEDTLS_PK_RSA)); rsa = mbedtls_pk_rsa(ctx); TEST_ASSERT(mbedtls_rsa_check_privkey(rsa) == 0); + + /* Test consistency between get_len and get_bitlen */ + size_t bitlen = mbedtls_pk_get_bitlen(&ctx); + TEST_EQUAL(mbedtls_pk_get_len(&ctx), (bitlen + 7) / 8); } exit: @@ -58,6 +62,10 @@ void pk_parse_public_keyfile_rsa(char *key_file, int result) TEST_ASSERT(mbedtls_pk_can_do(&ctx, MBEDTLS_PK_RSA)); rsa = mbedtls_pk_rsa(ctx); TEST_ASSERT(mbedtls_rsa_check_pubkey(rsa) == 0); + + /* Test consistency between get_len and get_bitlen */ + size_t bitlen = mbedtls_pk_get_bitlen(&ctx); + TEST_EQUAL(mbedtls_pk_get_len(&ctx), (bitlen + 7) / 8); } exit: From 7c1cd5ae1c17931f45baf3e39fd2696c16704c2c Mon Sep 17 00:00:00 2001 From: PiotrBzdrega Date: Tue, 13 Feb 2024 16:59:05 +0100 Subject: [PATCH 08/53] move entropy init prior arguments number recognition Signed-off-by: PiotrBzdrega --- ChangeLog.d/gen-key-segfault.txt | 3 +++ programs/pkey/gen_key.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 ChangeLog.d/gen-key-segfault.txt diff --git a/ChangeLog.d/gen-key-segfault.txt b/ChangeLog.d/gen-key-segfault.txt new file mode 100644 index 0000000000..7f8c39b098 --- /dev/null +++ b/ChangeLog.d/gen-key-segfault.txt @@ -0,0 +1,3 @@ +Bugfix + * Avoid segmentation fault caused by releasing not initialized + entropy resource in gen_key example. Fixes #8809 \ No newline at end of file diff --git a/programs/pkey/gen_key.c b/programs/pkey/gen_key.c index 8ad2627667..eab5c30ac3 100644 --- a/programs/pkey/gen_key.c +++ b/programs/pkey/gen_key.c @@ -188,6 +188,7 @@ int main(int argc, char *argv[]) mbedtls_mpi_init(&D); mbedtls_mpi_init(&E); mbedtls_mpi_init(&DP); mbedtls_mpi_init(&DQ); mbedtls_mpi_init(&QP); + mbedtls_entropy_init(&entropy); mbedtls_pk_init(&key); mbedtls_ctr_drbg_init(&ctr_drbg); memset(buf, 0, sizeof(buf)); @@ -275,7 +276,6 @@ usage: mbedtls_printf("\n . Seeding the random number generator..."); fflush(stdout); - mbedtls_entropy_init(&entropy); #if !defined(_WIN32) && defined(MBEDTLS_FS_IO) if (opt.use_dev_random) { if ((ret = mbedtls_entropy_add_source(&entropy, dev_random_entropy_poll, From 14e4727d0ededb1a0174f1d48ecc856d31a29063 Mon Sep 17 00:00:00 2001 From: PiotrBzdrega Date: Tue, 13 Feb 2024 17:09:40 +0100 Subject: [PATCH 09/53] fill out missing dot in changelog Signed-off-by: PiotrBzdrega --- ChangeLog.d/gen-key-segfault.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/gen-key-segfault.txt b/ChangeLog.d/gen-key-segfault.txt index 7f8c39b098..4fb2d1f858 100644 --- a/ChangeLog.d/gen-key-segfault.txt +++ b/ChangeLog.d/gen-key-segfault.txt @@ -1,3 +1,3 @@ Bugfix * Avoid segmentation fault caused by releasing not initialized - entropy resource in gen_key example. Fixes #8809 \ No newline at end of file + entropy resource in gen_key example. Fixes #8809. \ No newline at end of file From eb77b6f418b44e3cb0a03dcf7b78431114e3f234 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 13 Feb 2024 17:53:35 +0000 Subject: [PATCH 10/53] Add session config bit for KEEP_PEER_CERTIFICATE This config option decides whether the session stores the entire certificate or just a digest of it, but was missing from the serialization config bitflag. Signed-off-by: David Horstmann --- library/ssl_tls.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index fc8e8c6237..13edabc091 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5204,6 +5204,12 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con #define SSL_SERIALIZED_SESSION_CONFIG_CRT 0 #endif /* MBEDTLS_X509_CRT_PARSE_C */ +#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +#define SSL_SERIALIZED_SESSION_KEEP_PEER_CRT 1 +#else +#define SSL_SERIALIZED_SESSION_KEEP_PEER_CRT 0 +#endif /* MBEDTLS_SSL_SESSION_TICKETS */ + #if defined(MBEDTLS_SSL_CLI_C) && defined(MBEDTLS_SSL_SESSION_TICKETS) #define SSL_SERIALIZED_SESSION_CONFIG_CLIENT_TICKET 1 #else @@ -5241,6 +5247,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con #define SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT 4 #define SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT 5 #define SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT 6 +#define SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT 7 #define SSL_SERIALIZED_SESSION_CONFIG_BITFLAG \ ((uint16_t) ( \ @@ -5252,7 +5259,8 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con (SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC << \ SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_ETM << SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT) | \ - (SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT))) + (SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT) | \ + (SSL_SERIALIZED_SESSION_KEEP_PEER_CRT << SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT))) static const unsigned char ssl_serialized_session_header[] = { MBEDTLS_VERSION_MAJOR, From c609654665b2a0c49ce1aca3748c0edb164729a2 Mon Sep 17 00:00:00 2001 From: PiotrBzdrega Date: Tue, 13 Feb 2024 22:12:23 +0100 Subject: [PATCH 11/53] newline at end of changelog file Signed-off-by: PiotrBzdrega --- ChangeLog.d/gen-key-segfault.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/gen-key-segfault.txt b/ChangeLog.d/gen-key-segfault.txt index 4fb2d1f858..fefc702726 100644 --- a/ChangeLog.d/gen-key-segfault.txt +++ b/ChangeLog.d/gen-key-segfault.txt @@ -1,3 +1,3 @@ Bugfix * Avoid segmentation fault caused by releasing not initialized - entropy resource in gen_key example. Fixes #8809. \ No newline at end of file + entropy resource in gen_key example. Fixes #8809. From 049ea32931ab7acba350fcda2b845bd1065dd657 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 15 Feb 2024 15:32:12 +0100 Subject: [PATCH 12/53] Fix copypasta Signed-off-by: Gilles Peskine --- include/psa/crypto_values.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h index b528251228..773c01e0ef 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -400,7 +400,7 @@ ((type) | PSA_KEY_TYPE_CATEGORY_FLAG_PAIR) /** The public key type corresponding to a key pair type. * - * You may also pass a key pair type as \p type, it will be left unchanged. + * You may also pass a public key type as \p type, it will be left unchanged. * * \param type A public key type or key pair type. * From 6805ff78920e48926aea5041117a661cecb21ea2 Mon Sep 17 00:00:00 2001 From: Benson Liou Date: Wed, 27 Dec 2023 22:03:24 +0800 Subject: [PATCH 13/53] use mbedtls_ssl_session_init() to init session variable Use mbedtls_ssl_session_init() to init variable just like session-family APIs described Signed-off-by: Benson Liou --- programs/ssl/ssl_client2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 161ff3a178..d01202fb68 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -731,7 +731,7 @@ int main(int argc, char *argv[]) mbedtls_net_init(&server_fd); mbedtls_ssl_init(&ssl); mbedtls_ssl_config_init(&conf); - memset(&saved_session, 0, sizeof(mbedtls_ssl_session)); + mbedtls_ssl_session_init(&saved_session); rng_init(&rng); #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) mbedtls_x509_crt_init(&cacert); From c2c74b9cef46f02f889dcb9a435366d54531bffa Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 13:24:41 +0100 Subject: [PATCH 14/53] mbedtls_ecp_write_key: no FEATURE_UNAVAILABLE error When exporting a key, MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE should not happen. This error indicates that the curve is not supported, but that would prevent the creation of the key. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 2 -- library/ecp.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index e4e40c003c..df59891de2 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1295,8 +1295,6 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the \p key representation is larger than the available space in \p buf. - * \return #MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE if the operation for - * the group is not implemented. * \return Another negative error code on different kinds of failure. */ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, diff --git a/library/ecp.c b/library/ecp.c index f67b4d057c..c98eb7b79a 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -3358,7 +3358,7 @@ cleanup: int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, unsigned char *buf, size_t buflen) { - int ret = MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ECP_VALIDATE_RET(key != NULL); ECP_VALIDATE_RET(buf != NULL); From 75bb596de80662f846f78b3cafa8335e15e4d8c4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 13:30:31 +0100 Subject: [PATCH 15/53] mbedtls_ecp_write_key: document and test larger output buffer Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 21 ++++++++++-- tests/suites/test_suite_ecp.data | 48 ++++++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 47 +++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index df59891de2..231832b877 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1286,11 +1286,28 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, /** * \brief This function exports an elliptic curve private key. * + * \note Note that although this function accepts an output + * buffer that is larger than the key, most key import + * interfaces require the output to be trimmed to the + * key's nominal length. It is generally simplest to + * pass the key's nominal length as \c buflen, after + * checking that the output buffer is large enough. + * See the description of the \p buflen parameter for + * how to calculate the nominal length. + * * \param key The private key. * \param buf The output buffer for containing the binary representation - * of the key. (Big endian integer for Weierstrass curves, byte - * string for Montgomery curves.) + * of the key. + * For Weierstrass curves, this is the big-endian + * representation, padded with null bytes at the beginning + * to reach \p buflen bytes. + * For Montgomery curves, this is the standard byte string + * representation (which is little-endian), padded with + * null bytes at the end to reach \p buflen bytes. * \param buflen The total length of the buffer in bytes. + * The length of the output is always + * (`grp->nbits` + 7) / 8 bytes + * where `grp->nbits` is the private key size in bits. * * \return \c 0 on success. * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the \p key diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 6211fb7297..7d3dd1a41d 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -550,6 +550,54 @@ ECP read key #16 (Curve25519 RFC, OK) depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED mbedtls_ecp_read_key:MBEDTLS_ECP_DP_CURVE25519:"70076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c6a":0:1 +ECP write key: secp256r1, nominal +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":32:0 + +ECP write key: secp256r1, output longer by 1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":33:0 + +ECP write key: secp256r1, output longer by 32 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":64:0 + +ECP write key: secp256r1, output longer by 33 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":65:0 + +ECP write key: secp384r1, nominal +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":48:0 + +ECP write key: secp384r1, output longer by 1 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":49:0 + +ECP write key: secp384r1, output longer by 48 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":96:0 + +ECP write key: secp384r1, output longer by 49 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":97:0 + +ECP write key: Curve25519, nominal +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":32:0 + +ECP write key: Curve25519, output longer by 1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":33:0 + +ECP write key: Curve25519, output longer by 32 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":64:0 + +ECP write key: Curve25519, output longer by 33 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":65:0 + ECP mod p192 small (more than 192 bits, less limbs than 2 * 192 bits) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_fast_mod:MBEDTLS_ECP_DP_SECP192R1:"0100000000000103010000000000010201000000000001010100000000000100" diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index 443bc0861b..aff5ba4b49 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1383,6 +1383,53 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void ecp_write_key(int grp_id, data_t *in_key, + int exported_size, int expected_ret) +{ + mbedtls_ecp_keypair key; + mbedtls_ecp_keypair_init(&key); + unsigned char *exported = NULL; + + TEST_EQUAL(mbedtls_ecp_read_key(grp_id, &key, in_key->x, in_key->len), 0); + + TEST_CALLOC(exported, exported_size); + TEST_EQUAL(mbedtls_ecp_write_key(&key, exported, exported_size), + expected_ret); + + if (expected_ret == 0) { + size_t length = (key.grp.nbits + 7) / 8; + TEST_LE_U(length, exported_size); + + const unsigned char *key_start = NULL; + const unsigned char *zeros_start = NULL; + switch (mbedtls_ecp_get_type(&key.grp)) { + case MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS: + key_start = exported + exported_size - length; + zeros_start = exported; + break; + case MBEDTLS_ECP_TYPE_MONTGOMERY: + key_start = exported; + zeros_start = exported + length; + break; + default: + TEST_FAIL("Unknown ECP curve type"); + break; + } + TEST_MEMORY_COMPARE(in_key->x, in_key->len, + key_start, length); + for (size_t i = 0; i < exported_size - length; i++) { + mbedtls_test_set_step(i); + TEST_EQUAL(zeros_start[i], 0); + } + } + +exit: + mbedtls_ecp_keypair_free(&key); + mbedtls_free(exported); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:HAVE_FIX_NEGATIVE */ void fix_negative(data_t *N_bin, int c, int bits) { From e65e98a1dc8b2a609ee89e3dd6901dce875c394e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 13:49:45 +0100 Subject: [PATCH 16/53] mbedtls_ecp_write_key: document and test smaller output buffer Document and test the current behavior, even if it is weird: * For Weierstrass keys, the error is MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL, not MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL. * For Weierstrass keys, a smaller output buffer is ok if the output fits. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 15 ++++-- tests/suites/test_suite_ecp.data | 68 ++++++++++++++++++++++++++++ tests/suites/test_suite_ecp.function | 27 ++++++++--- 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 231832b877..047e4944c8 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1287,8 +1287,8 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * \brief This function exports an elliptic curve private key. * * \note Note that although this function accepts an output - * buffer that is larger than the key, most key import - * interfaces require the output to be trimmed to the + * buffer that is smaller or larger than the key, most key + * import interfaces require the output to have exactly * key's nominal length. It is generally simplest to * pass the key's nominal length as \c buflen, after * checking that the output buffer is large enough. @@ -1305,13 +1305,18 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * representation (which is little-endian), padded with * null bytes at the end to reach \p buflen bytes. * \param buflen The total length of the buffer in bytes. - * The length of the output is always + * The length of the output is * (`grp->nbits` + 7) / 8 bytes * where `grp->nbits` is the private key size in bits. + * For Weierstrass keys, if the output buffer is smaller, + * leading zeros are trimmed to fit if possible. For + * Montgomery keys, the output buffer must always be large + * enough for the nominal length. * * \return \c 0 on success. - * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL if the \p key - representation is larger than the available space in \p buf. + * \return #MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL or + * #MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL if the \p key + * representation is larger than the available space in \p buf. * \return Another negative error code on different kinds of failure. */ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, diff --git a/tests/suites/test_suite_ecp.data b/tests/suites/test_suite_ecp.data index 7d3dd1a41d..a244bc4602 100644 --- a/tests/suites/test_suite_ecp.data +++ b/tests/suites/test_suite_ecp.data @@ -566,6 +566,42 @@ ECP write key: secp256r1, output longer by 33 depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":65:0 +ECP write key: secp256r1, output short by 1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":31:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp256r1, output_size=1 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":1:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp256r1, output_size=0 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"f12a1320760270a83cbffd53f6031ef76a5d86c8a204f2c30ca9ebf51f0f0ea7":0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp256r1, top byte = 0, output_size=32 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":32:0 + +ECP write key: secp256r1, top byte = 0, output_size=31 (fits) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":31:0 + +ECP write key: secp256r1, top byte = 0, output_size=30 (too small) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"00ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff":30:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp256r1, mostly-0 key, output_size=32 +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":32:0 + +ECP write key: secp256r1, mostly-0 key, output_size=31 (fits) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":31:0 + +ECP write key: secp256r1, mostly-0 key, output_size=1 (fits) +depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP256R1:"0000000000000000000000000000000000000000000000000000000000000001":1:0 + ECP write key: secp384r1, nominal depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":48:0 @@ -582,6 +618,18 @@ ECP write key: secp384r1, output longer by 49 depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":97:0 +ECP write key: secp384r1, output short by 1 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":47:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp384r1, output_size=1 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":1:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + +ECP write key: secp384r1, output_size=0 +depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_SECP384R1:"d27335ea71664af244dd14e9fd1260715dfd8a7965571c48d709ee7a7962a156d706a90cbcb5df2986f05feadb9376f1":0:MBEDTLS_ERR_MPI_BUFFER_TOO_SMALL + ECP write key: Curve25519, nominal depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":32:0 @@ -598,6 +646,26 @@ ECP write key: Curve25519, output longer by 33 depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":65:0 +ECP write key: Curve25519, output short by 1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key: Curve25519, output_size=1 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":1:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key: Curve25519, output_size=0 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"a046e36bf0527c9d3b16154b82465edd62144c0ac1fc5a18506a2244ba449a44":0:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + +ECP write key: Curve25519, mostly-0 key, output_size=32 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":32:0 + +ECP write key: Curve25519, mostly-0 key, output_size=31 +depends_on:MBEDTLS_ECP_DP_CURVE25519_ENABLED +ecp_write_key:MBEDTLS_ECP_DP_CURVE25519:"0000000000000000000000000000000000000000000000000000000000000040":31:MBEDTLS_ERR_ECP_BUFFER_TOO_SMALL + ECP mod p192 small (more than 192 bits, less limbs than 2 * 192 bits) depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED ecp_fast_mod:MBEDTLS_ECP_DP_SECP192R1:"0100000000000103010000000000010201000000000001010100000000000100" diff --git a/tests/suites/test_suite_ecp.function b/tests/suites/test_suite_ecp.function index aff5ba4b49..da137200ba 100644 --- a/tests/suites/test_suite_ecp.function +++ b/tests/suites/test_suite_ecp.function @@ -1399,16 +1399,18 @@ void ecp_write_key(int grp_id, data_t *in_key, if (expected_ret == 0) { size_t length = (key.grp.nbits + 7) / 8; - TEST_LE_U(length, exported_size); - const unsigned char *key_start = NULL; const unsigned char *zeros_start = NULL; switch (mbedtls_ecp_get_type(&key.grp)) { case MBEDTLS_ECP_TYPE_SHORT_WEIERSTRASS: + if ((size_t) exported_size < length) { + length = exported_size; + } key_start = exported + exported_size - length; zeros_start = exported; break; case MBEDTLS_ECP_TYPE_MONTGOMERY: + TEST_LE_U(length, exported_size); key_start = exported; zeros_start = exported + length; break; @@ -1416,11 +1418,22 @@ void ecp_write_key(int grp_id, data_t *in_key, TEST_FAIL("Unknown ECP curve type"); break; } - TEST_MEMORY_COMPARE(in_key->x, in_key->len, - key_start, length); - for (size_t i = 0; i < exported_size - length; i++) { - mbedtls_test_set_step(i); - TEST_EQUAL(zeros_start[i], 0); + + if (length < in_key->len) { + /* Shorter output (only possible with Weierstrass keys) */ + for (size_t i = 0; i < in_key->len - length; i++) { + mbedtls_test_set_step(i); + TEST_EQUAL(in_key->x[i], 0); + } + TEST_MEMORY_COMPARE(in_key->x + in_key->len - length, length, + key_start, length); + } else { + TEST_MEMORY_COMPARE(in_key->x, in_key->len, + key_start, length); + for (size_t i = 0; i < exported_size - length; i++) { + mbedtls_test_set_step(i); + TEST_EQUAL(zeros_start[i], 0); + } } } From 7ce99c0f3af26d93ba5b513e9ba814c050044814 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 19 Feb 2024 13:56:39 +0100 Subject: [PATCH 17/53] mbedtls_ecp_write_key: document that this function doesn't detect unset data Fixes #8803. Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 047e4944c8..0c23f5dbf3 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1295,6 +1295,10 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * See the description of the \p buflen parameter for * how to calculate the nominal length. * + * \note If the private key was not set in \p key, + * the output is unspecified. Future versions + * may return an error in that case. + * * \param key The private key. * \param buf The output buffer for containing the binary representation * of the key. From 84dc44b9b53239123480ba7314764b7aa4895fda Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 26 Feb 2024 09:05:45 +0100 Subject: [PATCH 18/53] Note that ecp read/write functions don't support Curve448 yet Signed-off-by: Gilles Peskine --- include/mbedtls/ecp.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h index 0c23f5dbf3..33ea14d7e2 100644 --- a/include/mbedtls/ecp.h +++ b/include/mbedtls/ecp.h @@ -1265,6 +1265,8 @@ int mbedtls_ecp_gen_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, /** * \brief This function reads an elliptic curve private key. * + * \note This function does not support Curve448 yet. + * * \param grp_id The ECP group identifier. * \param key The destination key. * \param buf The buffer containing the binary representation of the @@ -1299,6 +1301,8 @@ int mbedtls_ecp_read_key(mbedtls_ecp_group_id grp_id, mbedtls_ecp_keypair *key, * the output is unspecified. Future versions * may return an error in that case. * + * \note This function does not support Curve448 yet. + * * \param key The private key. * \param buf The output buffer for containing the binary representation * of the key. From 9721b868a29d4306d2f63a67e71e84596e7721e0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 26 Feb 2024 11:41:22 +0100 Subject: [PATCH 19/53] Allow null buffers when the length is 0 Signed-off-by: Gilles Peskine --- library/ecp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index c98eb7b79a..cfe02b0d2c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -927,7 +927,7 @@ int mbedtls_ecp_point_read_binary(const mbedtls_ecp_group *grp, size_t plen; ECP_VALIDATE_RET(grp != NULL); ECP_VALIDATE_RET(pt != NULL); - ECP_VALIDATE_RET(buf != NULL); + ECP_VALIDATE_RET(ilen == 0 || buf != NULL); if (ilen < 1) { return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; @@ -996,7 +996,7 @@ int mbedtls_ecp_tls_read_point(const mbedtls_ecp_group *grp, ECP_VALIDATE_RET(grp != NULL); ECP_VALIDATE_RET(pt != NULL); ECP_VALIDATE_RET(buf != NULL); - ECP_VALIDATE_RET(*buf != NULL); + ECP_VALIDATE_RET(buf_len == 0 || *buf != NULL); /* * We must have at least two bytes (1 for length, at least one for data) @@ -1068,7 +1068,7 @@ int mbedtls_ecp_tls_read_group(mbedtls_ecp_group *grp, mbedtls_ecp_group_id grp_id; ECP_VALIDATE_RET(grp != NULL); ECP_VALIDATE_RET(buf != NULL); - ECP_VALIDATE_RET(*buf != NULL); + ECP_VALIDATE_RET(len == 0 || *buf != NULL); if ((ret = mbedtls_ecp_tls_read_group_id(&grp_id, buf, len)) != 0) { return ret; @@ -1088,7 +1088,7 @@ int mbedtls_ecp_tls_read_group_id(mbedtls_ecp_group_id *grp, const mbedtls_ecp_curve_info *curve_info; ECP_VALIDATE_RET(grp != NULL); ECP_VALIDATE_RET(buf != NULL); - ECP_VALIDATE_RET(*buf != NULL); + ECP_VALIDATE_RET(len == 0 || *buf != NULL); /* * We expect at least three bytes (see below) @@ -3361,7 +3361,7 @@ int mbedtls_ecp_write_key(mbedtls_ecp_keypair *key, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ECP_VALIDATE_RET(key != NULL); - ECP_VALIDATE_RET(buf != NULL); + ECP_VALIDATE_RET(buflen == 0 || buf != NULL); #if defined(MBEDTLS_ECP_MONTGOMERY_ENABLED) if (mbedtls_ecp_get_type(&key->grp) == MBEDTLS_ECP_TYPE_MONTGOMERY) { From 99fa0d08d327eaa0aa09000785ce2df3f0cc384c Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 4 Jan 2024 16:20:20 +0000 Subject: [PATCH 20/53] Create quiet wrappers for make and cmake Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 44 +++++++++++++++++++++++++++++++++++++++ tests/scripts/quiet/make | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100755 tests/scripts/quiet/cmake create mode 100755 tests/scripts/quiet/make diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake new file mode 100755 index 0000000000..ea1474f877 --- /dev/null +++ b/tests/scripts/quiet/cmake @@ -0,0 +1,44 @@ +#! /usr/bin/env bash +# +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later +# +# This swallows the output of the wrapped tool, unless there is an error. +# This helps reduce excess logging in the CI. + +# If you are debugging a build / CI issue, you can get complete unsilenced logs +# by un-commenting the following line (or setting VERBOSE_LOGS in your environment): +# VERBOSE_LOGS=1 + +# don't silence invocations containing these arguments +NO_SILENCE=" --version " + +TOOL=$(basename "$0") + +# Locate original tool +ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) + +if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then + ${ORIGINAL_TOOL} "$@" + EXIT_STATUS=$? +else + # Display the command being invoked - if it succeeds, this is all that will + # be displayed. + echo "${TOOL} $@" + + # Run original command and capture output & exit status + TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) + ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 + EXIT_STATUS=$? + + if [[ $EXIT_STATUS -ne 0 ]]; then + # On error, display the full output + cat ${TMPFILE} + fi + + # Remove tmpfile + rm ${TMPFILE} +fi + +# Propagate the exit status +exit $EXIT_STATUS diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make new file mode 100755 index 0000000000..633758ae6f --- /dev/null +++ b/tests/scripts/quiet/make @@ -0,0 +1,44 @@ +#! /usr/bin/env bash +# +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later +# +# This swallows the output of the wrapped tool, unless there is an error. +# This helps reduce excess logging in the CI. + +# If you are debugging a build / CI issue, you can get complete unsilenced logs +# by un-commenting the following line (or setting VERBOSE_LOGS in your environment): +# VERBOSE_LOGS=1 + +# don't silence invocations containing these arguments +NO_SILENCE=" --version | test " + +TOOL=$(basename "$0") + +# Locate original tool +ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) + +if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then + ${ORIGINAL_TOOL} "$@" + EXIT_STATUS=$? +else + # Display the command being invoked - if it succeeds, this is all that will + # be displayed. + echo "${TOOL} $@" + + # Run original command and capture output & exit status + TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) + ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 + EXIT_STATUS=$? + + if [[ $EXIT_STATUS -ne 0 ]]; then + # On error, display the full output + cat ${TMPFILE} + fi + + # Remove tmpfile + rm ${TMPFILE} +fi + +# Propagate the exit status +exit $EXIT_STATUS From a0b7c08b60ca3be3fbee49462483eaf01838beb1 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 16 Jan 2024 17:33:27 +0000 Subject: [PATCH 21/53] Use quiet make wrappers from all.sh Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index ca2df4d304..022e72f1eb 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -124,6 +124,25 @@ pre_check_environment () { } pre_initialize_variables () { + special_options="--list-components|--list-all-components|-h|--help" + if [[ ! "$@" =~ $special_options ]]; then + # skip wrappers for "special options" which don't actually run any tests + + # Pick up "quiet" wrappers for make and cmake, which don't output very much + # unless there is an error. This reduces logging overhead in the CI. + # + # Note that the cmake wrapper breaks unless we use an absolute path here. + export PATH=${PWD}/tests/scripts/quiet:$PATH + if [[ ! -x ${PWD}/tests/scripts/quiet/make ]]; then + echo "can't find quiet/make" + exit 1 + fi + if [[ ! -x ${PWD}/tests/scripts/quiet/cmake ]]; then + echo "can't find quiet/cmake" + exit 1 + fi + fi + CONFIG_H='include/mbedtls/config.h' CRYPTO_CONFIG_H='include/psa/crypto_config.h' @@ -3730,7 +3749,7 @@ run_component () { # Preliminary setup pre_check_environment -pre_initialize_variables +pre_initialize_variables "$@" pre_parse_command_line "$@" pre_check_git From 8470d114e2144480c05e20463d3ff5f74a95b5da Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Tue, 16 Jan 2024 17:33:34 +0000 Subject: [PATCH 22/53] Spelling fix Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 022e72f1eb..229478ec7d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -214,7 +214,7 @@ pre_initialize_variables () { # defined in this script whose name starts with "component_". ALL_COMPONENTS=$(compgen -A function component_ | sed 's/component_//') - # Delay determinig SUPPORTED_COMPONENTS until the command line options have a chance to override + # Delay determining SUPPORTED_COMPONENTS until the command line options have a chance to override # the commands set by the environment } From 75da31316620d890666f927a7d3443b45a7b97e7 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Wed, 17 Jan 2024 09:59:10 +0000 Subject: [PATCH 23/53] Pacify check_files Signed-off-by: Dave Rodgman --- tests/scripts/check_files.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index a2a9dfa8d0..837905eada 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -172,6 +172,8 @@ class ShebangIssueTracker(FileIssueTracker): b'sh': 'sh', } + path_exemptions = re.compile(r'tests/scripts/quiet/.*') + def is_valid_shebang(self, first_line, filepath): m = re.match(self._shebang_re, first_line) if not m: From 59f9df999ddc054382f24665350dc4cf645d813c Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 15 Feb 2024 12:27:03 +0000 Subject: [PATCH 24/53] Always display make/cmake invocation command Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 14 ++++++++++---- tests/scripts/quiet/make | 14 ++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index ea1474f877..2e95c6b938 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -18,14 +18,20 @@ TOOL=$(basename "$0") # Locate original tool ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +if [[ ! " $@ " =~ " --version " ]]; then + # Display the command being invoked - if it succeeds, this is all that will + # be displayed. Don't do this for invocations with --version, because + # this output is often parsed by scripts, so we don't want to modify it. + echo -n "${TOOL} " + printf '%q ' "$@" + echo +fi + if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then + # Run original command with no output supression ${ORIGINAL_TOOL} "$@" EXIT_STATUS=$? else - # Display the command being invoked - if it succeeds, this is all that will - # be displayed. - echo "${TOOL} $@" - # Run original command and capture output & exit status TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 633758ae6f..0c4869644e 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -18,14 +18,20 @@ TOOL=$(basename "$0") # Locate original tool ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +if [[ ! " $@ " =~ " --version " ]]; then + # Display the command being invoked - if it succeeds, this is all that will + # be displayed. Don't do this for invocations with --version, because + # this output is often parsed by scripts, so we don't want to modify it. + echo -n "${TOOL} " + printf '%q ' "$@" + echo +fi + if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then + # Run original command with no output supression ${ORIGINAL_TOOL} "$@" EXIT_STATUS=$? else - # Display the command being invoked - if it succeeds, this is all that will - # be displayed. - echo "${TOOL} $@" - # Run original command and capture output & exit status TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 From d4a5563417a9c0252aae3720f8e087822510a7fb Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 15 Feb 2024 14:39:48 +0000 Subject: [PATCH 25/53] Improve output from make/cmake wrapper Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 25 ++++++++++++++++++++++--- tests/scripts/quiet/make | 25 ++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 2e95c6b938..e3114a86fc 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -18,13 +18,32 @@ TOOL=$(basename "$0") # Locate original tool ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +quote_args() { + # similar to printf '%q' "$@" + # but produce more human-readable results for common/simple cases like "a b" + local args=("$@") + s="" + for a in "${args[@]}"; do + simple_pattern='^[[:alnum:] _=+-]*$' + if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then + # a has spaces, but no other special characters that need escaping + # (quoting after removing spaces yields no backslashes) + # simplify quoted form to "$a" - e.g. yield "a b c" instead of a\ b\ c + q="\"$a\"" + else + # get bash to do the quoting + q=$(printf '%q' "$a") + fi + s="$s $q" + done + echo $s +} + if [[ ! " $@ " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - echo -n "${TOOL} " - printf '%q ' "$@" - echo + echo "${TOOL} $(quote_args "$@")" fi if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 0c4869644e..257703ec93 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -18,13 +18,32 @@ TOOL=$(basename "$0") # Locate original tool ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +quote_args() { + # similar to printf '%q' "$@" + # but produce more human-readable results for common/simple cases like "a b" + local args=("$@") + s="" + for a in "${args[@]}"; do + simple_pattern='^[[:alnum:] _=+-]*$' + if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then + # a has spaces, but no other special characters that need escaping + # (quoting after removing spaces yields no backslashes) + # simplify quoted form to "$a" - e.g. yield "a b c" instead of a\ b\ c + q="\"$a\"" + else + # get bash to do the quoting + q=$(printf '%q' "$a") + fi + s="$s $q" + done + echo $s +} + if [[ ! " $@ " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - echo -n "${TOOL} " - printf '%q ' "$@" - echo + echo "${TOOL} $(quote_args "$@")" fi if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then From 634fe908b73103f9816e0bad74ee2ec96abed2c0 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 15 Feb 2024 16:04:36 +0000 Subject: [PATCH 26/53] Improve quote_args output readability Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 8 +++++--- tests/scripts/quiet/make | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index e3114a86fc..a79d7578bf 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -24,12 +24,14 @@ quote_args() { local args=("$@") s="" for a in "${args[@]}"; do - simple_pattern='^[[:alnum:] _=+-]*$' + simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-]*)$' if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then # a has spaces, but no other special characters that need escaping # (quoting after removing spaces yields no backslashes) - # simplify quoted form to "$a" - e.g. yield "a b c" instead of a\ b\ c - q="\"$a\"" + # simplify quoted form - e.g.: + # a b -> "a b" + # CFLAGS=a b -> CFLAGS="a b" + q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" else # get bash to do the quoting q=$(printf '%q' "$a") diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 257703ec93..9722029083 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -24,12 +24,14 @@ quote_args() { local args=("$@") s="" for a in "${args[@]}"; do - simple_pattern='^[[:alnum:] _=+-]*$' + simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-]*)$' if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then # a has spaces, but no other special characters that need escaping # (quoting after removing spaces yields no backslashes) - # simplify quoted form to "$a" - e.g. yield "a b c" instead of a\ b\ c - q="\"$a\"" + # simplify quoted form - e.g.: + # a b -> "a b" + # CFLAGS=a b -> CFLAGS="a b" + q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" else # get bash to do the quoting q=$(printf '%q' "$a") From 4cb98a930c930007ae7308c2fb976b3c30827832 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 11:41:19 +0000 Subject: [PATCH 27/53] Move quiet wrapper setup Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 229478ec7d..b1a3462ee8 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -124,25 +124,6 @@ pre_check_environment () { } pre_initialize_variables () { - special_options="--list-components|--list-all-components|-h|--help" - if [[ ! "$@" =~ $special_options ]]; then - # skip wrappers for "special options" which don't actually run any tests - - # Pick up "quiet" wrappers for make and cmake, which don't output very much - # unless there is an error. This reduces logging overhead in the CI. - # - # Note that the cmake wrapper breaks unless we use an absolute path here. - export PATH=${PWD}/tests/scripts/quiet:$PATH - if [[ ! -x ${PWD}/tests/scripts/quiet/make ]]; then - echo "can't find quiet/make" - exit 1 - fi - if [[ ! -x ${PWD}/tests/scripts/quiet/cmake ]]; then - echo "can't find quiet/cmake" - exit 1 - fi - fi - CONFIG_H='include/mbedtls/config.h' CRYPTO_CONFIG_H='include/psa/crypto_config.h' @@ -218,6 +199,23 @@ pre_initialize_variables () { # the commands set by the environment } +setup_quiet_wrappers() +{ + # Pick up "quiet" wrappers for make and cmake, which don't output very much + # unless there is an error. This reduces logging overhead in the CI. + # + # Note that the cmake wrapper breaks unless we use an absolute path here. + export PATH=${PWD}/tests/scripts/quiet:$PATH + if [[ ! -x ${PWD}/tests/scripts/quiet/make ]]; then + echo "can't find quiet/make" + exit 1 + fi + if [[ ! -x ${PWD}/tests/scripts/quiet/cmake ]]; then + echo "can't find quiet/cmake" + exit 1 + fi +} + # Test whether the component $1 is included in the command line patterns. is_component_included() { @@ -3752,6 +3750,7 @@ pre_check_environment pre_initialize_variables "$@" pre_parse_command_line "$@" +setup_quiet_wrappers pre_check_git pre_restore_files pre_back_up From 6529f12d35989189ce42cd77cc4ed1301a4d07aa Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 11:43:11 +0000 Subject: [PATCH 28/53] Tidy up quiet wrappers Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 39 ++++++++++++++++++------------------- tests/scripts/quiet/make | 41 +++++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 41 deletions(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index a79d7578bf..4804637f8e 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -16,15 +16,13 @@ NO_SILENCE=" --version " TOOL=$(basename "$0") # Locate original tool -ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1 ) -quote_args() { +print_quoted_args() { # similar to printf '%q' "$@" # but produce more human-readable results for common/simple cases like "a b" - local args=("$@") - s="" - for a in "${args[@]}"; do - simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-]*)$' + for a in "$@"; do + simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then # a has spaces, but no other special characters that need escaping # (quoting after removing spaces yields no backslashes) @@ -33,39 +31,40 @@ quote_args() { # CFLAGS=a b -> CFLAGS="a b" q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" else - # get bash to do the quoting + # get bash to do the quoting (which may result in no quotes or escaping, + # if none is needed). q=$(printf '%q' "$a") fi - s="$s $q" + printf "%s " "$q" done - echo $s } -if [[ ! " $@ " =~ " --version " ]]; then +if [[ ! " $* " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - echo "${TOOL} $(quote_args "$@")" + printf %s "${TOOL} " + print_quoted_args "$@" + echo fi if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then # Run original command with no output supression - ${ORIGINAL_TOOL} "$@" - EXIT_STATUS=$? + exec "${ORIGINAL_TOOL}" "$@" else # Run original command and capture output & exit status - TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) - ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 + TMPFILE=$(mktemp "quiet-${TOOL}.XXXXXX") + "${ORIGINAL_TOOL}" "$@" > "${TMPFILE}" 2>&1 EXIT_STATUS=$? if [[ $EXIT_STATUS -ne 0 ]]; then # On error, display the full output - cat ${TMPFILE} + cat "${TMPFILE}" fi # Remove tmpfile - rm ${TMPFILE} -fi + rm "${TMPFILE}" -# Propagate the exit status -exit $EXIT_STATUS + # Propagate the exit status + exit $EXIT_STATUS +fi diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 9722029083..3eae91c4f0 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -11,20 +11,18 @@ # VERBOSE_LOGS=1 # don't silence invocations containing these arguments -NO_SILENCE=" --version | test " +NO_SILENCE=" --version | test" TOOL=$(basename "$0") # Locate original tool -ORIGINAL_TOOL=$(type -ap ${TOOL} | grep -v "$0" | head -n1 ) +ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1 ) -quote_args() { +print_quoted_args() { # similar to printf '%q' "$@" # but produce more human-readable results for common/simple cases like "a b" - local args=("$@") - s="" - for a in "${args[@]}"; do - simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-]*)$' + for a in "$@"; do + simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then # a has spaces, but no other special characters that need escaping # (quoting after removing spaces yields no backslashes) @@ -33,39 +31,40 @@ quote_args() { # CFLAGS=a b -> CFLAGS="a b" q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" else - # get bash to do the quoting + # get bash to do the quoting (which may result in no quotes or escaping, + # if none is needed). q=$(printf '%q' "$a") fi - s="$s $q" + printf "%s " "$q" done - echo $s } -if [[ ! " $@ " =~ " --version " ]]; then +if [[ ! " $* " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - echo "${TOOL} $(quote_args "$@")" + printf %s "${TOOL} " + print_quoted_args "$@" + echo fi if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then # Run original command with no output supression - ${ORIGINAL_TOOL} "$@" - EXIT_STATUS=$? + exec "${ORIGINAL_TOOL}" "$@" else # Run original command and capture output & exit status - TMPFILE=$(mktemp /tmp/quiet-${TOOL}.XXXXXX) - ${ORIGINAL_TOOL} "$@" > ${TMPFILE} 2>&1 + TMPFILE=$(mktemp "quiet-${TOOL}.XXXXXX") + "${ORIGINAL_TOOL}" "$@" > "${TMPFILE}" 2>&1 EXIT_STATUS=$? if [[ $EXIT_STATUS -ne 0 ]]; then # On error, display the full output - cat ${TMPFILE} + cat "${TMPFILE}" fi # Remove tmpfile - rm ${TMPFILE} -fi + rm "${TMPFILE}" -# Propagate the exit status -exit $EXIT_STATUS + # Propagate the exit status + exit $EXIT_STATUS +fi From 57783d74599b995885ef2b2fb7eb977a3879d7af Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 12:37:44 +0000 Subject: [PATCH 29/53] Extract common parts of quiet wrapper Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 58 ++-------------------------------- tests/scripts/quiet/make | 58 ++-------------------------------- tests/scripts/quiet/quiet | 65 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 110 deletions(-) create mode 100755 tests/scripts/quiet/quiet diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 4804637f8e..10a0b25f43 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -11,60 +11,8 @@ # VERBOSE_LOGS=1 # don't silence invocations containing these arguments -NO_SILENCE=" --version " +export NO_SILENCE=" --version " -TOOL=$(basename "$0") +export TOOL="cmake" -# Locate original tool -ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1 ) - -print_quoted_args() { - # similar to printf '%q' "$@" - # but produce more human-readable results for common/simple cases like "a b" - for a in "$@"; do - simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' - if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then - # a has spaces, but no other special characters that need escaping - # (quoting after removing spaces yields no backslashes) - # simplify quoted form - e.g.: - # a b -> "a b" - # CFLAGS=a b -> CFLAGS="a b" - q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" - else - # get bash to do the quoting (which may result in no quotes or escaping, - # if none is needed). - q=$(printf '%q' "$a") - fi - printf "%s " "$q" - done -} - -if [[ ! " $* " =~ " --version " ]]; then - # Display the command being invoked - if it succeeds, this is all that will - # be displayed. Don't do this for invocations with --version, because - # this output is often parsed by scripts, so we don't want to modify it. - printf %s "${TOOL} " - print_quoted_args "$@" - echo -fi - -if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then - # Run original command with no output supression - exec "${ORIGINAL_TOOL}" "$@" -else - # Run original command and capture output & exit status - TMPFILE=$(mktemp "quiet-${TOOL}.XXXXXX") - "${ORIGINAL_TOOL}" "$@" > "${TMPFILE}" 2>&1 - EXIT_STATUS=$? - - if [[ $EXIT_STATUS -ne 0 ]]; then - # On error, display the full output - cat "${TMPFILE}" - fi - - # Remove tmpfile - rm "${TMPFILE}" - - # Propagate the exit status - exit $EXIT_STATUS -fi +exec $(dirname "$0")/quiet "$@" diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 3eae91c4f0..a1ef3e565d 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -11,60 +11,8 @@ # VERBOSE_LOGS=1 # don't silence invocations containing these arguments -NO_SILENCE=" --version | test" +export NO_SILENCE=" --version | test " -TOOL=$(basename "$0") +export TOOL="make" -# Locate original tool -ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1 ) - -print_quoted_args() { - # similar to printf '%q' "$@" - # but produce more human-readable results for common/simple cases like "a b" - for a in "$@"; do - simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' - if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then - # a has spaces, but no other special characters that need escaping - # (quoting after removing spaces yields no backslashes) - # simplify quoted form - e.g.: - # a b -> "a b" - # CFLAGS=a b -> CFLAGS="a b" - q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" - else - # get bash to do the quoting (which may result in no quotes or escaping, - # if none is needed). - q=$(printf '%q' "$a") - fi - printf "%s " "$q" - done -} - -if [[ ! " $* " =~ " --version " ]]; then - # Display the command being invoked - if it succeeds, this is all that will - # be displayed. Don't do this for invocations with --version, because - # this output is often parsed by scripts, so we don't want to modify it. - printf %s "${TOOL} " - print_quoted_args "$@" - echo -fi - -if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then - # Run original command with no output supression - exec "${ORIGINAL_TOOL}" "$@" -else - # Run original command and capture output & exit status - TMPFILE=$(mktemp "quiet-${TOOL}.XXXXXX") - "${ORIGINAL_TOOL}" "$@" > "${TMPFILE}" 2>&1 - EXIT_STATUS=$? - - if [[ $EXIT_STATUS -ne 0 ]]; then - # On error, display the full output - cat "${TMPFILE}" - fi - - # Remove tmpfile - rm "${TMPFILE}" - - # Propagate the exit status - exit $EXIT_STATUS -fi +exec $(dirname "$0")/quiet "$@" diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet new file mode 100755 index 0000000000..2279e6a717 --- /dev/null +++ b/tests/scripts/quiet/quiet @@ -0,0 +1,65 @@ +#! /usr/bin/env bash +# +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later +# +# This swallows the output of the wrapped tool, unless there is an error. +# This helps reduce excess logging in the CI. + +# If you are debugging a build / CI issue, you can get complete unsilenced logs +# by un-commenting the following line (or setting VERBOSE_LOGS in your environment): +# VERBOSE_LOGS=1 + +# Locate original tool +ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1) + +print_quoted_args() { + # similar to printf '%q' "$@" + # but produce more human-readable results for common/simple cases like "a b" + for a in "$@"; do + simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' + if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then + # a has spaces, but no other special characters that need escaping + # (quoting after removing spaces yields no backslashes) + # simplify quoted form - e.g.: + # a b -> "a b" + # CFLAGS=a b -> CFLAGS="a b" + q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" + else + # get bash to do the quoting (which may result in no quotes or escaping, + # if none is needed). + q=$(printf '%q' "$a") + fi + printf "%s " "$q" + done +} + +if [[ ! " $* " =~ " --version " ]]; then + # Display the command being invoked - if it succeeds, this is all that will + # be displayed. Don't do this for invocations with --version, because + # this output is often parsed by scripts, so we don't want to modify it. + printf %s "${TOOL} " + print_quoted_args "$@" + echo +fi + +if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then + # Run original command with no output supression + exec "${ORIGINAL_TOOL}" "$@" +else + # Run original command and capture output & exit status + TMPFILE=$(mktemp "quiet-${TOOL}.XXXXXX") + "${ORIGINAL_TOOL}" "$@" > "${TMPFILE}" 2>&1 + EXIT_STATUS=$? + + if [[ $EXIT_STATUS -ne 0 ]]; then + # On error, display the full output + cat "${TMPFILE}" + fi + + # Remove tmpfile + rm "${TMPFILE}" + + # Propagate the exit status + exit $EXIT_STATUS +fi From 0b069bd5b15e978800a6131d8886eddcc68a03e1 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 12:48:49 +0000 Subject: [PATCH 30/53] Avoid infinite loop Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index 2279e6a717..bbc685cd8b 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -11,7 +11,8 @@ # VERBOSE_LOGS=1 # Locate original tool -ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$0" | head -n1) +TOOL_WITH_PATH=$(dirname "$0")/$TOOL +ORIGINAL_TOOL=$(type -ap "${TOOL}" | grep -v -Fx "$TOOL_WITH_PATH" | head -n1) print_quoted_args() { # similar to printf '%q' "$@" From e2317649dd41a58911d95265f187082c6c29673c Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:27:18 +0000 Subject: [PATCH 31/53] Allow wrappers to be missing; quote directory name from make Co-authored-by: Gilles Peskine Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 10 ++-------- tests/scripts/quiet/make | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index b1a3462ee8..4065e7129d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -205,14 +205,8 @@ setup_quiet_wrappers() # unless there is an error. This reduces logging overhead in the CI. # # Note that the cmake wrapper breaks unless we use an absolute path here. - export PATH=${PWD}/tests/scripts/quiet:$PATH - if [[ ! -x ${PWD}/tests/scripts/quiet/make ]]; then - echo "can't find quiet/make" - exit 1 - fi - if [[ ! -x ${PWD}/tests/scripts/quiet/cmake ]]; then - echo "can't find quiet/cmake" - exit 1 + if [[ -e ${PWD}/tests/scripts/quiet ]]; then + export PATH=${PWD}/tests/scripts/quiet:$PATH fi } diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index a1ef3e565d..162d44de6c 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -15,4 +15,4 @@ export NO_SILENCE=" --version | test " export TOOL="make" -exec $(dirname "$0")/quiet "$@" +exec "$(dirname "$0")/quiet" "$@" From 62ba696baea3dea462d80d0ebf19fdc6af4679c6 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:28:13 +0000 Subject: [PATCH 32/53] Undo not-needed change Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4065e7129d..1bae7f9ef7 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -3741,7 +3741,7 @@ run_component () { # Preliminary setup pre_check_environment -pre_initialize_variables "$@" +pre_initialize_variables pre_parse_command_line "$@" setup_quiet_wrappers From 6122cb1013d57cbdf50ad179842392afd6c86f6c Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:28:42 +0000 Subject: [PATCH 33/53] Quote directory name from cmake wrapper Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 10a0b25f43..3723473f53 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -15,4 +15,4 @@ export NO_SILENCE=" --version " export TOOL="cmake" -exec $(dirname "$0")/quiet "$@" +exec "$(dirname "$0")/quiet" "$@" From 043325d191395e6885fa5220ac8fa61bce8fd6dc Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:28:56 +0000 Subject: [PATCH 34/53] Improve docs Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index bbc685cd8b..8f83d07638 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -9,6 +9,17 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): # VERBOSE_LOGS=1 +# +# This script provides most of the functionality for the adjacent make and cmake +# wrappers. +# +# It requires two variables to be set: +# +# TOOL - the name of the tool that is being wrapped (with no path), e.g. "make" +# +# NO_SILENCE - a regex that describes the commandline arguments for which output will not +# be silenced, e.g. " --version | test ". In this example, "make test" will +# not be silent, but "make lib" will be. # Locate original tool TOOL_WITH_PATH=$(dirname "$0")/$TOOL From a8e671d7bb24204e2976b317c4a871175b617f34 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:29:10 +0000 Subject: [PATCH 35/53] remove shebang from quiet Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index 8f83d07638..00e2f6342f 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -1,5 +1,3 @@ -#! /usr/bin/env bash -# # Copyright The Mbed TLS Contributors # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later # From f57529903d819acb429bc7c364fb285f6c95fb4b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:29:31 +0000 Subject: [PATCH 36/53] Improve simplified quoting Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index 00e2f6342f..7b54bdba86 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -27,18 +27,15 @@ print_quoted_args() { # similar to printf '%q' "$@" # but produce more human-readable results for common/simple cases like "a b" for a in "$@"; do - simple_pattern='^([[:alnum:]_+-]+=)?([[:alnum:] _=+-./:@]*)$' - if [[ $a =~ ' ' && $a =~ $simple_pattern ]]; then - # a has spaces, but no other special characters that need escaping - # (quoting after removing spaces yields no backslashes) - # simplify quoted form - e.g.: - # a b -> "a b" - # CFLAGS=a b -> CFLAGS="a b" - q="${BASH_REMATCH[1]}\"${BASH_REMATCH[2]}\"" - else - # get bash to do the quoting (which may result in no quotes or escaping, - # if none is needed). - q=$(printf '%q' "$a") + # Get bash to quote the string + q=$(printf '%q' "$a") + simple_pattern="^([-[:alnum:]_+./:@]+=)?([^']*)$" + if [[ "$a" != "$q" && $a =~ $simple_pattern ]]; then + # a requires some quoting (a != q), but has no single quotes, so we can + # simplify the quoted form - e.g.: + # a b -> 'a b' + # CFLAGS=a b -> CFLAGS='a b' + q="${BASH_REMATCH[1]}'${BASH_REMATCH[2]}'" fi printf "%s " "$q" done From cdf57d1ddc0005dcb7d6c43dd101104c681685ed Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:30:37 +0000 Subject: [PATCH 37/53] remove trailing space from printed command Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index 7b54bdba86..9c4939ed24 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -37,7 +37,7 @@ print_quoted_args() { # CFLAGS=a b -> CFLAGS='a b' q="${BASH_REMATCH[1]}'${BASH_REMATCH[2]}'" fi - printf "%s " "$q" + printf " %s" "$q" done } @@ -45,7 +45,7 @@ if [[ ! " $* " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - printf %s "${TOOL} " + printf %s "${TOOL}" print_quoted_args "$@" echo fi From 04e0f41f085b9e80ab8ecdcaeec836e1cd46fc6a Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 26 Feb 2024 17:30:56 +0000 Subject: [PATCH 38/53] Send printed command to stderr Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet index 9c4939ed24..0f5bf06ecd 100755 --- a/tests/scripts/quiet/quiet +++ b/tests/scripts/quiet/quiet @@ -45,9 +45,9 @@ if [[ ! " $* " =~ " --version " ]]; then # Display the command being invoked - if it succeeds, this is all that will # be displayed. Don't do this for invocations with --version, because # this output is often parsed by scripts, so we don't want to modify it. - printf %s "${TOOL}" - print_quoted_args "$@" - echo + printf %s "${TOOL}" 1>&2 + print_quoted_args "$@" 1>&2 + echo 1>&2 fi if [[ " $@ " =~ $NO_SILENCE || -n "${VERBOSE_LOGS}" ]]; then From 7c84471ed7b7fa1bba186f07d90ced29b93f0da7 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:00:58 +0000 Subject: [PATCH 39/53] Rename quiet to quiet.sh Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 2 +- tests/scripts/quiet/make | 2 +- tests/scripts/quiet/{quiet => quiet.sh} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename tests/scripts/quiet/{quiet => quiet.sh} (100%) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 3723473f53..586397887f 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -15,4 +15,4 @@ export NO_SILENCE=" --version " export TOOL="cmake" -exec "$(dirname "$0")/quiet" "$@" +exec "$(dirname "$0")/quiet.sh" "$@" diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 162d44de6c..24e77951c2 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -15,4 +15,4 @@ export NO_SILENCE=" --version | test " export TOOL="make" -exec "$(dirname "$0")/quiet" "$@" +exec "$(dirname "$0")/quiet.sh" "$@" diff --git a/tests/scripts/quiet/quiet b/tests/scripts/quiet/quiet.sh similarity index 100% rename from tests/scripts/quiet/quiet rename to tests/scripts/quiet/quiet.sh From 03b232ae4e15c722b2110eb2d4714599b6fb33cb Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:06:19 +0000 Subject: [PATCH 40/53] Add editor hint for emacs Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/scripts/quiet/quiet.sh b/tests/scripts/quiet/quiet.sh index 0f5bf06ecd..be065f4d78 100755 --- a/tests/scripts/quiet/quiet.sh +++ b/tests/scripts/quiet/quiet.sh @@ -1,3 +1,5 @@ +# -*-mode: sh; sh-shell: bash -*- +# # Copyright The Mbed TLS Contributors # SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later # From b93ae3b4538c96f10c5b640ea5c7fab6374465e3 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:06:36 +0000 Subject: [PATCH 41/53] improve docs Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/quiet/quiet.sh b/tests/scripts/quiet/quiet.sh index be065f4d78..5cdcd1ea64 100755 --- a/tests/scripts/quiet/quiet.sh +++ b/tests/scripts/quiet/quiet.sh @@ -19,7 +19,7 @@ # # NO_SILENCE - a regex that describes the commandline arguments for which output will not # be silenced, e.g. " --version | test ". In this example, "make test" will -# not be silent, but "make lib" will be. +# not be silent, but "make lib test" will be. # Locate original tool TOOL_WITH_PATH=$(dirname "$0")/$TOOL From c2a27492bca2c0445556c1267acc43a6b5c95d1e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:06:49 +0000 Subject: [PATCH 42/53] simplify printf call Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/quiet/quiet.sh b/tests/scripts/quiet/quiet.sh index 5cdcd1ea64..5b341294f7 100755 --- a/tests/scripts/quiet/quiet.sh +++ b/tests/scripts/quiet/quiet.sh @@ -30,7 +30,7 @@ print_quoted_args() { # but produce more human-readable results for common/simple cases like "a b" for a in "$@"; do # Get bash to quote the string - q=$(printf '%q' "$a") + printf -v q '%q' "$a" simple_pattern="^([-[:alnum:]_+./:@]+=)?([^']*)$" if [[ "$a" != "$q" && $a =~ $simple_pattern ]]; then # a requires some quoting (a != q), but has no single quotes, so we can From f4aa1ce006159d0bf7102b1d135f9ace92d9b95b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:14:37 +0000 Subject: [PATCH 43/53] Fix docs Co-authored-by: Gilles Peskine Signed-off-by: Dave Rodgman --- tests/scripts/quiet/quiet.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/quiet/quiet.sh b/tests/scripts/quiet/quiet.sh index 5b341294f7..01ef422e32 100755 --- a/tests/scripts/quiet/quiet.sh +++ b/tests/scripts/quiet/quiet.sh @@ -18,8 +18,8 @@ # TOOL - the name of the tool that is being wrapped (with no path), e.g. "make" # # NO_SILENCE - a regex that describes the commandline arguments for which output will not -# be silenced, e.g. " --version | test ". In this example, "make test" will -# not be silent, but "make lib test" will be. +# be silenced, e.g. " --version | test ". In this example, "make lib test" will +# not be silent, but "make lib" will be. # Locate original tool TOOL_WITH_PATH=$(dirname "$0")/$TOOL From 7a659102f559a2fa375332f9f492693961a287a9 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 14:59:40 +0000 Subject: [PATCH 44/53] Use export to set VERBOSE_LOGS Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 2 +- tests/scripts/quiet/make | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 586397887f..854375ecb4 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -8,7 +8,7 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): -# VERBOSE_LOGS=1 +# export VERBOSE_LOGS=1 # don't silence invocations containing these arguments export NO_SILENCE=" --version " diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index 24e77951c2..b2323166df 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -8,7 +8,7 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): -# VERBOSE_LOGS=1 +# export VERBOSE_LOGS=1 # don't silence invocations containing these arguments export NO_SILENCE=" --version | test " From 9f1003b381d1befe8453aee7150902979063416e Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 15:01:29 +0000 Subject: [PATCH 45/53] blank line for readability Signed-off-by: Dave Rodgman --- tests/scripts/quiet/cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/quiet/cmake b/tests/scripts/quiet/cmake index 854375ecb4..930931d539 100755 --- a/tests/scripts/quiet/cmake +++ b/tests/scripts/quiet/cmake @@ -8,6 +8,7 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): + # export VERBOSE_LOGS=1 # don't silence invocations containing these arguments From 422f9bcea04af64625f00364372b8ba6dc1a2258 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 18:41:36 +0000 Subject: [PATCH 46/53] Fix formatting Signed-off-by: Dave Rodgman --- tests/scripts/quiet/make | 1 + tests/scripts/quiet/quiet.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/scripts/quiet/make b/tests/scripts/quiet/make index b2323166df..d022551df1 100755 --- a/tests/scripts/quiet/make +++ b/tests/scripts/quiet/make @@ -8,6 +8,7 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): + # export VERBOSE_LOGS=1 # don't silence invocations containing these arguments diff --git a/tests/scripts/quiet/quiet.sh b/tests/scripts/quiet/quiet.sh index 01ef422e32..30ee569a22 100755 --- a/tests/scripts/quiet/quiet.sh +++ b/tests/scripts/quiet/quiet.sh @@ -8,6 +8,7 @@ # If you are debugging a build / CI issue, you can get complete unsilenced logs # by un-commenting the following line (or setting VERBOSE_LOGS in your environment): +# # VERBOSE_LOGS=1 # # This script provides most of the functionality for the adjacent make and cmake From e264a7dcd1481fe03bc3bcf0a12a35da1fbe5495 Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Thu, 29 Feb 2024 21:22:59 +0000 Subject: [PATCH 47/53] Fix generate_visualc_files.pl Signed-off-by: Dave Rodgman --- scripts/generate_visualc_files.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/generate_visualc_files.pl b/scripts/generate_visualc_files.pl index 11604cd9c0..b27f795fca 100755 --- a/scripts/generate_visualc_files.pl +++ b/scripts/generate_visualc_files.pl @@ -168,7 +168,7 @@ sub gen_app { } sub get_app_list { - my $app_list = `cd $programs_dir && make list`; + my $app_list = `cd $programs_dir && VERBOSE_LOGS=1 make list`; die "make list failed: $!\n" if $?; return split /\s+/, $app_list; From 363db7759a5c40ec463459f4191202ad0a9cf1bd Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Fri, 1 Mar 2024 12:11:24 +0000 Subject: [PATCH 48/53] Add config guards to ssl session comment Show which members of the session structure are dependent on configuration options and which aren't. Signed-off-by: David Horstmann --- library/ssl_tls.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 13edabc091..fa2d88f517 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5286,19 +5286,34 @@ static const unsigned char ssl_serialized_session_header[] = { * // the setting of those compile-time * // configuration options which influence * // the structure of mbedtls_ssl_session. - * uint64 start_time; - * uint8 ciphersuite[2]; // defined by the standard - * uint8 compression; // 0 or 1 - * uint8 session_id_len; // at most 32 - * opaque session_id[32]; - * opaque master[48]; // fixed length in the standard - * uint32 verify_result; - * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert - * opaque ticket<0..2^24-1>; // length 0 means no ticket - * uint32 ticket_lifetime; - * uint8 mfl_code; // up to 255 according to standard - * uint8 trunc_hmac; // 0 or 1 - * uint8 encrypt_then_mac; // 0 or 1 + * #if defined(MBEDTLS_HAVE_TIME) + * uint64 start_time; + * #endif + * uint8 ciphersuite[2]; // defined by the standard + * uint8 compression; // 0 or 1 + * uint8 session_id_len; // at most 32 + * opaque session_id[32]; + * opaque master[48]; // fixed length in the standard + * uint32 verify_result; + * #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) + * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert + * #else + * uint8 peer_cert_digest_type; + * opaque peer_cert_digest<0..2^8-1> + * #endif + * #if defined(MBEDTLS_SSL_SESSION_TICKETS) + * opaque ticket<0..2^24-1>; // length 0 means no ticket + * uint32 ticket_lifetime; + * #endif + * #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + * uint8 mfl_code; // up to 255 according to standard + * #endif + * #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) + * uint8 trunc_hmac; // 0 or 1 + * #endif + * #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) + * uint8 encrypt_then_mac; // 0 or 1 + * #endif * * The order is the same as in the definition of the structure, except * verify_result is put before peer_cert so that all mandatory fields come From 11def974723ac2ea28ff964a9345342dab29d42a Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Fri, 1 Mar 2024 11:20:32 +0000 Subject: [PATCH 49/53] Fix naming inconsistencies in config bits Signed-off-by: David Horstmann --- library/ssl_tls.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index fa2d88f517..c4f8bd3e04 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5205,9 +5205,9 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) -#define SSL_SERIALIZED_SESSION_KEEP_PEER_CRT 1 +#define SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT 1 #else -#define SSL_SERIALIZED_SESSION_KEEP_PEER_CRT 0 +#define SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT 0 #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_CLI_C) && defined(MBEDTLS_SSL_SESSION_TICKETS) @@ -5247,7 +5247,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con #define SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT 4 #define SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT 5 #define SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT 6 -#define SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT 7 +#define SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT_BIT 7 #define SSL_SERIALIZED_SESSION_CONFIG_BITFLAG \ ((uint16_t) ( \ @@ -5260,7 +5260,7 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_ETM << SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT) | \ - (SSL_SERIALIZED_SESSION_KEEP_PEER_CRT << SSL_SERIALIZED_SESSION_CONFIG_KEEP_CRT_BIT))) + (SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT << SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT_BIT))) static const unsigned char ssl_serialized_session_header[] = { MBEDTLS_VERSION_MAJOR, From ec8a5b175ec4735e905ca342a8405c0adce2db1e Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Fri, 1 Mar 2024 11:29:36 +0000 Subject: [PATCH 50/53] Add ChangeLog entry for ssl serialization bitflags Signed-off-by: David Horstmann --- ChangeLog.d/fix-ssl-session-serialization-config.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix-ssl-session-serialization-config.txt diff --git a/ChangeLog.d/fix-ssl-session-serialization-config.txt b/ChangeLog.d/fix-ssl-session-serialization-config.txt new file mode 100644 index 0000000000..ca1cc81f5e --- /dev/null +++ b/ChangeLog.d/fix-ssl-session-serialization-config.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix missing bitflags in SSL session serialization headers. Their absence + allowed SSL sessions saved in one configuration to be loaded in a + different, incompatible configuration. From f5a6fa2e4ac233898dae8c3b8954d290a3c06cf6 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Fri, 1 Mar 2024 12:31:35 +0000 Subject: [PATCH 51/53] Fix code style Signed-off-by: David Horstmann --- library/ssl_tls.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c4f8bd3e04..a7735c394a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5260,7 +5260,8 @@ const mbedtls_ssl_session *mbedtls_ssl_get_session_pointer(const mbedtls_ssl_con SSL_SERIALIZED_SESSION_CONFIG_TRUNC_HMAC_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_ETM << SSL_SERIALIZED_SESSION_CONFIG_ETM_BIT) | \ (SSL_SERIALIZED_SESSION_CONFIG_TICKET << SSL_SERIALIZED_SESSION_CONFIG_TICKET_BIT) | \ - (SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT << SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT_BIT))) + (SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT << \ + SSL_SERIALIZED_SESSION_CONFIG_KEEP_PEER_CRT_BIT))) static const unsigned char ssl_serialized_session_header[] = { MBEDTLS_VERSION_MAJOR, From dff18da29a26085761cf7eb58eca753977a5b24b Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Fri, 1 Mar 2024 15:53:52 +0000 Subject: [PATCH 52/53] fix zlib test Signed-off-by: Dave Rodgman --- tests/scripts/all.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 1bae7f9ef7..01ce3b9c2d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1008,6 +1008,9 @@ EOF } component_test_zlib_cmake() { + # This is needed due to something parsing the output from make + export VERBOSE_LOGS=1 + msg "build: zlib enabled, cmake" scripts/config.py set MBEDTLS_ZLIB_SUPPORT cmake -D ENABLE_ZLIB_SUPPORT=On -D CMAKE_BUILD_TYPE:String=Release . From fc8cacf9a277a5e53d721e60df089c1b3311ff4c Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 4 Mar 2024 16:37:02 +0000 Subject: [PATCH 53/53] Add missing config guards in comment Signed-off-by: David Horstmann --- library/ssl_tls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index a7735c394a..235959a9ee 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5296,13 +5296,15 @@ static const unsigned char ssl_serialized_session_header[] = { * opaque session_id[32]; * opaque master[48]; // fixed length in the standard * uint32 verify_result; + * #if defined(MBEDTLS_X509_CRT_PARSE_C) * #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) * opaque peer_cert<0..2^24-1>; // length 0 means no peer cert * #else * uint8 peer_cert_digest_type; * opaque peer_cert_digest<0..2^8-1> * #endif - * #if defined(MBEDTLS_SSL_SESSION_TICKETS) + * #endif + * #if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C) * opaque ticket<0..2^24-1>; // length 0 means no ticket * uint32 ticket_lifetime; * #endif