OpenSSL: Use BN_clear_free instead of BN_free

This changes OpenSSL calls to explicitly clear the bignum 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 <nextgens@freenetproject.org>

diff --git a/src/crypto/crypto_openssl.c b/src/crypto/crypto_openssl.c
index 817ee2d00cbfcccf80d42f28f33ca14c374fdaec..d04594febce38a0f0883cae0da089657d6e0c0a6 100644 (file)
--- a/src/crypto/crypto_openssl.c
+++ b/src/crypto/crypto_openssl.c
@@ -340,10 +340,10 @@ int crypto_mod_exp(const u8 *base, size_t base_len,
        ret = 0;
 
 error:
-       BN_free(bn_base);
-       BN_free(bn_exp);
-       BN_free(bn_modulus);
-       BN_free(bn_result);
+       BN_clear_free(bn_base);
+       BN_clear_free(bn_exp);
+       BN_clear_free(bn_modulus);
+       BN_clear_free(bn_result);
        BN_CTX_free(ctx);
        return ret;
 }
@@ -571,12 +571,12 @@ struct wpabuf * dh5_derive_shared(void *ctx, const struct wpabuf *peer_public,
        if (keylen < 0)
                goto err;
        wpabuf_put(res, keylen);
-       BN_free(pub_key);
+       BN_clear_free(pub_key);
 
        return res;
 
 err:
-       BN_free(pub_key);
+       BN_clear_free(pub_key);
        wpabuf_free(res);
        return NULL;
 }
@@ -1066,7 +1066,7 @@ void crypto_ec_deinit(struct crypto_ec *e)
 {
        if (e == NULL)
                return;
-       BN_free(e->order);
+       BN_clear_free(e->order);
        EC_GROUP_free(e->group);
        BN_CTX_free(e->bnctx);
        os_free(e);
@@ -1138,8 +1138,8 @@ int crypto_ec_point_to_bin(struct crypto_ec *e,
                ret = 0;
        }
 
-       BN_free(x_bn);
-       BN_free(y_bn);
+       BN_clear_free(x_bn);
+       BN_clear_free(y_bn);
        return ret;
 }
 
@@ -1155,8 +1155,8 @@ struct crypto_ec_point * crypto_ec_point_from_bin(struct crypto_ec *e,
        y = BN_bin2bn(val + len, len, NULL);
        elem = EC_POINT_new(e->group);
        if (x == NULL || y == NULL || elem == NULL) {
-               BN_free(x);
-               BN_free(y);
+               BN_clear_free(x);
+               BN_clear_free(y);
                EC_POINT_free(elem);
                return NULL;
        }
@@ -1167,8 +1167,8 @@ struct crypto_ec_point * crypto_ec_point_from_bin(struct crypto_ec *e,
                elem = NULL;
        }
 
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(x);
+       BN_clear_free(y);
 
        return (struct crypto_ec_point *) elem;
 }

diff --git a/src/eap_common/eap_pwd_common.c b/src/eap_common/eap_pwd_common.c
index 96c9efd82cce6b3e5660f0e04af43ec4b284c57e..a1a24e3719d049b296e183df27370576c08f3419 100644 (file)
--- a/src/eap_common/eap_pwd_common.c
+++ b/src/eap_common/eap_pwd_common.c
@@ -265,16 +265,16 @@ int compute_password_element(EAP_PWD_group *grp, u16 num,
                grp->group = NULL;
                EC_POINT_free(grp->pwe);
                grp->pwe = NULL;
-               BN_free(grp->order);
+               BN_clear_free(grp->order);
                grp->order = NULL;
-               BN_free(grp->prime);
+               BN_clear_free(grp->prime);
                grp->prime = NULL;
                ret = 1;
        }
        /* cleanliness and order.... */
-       BN_free(cofactor);
-       BN_free(x_candidate);
-       BN_free(rnd);
+       BN_clear_free(cofactor);
+       BN_clear_free(x_candidate);
+       BN_clear_free(rnd);
        os_free(prfbuf);
 
        return ret;

diff --git a/src/eap_peer/eap_pwd.c b/src/eap_peer/eap_pwd.c
index bdcca0b815bbc52ccf9c787e4cc3c24559b286ef..b9eae3773ed0748ceeda293e110949f6450ac5d0 100644 (file)
--- a/src/eap_peer/eap_pwd.c
+++ b/src/eap_peer/eap_pwd.c
@@ -148,10 +148,10 @@ static void eap_pwd_deinit(struct eap_sm *sm, void *priv)
 {
        struct eap_pwd_data *data = priv;
 
-       BN_free(data->private_value);
-       BN_free(data->server_scalar);
-       BN_free(data->my_scalar);
-       BN_free(data->k);
+       BN_clear_free(data->private_value);
+       BN_clear_free(data->server_scalar);
+       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);
@@ -161,8 +161,8 @@ static void eap_pwd_deinit(struct eap_sm *sm, void *priv)
        if (data->grp) {
                EC_GROUP_free(data->grp->group);
                EC_POINT_free(data->grp->pwe);
-               BN_free(data->grp->order);
-               BN_free(data->grp->prime);
+               BN_clear_free(data->grp->order);
+               BN_clear_free(data->grp->prime);
                os_free(data->grp);
        }
        wpabuf_free(data->inbuf);
@@ -336,7 +336,7 @@ eap_pwd_perform_commit_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
                wpa_printf(MSG_INFO, "EAP-PWD (peer): element inversion fail");
                goto fin;
        }
-       BN_free(mask);
+       BN_clear_free(mask);
 
        if (((x = BN_new()) == NULL) ||
            ((y = BN_new()) == NULL)) {
@@ -471,9 +471,9 @@ eap_pwd_perform_commit_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 fin:
        os_free(scalar);
        os_free(element);
-       BN_free(x);
-       BN_free(y);
-       BN_free(cofactor);
+       BN_clear_free(x);
+       BN_clear_free(y);
+       BN_clear_free(cofactor);
        EC_POINT_free(K);
        EC_POINT_free(point);
        if (data->outbuf == NULL)
@@ -681,8 +681,8 @@ eap_pwd_perform_confirm_exchange(struct eap_sm *sm, struct eap_pwd_data *data,
 
 fin:
        os_free(cruft);
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(x);
+       BN_clear_free(y);
        if (data->outbuf == NULL) {
                ret->methodState = METHOD_DONE;
                ret->decision = DECISION_FAIL;

diff --git a/src/eap_server/eap_server_pwd.c b/src/eap_server/eap_server_pwd.c
index 9154ab17b57079678951b2475e594c95fa4f3cf5..e86d3b1119d0c12d73a8207b389b4d4fec43ad87 100644 (file)
--- a/src/eap_server/eap_server_pwd.c
+++ b/src/eap_server/eap_server_pwd.c
@@ -135,10 +135,10 @@ static void eap_pwd_reset(struct eap_sm *sm, void *priv)
 {
        struct eap_pwd_data *data = priv;
 
-       BN_free(data->private_value);
-       BN_free(data->peer_scalar);
-       BN_free(data->my_scalar);
-       BN_free(data->k);
+       BN_clear_free(data->private_value);
+       BN_clear_free(data->peer_scalar);
+       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);
@@ -148,8 +148,8 @@ static void eap_pwd_reset(struct eap_sm *sm, void *priv)
        if (data->grp) {
                EC_GROUP_free(data->grp->group);
                EC_POINT_free(data->grp->pwe);
-               BN_free(data->grp->order);
-               BN_free(data->grp->prime);
+               BN_clear_free(data->grp->order);
+               BN_clear_free(data->grp->prime);
                os_free(data->grp);
        }
        wpabuf_free(data->inbuf);
@@ -230,7 +230,7 @@ static void eap_pwd_build_commit_req(struct eap_sm *sm,
                           "fail");
                goto fin;
        }
-       BN_free(mask);
+       BN_clear_free(mask);
 
        if (((x = BN_new()) == NULL) ||
            ((y = BN_new()) == NULL)) {
@@ -282,8 +282,8 @@ static void eap_pwd_build_commit_req(struct eap_sm *sm,
 fin:
        os_free(scalar);
        os_free(element);
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(x);
+       BN_clear_free(y);
        if (data->outbuf == NULL)
                eap_pwd_state(data, FAILURE);
 }
@@ -407,8 +407,8 @@ static void eap_pwd_build_confirm_req(struct eap_sm *sm,
 
 fin:
        os_free(cruft);
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(x);
+       BN_clear_free(y);
        if (data->outbuf == NULL)
                eap_pwd_state(data, FAILURE);
 }
@@ -726,9 +726,9 @@ eap_pwd_process_commit_resp(struct eap_sm *sm, struct eap_pwd_data *data,
 fin:
        EC_POINT_free(K);
        EC_POINT_free(point);
-       BN_free(cofactor);
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(cofactor);
+       BN_clear_free(x);
+       BN_clear_free(y);
 
        if (res)
                eap_pwd_state(data, PWD_Confirm_Req);
@@ -852,8 +852,8 @@ eap_pwd_process_confirm_resp(struct eap_sm *sm, struct eap_pwd_data *data,
 
 fin:
        os_free(cruft);
-       BN_free(x);
-       BN_free(y);
+       BN_clear_free(x);
+       BN_clear_free(y);
 }