From b7ae23856a5929fd3dbad75a0165489f7e8111ca Mon Sep 17 00:00:00 2001 From: Laukik Hase Date: Thu, 15 Sep 2022 12:40:53 +0530 Subject: [PATCH] protocomm: `MBEDTLS_PRIVATE` & `MBEDTLS_ALLOW_PRIVATE_ACCESS`-related cleanup --- components/protocomm/src/security/security1.c | 28 +++++------- .../protocomm/test_apps/main/test_protocomm.c | 45 +++++++++++-------- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/components/protocomm/src/security/security1.c b/components/protocomm/src/security/security1.c index 316747b673..f530055fab 100644 --- a/components/protocomm/src/security/security1.c +++ b/components/protocomm/src/security/security1.c @@ -10,23 +10,18 @@ #include #include -/* ToDo - Remove this once appropriate solution is available. -We need to define this for the file as ssl_misc.h uses private structures from mbedtls, -which are undefined if the following flag is not defined */ -/* Many APIs in the file make use of this flag instead of `MBEDTLS_PRIVATE` */ -/* ToDo - Replace them with proper getter-setter once they are added */ -#define MBEDTLS_ALLOW_PRIVATE_ACCESS - -/* ToDo - Remove this once appropriate solution is available. - * Currently MBEDTLS_LEGACY_CONTEXT is enabled by default for MBEDTLS_ECP_RESTARTABLE +/* TODO: Currently MBEDTLS_ECDH_LEGACY_CONTEXT is enabled by default + * when MBEDTLS_ECP_RESTARTABLE is enabled. * This is a temporary workaround to allow that. - * The LEGACY option is soon going to be removed in future mbedtls - * once it is removed we can remove the workaround. + * + * The legacy option is soon going to be removed in future mbedtls + * versions and this workaround will be removed once the appropriate + * solution is available. */ #ifdef CONFIG_MBEDTLS_ECDH_LEGACY_CONTEXT -#define ACCESS_ECDH(S, var) S->var +#define ACCESS_ECDH(S, var) S->MBEDTLS_PRIVATE(var) #else -#define ACCESS_ECDH(S, var) S->ctx.mbed_ecdh.var +#define ACCESS_ECDH(S, var) S->MBEDTLS_PRIVATE(ctx).MBEDTLS_PRIVATE(mbed_ecdh).MBEDTLS_PRIVATE(var) #endif #include @@ -36,7 +31,6 @@ which are undefined if the following flag is not defined */ #include #include #include -#include #include #include @@ -249,7 +243,7 @@ static esp_err_t handle_session_command0(session_t *cur_session, goto exit_cmd0; } - mbed_err = mbedtls_mpi_write_binary(ACCESS_ECDH(&ctx_server, Q).X, + mbed_err = mbedtls_mpi_write_binary(ACCESS_ECDH(&ctx_server, Q).MBEDTLS_PRIVATE(X), cur_session->device_pubkey, PUBLIC_KEY_LEN); if (mbed_err != 0) { @@ -266,7 +260,7 @@ static esp_err_t handle_session_command0(session_t *cur_session, hexdump("Device pubkey", dev_pubkey, PUBLIC_KEY_LEN); hexdump("Client pubkey", cli_pubkey, PUBLIC_KEY_LEN); - mbed_err = mbedtls_mpi_lset(ACCESS_ECDH(&ctx_server, Qp).Z, 1); + mbed_err = mbedtls_mpi_lset(ACCESS_ECDH(&ctx_server, Qp).MBEDTLS_PRIVATE(Z), 1); if (mbed_err != 0) { ESP_LOGE(TAG, "Failed at mbedtls_mpi_lset with error code : -0x%x", -mbed_err); ret = ESP_FAIL; @@ -274,7 +268,7 @@ static esp_err_t handle_session_command0(session_t *cur_session, } flip_endian(cur_session->client_pubkey, PUBLIC_KEY_LEN); - mbed_err = mbedtls_mpi_read_binary(ACCESS_ECDH(&ctx_server, Qp).X, cli_pubkey, PUBLIC_KEY_LEN); + mbed_err = mbedtls_mpi_read_binary(ACCESS_ECDH(&ctx_server, Qp).MBEDTLS_PRIVATE(X), cli_pubkey, PUBLIC_KEY_LEN); flip_endian(cur_session->client_pubkey, PUBLIC_KEY_LEN); if (mbed_err != 0) { ESP_LOGE(TAG, "Failed at mbedtls_mpi_read_binary with error code : -0x%x", -mbed_err); diff --git a/components/protocomm/test_apps/main/test_protocomm.c b/components/protocomm/test_apps/main/test_protocomm.c index dbf7ea4057..bb8adf2d47 100644 --- a/components/protocomm/test_apps/main/test_protocomm.c +++ b/components/protocomm/test_apps/main/test_protocomm.c @@ -14,12 +14,19 @@ #include #include -/* ToDo - Remove this once appropriate solution is available. -We need to define this for the file as ssl_misc.h uses private structures from mbedtls, -which are undefined if the following flag is not defined */ -/* Many APIs in the file make use of this flag instead of `MBEDTLS_PRIVATE()` */ -/* ToDo - Replace them with proper getter-setter once they are added */ -#define MBEDTLS_ALLOW_PRIVATE_ACCESS +/* TODO: Currently MBEDTLS_ECDH_LEGACY_CONTEXT is enabled by default + * when MBEDTLS_ECP_RESTARTABLE is enabled. + * This is a temporary workaround to allow that. + * + * The legacy option is soon going to be removed in future mbedtls + * versions and this workaround will be removed once the appropriate + * solution is available. + */ +#ifdef CONFIG_MBEDTLS_ECDH_LEGACY_CONTEXT +#define ACCESS_ECDH(S, var) S.MBEDTLS_PRIVATE(var) +#else +#define ACCESS_ECDH(S, var) S.MBEDTLS_PRIVATE(ctx).MBEDTLS_PRIVATE(mbed_ecdh).MBEDTLS_PRIVATE(var) +#endif #include #include @@ -155,24 +162,24 @@ static esp_err_t verify_response0(session_t *session, SessionData *resp) hexdump("Device pubkey", dev_pubkey, PUBLIC_KEY_LEN); hexdump("Client pubkey", cli_pubkey, PUBLIC_KEY_LEN); - ret = mbedtls_mpi_lset(&session->ctx_client.ctx.mbed_ecdh.Qp.Z, 1); + ret = mbedtls_mpi_lset(ACCESS_ECDH(&session->ctx_client, Qp).MBEDTLS_PRIVATE(Z), 1); if (ret != 0) { ESP_LOGE(TAG, "Failed at mbedtls_mpi_lset with error code : %d", ret); return ESP_FAIL; } flip_endian(session->device_pubkey, PUBLIC_KEY_LEN); - ret = mbedtls_mpi_read_binary(&session->ctx_client.ctx.mbed_ecdh.Qp.X, dev_pubkey, PUBLIC_KEY_LEN); + ret = mbedtls_mpi_read_binary(ACCESS_ECDH(&session->ctx_client, Qp).MBEDTLS_PRIVATE(X), dev_pubkey, PUBLIC_KEY_LEN); flip_endian(session->device_pubkey, PUBLIC_KEY_LEN); if (ret != 0) { ESP_LOGE(TAG, "Failed at mbedtls_mpi_read_binary with error code : %d", ret); return ESP_FAIL; } - ret = mbedtls_ecdh_compute_shared(&session->ctx_client.ctx.mbed_ecdh.grp, - &session->ctx_client.ctx.mbed_ecdh.z, - &session->ctx_client.ctx.mbed_ecdh.Qp, - &session->ctx_client.ctx.mbed_ecdh.d, + ret = mbedtls_ecdh_compute_shared(ACCESS_ECDH(&session->ctx_client, grp), + ACCESS_ECDH(&session->ctx_client, z), + ACCESS_ECDH(&session->ctx_client, Qp), + ACCESS_ECDH(&session->ctx_client, d), mbedtls_ctr_drbg_random, &session->ctr_drbg); if (ret != 0) { @@ -180,7 +187,7 @@ static esp_err_t verify_response0(session_t *session, SessionData *resp) return ESP_FAIL; } - ret = mbedtls_mpi_write_binary(&session->ctx_client.ctx.mbed_ecdh.z, session->sym_key, PUBLIC_KEY_LEN); + ret = mbedtls_mpi_write_binary(ACCESS_ECDH(&session->ctx_client, z), session->sym_key, PUBLIC_KEY_LEN); if (ret != 0) { ESP_LOGE(TAG, "Failed at mbedtls_mpi_write_binary with error code : %d", ret); return ESP_FAIL; @@ -382,15 +389,15 @@ static esp_err_t test_sec_endpoint(session_t *session) goto abort_test_sec_endpoint; } - ret = mbedtls_ecp_group_load(&session->ctx_client.ctx.mbed_ecdh.grp, MBEDTLS_ECP_DP_CURVE25519); + ret = mbedtls_ecp_group_load(ACCESS_ECDH(&session->ctx_client, grp), MBEDTLS_ECP_DP_CURVE25519); if (ret != 0) { ESP_LOGE(TAG, "Failed at mbedtls_ecp_group_load with error code : %d", ret); goto abort_test_sec_endpoint; } - ret = mbedtls_ecdh_gen_public(&session->ctx_client.ctx.mbed_ecdh.grp, - &session->ctx_client.ctx.mbed_ecdh.d, - &session->ctx_client.ctx.mbed_ecdh.Q, + ret = mbedtls_ecdh_gen_public(ACCESS_ECDH(&session->ctx_client, grp), + ACCESS_ECDH(&session->ctx_client, d), + ACCESS_ECDH(&session->ctx_client, Q), mbedtls_ctr_drbg_random, &session->ctr_drbg); if (ret != 0) { @@ -400,7 +407,7 @@ static esp_err_t test_sec_endpoint(session_t *session) if (session->weak) { /* Read zero client public key */ - ret = mbedtls_mpi_read_binary(&session->ctx_client.ctx.mbed_ecdh.Q.X, + ret = mbedtls_mpi_read_binary(ACCESS_ECDH(&session->ctx_client, Q).MBEDTLS_PRIVATE(X), session->client_pubkey, PUBLIC_KEY_LEN); if (ret != 0) { @@ -408,7 +415,7 @@ static esp_err_t test_sec_endpoint(session_t *session) goto abort_test_sec_endpoint; } } - ret = mbedtls_mpi_write_binary(&session->ctx_client.ctx.mbed_ecdh.Q.X, + ret = mbedtls_mpi_write_binary(ACCESS_ECDH(&session->ctx_client, Q).MBEDTLS_PRIVATE(X), session->client_pubkey, PUBLIC_KEY_LEN); if (ret != 0) {