Allocate SSL session ctx memory under the handler to avoid thread safety issues
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Thu, 19 Jun 2014 18:24:02 +0000 (19:24 +0100)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Thu, 19 Jun 2014 18:24:02 +0000 (19:24 +0100)
src/include/tls-h
src/main/listen.c
src/main/tls.c
src/main/tls_listen.c
src/modules/rlm_eap/libeap/eap_tls.c
src/modules/rlm_eap/libeap/eap_tls.h
src/modules/rlm_eap/types/rlm_eap_peap/rlm_eap_peap.c
src/modules/rlm_eap/types/rlm_eap_tls/rlm_eap_tls.c
src/modules/rlm_eap/types/rlm_eap_ttls/rlm_eap_ttls.c

index 10a8104..0ba9b63 100644 (file)
@@ -297,8 +297,7 @@ int         cbtls_verify(int ok, X509_STORE_CTX *ctx);
 void           tls_global_init(void);
 int            tls_global_version_check(char const *acknowledged);
 void           tls_global_cleanup(void);
-tls_session_t  *tls_new_session(fr_tls_server_conf_t *conf, REQUEST *request,
-                              int client_cert);
+tls_session_t  *tls_new_session(TALLOC_CTX *ctx, fr_tls_server_conf_t *conf, REQUEST *request, int client_cert);
 tls_session_t  *tls_new_client_session(fr_tls_server_conf_t *conf, int fd);
 fr_tls_server_conf_t *tls_server_conf_parse(CONF_SECTION *cs);
 fr_tls_server_conf_t *tls_client_conf_parse(CONF_SECTION *cs);
@@ -315,7 +314,6 @@ fr_tls_status_t tls_ack_handler(tls_session_t *tls_session, REQUEST *request);
 fr_tls_status_t tls_application_data(tls_session_t *ssn, REQUEST *request);
 
 /* Session */
-void           session_free(void *ssn);
 void           session_close(tls_session_t *ssn);
 void           session_init(tls_session_t *ssn);
 
index 492eb17..3b1ec30 100644 (file)
@@ -2542,7 +2542,7 @@ static int listener_free(void *ctx)
                 *      may be used by multiple listeners.
                 */
                if (this->tls) {
-                       if (sock->ssn) session_free(sock->ssn);
+                       TALLOC_FREE(sock->ssn);
                        TALLOC_FREE(sock->request);
 #ifdef HAVE_PTHREAD_H
                        pthread_mutex_destroy(&(sock->mutex));
index 50fe77a..426ff82 100644 (file)
@@ -182,8 +182,22 @@ tls_session_t *tls_new_client_session(fr_tls_server_conf_t *conf, int fd)
        return ssn;
 }
 
-tls_session_t *tls_new_session(fr_tls_server_conf_t *conf, REQUEST *request,
-                              int client_cert)
+static int _tls_session_free(tls_session_t *ssn)
+{
+       /*
+        *      Free any opaque TTLS or PEAP data.
+        */
+       if ((ssn->opaque) && (ssn->free_opaque)) {
+               ssn->free_opaque(ssn->opaque);
+               ssn->opaque = NULL;
+       }
+
+       session_close(ssn);
+
+       return 0;
+}
+
+tls_session_t *tls_new_session(TALLOC_CTX *ctx, fr_tls_server_conf_t *conf, REQUEST *request, int client_cert)
 {
        tls_session_t *state = NULL;
        SSL *new_tls = NULL;
@@ -217,8 +231,9 @@ tls_session_t *tls_new_session(fr_tls_server_conf_t *conf, REQUEST *request,
        /* We use the SSL's "app_data" to indicate a call-back */
        SSL_set_app_data(new_tls, NULL);
 
-       state = talloc_zero(conf, tls_session_t);
+       state = talloc_zero(ctx, tls_session_t);
        session_init(state);
+       talloc_set_destructor(state, _tls_session_free);
 
        state->ctx = conf->ctx;
        state->ssl = new_tls;
@@ -520,25 +535,6 @@ void session_close(tls_session_t *ssn)
        session_init(ssn);
 }
 
-void session_free(void *ssn)
-{
-       tls_session_t *sess = (tls_session_t *)ssn;
-
-       if (!ssn) return;
-
-       /*
-        *      Free any opaque TTLS or PEAP data.
-        */
-       if ((sess->opaque) && (sess->free_opaque)) {
-               sess->free_opaque(sess->opaque);
-               sess->opaque = NULL;
-       }
-
-       session_close(sess);
-
-       talloc_free(sess);
-}
-
 static void record_init(record_t *rec)
 {
        rec->used = 0;
index 1d64be5..6ed0330 100644 (file)
@@ -170,7 +170,7 @@ static int tls_socket_recv(rad_listen_t *listener)
 
                rad_assert(sock->ssn == NULL);
 
-               sock->ssn = tls_new_session(listener->tls, sock->request,
+               sock->ssn = tls_new_session(listener->tls, listener->tls, sock->request,
                                            listener->tls->require_client_cert);
                if (!sock->ssn) {
                        TALLOC_FREE(sock->request);
index 8b1ade7..26ec595 100644 (file)
@@ -44,7 +44,6 @@ USES_APPLE_DEPRECATED_API     /* OpenSSL API has been deprecated by Apple */
 #include <assert.h>
 
 #include "eap_tls.h"
-
 /*
  *     Send an initial eap-tls request to the peer.
  *
@@ -62,7 +61,7 @@ USES_APPLE_DEPRECATED_API     /* OpenSSL API has been deprecated by Apple */
  *
  *     Fragment length is Framed-MTU - 4.
  */
-tls_session_t *eaptls_session(fr_tls_server_conf_t *tls_conf, eap_handler_t *handler, int client_cert)
+tls_session_t *eaptls_session(eap_handler_t *handler, fr_tls_server_conf_t *tls_conf, bool client_cert)
 {
        tls_session_t   *ssn;
        int             verify_mode = 0;
@@ -78,7 +77,7 @@ tls_session_t *eaptls_session(fr_tls_server_conf_t *tls_conf, eap_handler_t *han
         *      in Opaque.  So that we can use these data structures
         *      when we get the response
         */
-       ssn = tls_new_session(tls_conf, request, client_cert);
+       ssn = tls_new_session(handler, tls_conf, request, client_cert);
        if (!ssn) {
                return NULL;
        }
index 4dbd69e..68fc5c2 100644 (file)
@@ -98,7 +98,7 @@ typedef struct tls_packet {
 /* EAP-TLS framework */
 EAPTLS_PACKET  *eaptls_alloc(void);
 void           eaptls_free(EAPTLS_PACKET **eaptls_packet_ptr);
-tls_session_t  *eaptls_session(fr_tls_server_conf_t *tls_conf, eap_handler_t *handler, int client_cert);
+tls_session_t  *eaptls_session(eap_handler_t *handler, fr_tls_server_conf_t *tls_conf, bool client_cert);
 int            eaptls_start(EAP_DS *eap_ds, int peap);
 int            eaptls_compose(EAP_DS *eap_ds, EAPTLS_PACKET *reply);
 
index 4793ee3..97973ea 100644 (file)
@@ -187,13 +187,12 @@ static int eappeap_initiate(void *type_arg, eap_handler_t *handler)
                client_cert = inst->req_client_cert;
        }
 
-       ssn = eaptls_session(inst->tls_conf, handler, client_cert);
+       ssn = eaptls_session(handler, inst->tls_conf, client_cert);
        if (!ssn) {
                return 0;
        }
 
        handler->opaque = ((void *)ssn);
-       handler->free_opaque = session_free;
 
        /*
         *      Set up type-specific information.
index 6d960f7..0f49eec 100644 (file)
@@ -95,13 +95,12 @@ static int eaptls_initiate(void *type_arg, eap_handler_t *handler)
        /*
         *      EAP-TLS always requires a client certificate.
         */
-       ssn = eaptls_session(inst->tls_conf, handler, true);
+       ssn = eaptls_session(handler, inst->tls_conf, true);
        if (!ssn) {
                return 0;
        }
 
        handler->opaque = ((void *)ssn);
-       handler->free_opaque = session_free;
 
        /*
         *      Set up type-specific information.
index 0c499e5..4a3e017 100644 (file)
@@ -156,8 +156,7 @@ static void ttls_free(void *p)
 /*
  *     Allocate the TTLS per-session data
  */
-static ttls_tunnel_t *ttls_alloc(rlm_eap_ttls_t *inst,
-                                eap_handler_t *handler)
+static ttls_tunnel_t *ttls_alloc(rlm_eap_ttls_t *inst, eap_handler_t *handler)
 {
        ttls_tunnel_t *t;
 
@@ -203,13 +202,12 @@ static int eapttls_initiate(void *type_arg, eap_handler_t *handler)
                client_cert = inst->req_client_cert;
        }
 
-       ssn = eaptls_session(inst->tls_conf, handler, client_cert);
+       ssn = eaptls_session(handler, inst->tls_conf, client_cert);
        if (!ssn) {
                return 0;
        }
 
        handler->opaque = ((void *)ssn);
-       handler->free_opaque = session_free;
 
        /*
         *      Set up type-specific information.