Call finish_select_query if we experience an error retrieving the result
[freeradius.git] / src / modules / rlm_sql / rlm_sql.c
index 8804a77..c703658 100644 (file)
@@ -44,7 +44,7 @@ RCSID("$Id$")
 static const CONF_PARSER query_config[] = {
        { "query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_MULTI, rlm_sql_config_t, accounting.query), NULL },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -58,7 +58,7 @@ static const CONF_PARSER type_config[] = {
        { "interim-update", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
        { "stop", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) query_config },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 static const CONF_PARSER acct_config[] = {
@@ -67,7 +67,7 @@ static const CONF_PARSER acct_config[] = {
 
        { "type", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) type_config },
 
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 static const CONF_PARSER postauth_config[] = {
@@ -75,13 +75,12 @@ static const CONF_PARSER postauth_config[] = {
        { "logfile", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT, rlm_sql_config_t, postauth.logfile), NULL },
 
        { "query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_MULTI, rlm_sql_config_t, postauth.query), NULL },
-
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 static const CONF_PARSER module_config[] = {
        { "driver", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_driver_name), "rlm_sql_null" },
-       { "server", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_server), "localhost" },
+       { "server", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_server), "" }, /* Must be zero length so drivers can determine if it was set */
        { "port", FR_CONF_OFFSET(PW_TYPE_INTEGER, rlm_sql_config_t, sql_port), "0" },
        { "login", FR_CONF_OFFSET(PW_TYPE_STRING, rlm_sql_config_t, sql_login), "" },
        { "password", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_SECRET, rlm_sql_config_t, sql_password), "" },
@@ -120,8 +119,7 @@ static const CONF_PARSER module_config[] = {
        { "accounting", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) acct_config },
 
        { "post-auth", FR_CONF_POINTER(PW_TYPE_SUBSECTION, NULL), (void const *) postauth_config },
-
-       {NULL, -1, 0, NULL, NULL}
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -191,6 +189,7 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
                numaffected = (inst->module->sql_affected_rows)(handle, inst->config);
                if (numaffected < 1) {
                        RDEBUG("SQL query affected no rows");
+                       (inst->driver->sql_finish_query)(handle, inst->config);
 
                        goto finish;
                }
@@ -228,7 +227,7 @@ static ssize_t sql_xlat(void *instance, REQUEST *request, char const *query, cha
 
        rcode = rlm_sql_fetch_row(inst, request, &handle);
        if (rcode) {
-               (inst->module->sql_finish_select_query)(handle, inst->config);
+               (inst->driver->sql_finish_select_query)(handle, inst->config);
                goto query_error;
        }
 
@@ -288,9 +287,17 @@ static int generate_sql_clients(rlm_sql_t *inst)
        if (rlm_sql_select_query(inst, NULL, &handle, inst->config->client_query) != RLM_SQL_OK) return -1;
 
        while ((rlm_sql_fetch_row(inst, NULL, &handle) == 0) && (row = handle->row)) {
+               int num_rows;
                char *server = NULL;
+
                i++;
 
+               num_rows = (inst->module->sql_num_fields)(handle, inst->config);
+               if (num_rows < 5) {
+                       WARN("SELECT returned too few rows.  Please do not edit 'client_query'");
+                       continue;
+               }
+
                /*
                 *  The return data for each row MUST be in the following order:
                 *
@@ -318,7 +325,7 @@ static int generate_sql_clients(rlm_sql_t *inst)
                        continue;
                }
 
-               if (((inst->module->sql_num_fields)(handle, inst->config) > 5) && (row[5] != NULL) && *row[5]) {
+               if ((num_rows > 5) && (row[5] != NULL) && *row[5]) {
                        server = row[5];
                }
 
@@ -371,7 +378,7 @@ static size_t sql_escape_func(UNUSED REQUEST *request, char *out, size_t outlen,
                /*
                 *      Allow all multi-byte UTF8 characters.
                 */
-               utf8_len = fr_utf8_char((uint8_t const *) in);
+               utf8_len = fr_utf8_char((uint8_t const *) in, -1);
                if (utf8_len > 1) {
                        if (outlen <= utf8_len) break;
 
@@ -504,7 +511,12 @@ int sql_set_user(rlm_sql_t *inst, REQUEST *request, char const *username)
        fr_pair_value_strsteal(vp, expanded);
        RDEBUG2("SQL-User-Name set to '%s'", vp->vp_strvalue);
        vp->op = T_OP_SET;
-       radius_pairmove(request, &request->packet->vps, vp, false);     /* needs to be pair move else op is not respected */
+
+       /*
+        *      Delete any existing SQL-User-Name, and replace it with ours.
+        */
+       fr_pair_delete_by_num(&request->packet->vps, vp->da->attr, vp->da->vendor, TAG_ANY);
+       fr_pair_add(&request->packet->vps, vp);
 
        return 0;
 }
@@ -584,6 +596,14 @@ static int sql_groupcmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *req
        rlm_sql_t *inst = instance;
        rlm_sql_grouplist_t *head, *entry;
 
+       /*
+        *      No group queries, don't do group comparisons.
+        */
+       if (!inst->config->groupmemb_query) {
+               RWARN("Cannot do group comparison when group_membership_query is not set");
+               return 1;
+       }
+
        RDEBUG("sql_groupcmp");
 
        if (check->vp_length == 0){
@@ -645,6 +665,19 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
 
        rad_assert(request->packet != NULL);
 
+       if (!inst->config->groupmemb_query) {
+               RWARN("Cannot do check groups when group_membership_query is not set");
+
+       do_nothing:
+               *do_fall_through = FALL_THROUGH_DEFAULT;
+
+               /*
+                *      Didn't add group attributes or allocate
+                *      memory, so don't do anything else.
+                */
+               return RLM_MODULE_NOTFOUND;
+       }
+
        /*
         *      Get the list of groups this user is a member of
         */
@@ -656,10 +689,7 @@ static rlm_rcode_t rlm_sql_process_groups(rlm_sql_t *inst, REQUEST *request, rlm
        }
        if (rows == 0) {
                RDEBUG2("User not found in any groups");
-               rcode = RLM_MODULE_NOTFOUND;
-               *do_fall_through = FALL_THROUGH_DEFAULT;
-
-               goto finish;
+               goto do_nothing;
        }
        rad_assert(head);
 
@@ -825,7 +855,7 @@ static int mod_bootstrap(CONF_SECTION *conf, void *instance)
         *
         *      We need this to check if the sql_fields callback is provided.
         */
-       inst->handle = lt_dlopenext(inst->config->sql_driver_name);
+       inst->handle = fr_dlopenext(inst->config->sql_driver_name);
        if (!inst->handle) {
                ERROR("Could not link driver %s: %s", inst->config->sql_driver_name, fr_strerror());
                ERROR("Make sure it (and all its dependent libraries!) are in the search path of your system's ld");
@@ -867,6 +897,11 @@ static int mod_bootstrap(CONF_SECTION *conf, void *instance)
 
                        inst->group_da = dict_attrbyname("SQL-Group");
                }
+
+               if (!inst->group_da) {
+                       ERROR("Failed resolving group attribute");
+                       return -1;
+               }
        }
 
        /*
@@ -922,6 +957,12 @@ do { \
                        WARN("rlm_sql (%s): Ignoring authorize_group_check_query as group_membership_query "
                             "is not configured", inst->name);
                }
+
+               if (!inst->config->read_groups) {
+                       WARN("rlm_sql (%s): Ignoring read_groups as group_membership_query "
+                            "is not configured", inst->name);
+                       inst->config->read_groups = false;
+               }
        } /* allow the group check / reply queries to be NULL */
 
        /*
@@ -982,7 +1023,7 @@ do { \
                }
        }
 
-       inst->ef = exfile_init(inst, 64, 30, true);
+       inst->ef = exfile_init(inst, 256, 30, true);
        if (!inst->ef) {
                cf_log_err_cs(conf, "Failed creating log file context");
                return -1;
@@ -1476,10 +1517,11 @@ static rlm_rcode_t mod_checksimul(void *instance, REQUEST * request)
 
        /* If simul_count_query is not defined, we don't do any checking */
        if (!inst->config->simul_count_query) {
+               RWDEBUG("Simultaneous-Use checking requires 'simul_count_query' to be configured");
                return RLM_MODULE_NOOP;
        }
 
-       if ((!request->username) || (request->username->vp_length == '\0')) {
+       if ((!request->username) || (request->username->vp_length == 0)) {
                REDEBUG("Zero Length username not permitted");
 
                return RLM_MODULE_INVALID;
@@ -1504,7 +1546,7 @@ static rlm_rcode_t mod_checksimul(void *instance, REQUEST * request)
 
        if (rlm_sql_select_query(inst, request, &handle, expanded) != RLM_SQL_OK) {
                rcode = RLM_MODULE_FAIL;
-               goto finish;
+               goto release;   /* handle may no longer be valid */
        }
 
        ret = rlm_sql_fetch_row(inst, request, &handle);
@@ -1561,11 +1603,21 @@ static rlm_rcode_t mod_checksimul(void *instance, REQUEST * request)
        }
 
        while (rlm_sql_fetch_row(inst, request, &handle) == 0) {
+               int num_rows;
+
                row = handle->row;
                if (!row) {
                        break;
                }
 
+               num_rows = (inst->module->sql_num_fields)(handle, inst->config);
+               if (num_rows < 8) {
+                       RDEBUG("Too few rows returned.  Please do not edit 'simul_verify_query'");
+                       rcode = RLM_MODULE_FAIL;
+
+                       goto finish;
+               }
+
                if (!row[2]){
                        RDEBUG("Cannot zap stale entry. No username present in entry");
                        rcode = RLM_MODULE_FAIL;
@@ -1606,7 +1658,7 @@ static rlm_rcode_t mod_checksimul(void *instance, REQUEST * request)
                                        else if (strcmp(row[7], "SLIP") == 0)
                                                proto = 'S';
                                }
-                               if (row[8])
+                               if ((num_rows > 8) && row[8])
                                        sess_time = atoi(row[8]);
                                session_zap(request, nas_addr, nas_port,
                                            row[2], row[1], framed_addr,