Remove 'caseless' from VALUE_PAIR flags. It's not needed.
authoraland <aland>
Wed, 5 Dec 2007 10:22:41 +0000 (10:22 +0000)
committeraland <aland>
Wed, 5 Dec 2007 10:22:41 +0000 (10:22 +0000)
Added 'unknown_attr' to VALUE_PAIR flags, which tracks if
vp->name points to a DICT_ATTR entry name or not.

vp->name is now a pointer, rather than a character array.

Updated code to have "vp->name = da->name" for known attributes.
Otherwise, the memory allocated for the VALUE_PAIR is increased
by ~24 characters.  The name is printed there (Vendor-X-Attr-Y),
and vp->name is pointed to the string.

Updated paircopy() to look at vp->flags.unknown_attr,
if set, it allocates more room for the name, and does
a memcpy() of the VALUE_PAIR + the name.

Updated rlm_preprocess to NOT print to vp->name.

Nothing else in the code should now write to vp->name

Updated paircreate() to simplify printing of Vendor-X-Attr-Y

Updated pairmake_any() to simplify parsing of Vendor-X-Attr-Y.
It now also checks size of attribute values (e.g. 1-octet,
2-octet, etc).  It now parses the octet string as an octet
string, no matter what the final type is.  So you can
have "Attr-5  = 0x00000001", and have it show up as
"NAS-Port = 1".

src/include/libradius.h
src/lib/valuepair.c
src/modules/rlm_preprocess/rlm_preprocess.c

index d555397..fca0e4a 100644 (file)
@@ -94,7 +94,7 @@ typedef struct attr_flags {
        unsigned int            addport : 1;  /* add NAS-Port to IP address */
        unsigned int            has_tag : 1;  /* tagged attribute */
        unsigned int            do_xlat : 1;  /* strvalue is dynamic */
-       unsigned int            caseless : 1; /* case insensitive compares */
+       unsigned int            unknown_attr : 1; /* not in dictionary */
        unsigned int            array : 1; /* pack multiples into 1 attr */
        unsigned int            has_value : 1; /* has a value */
        unsigned int            has_value_alias : 1; /* has a value alias */
@@ -146,7 +146,7 @@ typedef union value_pair_data {
 } VALUE_PAIR_DATA;
 
 typedef struct value_pair {
-       char                    name[40];
+       const char              *name;
        int                     attribute;
        int                     vendor;
        int                     type;
index 7f06a4f..6889f45 100644 (file)
@@ -39,11 +39,37 @@ static const char *months[] = {
         "jan", "feb", "mar", "apr", "may", "jun",
         "jul", "aug", "sep", "oct", "nov", "dec" };
 
+/*
+ *     This padding is necessary only for attributes that are NOT
+ *     in the dictionary, and then only because the rest of the
+ *     code accesses vp->name directly, rather than through an
+ *     accessor function.
+ *
+ *     The name padding only has to large enough for:
+ *
+ *             Vendor-65535-Attr-65535
+ *
+ *     i.e. 23 characters, plus a zero.  We add another 8 bytes for
+ *     padding, because the VALUE_PAIR structure may be un-aligned.
+ *
+ *     The result is that for the normal case, the server uses a less
+ *     memory (36 bytes * number of VALUE_PAIRs).
+ */
+#define FR_VP_NAME_PAD (32)
+#define FR_VP_NAME_LEN (24)
+
 VALUE_PAIR *pairalloc(DICT_ATTR *da)
 {
+       size_t name_len = 0;
        VALUE_PAIR *vp;
 
-       vp = malloc(sizeof(*vp));
+       /*
+        *      Not in the dictionary: the name is allocated AFTER
+        *      the VALUE_PAIR struct.
+        */
+       if (!da) name_len = FR_VP_NAME_PAD;
+
+       vp = malloc(sizeof(*vp) + name_len);
        if (!vp) return NULL;
        memset(vp, 0, sizeof(*vp));
 
@@ -51,14 +77,15 @@ VALUE_PAIR *pairalloc(DICT_ATTR *da)
                vp->attribute = da->attr;
                vp->vendor = da->vendor;
                vp->type = da->type;
-               strlcpy(vp->name, da->name, sizeof(vp->name));
+               vp->name = da->name;
                vp->flags = da->flags;
        } else {
                vp->attribute = 0;
                vp->vendor = 0;
                vp->type = PW_TYPE_OCTETS;
-               vp->name[0] = '\0';
+               vp->name = NULL;
                memset(&vp->flags, 0, sizeof(vp->flags));
+               vp->flags.unknown_attr = 1;
        }
 
        switch (vp->type) {
@@ -117,30 +144,41 @@ VALUE_PAIR *paircreate(int attr, int type)
        vp->operator = T_OP_EQ;
 
        /*
-        *      Update the name...
+        *      It isn't in the dictionary: update the name.
         */
        if (!da) {
-               if (VENDOR(attr) == 0) {
-                       snprintf(vp->name, sizeof(vp->name), "Attr-%u", attr);
-
-               } else {
+               size_t len = 0;
+               char *p = (char *) (vp + 1);
+               
+               vp->vendor = VENDOR(attr);
+               vp->attribute = attr;
+               vp->name = p;
+               vp->type = type; /* be forgiving */
+
+               if (vp->vendor) {
                        DICT_VENDOR *v;
 
-                       v = dict_vendorbyvalue(VENDOR(attr));
+                       v = dict_vendorbyvalue(vp->vendor);
                        if (v) {
-                               snprintf(vp->name, sizeof(vp->name),
-                                        "%s-Attr-%u",
-                                        v->name, attr & 0xffff);
+                               snprintf(p, FR_VP_NAME_LEN, "%s-", v->name);
                        } else {
-                               snprintf(vp->name, sizeof(vp->name),
-                                        "Vendor-%u-Attr-%u",
-                                        VENDOR(attr), attr & 0xffff);
+                               snprintf(p, FR_VP_NAME_LEN, "Vendor-%u-", vp->vendor);
                        }
-               }
-               vp->type = type;
 
-       } else {
-               vp->type = da->type;
+                       len = strlen(p);
+                       if (len == FR_VP_NAME_LEN) {
+                               free(vp);
+                               return NULL;
+                       }
+               }
+               
+               snprintf(p + len, FR_VP_NAME_LEN - len, "Attr-%u",
+                        attr & 0xffff);
+               len += strlen(p + len);
+               if (len == FR_VP_NAME_LEN) {
+                       free(vp);
+                       return NULL;
+               }
        }
 
        return vp;
@@ -286,15 +324,24 @@ VALUE_PAIR *paircopy2(VALUE_PAIR *vp, int attr)
        last = &first;
 
        while (vp) {
+               size_t name_len;
+
                if (attr >= 0 && vp->attribute != attr) {
                        vp = vp->next;
                        continue;
                }
+
+               if (!vp->flags.unknown_attr) {
+                       name_len = 0;
+               } else {
+                       name_len = FR_VP_NAME_PAD;
+               }
+               
                if ((n = malloc(sizeof(*n))) == NULL) {
                        librad_log("out of memory");
                        return first;
                }
-               memcpy(n, vp, sizeof(VALUE_PAIR));
+               memcpy(n, vp, sizeof(*n) + name_len);
                n->next = NULL;
                *last = n;
                last = &n->next;
@@ -779,7 +826,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
         *      Even for integers, dates and ip addresses we
         *      keep the original string in vp->vp_strvalue.
         */
-       strlcpy((char *)vp->vp_strvalue, value, sizeof(vp->vp_strvalue));
+       strlcpy(vp->vp_strvalue, value, sizeof(vp->vp_strvalue));
        vp->length = strlen(vp->vp_strvalue);
 
        switch(vp->type) {
@@ -1038,23 +1085,21 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                        break;
 
                case PW_TYPE_IFID:
-                       if (ifid_aton(value, (unsigned char *) vp->vp_strvalue) == NULL) {
+                       if (ifid_aton(value, (void *) &vp->vp_ifid) == NULL) {
                                librad_log("failed to parse interface-id "
                                           "string \"%s\"", value);
                                return NULL;
                        }
                        vp->length = 8;
-                       vp->vp_strvalue[vp->length] = '\0';
                        break;
 
                case PW_TYPE_IPV6ADDR:
-                       if (inet_pton(AF_INET6, value, vp->vp_strvalue) <= 0) {
+                       if (inet_pton(AF_INET6, value, &vp->vp_ipv6addr) <= 0) {
                                librad_log("failed to parse IPv6 address "
                                           "string \"%s\"", value);
                                return NULL;
                        }
                        vp->length = 16; /* length of IPv6 address */
-                       vp->vp_strvalue[vp->length] = '\0';
                        break;
 
                case PW_TYPE_IPV6PREFIX:
@@ -1070,7 +1115,7 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                                memcpy(buffer, value, p - value);
                                buffer[p - value] = '\0';
 
-                               if (inet_pton(AF_INET6, buffer, vp->vp_strvalue + 2) <= 0) {
+                               if (inet_pton(AF_INET6, buffer, vp->vp_octets + 2) <= 0) {
                                        librad_log("failed to parse IPv6 address "
                                                   "string \"%s\"", value);
                                        return NULL;
@@ -1082,9 +1127,9 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
                                                   "string \"%s\"", value);
                                        return NULL;
                                }
-                               vp->vp_strvalue[1] = prefix;
+                               vp->vp_octets[1] = prefix;
                        }
-                       vp->vp_strvalue[0] = '\0';
+                       vp->vp_octets[0] = '\0';
                        vp->length = 16 + 2;
                        break;
 
@@ -1137,89 +1182,125 @@ VALUE_PAIR *pairparsevalue(VALUE_PAIR *vp, const char *value)
  *
  *     Attr-%d
  *     Vendor-%d-Attr-%d
+ *     VendorName-Attr-%d
  */
 static VALUE_PAIR *pairmake_any(const char *attribute, const char *value,
                                int operator)
 {
-       int             attr;
-       const char      *p;
+       int             attr, vendor;
+       size_t          size;
+       const char      *p = attribute;
+       char            *q;
        VALUE_PAIR      *vp;
 
        /*
         *      Unknown attributes MUST be of type 'octets'
         */
        if (value && (strncasecmp(value, "0x", 2) != 0)) {
-               goto error;
+               librad_log("Invalid octet string \"%s\" for attribute name \"%s\"", value, attribute);
+               return NULL;
        }
 
+       attr = vendor = 0;
+
        /*
-        *      Attr-%d
+        *      Pull off vendor prefix first.
         */
-       if (strncasecmp(attribute, "Attr-", 5) == 0) {
-               attr = atoi(attribute + 5);
-               p = attribute + 5;
-               p += strspn(p, "0123456789");
-               if (*p != 0) goto error;
+       if (strncasecmp(p, "Attr-", 5) != 0) {
+               if (strncasecmp(p, "Vendor-", 7) == 0) {
+                       vendor = (int) strtol(p + 7, &q, 10);
+                       if ((vendor == 0) || (vendor > 65535)) {
+                               librad_log("Invalid vendor value in attribute name \"%s\"", attribute);
+                               return NULL;
+                       }
 
-               /*
-                *      Vendor-%d-Attr-%d
-                */
-       } else if (strncasecmp(attribute, "Vendor-", 7) == 0) {
-               int vendor;
+                       p = q;
 
-               vendor = atoi(attribute + 7);
-               if ((vendor == 0) || (vendor > 65535)) goto error;
+               } else {        /* must be vendor name */
+                       q = strchr(p, '-');
+                       char buffer[256];
 
-               p = attribute + 7;
-               p += strspn(p, "0123456789");
+                       if (!q) {
+                               librad_log("Invalid vendor name in attribute name \"%s\"", attribute);
+                               return NULL;
+                       }
 
-               /*
-                *      Not Vendor-%d-Attr-%d
-                */
-               if (strncasecmp(p, "-Attr-", 6) != 0) goto error;
+                       if ((q - p) >= sizeof(buffer)) {
+                               librad_log("Vendor name too long in attribute name \"%s\"", attribute);
+                               return NULL;
+                       }
 
-               p += 6;
-               attr = atoi(p);
+                       memcpy(buffer, p, (q - p));
+                       buffer[q - p] = '\0';
 
-               p += strspn(p, "0123456789");
-               if (*p != 0) goto error;
+                       vendor = dict_vendorbyname(buffer);
+                       if (!vendor) {
+                               librad_log("Unknown vendor name in attribute name \"%s\"", attribute);
+                               return NULL;
+                       }
 
-               if ((attr == 0) || (attr > 65535)) goto error;
+                       p = q;
+               }
 
-               attr |= (vendor << 16);
+               if (*p != '-') {
+                       librad_log("Invalid text following vendor definition in attribute name \"%s\"", attribute);
+                       return NULL;
+               }
+               p++;
+       }
 
-               /*
-                *      VendorName-Attr-%d
-                */
-       } else if (((p = strchr(attribute, '-')) != NULL) &&
-                  (strncasecmp(p, "-Attr-", 6) == 0)) {
-               int vendor;
-               char buffer[256];
+       /*
+        *      Attr-%d
+        */
+       if (strncasecmp(p, "Attr-", 5) != 0) {
+               librad_log("Invalid format in attribute name \"%s\"", attribute);
+               return NULL;
+       }
 
-               if (((size_t) (p - attribute)) >= sizeof(buffer)) goto error;
+       attr = strtol(p + 5, &q, 10);
 
-               memcpy(buffer, attribute, p - attribute);
-               buffer[p - attribute] = '\0';
+       /*
+        *      Invalid, or trailing text after number.
+        */
+       if ((attr == 0) || *q) {
+               librad_log("Invalid value in attribute name \"%s\"", attribute);
+               return NULL;
+       }
 
-               vendor = dict_vendorbyname(buffer);
-               if (vendor == 0) goto error;
+       /*
+        *      Double-check the size of attr.
+        */
+       if (vendor) {
+               DICT_VENDOR *dv = dict_vendorbyvalue(vendor);
 
-               p += 6;
-               attr = atoi(p);
+               if (!dv) {
+                       if (attr > 255) {
+                       attr_error:
+                               librad_log("Invalid attribute number in attribute name \"%s\"", attribute);
+                               return NULL;
+                       }
 
-               p += strspn(p, "0123456789");
-               if (*p != 0) goto error;
+               } else switch (dv->type) {
+                       case 1:
+                               if (attr > 255) goto attr_error;
+                               break;
 
-               if ((attr == 0) || (attr > 65535)) goto error;
+                       case 2:
+                               if (attr > 65535) goto attr_error;
+                               break;
 
-               attr |= (vendor << 16);
+                       case 4: /* Internal limitations! */
+                               if (attr > 65535) goto attr_error;
+                               break;
 
-       } else {                /* very much unknown: die */
-       error:
-               librad_log("Unknown attribute \"%s\"", attribute);
-               return NULL;
+                       default:
+                               librad_log("Internal sanity check failed");
+                               return NULL;
+               }
        }
 
+       attr |= vendor << 16;
+
        /*
         *      We've now parsed the attribute properly, Let's create
         *      it.  This next stop also looks the attribute up in the
@@ -1230,10 +1311,48 @@ static VALUE_PAIR *pairmake_any(const char *attribute, const char *value,
                return NULL;
        }
 
-       if (pairparsevalue(vp, value) == NULL) {
-               pairfree(&vp);
+       size = strlen(value + 2);
+
+       /*
+        *      We may be reading something like Attr-5.  i.e.
+        *      who-ever wrote the text didn't understand it, but we
+        *      do.
+        */
+       switch (vp->type) {
+       default:
+               if (size == (vp->length * 2)) break;
+               vp->type = PW_TYPE_OCTETS;
+               /* FALL-THROUGH */
+               
+       case PW_TYPE_STRING:
+       case PW_TYPE_OCTETS:
+       case PW_TYPE_ABINARY:
+               vp->length = size >> 1;
+               break;
+       }
+
+       if (fr_hex2bin(value + 2, vp->vp_octets, size) != vp->length) {
+               librad_log("Invalid hex string");
+               free(vp);
                return NULL;
        }
+
+       /*
+        *      Move contents around based on type.  This is
+        *      to work around the historical use of "lvalue".
+        */
+       switch (vp->type) {
+       case PW_TYPE_DATE:
+       case PW_TYPE_IPADDR:
+       case PW_TYPE_INTEGER:
+               memcpy(&vp->lvalue, vp->vp_octets, sizeof(vp->lvalue));
+               vp->vp_strvalue[0] = '\0';
+               break;
+               
+       default:
+               break;
+       }
+       
        vp->operator = (operator == 0) ? T_OP_EQ : operator;
 
        return vp;
@@ -1775,13 +1894,7 @@ int paircmp(VALUE_PAIR *one, VALUE_PAIR *two)
                break;
 
        case PW_TYPE_STRING:
-               if (one->flags.caseless) {
-                       compare = strcasecmp(two->vp_strvalue,
-                                            one->vp_strvalue);
-               } else {
-                       compare = strcmp(two->vp_strvalue,
-                                        one->vp_strvalue);
-               }
+               compare = strcmp(two->vp_strvalue, one->vp_strvalue);
                break;
 
        case PW_TYPE_BYTE:
index 67f66fa..5c1f6f8 100644 (file)
@@ -178,12 +178,17 @@ static void alvarion_vsa_hack(VALUE_PAIR *vp)
 
        for ( ; vp != NULL; vp = vp->next) {
                vendorcode = VENDOR(vp->attribute);
+               DICT_ATTR *da;
                if (vendorcode != 12394) continue;
                if (vp->type != PW_TYPE_STRING) continue;
 
-               vp->attribute = number | (12394 << 16);
-               snprintf(vp->name, sizeof(vp->name),
-                        "Breezecom-Attr%d", number++);
+               da = dict_attrbyvalue(number | (12394 << 16));
+               if (!da) continue;
+
+               vp->attribute = da->attr;
+               vp->name = da->name;
+
+               number++;
        }
 }