From 26c10f797cced4eab68590accc96508d70325ff7 Mon Sep 17 00:00:00 2001 From: Florent Daigniere Date: Fri, 27 Jun 2014 11:59:45 +0200 Subject: [PATCH] OpenSSL: Use EC_POINT_clear_free instead of EC_POINT_free This changes OpenSSL calls to explicitly clear the EC_POINT memory allocations when freeing them. This adds an extra layer of security by avoiding leaving potentially private keys into local memory after they are not needed anymore. While some of these variables are not really private (e.g., they are sent in clear anyway), the extra cost of clearing them is not significant and it is simpler to just clear these explicitly rather than review each possible code path to confirm where this does not help. Signed-off-by: Florent Daigniere --- src/crypto/crypto_openssl.c | 4 ++-- src/eap_common/eap_pwd_common.c | 2 +- src/eap_peer/eap_pwd.c | 10 +++++----- src/eap_server/eap_server_pwd.c | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c index d04594feb..f02aaacb7 100644 --- a/src/crypto/crypto_openssl.c +++ b/src/crypto/crypto_openssl.c @@ -1157,13 +1157,13 @@ struct crypto_ec_point * crypto_ec_point_from_bin(struct crypto_ec *e, if (x == NULL || y == NULL || elem == NULL) { BN_clear_free(x); BN_clear_free(y); - EC_POINT_free(elem); + EC_POINT_clear_free(elem); return NULL; } if (!EC_POINT_set_affine_coordinates_GFp(e->group, elem, x, y, e->bnctx)) { - EC_POINT_free(elem); + EC_POINT_clear_free(elem); elem = NULL; } diff --git a/src/eap_common/eap_pwd_common.c b/src/eap_common/eap_pwd_common.c index a1a24e371..fdcff7fa8 100644 --- a/src/eap_common/eap_pwd_common.c +++ b/src/eap_common/eap_pwd_common.c @@ -263,7 +263,7 @@ int compute_password_element(EAP_PWD_group *grp, u16 num, fail: EC_GROUP_free(grp->group); grp->group = NULL; - EC_POINT_free(grp->pwe); + EC_POINT_clear_free(grp->pwe); grp->pwe = NULL; BN_clear_free(grp->order); grp->order = NULL; diff --git a/src/eap_peer/eap_pwd.c b/src/eap_peer/eap_pwd.c index b9eae3773..089aec36d 100644 --- a/src/eap_peer/eap_pwd.c +++ b/src/eap_peer/eap_pwd.c @@ -153,14 +153,14 @@ static void eap_pwd_deinit(struct eap_sm *sm, void *priv) BN_clear_free(data->my_scalar); BN_clear_free(data->k); BN_CTX_free(data->bnctx); - EC_POINT_free(data->my_element); - EC_POINT_free(data->server_element); + EC_POINT_clear_free(data->my_element); + EC_POINT_clear_free(data->server_element); os_free(data->id_peer); os_free(data->id_server); bin_clear_free(data->password, data->password_len); if (data->grp) { EC_GROUP_free(data->grp->group); - EC_POINT_free(data->grp->pwe); + EC_POINT_clear_free(data->grp->pwe); BN_clear_free(data->grp->order); BN_clear_free(data->grp->prime); os_free(data->grp); @@ -474,8 +474,8 @@ fin: BN_clear_free(x); BN_clear_free(y); BN_clear_free(cofactor); - EC_POINT_free(K); - EC_POINT_free(point); + EC_POINT_clear_free(K); + EC_POINT_clear_free(point); if (data->outbuf == NULL) eap_pwd_state(data, FAILURE); else diff --git a/src/eap_server/eap_server_pwd.c b/src/eap_server/eap_server_pwd.c index e86d3b111..38fa0f201 100644 --- a/src/eap_server/eap_server_pwd.c +++ b/src/eap_server/eap_server_pwd.c @@ -140,14 +140,14 @@ static void eap_pwd_reset(struct eap_sm *sm, void *priv) BN_clear_free(data->my_scalar); BN_clear_free(data->k); BN_CTX_free(data->bnctx); - EC_POINT_free(data->my_element); - EC_POINT_free(data->peer_element); + EC_POINT_clear_free(data->my_element); + EC_POINT_clear_free(data->peer_element); os_free(data->id_peer); os_free(data->id_server); bin_clear_free(data->password, data->password_len); if (data->grp) { EC_GROUP_free(data->grp->group); - EC_POINT_free(data->grp->pwe); + EC_POINT_clear_free(data->grp->pwe); BN_clear_free(data->grp->order); BN_clear_free(data->grp->prime); os_free(data->grp); @@ -724,8 +724,8 @@ eap_pwd_process_commit_resp(struct eap_sm *sm, struct eap_pwd_data *data, res = 1; fin: - EC_POINT_free(K); - EC_POINT_free(point); + EC_POINT_clear_free(K); + EC_POINT_clear_free(point); BN_clear_free(cofactor); BN_clear_free(x); BN_clear_free(y);