From 6b522f8780813726799e6b8cf0f1f8e0ce2c8ebf Mon Sep 17 00:00:00 2001

From: Mathy Vanhoef <Mathy.Vanhoef@nyu.edu>

Date: Fri, 4 Oct 2019 17:53:52 +0400

Subject: [PATCH] EAP-pwd: fix DoS due to multithreaded BN_CTX access

The EAP-pwd module created one global OpenSSL BN_CTX instance, and

used this instance in all incoming requests. This means that different

threads used the same BN_CTX instance, which can result in a crash.

An adversary can trigger these crashes by concurrently initiating

multiple EAP-pwd handshakes from different clients.

Fix this bug by creating a separate BN_CTX instance for each request.

---

 .../rlm_eap/types/rlm_eap_pwd/eap_pwd.h       |  1 +

 .../rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.c   | 24 +++++++++----------

 .../rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.h   |  2 --

 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/src/modules/rlm_eap/types/rlm_eap_pwd/eap_pwd.h b/src/modules/rlm_eap/types/rlm_eap_pwd/eap_pwd.h

index 013a6e7992..ca12778f61 100644

--- a/src/modules/rlm_eap/types/rlm_eap_pwd/eap_pwd.h

+++ b/src/modules/rlm_eap/types/rlm_eap_pwd/eap_pwd.h

@@ -90,6 +90,7 @@ typedef struct _pwd_session_t {

     uint8_t *out;     /* message to fragment */

     size_t out_pos;

     size_t out_len;

+    BN_CTX *bnctx;

     EC_GROUP *group;

     EC_POINT *pwe;

     BIGNUM *order;

diff --git a/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.c b/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.c

index 76cc57023e..eefca985d7 100644

--- a/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.c

+++ b/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.c

@@ -55,8 +55,6 @@ static int mod_detach (void *arg)

 	inst = (eap_pwd_t *) arg;

-	if (inst->bnctx) BN_CTX_free(inst->bnctx);

-

 	return 0;

 }

@@ -76,11 +74,6 @@ static int mod_instantiate (CONF_SECTION *cs, void **instance)

 		return -1;

 	}

-	if ((inst->bnctx = BN_CTX_new()) == NULL) {

-		cf_log_err_cs(cs, "Failed to get BN context");

-		return -1;

-	}

-

 	return 0;

 }

@@ -96,6 +89,7 @@ static int _free_pwd_session (pwd_session_t *session)

 	EC_POINT_clear_free(session->pwe);

 	BN_clear_free(session->order);

 	BN_clear_free(session->prime);

+	BN_CTX_free(session->bnctx);

 	return 0;

 }

@@ -217,6 +211,12 @@ static int mod_session_init (void *instance, eap_handler_t *handler)

 	session->order = NULL;

 	session->prime = NULL;

+	session->bnctx = BN_CTX_new();

+	if (session->bnctx == NULL) {

+		ERROR("rlm_eap_pwd: Failed to get BN context");

+		return 0;

+	}

+

 	/*

 	 *	The admin can dynamically change the MTU.

 	 */

@@ -496,7 +496,7 @@ static int mod_process(void *arg, eap_handler_t *handler)

 		/*

 		 * compute our scalar and element

 		 */

-		if (compute_scalar_element(session, inst->bnctx)) {

+		if (compute_scalar_element(session, session->bnctx)) {

 			DEBUG2("failed to compute server's scalar and element");

 			return 0;

 		}

@@ -508,7 +508,7 @@ static int mod_process(void *arg, eap_handler_t *handler)

 		 * element is a point, get both coordinates: x and y

 		 */

 		if (!EC_POINT_get_affine_coordinates_GFp(session->group, session->my_element, x, y,

-							 inst->bnctx)) {

+							 session->bnctx)) {

 			DEBUG2("server point assignment failed");

 			BN_clear_free(x);

 			BN_clear_free(y);

@@ -552,7 +552,7 @@ static int mod_process(void *arg, eap_handler_t *handler)

 		/*

 		 * process the peer's commit and generate the shared key, k

 		 */

-		if (process_peer_commit(session, in, in_len, inst->bnctx)) {

+		if (process_peer_commit(session, in, in_len, session->bnctx)) {

 			RDEBUG2("failed to process peer's commit");

 			return 0;

 		}

@@ -560,7 +560,7 @@ static int mod_process(void *arg, eap_handler_t *handler)

 		/*

 		 * compute our confirm blob

 		 */

-		if (compute_server_confirm(session, session->my_confirm, inst->bnctx)) {

+		if (compute_server_confirm(session, session->my_confirm, session->bnctx)) {

 			ERROR("rlm_eap_pwd: failed to compute confirm!");

 			return 0;

 		}

@@ -591,7 +591,7 @@ static int mod_process(void *arg, eap_handler_t *handler)

 			RDEBUG2("pwd exchange is incorrect: not commit!");

 			return 0;

 		}

-		if (compute_peer_confirm(session, peer_confirm, inst->bnctx)) {

+		if (compute_peer_confirm(session, peer_confirm, session->bnctx)) {

 			RDEBUG2("pwd exchange cannot compute peer's confirm");

 			return 0;

 		}

diff --git a/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.h b/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.h

index 189530d066..2264566bb6 100644

--- a/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.h

+++ b/src/modules/rlm_eap/types/rlm_eap_pwd/rlm_eap_pwd.h

@@ -40,8 +40,6 @@

 #include <freeradius-devel/modules.h>

 typedef struct _eap_pwd_t {

-    BN_CTX *bnctx;

-

     uint32_t	group;

     uint32_t	fragment_size;

     char const	*server_id;