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;