135 lines
4.7 KiB
Diff
135 lines
4.7 KiB
Diff
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 013a6e79926..ca12778f612 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 76cc57023e2..eefca985d78 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 189530d0667..2264566bb64 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;
|