From 7b0ee1e655c3bf2204e979aeb858e6fe2505667f Mon Sep 17 00:00:00 2001 From: Agathiyan Bragadeesh Date: Thu, 27 Jul 2023 15:51:46 +0100 Subject: [PATCH 1/3] Fix control bypass warnings Declarations have been moved to the top of functions to fix this Signed-off-by: Agathiyan Bragadeesh --- library/ecp.c | 3 ++- library/psa_crypto.c | 34 +++++++++++++++++++--------- tests/src/test_helpers/ssl_helpers.c | 6 +++-- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 2d80b6f33f0b..1cf242ae2d80 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2593,6 +2593,7 @@ static int ecp_mul_mxz(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, void *p_rng) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + int have_rng; size_t i; unsigned char b; mbedtls_ecp_point RP; @@ -2626,7 +2627,7 @@ static int ecp_mul_mxz(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, MOD_ADD(RP.X); /* Randomize coordinates of the starting point */ - int have_rng = 1; + have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) if (f_rng == NULL) { have_rng = 0; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index fdcdd43d50cb..d1e6b5cc46bd 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1472,6 +1472,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot; /* Reject a zero-length output buffer now, since this can never be a @@ -1498,7 +1499,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; status = psa_driver_wrapper_export_public_key( @@ -2406,6 +2407,7 @@ static psa_status_t psa_mac_setup(psa_mac_operation_t *operation, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot = NULL; /* A context must be freshly initialized before it can be set up. */ @@ -2423,7 +2425,7 @@ static psa_status_t psa_mac_setup(psa_mac_operation_t *operation, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -2594,6 +2596,7 @@ static psa_status_t psa_mac_compute_internal(mbedtls_svc_key_id_t key, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot; uint8_t operation_mac_size = 0; @@ -2606,7 +2609,7 @@ static psa_status_t psa_mac_compute_internal(mbedtls_svc_key_id_t key, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -2732,6 +2735,7 @@ static psa_status_t psa_sign_internal(mbedtls_svc_key_id_t key, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot; *signature_length = 0; @@ -2764,7 +2768,7 @@ static psa_status_t psa_sign_internal(mbedtls_svc_key_id_t key, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -3303,6 +3307,7 @@ static psa_status_t psa_cipher_setup(psa_cipher_operation_t *operation, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot = NULL; psa_key_usage_t usage = (cipher_operation == MBEDTLS_ENCRYPT ? PSA_KEY_USAGE_ENCRYPT : @@ -3338,7 +3343,7 @@ static psa_status_t psa_cipher_setup(psa_cipher_operation_t *operation, } operation->default_iv_length = PSA_CIPHER_IV_LENGTH(slot->attr.type, alg); - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -3561,6 +3566,7 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot = NULL; uint8_t local_iv[PSA_CIPHER_IV_MAX_SIZE]; size_t default_iv_length = 0; @@ -3577,7 +3583,7 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -3633,6 +3639,7 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; + psa_key_attributes_t attributes; psa_key_slot_t *slot = NULL; if (!PSA_ALG_IS_CIPHER(alg)) { @@ -3647,7 +3654,7 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, goto exit; } - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -4251,6 +4258,7 @@ static psa_status_t psa_generate_derived_key_internal( uint8_t *data = NULL; size_t bytes = PSA_BITS_TO_BYTES(bits); psa_status_t status; + psa_key_attributes_t attributes; if (!key_type_is_raw_bytes(slot->attr.type)) { return PSA_ERROR_INVALID_ARGUMENT; @@ -4279,7 +4287,7 @@ static psa_status_t psa_generate_derived_key_internal( } slot->attr.bits = (psa_key_bits_t) bits; - psa_key_attributes_t attributes = { + attributes = (psa_key_attributes_t) { .core = slot->attr }; @@ -4891,14 +4899,15 @@ static psa_status_t psa_key_agreement_raw_internal(psa_algorithm_t alg, size_t shared_secret_size, size_t *shared_secret_length) { + mbedtls_ecp_keypair *ecp = NULL; + psa_status_t status; switch (alg) { #if defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH) case PSA_ALG_ECDH: if (!PSA_KEY_TYPE_IS_ECC_KEY_PAIR(private_key->attr.type)) { return PSA_ERROR_INVALID_ARGUMENT; } - mbedtls_ecp_keypair *ecp = NULL; - psa_status_t status = mbedtls_psa_ecp_load_representation( + status = mbedtls_psa_ecp_load_representation( private_key->attr.type, private_key->attr.bits, private_key->key.data, @@ -4916,6 +4925,8 @@ static psa_status_t psa_key_agreement_raw_internal(psa_algorithm_t alg, return status; #endif /* MBEDTLS_PSA_BUILTIN_ALG_ECDH */ default: + (void) ecp; + (void) status; (void) private_key; (void) peer_key; (void) peer_key_length; @@ -5011,6 +5022,7 @@ psa_status_t psa_raw_key_agreement(psa_algorithm_t alg, psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot = NULL; + size_t expected_length; if (!PSA_ALG_IS_KEY_AGREEMENT(alg)) { status = PSA_ERROR_INVALID_ARGUMENT; @@ -5030,7 +5042,7 @@ psa_status_t psa_raw_key_agreement(psa_algorithm_t alg, * PSA_RAW_KEY_AGREEMENT_OUTPUT_SIZE() is exact so the point is moot. * If FFDH is implemented, PSA_RAW_KEY_AGREEMENT_OUTPUT_SIZE() can easily * be exact for it as well. */ - size_t expected_length = + expected_length = PSA_RAW_KEY_AGREEMENT_OUTPUT_SIZE(slot->attr.type, slot->attr.bits); if (output_size < expected_length) { status = PSA_ERROR_BUFFER_TOO_SMALL; diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index bc9a2049837e..52759aac9f28 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -806,13 +806,14 @@ int mbedtls_ssl_write_fragment(mbedtls_ssl_context *ssl, int *written, const int expected_fragments) { + int ret; /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is * a valid no-op for TLS connections. */ if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) { TEST_ASSERT(mbedtls_ssl_write(ssl, NULL, 0) == 0); } - int ret = mbedtls_ssl_write(ssl, buf + *written, buf_len - *written); + ret = mbedtls_ssl_write(ssl, buf + *written, buf_len - *written); if (ret > 0) { *written += ret; } @@ -852,13 +853,14 @@ int mbedtls_ssl_read_fragment(mbedtls_ssl_context *ssl, int *read, int *fragments, const int expected_fragments) { + int ret; /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is * a valid no-op for TLS connections. */ if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) { TEST_ASSERT(mbedtls_ssl_read(ssl, NULL, 0) == 0); } - int ret = mbedtls_ssl_read(ssl, buf + *read, buf_len - *read); + ret = mbedtls_ssl_read(ssl, buf + *read, buf_len - *read); if (ret > 0) { (*fragments)++; *read += ret; From 5521b4ce37b2b7768a66c3a2b5a2fcff200ca11a Mon Sep 17 00:00:00 2001 From: Agathiyan Bragadeesh Date: Mon, 31 Jul 2023 16:15:56 +0100 Subject: [PATCH 2/3] Assign have_rng in declaration of ecp_mul_mxz Signed-off-by: Agathiyan Bragadeesh --- library/ecp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/ecp.c b/library/ecp.c index 1cf242ae2d80..5b60291cc80f 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -2593,7 +2593,7 @@ static int ecp_mul_mxz(mbedtls_ecp_group *grp, mbedtls_ecp_point *R, void *p_rng) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - int have_rng; + int have_rng = 1; size_t i; unsigned char b; mbedtls_ecp_point RP; @@ -2626,9 +2626,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 */ - have_rng = 1; #if defined(MBEDTLS_ECP_NO_INTERNAL_RNG) + /* Derandomize coordinates of the starting point */ if (f_rng == NULL) { have_rng = 0; } From e7eb8052befbce14917cc1dbe8e7a8bdc854f245 Mon Sep 17 00:00:00 2001 From: Agathiyan Bragadeesh Date: Mon, 31 Jul 2023 16:16:38 +0100 Subject: [PATCH 3/3] Seperate declarations from function body Signed-off-by: Agathiyan Bragadeesh --- library/psa_crypto.c | 1 + tests/src/test_helpers/ssl_helpers.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d1e6b5cc46bd..fade286ecd74 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4901,6 +4901,7 @@ static psa_status_t psa_key_agreement_raw_internal(psa_algorithm_t alg, { mbedtls_ecp_keypair *ecp = NULL; psa_status_t status; + switch (alg) { #if defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH) case PSA_ALG_ECDH: diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 52759aac9f28..bd1f46c96845 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -807,6 +807,7 @@ int mbedtls_ssl_write_fragment(mbedtls_ssl_context *ssl, const int expected_fragments) { int ret; + /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is * a valid no-op for TLS connections. */ if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) { @@ -854,6 +855,7 @@ int mbedtls_ssl_read_fragment(mbedtls_ssl_context *ssl, const int expected_fragments) { int ret; + /* Verify that calling mbedtls_ssl_write with a NULL buffer and zero length is * a valid no-op for TLS connections. */ if (ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM) {