From 6b522f8780813726799e6b8cf0f1f8e0ce2c8ebf Mon Sep 17 00:00:00 2001 From: Mathy Vanhoef 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 typedef struct _eap_pwd_t { - BN_CTX *bnctx; - uint32_t group; uint32_t fragment_size; char const *server_id;