Put the eap sessions into a tree, so that looking them up is
authoraland <aland>
Tue, 31 Aug 2004 19:24:48 +0000 (19:24 +0000)
committeraland <aland>
Tue, 31 Aug 2004 19:24:48 +0000 (19:24 +0000)
very fast, and no longer O(n) in the number of sessions.

src/modules/rlm_eap/eap.c
src/modules/rlm_eap/eap.h
src/modules/rlm_eap/mem.c
src/modules/rlm_eap/rlm_eap.c
src/modules/rlm_eap/rlm_eap.h

index b1c8e37..1b8cf07 100644 (file)
@@ -1056,7 +1056,7 @@ EAP_HANDLER *eap_handler(rlm_eap_t *inst, eap_packet_t **eap_packet_p,
                        radlog(L_ERR, "rlm_eap: Identity Unknown, authentication failed");
                        free(*eap_packet_p);
                        *eap_packet_p = NULL;
-                       eap_handler_free(&handler);
+                       eap_handler_free(handler);
                        return NULL;
                }
 
@@ -1091,7 +1091,7 @@ EAP_HANDLER *eap_handler(rlm_eap_t *inst, eap_packet_t **eap_packet_p,
                                radlog(L_ERR, "rlm_eap: Identity does not match User-Name, setting from EAP Identity.");
                                free(*eap_packet_p);
                                *eap_packet_p = NULL;
-                               eap_handler_free(&handler);
+                               eap_handler_free(handler);
                                return NULL;
                        }
               }
@@ -1101,7 +1101,7 @@ EAP_HANDLER *eap_handler(rlm_eap_t *inst, eap_packet_t **eap_packet_p,
        if (handler->eap_ds == NULL) {
                free(*eap_packet_p);
                *eap_packet_p = NULL;
-               eap_handler_free(&handler);
+               eap_handler_free(handler);
                return NULL;
        }
 
index 6a1c194..2c0c8f9 100644 (file)
@@ -100,8 +100,7 @@ typedef enum operation_t {
  */
 #define EAP_STATE_LEN (AUTH_VECTOR_LEN)
 typedef struct _eap_handler {
-       struct _eap_handler *next;
-
+       struct _eap_handler *prev, *next;
        uint8_t         state[EAP_STATE_LEN];
        uint32_t        src_ipaddr;
        unsigned int    eap_id;
index 177ef15..8d5a186 100644 (file)
@@ -116,14 +116,11 @@ EAP_HANDLER *eap_handler_alloc(void)
        return handler;
 }
 
-void eap_handler_free(EAP_HANDLER **handler_p)
+void eap_handler_free(EAP_HANDLER *handler)
 {
-        EAP_HANDLER *handler;
-
-       if ((handler_p == NULL) || (*handler_p == NULL))
+       if (!handler)
                return;
 
-       handler = *handler_p;
        if (handler->identity) {
                free(handler->identity);
                handler->identity = NULL;
@@ -141,10 +138,8 @@ void eap_handler_free(EAP_HANDLER **handler_p)
 
        handler->opaque = NULL;
        handler->free_opaque = NULL;
-       handler->next = NULL;
 
        free(handler);
-       *handler_p = NULL;
 }
 
 void eaptype_free(EAP_TYPES *i)
@@ -154,28 +149,17 @@ void eaptype_free(EAP_TYPES *i)
        if (i->handle) lt_dlclose(i->handle);
 }
 
+
 void eaplist_free(rlm_eap_t *inst)
 {
-       int i;
-
-       /*
-        *      The sessions are split out into an array, which makes
-        *      looking them up a bit faster.
-        */
-       for (i = 0; i < 256; i++) {
-               EAP_HANDLER *node, *next;
+       EAP_HANDLER *node, *next;
 
-               if (inst->sessions[i]) continue;
-
-               node = inst->sessions[i];
-               while (node) {
-                       next = node->next;
-                       eap_handler_free(&node);
-                       node = next;
-               }
-
-               inst->sessions[i] = NULL;
+               for (node = inst->session_head; node != NULL; node = next) {
+               next = node->next;
+               eap_handler_free(node);
        }
+
+       inst->session_head = inst->session_tail = NULL;
 }
 
 /*
@@ -186,9 +170,9 @@ void eaplist_free(rlm_eap_t *inst)
  */
 int eaplist_add(rlm_eap_t *inst, EAP_HANDLER *handler)
 {
-       EAP_HANDLER     **last;
+       int             status;
        VALUE_PAIR      *state;
-
+       
        rad_assert(handler != NULL);
        rad_assert(handler->request != NULL);
 
@@ -205,50 +189,56 @@ int eaplist_add(rlm_eap_t *inst, EAP_HANDLER *handler)
         */
        rad_assert(state->length == EAP_STATE_LEN);
 
+       /*
+        *      The time at which this request was made was the time
+        *      at which it was received by the RADIUS server.
+        */
+       handler->timestamp = handler->request->timestamp;
+       handler->status = 1;
+
        memcpy(handler->state, state->strvalue, sizeof(handler->state));
        handler->src_ipaddr = handler->request->packet->src_ipaddr;
        handler->eap_id = handler->eap_ds->request->id;
 
-#ifdef HAVE_PTHREAD_H
+       /*
+        *      We don't need this any more.
+        */
+       handler->request = NULL;
+
        /*
         *      Playing with a data structure shared among threads
         *      means that we need a lock, to avoid conflict.
         */
        pthread_mutex_lock(&(inst->session_mutex));
-#endif
 
        /*
-        *      We key the array based on the challenge, which is
-        *      a random number.  This "fans out" the sessions, and
-        *      helps to minimize the amount of work we've got to do
-        *      under heavy load.
+        *      Big-time failure.
         */
-       last = &(inst->sessions[state->strvalue[0]]);
+       status = rbtree_insert(inst->session_tree, handler);
 
-       while (*last) last = &((*last)->next);
+       if (status) {
+               EAP_HANDLER *prev;
 
-       *last = handler;
-
-       /*
-        *      The time at which this request was made was the time
-        *      at which it was received by the RADIUS server.
-        */
-       handler->timestamp = handler->request->timestamp;
-       handler->status = 1;
-       handler->next = NULL;
+               prev = inst->session_tail;
+               if (prev) {
+                       prev->next = handler;
+                       handler->prev = prev;
+               } else {
+                       inst->session_head = inst->session_tail = handler;
+               }
+       }
 
-#ifdef HAVE_PTHREAD_H
        /*
         *      Now that we've finished mucking with the list,
         *      unlock it.
         */
        pthread_mutex_unlock(&(inst->session_mutex));
-#endif
 
-       /*
-        *      We don't need this any more.
-        */
-       handler->request = NULL;
+       if (!status) {
+               radlog(L_ERR, "rlm_eap: Failed to remember handler!");
+               eap_handler_free(handler);
+               return 0;
+       }
 
        return 1;
 }
@@ -266,9 +256,10 @@ int eaplist_add(rlm_eap_t *inst, EAP_HANDLER *handler)
 EAP_HANDLER *eaplist_find(rlm_eap_t *inst, REQUEST *request,
                          eap_packet_t *eap_packet)
 {
-       EAP_HANDLER     *node, *next;
+       int             i;
        VALUE_PAIR      *state;
-       EAP_HANDLER     **first,  **last;
+       rbnode_t        *node;
+       EAP_HANDLER     *handler, myHandler;
 
        /*
         *      We key the sessions off of the 'state' attribute, so it
@@ -280,91 +271,103 @@ EAP_HANDLER *eaplist_find(rlm_eap_t *inst, REQUEST *request,
                return NULL;
        }
 
-#ifdef HAVE_PTHREAD_H
+       myHandler.src_ipaddr = request->packet->src_ipaddr;
+       myHandler.eap_id = eap_packet->id;
+       memcpy(myHandler.state, state->strvalue, sizeof(myHandler.state));
+
        /*
         *      Playing with a data structure shared among threads
         *      means that we need a lock, to avoid conflict.
         */
        pthread_mutex_lock(&(inst->session_mutex));
-#endif
-
-       last = first = &(inst->sessions[state->strvalue[0]]);
-
-       for (node = *first; node; node = next) {
-               next = node->next;
 
-               /*
-                *      If the time on this entry has expired,
-                *      delete it.  We do this while walking the list,
-                *      in order to spread out the work of deleting old
-                *      sessions.
-                */
-               if ((request->timestamp - node->timestamp) > inst->timer_limit) {
-                       *last = next;
-                       eap_handler_free(&node);
-                       continue;
+       /*
+        *      Check the first few handlers in the list, and delete
+        *      them if they're too old.  We don't need to check them
+        *      all, as incoming requests will quickly cause older
+        *      handlers to be deleted.
+        *
+        */
+       for (i = 0; i < 2; i++) {
+               handler = inst->session_head;
+               if (handler &&
+                   ((request->timestamp - handler->timestamp) > inst->timer_limit)) {
+                       node = rbtree_find(inst->session_tree, handler);
+                       rad_assert(node != NULL);
+                       rbtree_delete(inst->session_tree, node);
+                       
+                       inst->session_head = handler->next;
+                       if (handler->next) handler->next->prev = NULL;
+                       eap_handler_free(handler);
                }
+       }
+
+       handler = NULL;
+       node = rbtree_find(inst->session_tree, &myHandler);
+       if (node) {
+               handler = rbtree_node2data(inst->session_tree, node);
 
                /*
-                *      Find the previous part of the same conversation,
-                *      keying off of the EAP ID, the client IP, and
-                *      the State attribute.
+                *      Check against replays.  The client can re-play
+                *      a State attribute verbatim, so we wish to
+                *      ensure that the attribute falls within the
+                *      valid time window, which is the second at
+                *      which it was sent out.
                 *
-                *      If we've found a conversation, then we don't
-                *      have to check entries later in the list for
-                *      timeout, as they're guaranteed to be newer than
-                *      the one we found.
+                *      Hmm... I'm not sure that this step is
+                *      necessary, or even that it does anything.
                 */
-               if ((node->eap_id == eap_packet->id) &&
-                   (node->src_ipaddr == request->packet->src_ipaddr) &&
-                   (memcmp(node->state, state->strvalue, state->length) == 0)) {
-                       /*
-                        *      Check against replays.  The client can
-                        *      re-play a State attribute verbatim, so
-                        *      we wish to ensure that the attribute falls
-                        *      within the valid time window, which is
-                        *      the second at which it was sent out.
-                        */
-                       if (verify_state(state, node->timestamp) != 0) {
-                               radlog(L_ERR, "rlm_eap: State verification failed.");
-                               node = NULL;
-                               break;
-                       }
-
-                       DEBUG2("  rlm_eap: Request found, released from the list");
-                       /*
-                        *      detach the node from the list
-                        */
-                       *last = next;
-                       node->next = NULL;
-
+               if (verify_state(state, handler->timestamp) != 0) {
+                       handler = NULL;
+               } else {
                        /*
-                        *      Don't bother updating handler->request, etc.
-                        *      eap_handler() will do that for us.
+                        *      It's OK, delete it from the tree.
                         */
+                       rbtree_delete(inst->session_tree, node);
 
                        /*
-                        *      Remember what the previous request was.
+                        *      And unsplice it from the linked list.
                         */
-                       eap_ds_free(&(node->prev_eapds));
-                       node->prev_eapds = node->eap_ds;
-                       node->eap_ds = NULL;
-
-                       /*
-                        *      Stop here.
-                        */
-                       break;
-               } else  {
-                       last = &(node->next);
+                       if (handler->prev) {
+                               handler->prev->next = handler->next;
+                       } else {
+                               inst->session_head = NULL;
+                       }
+                       if (handler->next) {
+                               handler->next->prev = handler->prev;
+                       } else {
+                               inst->session_tail = NULL;
+                       }
+                       handler->prev = handler->next = NULL;
                }
        }
 
-#ifdef HAVE_PTHREAD_H
        pthread_mutex_unlock(&(inst->session_mutex));
-#endif
 
+       /*
+        *      Not found.
+        */
        if (!node) {
                DEBUG2("  rlm_eap: Request not found in the list");
+               return NULL;
+       }
+
+       /*
+        *      Found, but state verification failed.
+        */
+       if (!handler) {
+               radlog(L_ERR, "rlm_eap: State verification failed.");
+               return NULL;
        }
-       return node;
+
+       DEBUG2("  rlm_eap: Request found, released from the list");
+
+       /*
+        *      Remember what the previous request was.
+        */
+       eap_ds_free(&(handler->prev_eapds));
+       handler->prev_eapds = handler->eap_ds;
+       handler->eap_ds = NULL;
+       
+       return handler;
 }
index 5841f5c..18ce496 100644 (file)
@@ -57,6 +57,8 @@ static int eap_detach(void *instance)
 
        inst = (rlm_eap_t *)instance;
 
+       rbtree_free(inst->session_tree);
+       inst->session_tree = NULL;
        eaplist_free(inst);
 
        for (i = 0; i < PW_EAP_MAX_TYPES; i++) {
@@ -64,10 +66,8 @@ static int eap_detach(void *instance)
                inst->types[i] = NULL;
        }
 
-#ifdef HAVE_PTHREAD_H
        pthread_mutex_destroy(&(inst->session_mutex));
        pthread_mutex_destroy(&(inst->module_mutex));
-#endif
 
        if (inst->default_eap_type_name) free(inst->default_eap_type_name);
        free(inst);
@@ -77,6 +77,24 @@ static int eap_detach(void *instance)
 
 
 /*
+ *     Compare two handlers.
+ */
+static int eap_handler_cmp(const void *a, const void *b)
+{
+       const EAP_HANDLER *one = a;
+       const EAP_HANDLER *two = b;
+
+       if (one->src_ipaddr < two->src_ipaddr) return -1;
+       if (one->src_ipaddr > two->src_ipaddr) return +1;
+
+       if (one->eap_id < two->eap_id) return -1;
+       if (one->eap_id > two->eap_id) return +1;
+
+       return memcmp(one->state, two->state, sizeof(one->state));
+}
+
+
+/*
  * read the config section and load all the eap authentication types present.
  */
 static int eap_instantiate(CONF_SECTION *cs, void **instance)
@@ -172,24 +190,24 @@ static int eap_instantiate(CONF_SECTION *cs, void **instance)
        /* Generate a state key, specific to eap */
        generate_key();
 
-#ifdef HAVE_PTHREAD_H
+       /*
+        *      Lookup sessions in the tree.  We don't free them in
+        *      the tree, as that's taken care of elsewhere...
+        */
+       inst->session_tree = rbtree_create(eap_handler_cmp, NULL, 0);
+       if (!inst->session_tree) {
+               radlog(L_ERR|L_CONS, "rlm_eap: Cannot initialize tree");
+               eap_detach(inst);
+               return -1;
+       }
+
        pthread_mutex_init(&(inst->session_mutex), NULL);
        pthread_mutex_init(&(inst->module_mutex), NULL);
-#endif
 
        *instance = inst;
        return 0;
 }
 
-/*
- *     Dumb wrapper.
- *     FIXME: this should be done more intelligently...
- */
-static void my_handler_free(void *data)
-{
-  EAP_HANDLER *handler = (EAP_HANDLER *) data;
-  eap_handler_free(&handler);
-}
 
 /*
  *     For backwards compatibility.
@@ -237,7 +255,7 @@ static int eap_authenticate(void *instance, REQUEST *request)
                case PW_EAP_PEAP:
                        DEBUG2(" rlm_eap: Unable to tunnel TLS inside of TLS");
                        eap_fail(handler);
-                       eap_handler_free(&handler);
+                       eap_handler_free(handler);
                        return RLM_MODULE_INVALID;
                        break;
 
@@ -279,7 +297,7 @@ static int eap_authenticate(void *instance, REQUEST *request)
         */
        if (rcode == EAP_INVALID) {
                eap_fail(handler);
-               eap_handler_free(&handler);
+               eap_handler_free(handler);
                DEBUG2("  rlm_eap: Failed in EAP select");
                return RLM_MODULE_INVALID;
        }
@@ -296,7 +314,7 @@ static int eap_authenticate(void *instance, REQUEST *request)
                 */
                rcode = request_data_add(request,
                                         inst, REQUEST_DATA_EAP_HANDLER,
-                                        handler, my_handler_free);
+                                        handler, eap_handler_free);
                rad_assert(rcode == 0);
 
                return RLM_MODULE_HANDLED;
@@ -319,7 +337,7 @@ static int eap_authenticate(void *instance, REQUEST *request)
                 */
                rcode = request_data_add(request,
                                         inst, REQUEST_DATA_EAP_HANDLER,
-                                        handler, my_handler_free);
+                                        handler, eap_handler_free);
                rad_assert(rcode == 0);
 
                /*
@@ -381,7 +399,7 @@ static int eap_authenticate(void *instance, REQUEST *request)
        } else {
                DEBUG2("  rlm_eap: Freeing handler");
                /* handler is not required any more, free it now */
-               eap_handler_free(&handler);
+               eap_handler_free(handler);
        }
 
        /*
@@ -510,7 +528,7 @@ static int eap_post_proxy(void *inst, REQUEST *request)
                                                              REQUEST_DATA_EAP_TUNNEL_CALLBACK);
                if (!data) {
                        radlog(L_ERR, "rlm_eap: Failed to retrieve callback for tunneled session!");
-                       eap_handler_free(&handler);
+                       eap_handler_free(handler);
                        return RLM_MODULE_FAIL;
                }
 
@@ -520,7 +538,7 @@ static int eap_post_proxy(void *inst, REQUEST *request)
                rcode = data->callback(handler, data->tls_session);
                free(data);
                if (rcode == 0) {
-                       eap_handler_free(&handler);
+                       eap_handler_free(handler);
                        return RLM_MODULE_REJECT;
                }
 
@@ -541,7 +559,7 @@ static int eap_post_proxy(void *inst, REQUEST *request)
                } else {        /* couldn't have been LEAP, there's no tunnel */
                        DEBUG2("  rlm_eap: Freeing handler");
                        /* handler is not required any more, free it now */
-                       eap_handler_free(&handler);
+                       eap_handler_free(handler);
                }
 
                /*
index 07ae8f0..1ae8b23 100644 (file)
@@ -40,21 +40,20 @@ typedef struct eap_types_t {
 
 /*
  * This structure contains eap's persistent data.
- * sessions[] = EAP_HANDLERS, keyed by the first octet of the State
- *              attribute, and composed of a linked list, ordered from
- *              oldest to newest.
+ * sessions = remembered sessions, in a tree for speed.
  * types = All supported EAP-Types
  * mutex = ensure only one thread is updating the sessions[] struct
  */
 typedef struct rlm_eap_t {
-       EAP_HANDLER     *sessions[256];
+       rbtree_t        *session_tree;
+       EAP_HANDLER     *session_head, *session_tail;
        EAP_TYPES       *types[PW_EAP_MAX_TYPES + 1];
 
        /*
         *      Configuration items.
         */
-       char            *default_eap_type_name;
        int             timer_limit;
+       char            *default_eap_type_name;
        int             default_eap_type;
        int             ignore_unknown_eap_types;
        int             cisco_accounting_username_bug;
@@ -65,6 +64,19 @@ typedef struct rlm_eap_t {
 #endif
 } rlm_eap_t;
 
+/*
+ *     For simplicity in the rest of the code.
+ */
+#ifndef HAVE_PTHREAD_H
+/*
+ *     This is easier than ifdef's throughout the code.
+ */
+#define pthread_mutex_init(_x, _y)
+#define pthread_mutex_destroy(_x)
+#define pthread_mutex_lock(_x)
+#define pthread_mutex_unlock(_x)
+#endif
+
 /* function definitions */
 /* EAP-Type */
 int            eaptype_load(EAP_TYPES **type, int eap_type, CONF_SECTION *cs);
@@ -84,12 +96,12 @@ EAP_DS              *eap_ds_alloc(void);
 EAP_HANDLER    *eap_handler_alloc(void);
 void           eap_packet_free(EAP_PACKET **eap_packet);
 void           eap_ds_free(EAP_DS **eap_ds);
-void           eap_handler_free(EAP_HANDLER **handler);
+void           eap_handler_free(EAP_HANDLER *handler);
 
 int            eaplist_add(rlm_eap_t *inst, EAP_HANDLER *handler);
-void           eaplist_free(rlm_eap_t *inst);
 EAP_HANDLER    *eaplist_find(rlm_eap_t *inst, REQUEST *request,
                              eap_packet_t *eap_packet);
+void           eaplist_free(rlm_eap_t *inst);
 
 /* State */
 void           generate_key(void);