create string only if it's needed
[freeradius.git] / src / modules / rlm_sqlcounter / rlm_sqlcounter.c
index 5970746..8d43e73 100644 (file)
@@ -1,7 +1,8 @@
 /*
  *   This program is is free software; you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License, version 2 if the
- *   License as published by the Free Software Foundation.
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation; either version 2 of the License, or (at
+ *   your option) any later version.
  *
  *   This program is distributed in the hope that it will be useful,
  *   but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -29,7 +30,7 @@ RCSID("$Id$")
 
 #include <ctype.h>
 
-#define MAX_QUERY_LEN 1024
+#define MAX_QUERY_LEN 2048
 
 /*
  *     Note: When your counter spans more than 1 period (ie 3 months
@@ -57,21 +58,21 @@ RCSID("$Id$")
  *     be used as the instance handle.
  */
 typedef struct rlm_sqlcounter_t {
-       char            *counter_name;  //!< Daily-Session-Time.
-       char            *check_name;    //!< Max-Daily-Session.
-       char            *reply_name;    //!< Session-Timeout.
-       char            *key_name;      //!< User-Name.
-       char            *sqlmod_inst;   //!< Instance of SQL module to use,
+       char const      *counter_name;  //!< Daily-Session-Time.
+       char const      *limit_name;    //!< Max-Daily-Session.
+       char const      *reply_name;    //!< Session-Timeout.
+       char const      *key_name;      //!< User-Name.
+       char const      *sqlmod_inst;   //!< Instance of SQL module to use,
                                        //!< usually just 'sql'.
-       char            *query;         //!< SQL query to retrieve current
+       char const      *query;         //!< SQL query to retrieve current
                                        //!< session time.
-       char            *reset;         //!< Daily, weekly, monthly,
+       char const      *reset;         //!< Daily, weekly, monthly,
                                        //!< never or user defined.
        time_t          reset_time;
        time_t          last_reset;
-       const DICT_ATTR *key_attr;      //!< Attribute number for key field.
-       const DICT_ATTR *dict_attr;     //!< Attribute number for the counter.
-       const DICT_ATTR *reply_attr;    //!< Attribute number for the reply.
+       DICT_ATTR const *key_attr;      //!< Attribute number for key field.
+       DICT_ATTR const *dict_attr;     //!< Attribute number for the counter.
+       DICT_ATTR const *reply_attr;    //!< Attribute number for the reply.
 } rlm_sqlcounter_t;
 
 /*
@@ -84,17 +85,25 @@ typedef struct rlm_sqlcounter_t {
  *     buffer over-flows.
  */
 static const CONF_PARSER module_config[] = {
-  { "counter-name", PW_TYPE_STRING_PTR | PW_TYPE_REQUIRED, offsetof(rlm_sqlcounter_t,counter_name), NULL,  NULL },
-  { "check-name", PW_TYPE_STRING_PTR | PW_TYPE_REQUIRED, offsetof(rlm_sqlcounter_t,check_name), NULL, NULL },
-  { "reply-name", PW_TYPE_STRING_PTR | PW_TYPE_ATTRIBUTE, offsetof(rlm_sqlcounter_t,reply_name), NULL, "Session-Timeout" },
-  { "key", PW_TYPE_STRING_PTR | PW_TYPE_ATTRIBUTE, offsetof(rlm_sqlcounter_t,key_name), NULL, NULL },
-  { "sql-module-instance", PW_TYPE_STRING_PTR | PW_TYPE_REQUIRED, offsetof(rlm_sqlcounter_t,sqlmod_inst), NULL, NULL },
-  { "query", PW_TYPE_STRING_PTR | PW_TYPE_REQUIRED , offsetof(rlm_sqlcounter_t,query), NULL, NULL },
-  { "reset", PW_TYPE_STRING_PTR | PW_TYPE_REQUIRED, offsetof(rlm_sqlcounter_t,reset), NULL,  NULL },
-  { NULL, -1, 0, NULL, NULL }
+       { "sql-module-instance", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sqlcounter_t, sqlmod_inst), NULL },
+       { "sql_module_instance", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_REQUIRED, rlm_sqlcounter_t, sqlmod_inst), NULL },
+
+       { "key", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_ATTRIBUTE, rlm_sqlcounter_t, key_name), NULL },
+       { "query", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_REQUIRED, rlm_sqlcounter_t, query), NULL },
+       { "reset", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_REQUIRED, rlm_sqlcounter_t, reset), NULL },
+
+       { "counter-name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sqlcounter_t, counter_name), NULL },
+       { "counter_name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_REQUIRED, rlm_sqlcounter_t, counter_name), NULL },
+
+       { "check-name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sqlcounter_t, limit_name), NULL },
+       { "check_name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_REQUIRED, rlm_sqlcounter_t, limit_name), NULL },
+
+       { "reply-name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_DEPRECATED, rlm_sqlcounter_t, reply_name), NULL },
+       { "reply_name", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_ATTRIBUTE, rlm_sqlcounter_t, reply_name), "Session-Timeout" },
+       CONF_PARSER_TERMINATOR
 };
 
-static int find_next_reset(rlm_sqlcounter_t *inst, time_t timeval)
+static int find_next_reset(rlm_sqlcounter_t *inst, REQUEST *request, time_t timeval)
 {
        int ret = 0;
        size_t len;
@@ -104,28 +113,33 @@ static int find_next_reset(rlm_sqlcounter_t *inst, time_t timeval)
        char sCurrentTime[40], sNextTime[40];
 
        tm = localtime_r(&timeval, &s_tm);
-       len = strftime(sCurrentTime, sizeof(sCurrentTime), "%Y-%m-%d %H:%M:%S", tm);
-       if (len == 0) *sCurrentTime = '\0';
        tm->tm_sec = tm->tm_min = 0;
 
        rad_assert(inst->reset != NULL);
 
+       /*
+        *      Reset every N hours, days, weeks, months.
+        */
        if (isdigit((int) inst->reset[0])){
                len = strlen(inst->reset);
-               if (len == 0)
-                       return -1;
+               if (len == 0) return -1;
+
                last = inst->reset[len - 1];
-               if (!isalpha((int) last))
+               if (!isalpha((int) last)) {
                        last = 'd';
+               }
+
                num = atoi(inst->reset);
                DEBUG("rlm_sqlcounter: num=%d, last=%c",num,last);
        }
+
        if (strcmp(inst->reset, "hourly") == 0 || last == 'h') {
                /*
                 *  Round up to the next nearest hour.
                 */
                tm->tm_hour += num;
                inst->reset_time = mktime(tm);
+
        } else if (strcmp(inst->reset, "daily") == 0 || last == 'd') {
                /*
                 *  Round up to the next nearest day.
@@ -133,6 +147,7 @@ static int find_next_reset(rlm_sqlcounter_t *inst, time_t timeval)
                tm->tm_hour = 0;
                tm->tm_mday += num;
                inst->reset_time = mktime(tm);
+
        } else if (strcmp(inst->reset, "weekly") == 0 || last == 'w') {
                /*
                 *  Round up to the next nearest week.
@@ -140,21 +155,29 @@ static int find_next_reset(rlm_sqlcounter_t *inst, time_t timeval)
                tm->tm_hour = 0;
                tm->tm_mday += (7 - tm->tm_wday) +(7*(num-1));
                inst->reset_time = mktime(tm);
+
        } else if (strcmp(inst->reset, "monthly") == 0 || last == 'm') {
                tm->tm_hour = 0;
                tm->tm_mday = 1;
                tm->tm_mon += num;
                inst->reset_time = mktime(tm);
+
        } else if (strcmp(inst->reset, "never") == 0) {
                inst->reset_time = 0;
+
        } else {
                return -1;
        }
 
+       if (!request || (rad_debug_lvl < 2)) return ret;
+
+       len = strftime(sCurrentTime, sizeof(sCurrentTime), "%Y-%m-%d %H:%M:%S", tm);
+       if (len == 0) *sCurrentTime = '\0';
+
        len = strftime(sNextTime, sizeof(sNextTime),"%Y-%m-%d %H:%M:%S",tm);
        if (len == 0) *sNextTime = '\0';
-       DEBUG2("rlm_sqlcounter: Current Time: %li [%s], Next reset %li [%s]",
-               timeval, sCurrentTime, inst->reset_time, sNextTime);
+       RDEBUG2("rlm_sqlcounter: Current Time: %" PRId64 " [%s], Next reset %" PRId64 " [%s]",
+               (int64_t) timeval, sCurrentTime, (int64_t) inst->reset_time, sNextTime);
 
        return ret;
 }
@@ -196,6 +219,7 @@ static int find_prev_reset(rlm_sqlcounter_t *inst, time_t timeval)
                 */
                tm->tm_hour -= num - 1;
                inst->last_reset = mktime(tm);
+
        } else if (strcmp(inst->reset, "daily") == 0 || last == 'd') {
                /*
                 *  Round down to the prev nearest day.
@@ -203,27 +227,31 @@ static int find_prev_reset(rlm_sqlcounter_t *inst, time_t timeval)
                tm->tm_hour = 0;
                tm->tm_mday -= num - 1;
                inst->last_reset = mktime(tm);
+
        } else if (strcmp(inst->reset, "weekly") == 0 || last == 'w') {
                /*
                 *  Round down to the prev nearest week.
                 */
                tm->tm_hour = 0;
-               tm->tm_mday -= (7 - tm->tm_wday) +(7*(num-1));
+               tm->tm_mday -= tm->tm_wday +(7*(num-1));
                inst->last_reset = mktime(tm);
+
        } else if (strcmp(inst->reset, "monthly") == 0 || last == 'm') {
                tm->tm_hour = 0;
                tm->tm_mday = 1;
                tm->tm_mon -= num - 1;
                inst->last_reset = mktime(tm);
+
        } else if (strcmp(inst->reset, "never") == 0) {
                inst->reset_time = 0;
+
        } else {
                return -1;
        }
        len = strftime(sPrevTime, sizeof(sPrevTime), "%Y-%m-%d %H:%M:%S", tm);
        if (len == 0) *sPrevTime = '\0';
-       DEBUG2("rlm_sqlcounter: Current Time: %li [%s], Prev reset %li [%s]",
-              timeval, sCurrentTime, inst->last_reset, sPrevTime);
+       DEBUG2("rlm_sqlcounter: Current Time: %" PRId64 " [%s], Prev reset %" PRId64 " [%s]",
+              (int64_t) timeval, sCurrentTime, (int64_t) inst->last_reset, sPrevTime);
 
        return ret;
 }
@@ -241,67 +269,86 @@ static int find_prev_reset(rlm_sqlcounter_t *inst, time_t timeval)
 
 static size_t sqlcounter_expand(char *out, int outlen, char const *fmt, rlm_sqlcounter_t *inst)
 {
-       int c,freespace;
+       int freespace;
        char const *p;
        char *q;
        char tmpdt[40]; /* For temporary storing of dates */
 
        q = out;
-       for (p = fmt; *p ; p++) {
-       /* Calculate freespace in output */
-       freespace = outlen - (q - out);
-               if (freespace <= 1)
+       p = fmt;
+       while (*p) {
+               /* Calculate freespace in output */
+               freespace = outlen - (q - out);
+               if (freespace <= 1) {
                        return -1;
-               c = *p;
-               if ((c != '%') && (c != '\\')) {
-                       *q++ = *p;
+               }
+
+               /*
+                *      Non-% get copied as-is.
+                */
+               if (*p != '%') {
+                       *q++ = *p++;
                        continue;
                }
-               if (*++p == '\0') break;
-               if (c == '\\') switch(*p) {
-                       case '\\':
-                               *q++ = *p;
-                               break;
-                       case 't':
-                               *q++ = '\t';
-                               break;
-                       case 'n':
-                               *q++ = '\n';
-                               break;
-                       default:
-                               *q++ = c;
-                               *q++ = *p;
-                               break;
+               p++;
+               if (!*p) {      /* % and then EOS --> % */
+                       *q++ = '%';
+                       break;
+               }
 
-               } else if (c == '%') switch(*p) {
+               if (freespace <= 2) return -1;
 
-                       case '%':
-                               *q++ = *p;
-                               break;
+               /*
+                *      We need TWO %% in a row before we do our expansions.
+                *      If we only get one, just copy the %s as-is.
+                */
+               if (*p != '%') {
+                       *q++ = '%';
+                       *q++ = *p++;
+                       continue;
+               }
+               p++;
+               if (!*p) {
+                       *q++ = '%';
+                       *q++ = '%';
+                       break;
+               }
+
+               if (freespace <= 3) return -1;
+
+               switch (*p) {
                        case 'b': /* last_reset */
-                               snprintf(tmpdt, sizeof(tmpdt), "%lu", inst->last_reset);
+                               snprintf(tmpdt, sizeof(tmpdt), "%" PRId64, (int64_t) inst->last_reset);
                                strlcpy(q, tmpdt, freespace);
                                q += strlen(q);
+                               p++;
                                break;
                        case 'e': /* reset_time */
-                               snprintf(tmpdt, sizeof(tmpdt), "%lu", inst->reset_time);
+                               snprintf(tmpdt, sizeof(tmpdt), "%" PRId64, (int64_t) inst->reset_time);
                                strlcpy(q, tmpdt, freespace);
                                q += strlen(q);
+                               p++;
                                break;
                        case 'k': /* Key Name */
-                               WDEBUG2("Please replace '%%k' with '${key}'");
+                               WARN("Please replace '%%k' with '${key}'");
                                strlcpy(q, inst->key_name, freespace);
                                q += strlen(q);
+                               p++;
                                break;
+
+                               /*
+                                *      %%s gets copied over as-is.
+                                */
                        default:
                                *q++ = '%';
-                               *q++ = *p;
+                               *q++ = '%';
+                               *q++ = *p++;
                                break;
                }
        }
        *q = '\0';
 
-       DEBUG2("sqlcounter_expand:  '%s'", out);
+       DEBUG2("sqlcounter_expand: '%s'", out);
 
        return strlen(out);
 }
@@ -314,53 +361,91 @@ static int sqlcounter_cmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *r
                          UNUSED VALUE_PAIR *check_pairs, UNUSED VALUE_PAIR **reply_pairs)
 {
        rlm_sqlcounter_t *inst = instance;
-       int counter;
+       uint64_t counter;
 
-       char *p;
-       char query[MAX_QUERY_LEN];
+       char query[MAX_QUERY_LEN], subst[MAX_QUERY_LEN];
        char *expanded = NULL;
-       
        size_t len;
 
-       len = snprintf(query, sizeof(query), "%%{%s:%s}", inst->sqlmod_inst, query);
-       if (len >= sizeof(query) - 1) {
+       /* First, expand %k, %b and %e in query */
+       if (sqlcounter_expand(subst, sizeof(subst), inst->query, inst) <= 0) {
                REDEBUG("Insufficient query buffer space");
-               
-               return RLM_MODULE_FAIL;
-       }
-       
-       p = query + len;
-       
-       /* first, expand %k, %b and %e in query */
-       len = sqlcounter_expand(p, p - query, inst->query, inst);
-       if (len <= 0) {
-               REDEBUG("Insufficient query buffer space");
-               
+
                return RLM_MODULE_FAIL;
        }
-       
-       p += len;
-       
-       if ((p - query) < 2) {
+
+       /* Then combine that with the name of the module were using to do the query */
+       len = snprintf(query, sizeof(query), "%%{%s:%s}", inst->sqlmod_inst, subst);
+       if (len >= sizeof(query) - 1) {
                REDEBUG("Insufficient query buffer space");
-               
+
                return RLM_MODULE_FAIL;
        }
-       
-       p[0] = '}';
-       p[1] = '\0';
 
        /* Finally, xlat resulting SQL query */
        if (radius_axlat(&expanded, request, query, NULL, NULL) < 0) {
                return RLM_MODULE_FAIL;
        }
 
-       counter = atoi(expanded);
+       if (sscanf(expanded, "%" PRIu64, &counter) != 1) {
+               RDEBUG2("No integer found in string \"%s\"", expanded);
+       }
        talloc_free(expanded);
 
-       return counter - check->vp_integer;
+       if (counter < check->vp_integer64) {
+               return -1;
+       }
+       if (counter > check->vp_integer64) {
+               return 1;
+       }
+       return 0;
 }
 
+static int mod_bootstrap(CONF_SECTION *conf, void *instance)
+{
+       rlm_sqlcounter_t *inst = instance;
+       DICT_ATTR const *da;
+       ATTR_FLAGS flags;
+
+       memset(&flags, 0, sizeof(flags));
+       flags.compare = 1;      /* ugly hack */
+       da = dict_attrbyname(inst->counter_name);
+       if (da && (da->type != PW_TYPE_INTEGER64)) {
+               cf_log_err_cs(conf, "Counter attribute %s MUST be integer64", inst->counter_name);
+               return -1;
+       }
+
+       if (!da && (dict_addattr(inst->counter_name, -1, 0, PW_TYPE_INTEGER64, flags) < 0)) {
+               cf_log_err_cs(conf, "Failed to create counter attribute %s: %s", inst->counter_name, fr_strerror());
+               return -1;
+       }
+
+       /*
+        *  Register the counter comparison operation.
+        */
+       if (paircompare_register_byname(inst->counter_name, NULL, true, sqlcounter_cmp, inst) < 0) {
+               cf_log_err_cs(conf, "Failed registering counter attribute %s: %s", inst->counter_name, fr_strerror());
+               return -1;
+       }
+
+       inst->dict_attr = dict_attrbyname(inst->counter_name);
+       if (!inst->dict_attr) {
+               cf_log_err_cs(conf, "Failed to find counter attribute %s", inst->counter_name);
+               return -1;
+       }
+
+       /*
+        *  Create a new attribute for the check item.
+        */
+       flags.compare = 0;
+       if ((dict_addattr(inst->limit_name, -1, 0, PW_TYPE_INTEGER64, flags) < 0) ||
+           !dict_attrbyname(inst->limit_name)) {
+               cf_log_err_cs(conf, "Failed to create check attribute %s: %s", inst->limit_name, fr_strerror());
+               return -1;
+       }
+
+       return 0;
+}
 
 /*
  *     Do any per-module initialization that is separate to each
@@ -375,67 +460,29 @@ static int sqlcounter_cmp(void *instance, REQUEST *request, UNUSED VALUE_PAIR *r
 static int mod_instantiate(CONF_SECTION *conf, void *instance)
 {
        rlm_sqlcounter_t *inst = instance;
-       const DICT_ATTR *dattr;
-       ATTR_FLAGS flags;
+       DICT_ATTR const *da;
        time_t now;
 
        rad_assert(inst->query && *inst->query);
 
-       dattr = dict_attrbyname(inst->key_name);
-       rad_assert(dattr != NULL);
-       if (dattr->vendor != 0) {
-               cf_log_err_cs(conf, "Configuration item 'key' cannot be a VSA");
+       da = dict_attrbyname(inst->key_name);
+       if (!da) {
+               cf_log_err_cs(conf, "Invalid attribute '%s'", inst->key_name);
                return -1;
        }
-       inst->key_attr = dattr;
+       inst->key_attr = da;
 
-       dattr = dict_attrbyname(inst->reply_name);
-       rad_assert(dattr != NULL);
-       if (dattr->vendor != 0) {
-               cf_log_err_cs(conf, "Configuration item 'reply-name' cannot be a VSA");
+       da = dict_attrbyname(inst->reply_name);
+       if (!da) {
+               cf_log_err_cs(conf, "Invalid attribute '%s'", inst->reply_name);
                return -1;
        }
-       inst->reply_attr = dattr;
-
-       DEBUG2("rlm_sqlcounter: Reply attribute %s is number %d",
-                      inst->reply_name, dattr->attr);
-
-       /*
-        *  Create a new attribute for the counter.
-        */
-       rad_assert(inst->counter_name && *inst->counter_name);
-       memset(&flags, 0, sizeof(flags));
-       dict_addattr(inst->counter_name, -1, 0, PW_TYPE_INTEGER, flags);
-       dattr = dict_attrbyname(inst->counter_name);
-       if (!dattr) {
-               cf_log_err_cs(conf, "Failed to create counter attribute %s",
-                               inst->counter_name);
-               return -1;
-       }
-       if (dattr->vendor != 0) {
-               cf_log_err_cs(conf, "Counter attribute must not be a VSA");
-               return -1;
-       }
-       inst->dict_attr = dattr;
-
-       /*
-        * Create a new attribute for the check item.
-        */
-       rad_assert(inst->check_name && *inst->check_name);
-       dict_addattr(inst->check_name, 0, PW_TYPE_INTEGER, -1, flags);
-       dattr = dict_attrbyname(inst->check_name);
-       if (!dattr) {
-               cf_log_err_cs(conf, "Failed to create check attribute %s",
-                               inst->check_name);
-               return -1;
-       }
-       DEBUG2("rlm_sqlcounter: Check attribute %s is number %d",
-                       inst->check_name, dattr->attr);
+       inst->reply_attr = da;
 
        now = time(NULL);
        inst->reset_time = 0;
 
-       if (find_next_reset(inst,now) == -1) {
+       if (find_next_reset(inst, NULL, now) < 0) {
                cf_log_err_cs(conf, "Invalid reset '%s'", inst->reset);
                return -1;
        }
@@ -450,11 +497,6 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
                return -1;
        }
 
-       /*
-        *      Register the counter comparison operation.
-        */
-       paircompare_register(inst->dict_attr->attr, 0, sqlcounter_cmp, inst);
-
        return 0;
 }
 
@@ -464,20 +506,19 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
  *     from the database. The authentication code only needs to check
  *     the password, the rest is done here.
  */
-static rlm_rcode_t mod_authorize(UNUSED void *instance, UNUSED REQUEST *request)
+static rlm_rcode_t CC_HINT(nonnull) mod_authorize(void *instance, REQUEST *request)
 {
        rlm_sqlcounter_t *inst = instance;
        int rcode = RLM_MODULE_NOOP;
-       unsigned int counter;
-       const DICT_ATTR *dattr;
-       VALUE_PAIR *key_vp, *check_vp;
+       uint64_t counter, res;
+       DICT_ATTR const *da;
+       VALUE_PAIR *key_vp, *limit;
        VALUE_PAIR *reply_item;
        char msg[128];
-       
-       char *p;
-       char query[MAX_QUERY_LEN];
+
+       char query[MAX_QUERY_LEN], subst[MAX_QUERY_LEN];
        char *expanded = NULL;
-       
+
        size_t len;
 
        /*
@@ -489,139 +530,123 @@ static rlm_rcode_t mod_authorize(UNUSED void *instance, UNUSED REQUEST *request)
                 *      Re-set the next time and prev_time for this counters range
                 */
                inst->last_reset = inst->reset_time;
-               find_next_reset(inst,request->timestamp);
+               find_next_reset(inst, request, request->timestamp);
        }
 
        /*
         *      Look for the key.  User-Name is special.  It means
         *      The REAL username, after stripping.
         */
-       RDEBUG2("Entering module authorize code");
-       key_vp = ((inst->key_attr->vendor == 0) && (inst->key_attr->attr == PW_USER_NAME)) ?
-                       request->username :
-                       pairfind(request->packet->vps, inst->key_attr->attr, inst->key_attr->vendor, TAG_ANY);
+       if ((inst->key_attr->vendor == 0) && (inst->key_attr->attr == PW_USER_NAME)) {
+               key_vp = request->username;
+       } else {
+               key_vp = fr_pair_find_by_da(request->packet->vps, inst->key_attr, TAG_ANY);
+       }
        if (!key_vp) {
-               RDEBUG2("Could not find Key value pair");
+               RWDEBUG2("Couldn't find key attribute, request:%s, doing nothing...", inst->key_attr->name);
                return rcode;
        }
 
        /*
         *      Look for the check item
         */
-       if ((dattr = dict_attrbyname(inst->check_name)) == NULL) {
+       if ((da = dict_attrbyname(inst->limit_name)) == NULL) {
                return rcode;
        }
-       /* DEBUG2("rlm_sqlcounter: Found Check item attribute %d", dattr->attr); */
-       if ((check_vp = pairfind(request->config_items, dattr->attr, dattr->vendor, TAG_ANY)) == NULL) {
-               RDEBUG2("Could not find Check item value pair");
+
+       limit = fr_pair_find_by_da(request->config, da, TAG_ANY);
+       if (limit == NULL) {
+               /* Yes this really is 'check' as distinct from control */
+               RWDEBUG2("Couldn't find check attribute, control:%s, doing nothing...", inst->limit_name);
                return rcode;
        }
 
-       len = snprintf(query, sizeof(query), "%%{%s:%s}", inst->sqlmod_inst, query);
-       if (len >= sizeof(query) - 1) {
+       /* First, expand %k, %b and %e in query */
+       if (sqlcounter_expand(subst, sizeof(subst), inst->query, inst) <= 0) {
                REDEBUG("Insufficient query buffer space");
-               
-               return RLM_MODULE_FAIL;
-       }
-       
-       p = query + len;
-       
-       /* first, expand %k, %b and %e in query */
-       len = sqlcounter_expand(p, p - query, inst->query, inst);
-       if (len <= 0) {
-               REDEBUG("Insufficient query buffer space");
-               
+
                return RLM_MODULE_FAIL;
        }
-       
-       p += len;
-       
-       if ((p - query) < 2) {
+
+       /* Then combine that with the name of the module were using to do the query */
+       len = snprintf(query, sizeof(query), "%%{%s:%s}", inst->sqlmod_inst, subst);
+       if (len >= (sizeof(query) - 1)) {
                REDEBUG("Insufficient query buffer space");
-               
+
                return RLM_MODULE_FAIL;
        }
-       
-       p[0] = '}';
-       p[1] = '\0';
 
        /* Finally, xlat resulting SQL query */
        if (radius_axlat(&expanded, request, query, NULL, NULL) < 0) {
                return RLM_MODULE_FAIL;
        }
+       talloc_free(expanded);
 
-       if (sscanf(expanded, "%u", &counter) != 1) {
-               RDEBUG2("No integer found in string \"%s\"", expanded);
-               return RLM_MODULE_NOOP;
+       if (sscanf(expanded, "%" PRIu64, &counter) != 1) {
+               RDEBUG2("No integer found in result string \"%s\".  May be first session, setting counter to 0",
+                       expanded);
+               counter = 0;
        }
 
-       talloc_free(expanded);
-
        /*
-        * Check if check item > counter
+        *      Check if check item > counter
         */
-       if (check_vp->vp_integer > counter) {
-               unsigned int res = check_vp->vp_integer - counter;
+       if (limit->vp_integer64 <= counter) {
+               /* User is denied access, send back a reply message */
+               snprintf(msg, sizeof(msg), "Your maximum %s usage time has been reached", inst->reset);
+               pair_make_reply("Reply-Message", msg, T_OP_EQ);
 
-               RDEBUG2("Check item is greater than query result");
-               /*
-                *      We are assuming that simultaneous-use=1. But
-                *      even if that does not happen then our user
-                *      could login at max for 2*max-usage-time Is
-                *      that acceptable?
-                */
+               REDEBUG2("Maximum %s usage time reached", inst->reset);
+               REDEBUG2("Rejecting user, &control:%s value (%" PRIu64 ") is less than counter value (%" PRIu64 ")",
+                        inst->limit_name, limit->vp_integer64, counter);
 
-               /*
-                *      If we are near a reset then add the next
-                *      limit, so that the user will not need to login
-                *      again.  Do this only for Session-Timeout.
-                */
-               if ((inst->reply_attr->attr == PW_SESSION_TIMEOUT) && inst->reset_time &&
-                   (res >= (inst->reset_time - request->timestamp))) {
-                       res = inst->reset_time - request->timestamp;
-                       res += check_vp->vp_integer;
-               }
+               return RLM_MODULE_REJECT;
+       }
 
-               /*
-                *      Limit the reply attribute to the minimum of
-                *      the existing value, or this new one.
-                */
-               reply_item = pairfind(request->reply->vps, inst->reply_attr->attr, inst->reply_attr->vendor, TAG_ANY);
-               if (reply_item) {
-                       if (reply_item->vp_integer > res) {
-                               reply_item->vp_integer = res;
-                       }
-               } else {
-                       reply_item = radius_paircreate(request, &request->reply->vps, inst->reply_attr->attr,
-                                                      inst->reply_attr->vendor);
-                       reply_item->vp_integer = res;
-               }
+       res = limit->vp_integer64 - counter;
+       RDEBUG2("Allowing user, &control:%s value (%" PRIu64 ") is greater than counter value (%" PRIu64 ")",
+               inst->limit_name, limit->vp_integer64, counter);
+       /*
+        *      We are assuming that simultaneous-use=1. But
+        *      even if that does not happen then our user
+        *      could login at max for 2*max-usage-time Is
+        *      that acceptable?
+        */
 
-               rcode = RLM_MODULE_OK;
+       /*
+        *      If we are near a reset then add the next
+        *      limit, so that the user will not need to login
+        *      again.  Do this only for Session-Timeout.
+        */
+       if (((inst->reply_attr->vendor == 0) && (inst->reply_attr->attr == PW_SESSION_TIMEOUT)) &&
+           inst->reset_time && (res >= (uint64_t)(inst->reset_time - request->timestamp))) {
+               uint64_t to_reset = inst->reset_time - request->timestamp;
 
-               RDEBUG2("Authorized user %s, check_item=%u, counter=%u", key_vp->vp_strvalue, check_vp->vp_integer,
-                       counter);
-               RDEBUG2("Sent Reply-Item for user %s, Type=%s, value=%u", key_vp->vp_strvalue, inst->reply_name, 
-                       reply_item->vp_integer);
+               RDEBUG2("Time remaining (%" PRIu64 "s) is greater than time to reset (%" PRIu64 "s).  "
+                       "Adding %" PRIu64 "s to reply value", to_reset, res, to_reset);
+               res = to_reset + limit->vp_integer;
        }
-       else{
-               RDEBUG2("(Check item - counter) is less than zero");
 
-               /*
-                * User is denied access, send back a reply message
-                */
-               snprintf(msg, sizeof(msg), "Your maximum %s usage time has been reached", inst->reset);
-               pairmake_reply("Reply-Message", msg, T_OP_EQ);
-
-               REDEBUG("Maximum %s usage time reached",
-                                  inst->reset);
-               rcode = RLM_MODULE_REJECT;
+       /*
+        *      Limit the reply attribute to the minimum of the existing value, or this new one.
+        */
+       reply_item = fr_pair_find_by_da(request->reply->vps, inst->reply_attr, TAG_ANY);
+       if (reply_item) {
+               if (reply_item->vp_integer64 <= res) {
+                       RDEBUG2("Leaving existing &reply:%s value of %" PRIu64, inst->reply_attr->name,
+                               reply_item->vp_integer64);
 
-               RDEBUG2("Rejected user %s, check_item=%u, counter=%u", key_vp->vp_strvalue, 
-                       check_vp->vp_integer,counter);
+                       return RLM_MODULE_OK;
+               }
+       } else {
+               reply_item = radius_pair_create(request->reply, &request->reply->vps, inst->reply_attr->attr,
+                                              inst->reply_attr->vendor);
        }
+       reply_item->vp_integer64 = res;
+
+       RDEBUG2("Setting &reply:%s value to %" PRIu64, inst->reply_name, reply_item->vp_integer64);
 
-       return rcode;
+       return RLM_MODULE_OK;
 }
 
 /*
@@ -633,23 +658,17 @@ static rlm_rcode_t mod_authorize(UNUSED void *instance, UNUSED REQUEST *request)
  *     The server will then take care of ensuring that the module
  *     is single-threaded.
  */
+extern module_t rlm_sqlcounter;
 module_t rlm_sqlcounter = {
-       RLM_MODULE_INIT,
-       "SQL Counter",
-       RLM_TYPE_THREAD_SAFE,           /* type */
-       sizeof(rlm_sqlcounter_t),
-       module_config,
-       mod_instantiate,                /* instantiation */
-       NULL,                           /* detach */
-       {
-               NULL,                   /* authentication */
-               mod_authorize,  /* authorization */
-               NULL,                   /* preaccounting */
-               NULL,                   /* accounting */
-               NULL,                   /* checksimul */
-               NULL,                   /* pre-proxy */
-               NULL,                   /* post-proxy */
-               NULL                    /* post-auth */
+       .magic          = RLM_MODULE_INIT,
+       .name           = "sqlcounter",
+       .type           = RLM_TYPE_THREAD_SAFE,
+       .inst_size      = sizeof(rlm_sqlcounter_t),
+       .config         = module_config,
+       .bootstrap      = mod_bootstrap,
+       .instantiate    = mod_instantiate,
+       .methods = {
+               [MOD_AUTHORIZE]         = mod_authorize
        },
 };