Don't do tlv checks on 'internal' attributes
[freeradius.git] / src / lib / dict.c
index 8ec6a95..a4ea0aa 100644 (file)
@@ -37,13 +37,6 @@ RCSID("$Id$")
 #include       <sys/stat.h>
 #endif
 
-
-#define DICT_VALUE_MAX_NAME_LEN (128)
-#define DICT_VENDOR_MAX_NAME_LEN (128)
-#define DICT_ATTR_MAX_NAME_LEN (128)
-
-#define DICT_ATTR_SIZE sizeof(DICT_ATTR) + DICT_ATTR_MAX_NAME_LEN
-
 static fr_hash_table_t *vendors_byname = NULL;
 static fr_hash_table_t *vendors_byvalue = NULL;
 
@@ -84,17 +77,17 @@ static value_fixup_t *value_fixup = NULL;
 const FR_NAME_NUMBER dict_attr_types[] = {
        { "integer",    PW_TYPE_INTEGER },
        { "string",     PW_TYPE_STRING },
-       { "ipaddr",     PW_TYPE_IPADDR },
+       { "ipaddr",     PW_TYPE_IPV4_ADDR },
        { "date",       PW_TYPE_DATE },
        { "abinary",    PW_TYPE_ABINARY },
        { "octets",     PW_TYPE_OCTETS },
        { "ifid",       PW_TYPE_IFID },
-       { "ipv6addr",   PW_TYPE_IPV6ADDR },
-       { "ipv6prefix", PW_TYPE_IPV6PREFIX },
+       { "ipv6addr",   PW_TYPE_IPV6_ADDR },
+       { "ipv6prefix", PW_TYPE_IPV6_PREFIX },
        { "byte",       PW_TYPE_BYTE },
        { "short",      PW_TYPE_SHORT },
        { "ether",      PW_TYPE_ETHERNET },
-       { "combo-ip",   PW_TYPE_COMBO_IP },
+       { "combo-ip",   PW_TYPE_COMBO_IP_ADDR },
        { "tlv",        PW_TYPE_TLV },
        { "signed",     PW_TYPE_SIGNED },
        { "extended",   PW_TYPE_EXTENDED },
@@ -106,8 +99,8 @@ const FR_NAME_NUMBER dict_attr_types[] = {
        { "int32",      PW_TYPE_SIGNED },
        { "integer64",  PW_TYPE_INTEGER64 },
        { "uint64",     PW_TYPE_INTEGER64 },
-       { "ipv4prefix", PW_TYPE_IPV4PREFIX },
-       { "cidr",       PW_TYPE_IPV4PREFIX },
+       { "ipv4prefix", PW_TYPE_IPV4_PREFIX },
+       { "cidr",       PW_TYPE_IPV4_PREFIX },
        { "vsa",        PW_TYPE_VSA },
        { NULL, 0 }
 };
@@ -116,27 +109,27 @@ const FR_NAME_NUMBER dict_attr_types[] = {
  *     Map data types to min / max data sizes.
  */
 const size_t dict_attr_sizes[PW_TYPE_MAX][2] = {
-       [PW_TYPE_INVALID]       = { ~0, 0 },
-       [PW_TYPE_STRING]        = { 0, ~0 },
+       [PW_TYPE_INVALID]       = {~0, 0},
+       [PW_TYPE_STRING]        = {0, ~0},
        [PW_TYPE_INTEGER]       = {4, 4 },
-       [PW_TYPE_IPADDR]        = {4, 4},
+       [PW_TYPE_IPV4_ADDR]     = {4, 4},
        [PW_TYPE_DATE]          = {4, 4},
        [PW_TYPE_ABINARY]       = {32, ~0},
        [PW_TYPE_OCTETS]        = {0, ~0},
        [PW_TYPE_IFID]          = {8, 8},
-       [PW_TYPE_IPV6ADDR]      = { 16, 16},
-       [PW_TYPE_IPV6PREFIX]    = {2, 18},
+       [PW_TYPE_IPV6_ADDR]     = {16, 16},
+       [PW_TYPE_IPV6_PREFIX]   = {2, 18},
        [PW_TYPE_BYTE]          = {1, 1},
        [PW_TYPE_SHORT]         = {2, 2},
        [PW_TYPE_ETHERNET]      = {6, 6},
        [PW_TYPE_SIGNED]        = {4, 4},
-       [PW_TYPE_COMBO_IP]      = {4, 16},
+       [PW_TYPE_COMBO_IP_ADDR] = {4, 16},
        [PW_TYPE_TLV]           = {2, ~0},
        [PW_TYPE_EXTENDED]      = {2, ~0},
        [PW_TYPE_LONG_EXTENDED] = {3, ~0},
        [PW_TYPE_EVS]           = {6, ~0},
        [PW_TYPE_INTEGER64]     = {8, 8},
-       [PW_TYPE_IPV4PREFIX]    = {6, 6},
+       [PW_TYPE_IPV4_PREFIX]   = {6, 6},
        [PW_TYPE_VSA]           = {4, ~0}
 };
 
@@ -164,15 +157,15 @@ const size_t dict_attr_sizes[PW_TYPE_MAX][2] = {
  *     5 bits for nested TLV 3
  *     3 bits for nested TLV 4
  */
-const int fr_attr_max_tlv = MAX_TLV_NEST;
-const int fr_attr_shift[MAX_TLV_NEST + 1] = {
-  0, 8, 16, 24, 29
-};
+int const fr_attr_max_tlv = MAX_TLV_NEST;
+int const fr_attr_shift[MAX_TLV_NEST + 1] = { 0, 8, 16, 24, 29 };
 
-const int fr_attr_mask[MAX_TLV_NEST + 1] = {
-  0xff, 0xff, 0xff, 0x1f, 0x07
-};
+int const fr_attr_mask[MAX_TLV_NEST + 1] = { 0xff, 0xff, 0xff, 0x1f, 0x07 };
 
+/*
+ *     attr & fr_attr_parent_mask[i] == Nth parent of attr
+ */
+static unsigned int const fr_attr_parent_mask[MAX_TLV_NEST + 1] = { 0, 0x000000ff, 0x0000ffff, 0x00ffffff, 0x1fffffff };
 
 /*
  *     Create the hash of the name.
@@ -605,41 +598,125 @@ int dict_addvendor(char const *name, unsigned int value)
        return 0;
 }
 
+const int dict_attr_allowed_chars[256] = {
+/* 0x   0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f */
+/* 0 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* 1 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* 2 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1,
+/* 3 */ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0,
+/* 4 */ 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+/* 5 */ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1,
+/* 6 */ 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+/* 7 */ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0,
+/* 8 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* 9 */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* a */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* b */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* c */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* d */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* e */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+/* f */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
+};
 
 /*
  *     [a-zA-Z0-9_-:.]+
  */
-const int dict_attr_allowed_chars[256] = {
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1,
-       1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0,
-       0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-       1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 1,
-       0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-       1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-       0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
-};
+int dict_valid_name(char const *name)
+{
+       uint8_t const *p;
+
+       for (p = (uint8_t const *) name; *p != '\0'; p++) {
+               if (!dict_attr_allowed_chars[*p]) {
+                       char buff[5];
+
+                       fr_prints(buff, sizeof(buff), (char const *)p, 1, '\'');
+                       fr_strerror_printf("Invalid character '%s' in attribute", buff);
+
+                       return -(p - (uint8_t const *)name);
+               }
+       }
+
+       return 0;
+}
 
 
 /*
- *     Add an attribute to the dictionary.
+ *     Bamboo skewers under the fingernails in 5, 4, 3, 2, ...
+ */
+static DICT_ATTR const *dict_parent(unsigned int attr, unsigned int vendor)
+{
+       int i;
+       unsigned int base_vendor;
+
+       /*
+        *      RFC attributes can't be of type "tlv".
+        */
+       if (!vendor) return NULL;
+
+       base_vendor = vendor & (FR_MAX_VENDOR - 1);
+
+       /*
+        *      It's a real vendor.
+        */
+       if (base_vendor != 0) {
+               DICT_VENDOR const *dv;
+
+               dv = dict_vendorbyvalue(base_vendor);
+               if (!dv) return NULL;
+
+               /*
+                *      Only standard format attributes can be of type "tlv",
+                *      Except for DHCP.  <sigh>
+                */
+               if ((vendor != 54) && ((dv->type != 1) || (dv->length != 1))) return NULL;
+
+               for (i = MAX_TLV_NEST; i > 0; i--) {
+                       unsigned int parent;
+
+                       parent = attr & fr_attr_parent_mask[i];
+
+                       if (parent != attr) return dict_attrbyvalue(parent, vendor); /* not base_vendor */
+               }
+
+               /*
+                *      It was a top-level VSA.  There's no parent.
+                *      We COULD return the appropriate enclosing VSA
+                *      (26, or 241.26, etc.) but that's not what we
+                *      want.
+                */
+               return NULL;
+       }
+
+       /*
+        *      It's an extended attribute.  Return the base Extended-Attr-X
+        */
+       if (attr < 256) return dict_attrbyvalue((vendor / FR_MAX_VENDOR) & 0xff, 0);
+
+       /*
+        *      Figure out which attribute it is.
+        */
+       for (i = MAX_TLV_NEST; i > 0; i--) {
+               unsigned int parent;
+
+               parent = attr & fr_attr_parent_mask[i];
+               if (parent != attr) return dict_attrbyvalue(parent, vendor); /* not base_vendor */
+       }
+
+       return NULL;
+}
+
+
+/** Add an attribute to the dictionary
+ *
+ * @return 0 on success -1 on failure.
  */
 int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                 ATTR_FLAGS flags)
 {
        size_t namelen;
-       static int      max_attr = 0;
-       uint8_t const *p;
-       DICT_ATTR const *da;
+       DICT_ATTR const *parent;
        DICT_ATTR *n;
+       static int      max_attr = 0;
 
        namelen = strlen(name);
        if (namelen >= DICT_ATTR_MAX_NAME_LEN) {
@@ -647,12 +724,7 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                return -1;
        }
 
-       for (p = (uint8_t const *) name; *p != '\0'; p++) {
-               if (!dict_attr_allowed_chars[*p]) {
-                       fr_strerror_printf("dict_addattr: Invalid character '%c' in attribute name", *p);
-                       return -1;
-               }
-       }
+       if (dict_valid_name(name) < 0) return -1;
 
        if (flags.has_tag &&
            !((type == PW_TYPE_INTEGER) || (type == PW_TYPE_STRING))) {
@@ -660,6 +732,13 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                return -1;
        }
 
+       /*
+        *      Disallow attributes of type zero.
+        */
+       if (!attr && !vendor) {
+               fr_strerror_printf("dict_addattr: Attribute 0 is invalid and cannot be used");
+               return -1;
+       }
 
        /*
         *      If the attr is '-1', that means use a pre-existing
@@ -684,11 +763,37 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
        }
 
        /*
+        *      Check the parent attribute, and set the various flags
+        *      based on the parents values.  It's OK for the caller
+        *      to not set them, as we'll set them.  But if the caller
+        *      sets them when he's not supposed to set them, that's
+        *      an error.
+        */
+       parent = dict_parent(attr, vendor);
+       if (parent) {
+               /*
+                *      We're still in the same space and the parent isn't a TLV.  That's an error.
+                *
+                *      Otherwise, dict_parent() has taken us from an Extended sub-attribute to
+                *      a *the* Extended attribute, whish isn't what we want here.
+                */
+               if (!flags.internal && (vendor == parent->vendor) && (parent->type != PW_TYPE_TLV)) {
+                       fr_strerror_printf("dict_addattr: Attribute %s has parent attribute %s which is not of type 'tlv'",
+                                          name, parent->name);
+                       return -1;
+               }
+
+               flags.extended |= parent->flags.extended;
+               flags.long_extended |= parent->flags.long_extended;
+               flags.evs |= parent->flags.evs;
+       }
+
+       /*
         *      Additional checks for extended attributes.
         */
        if (flags.extended || flags.long_extended || flags.evs) {
                if (vendor && (vendor < FR_MAX_VENDOR)) {
-                       fr_strerror_printf("dict_addattr: VSAs cannot use the \"extended\" or \"evs\" attribute formats.");
+                       fr_strerror_printf("dict_addattr: VSAs cannot use the \"extended\" or \"evs\" attribute formats");
                        return -1;
                }
                if (flags.has_tag
@@ -696,7 +801,7 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                    || flags.array
 #endif
                    || (flags.encrypt != FLAG_ENCRYPT_NONE)) {
-                       fr_strerror_printf("dict_addattr: The \"extended\" attributes MUST NOT have any flags set.");
+                       fr_strerror_printf("dict_addattr: The \"extended\" attributes MUST NOT have any flags set");
                        return -1;
                }
        }
@@ -715,19 +820,8 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
        }
 
        /*
-        *      Allow for generic pointers
+        *      Do various sanity checks.
         */
-       switch (type) {
-       default:
-               break;
-
-       case PW_TYPE_STRING:
-       case PW_TYPE_OCTETS:
-       case PW_TYPE_TLV:
-               flags.is_pointer = true;
-               break;
-       }
-
        if (attr < 0) {
                fr_strerror_printf("dict_addattr: ATTRIBUTE has invalid number (less than zero)");
                return -1;
@@ -739,22 +833,134 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
        }
 
        if (vendor && flags.concat) {
-               fr_strerror_printf("VSAs cannot have the \"concat\" flag set.");
+               fr_strerror_printf("VSAs cannot have the \"concat\" flag set");
                return -1;
        }
 
        if (flags.concat && (type != PW_TYPE_OCTETS)) {
-               fr_strerror_printf("The \"concat\" flag can only be set for attributes of type \"octets\".");
+               fr_strerror_printf("The \"concat\" flag can only be set for attributes of type \"octets\"");
                return -1;
        }
 
        if (flags.concat && (flags.has_tag || flags.array || flags.is_tlv || flags.has_tlv ||
                             flags.length || flags.evs || flags.extended || flags.long_extended ||
                             (flags.encrypt != FLAG_ENCRYPT_NONE))) {
-               fr_strerror_printf("The \"concat\" flag cannot be used with any other flag.");
+               fr_strerror_printf("The \"concat\" flag cannot be used with any other flag");
                return -1;
        }
 
+       if (flags.length && (type != PW_TYPE_OCTETS)) {
+               fr_strerror_printf("The \"length\" flag can only be set for attributes of type \"octets\"");
+               return -1;
+       }
+
+       if (flags.length && (flags.has_tag || flags.array || flags.is_tlv || flags.has_tlv ||
+                            flags.concat || flags.evs || flags.extended || flags.long_extended ||
+                            (flags.encrypt > FLAG_ENCRYPT_USER_PASSWORD))) {
+               fr_strerror_printf("The \"length\" flag cannot be used with any other flag");
+               return -1;
+       }
+
+       /*
+        *      Force "length" for data types of fixed length;
+        */
+       switch (type) {
+       case PW_TYPE_BYTE:
+               flags.length = 1;
+               break;
+
+       case PW_TYPE_SHORT:
+               flags.length = 2;
+               break;
+
+       case PW_TYPE_DATE:
+       case PW_TYPE_IPV4_ADDR:
+       case PW_TYPE_INTEGER:
+       case PW_TYPE_SIGNED:
+               flags.length = 4;
+               break;
+
+       case PW_TYPE_INTEGER64:
+               flags.length = 8;
+               break;
+
+       case PW_TYPE_ETHERNET:
+               flags.length = 6;
+               break;
+
+       case PW_TYPE_IFID:
+               flags.length = 8;
+               break;
+
+       case PW_TYPE_IPV6_ADDR:
+               flags.length = 16;
+               break;
+
+       case PW_TYPE_EXTENDED:
+               if ((vendor != 0) || (attr < 241)) {
+                       fr_strerror_printf("Attributes of type \"extended\" MUST be "
+                                          "RFC attributes with value >= 241.");
+                       return -1;
+               }
+
+               flags.length = 0;
+               flags.extended = 1;
+               break;
+
+       case PW_TYPE_LONG_EXTENDED:
+               if ((vendor != 0) || (attr < 241)) {
+                       fr_strerror_printf("Attributes of type \"long-extended\" MUST "
+                                          "be RFC attributes with value >= 241.");
+                       return -1;
+               }
+
+               flags.length = 0;
+               flags.extended = 1;
+               flags.long_extended = 1;
+               break;
+
+       case PW_TYPE_EVS:
+               if (attr != PW_VENDOR_SPECIFIC) {
+                       fr_strerror_printf("Attributes of type \"evs\" MUST have "
+                                          "attribute code 26.");
+                       return -1;
+               }
+
+               flags.length = 0;
+               flags.extended = 1;
+               flags.evs = 1;
+               break;
+
+       case PW_TYPE_STRING:
+       case PW_TYPE_OCTETS:
+       case PW_TYPE_TLV:
+               flags.is_pointer = true;
+               break;
+
+       default:
+               break;
+       }
+
+       /*
+        *      Stupid hacks for MS-CHAP-MPPE-Keys.  The User-Password
+        *      encryption method has no provisions for encoding the
+        *      length of the data.  For User-Password, the data is
+        *      (presumably) all printable non-zero data.  For
+        *      MS-CHAP-MPPE-Keys, the data is binary crap.  So... we
+        *      MUST specify a length in the dictionary.
+        */
+       if ((flags.encrypt == FLAG_ENCRYPT_USER_PASSWORD) && (type != PW_TYPE_STRING)) {
+               if (type != PW_TYPE_OCTETS) {
+                       fr_strerror_printf("The \"encrypt=1\" flag cannot be used with non-string data types");
+                       return -1;
+               }
+
+               if (flags.length == 0) {
+                       fr_strerror_printf("The \"encrypt=1\" flag MUST be used with an explicit length for 'octets' data types");
+                       return -1;
+               }
+       }
+
        if ((vendor & (FR_MAX_VENDOR -1)) != 0) {
                DICT_VENDOR *dv;
                static DICT_VENDOR *last_vendor = NULL;
@@ -800,12 +1006,18 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                        return -1;
                }
 
+               if (!attr && dv->type != 1) {
+                       fr_strerror_printf("dict_addattr: Attribute %s cannot have value zero",
+                                          name);
+                       return -1;
+               }
+
                /*
                 *      FIXME: Switch over dv->type, and limit things
                 *      properly.
                 */
                if ((dv->type == 1) && (attr >= 256) && !flags.is_tlv) {
-                       fr_strerror_printf("dict_addattr: ATTRIBUTE has invalid number (larger than 255).");
+                       fr_strerror_printf("dict_addattr: ATTRIBUTE has invalid number (larger than 255)");
                        return -1;
                } /* else 256..65535 are allowed */
 
@@ -821,24 +1033,12 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
                 *      based on what we see.
                 */
                if (vendor >= FR_MAX_VENDOR) {
-                       unsigned int parent;
-
-                       parent = (vendor / FR_MAX_VENDOR) & 0xff;
-
-                       da = dict_attrbyvalue(parent, 0);
-                       if (!da) {
-                               fr_strerror_printf("dict_addattr: ATTRIBUTE refers to unknown parent attribute %u.", parent);
+                       if (!parent) {
+                               fr_strerror_printf("dict_addattr: ATTRIBUTE refers to unknown parent attribute");
                                return -1;
                        }
 
                        /*
-                        *      These flags are inherited from the
-                        *      parent.
-                        */
-                       flags.extended = da->flags.extended;
-                       flags.long_extended = da->flags.long_extended;
-
-                       /*
                         *      Non-extended attributes can't have VSAs.
                         */
                        if (!flags.extended &&
@@ -931,20 +1131,20 @@ int dict_addattr(char const *name, int attr, unsigned int vendor, PW_TYPE type,
        /*
         *      Hacks for combo-IP
         */
-       if (n->type == PW_TYPE_COMBO_IP) {
+       if (n->type == PW_TYPE_COMBO_IP_ADDR) {
                DICT_ATTR *v4, *v6;
 
-               v4 = fr_pool_alloc(sizeof(*v4));
+               v4 = fr_pool_alloc(sizeof(*v4) + namelen);
                if (!v4) goto oom;
 
-               v6 = fr_pool_alloc(sizeof(*v6));
+               v6 = fr_pool_alloc(sizeof(*v6) + namelen);
                if (!v6) goto oom;
 
-               memcpy(v4, n, sizeof(*v4));
-               v4->type = PW_TYPE_IPADDR;
+               memcpy(v4, n, sizeof(*v4) + namelen);
+               v4->type = PW_TYPE_IPV4_ADDR;
 
-               memcpy(v6, n, sizeof(*v6));
-               v6->type = PW_TYPE_IPV6ADDR;
+               memcpy(v6, n, sizeof(*v6) + namelen);
+               v6->type = PW_TYPE_IPV6_ADDR;
                if (!fr_hash_table_replace(attributes_combo, v4)) {
                        fr_strerror_printf("dict_addattr: Failed inserting attribute name %s - IPv4", name);
                        return -1;
@@ -1025,36 +1225,36 @@ int dict_addvalue(char const *namestr, char const *attrstr, int value)
                 *      Don't worry about fixups...
                 */
                switch (da->type) {
-                       case PW_TYPE_BYTE:
-                               if (value > 255) {
-                                       fr_pool_free(dval);
-                                       fr_strerror_printf("dict_addvalue: ATTRIBUTEs of type 'byte' cannot have VALUEs larger than 255");
-                                       return -1;
-                               }
-                               break;
-                       case PW_TYPE_SHORT:
-                               if (value > 65535) {
-                                       fr_pool_free(dval);
-                                       fr_strerror_printf("dict_addvalue: ATTRIBUTEs of type 'short' cannot have VALUEs larger than 65535");
-                                       return -1;
-                               }
-                               break;
+               case PW_TYPE_BYTE:
+                       if (value > 255) {
+                               fr_pool_free(dval);
+                               fr_strerror_printf("dict_addvalue: ATTRIBUTEs of type 'byte' cannot have VALUEs larger than 255");
+                               return -1;
+                       }
+                       break;
+               case PW_TYPE_SHORT:
+                       if (value > 65535) {
+                               fr_pool_free(dval);
+                               fr_strerror_printf("dict_addvalue: ATTRIBUTEs of type 'short' cannot have VALUEs larger than 65535");
+                               return -1;
+                       }
+                       break;
 
-                               /*
-                                *      Allow octets for now, because
-                                *      of dictionary.cablelabs
-                                */
-                       case PW_TYPE_OCTETS:
+                       /*
+                        *      Allow octets for now, because
+                        *      of dictionary.cablelabs
+                        */
+               case PW_TYPE_OCTETS:
 
-                       case PW_TYPE_INTEGER:
-                               break;
+               case PW_TYPE_INTEGER:
+                       break;
 
-                       case PW_TYPE_INTEGER64:
-                       default:
-                               fr_pool_free(dval);
-                               fr_strerror_printf("dict_addvalue: VALUEs cannot be defined for attributes of type '%s'",
-                                          fr_int2str(dict_attr_types, da->type, "?Unknown?"));
-                               return -1;
+               case PW_TYPE_INTEGER64:
+               default:
+                       fr_pool_free(dval);
+                       fr_strerror_printf("dict_addvalue: VALUEs cannot be defined for attributes of type '%s'",
+                                  fr_int2str(dict_attr_types, da->type, "?Unknown?"));
+                       return -1;
                }
        } else {
                value_fixup_t *fixup;
@@ -1186,7 +1386,7 @@ int dict_str2oid(char const *ptr, unsigned int *pvalue, unsigned int *pvendor,
        if (*pvalue) {
                da = dict_attrbyvalue(*pvalue, *pvendor);
                if (!da) {
-                       fr_strerror_printf("Parent attribute is undefined.");
+                       fr_strerror_printf("Parent attribute is undefined");
                        return -1;
                }
 
@@ -1279,22 +1479,6 @@ int dict_str2oid(char const *ptr, unsigned int *pvalue, unsigned int *pvendor,
        return tlv_depth;
 }
 
-/*
- *     Bamboo skewers under the fingernails in 5, 4, 3, 2, ...
- */
-static DICT_ATTR const *dict_parent(unsigned int attr, unsigned int vendor)
-{
-       if (vendor < FR_MAX_VENDOR) {
-               return dict_attrbyvalue(attr & 0xff, vendor);
-       }
-
-       if (attr < 256) {
-               return dict_attrbyvalue((vendor / FR_MAX_VENDOR) & 0xff, 0);
-       }
-
-       return dict_attrbyvalue(attr & 0xff, vendor);
-}
-
 
 /*
  *     Process the ATTRIBUTE command
@@ -1308,7 +1492,7 @@ static int process_attribute(char const* fn, int const line,
        unsigned int    vendor = 0;
        unsigned int    value;
        int             type;
-       unsigned int    length = 0;
+       unsigned int    length;
        ATTR_FLAGS      flags;
        char            *p;
 
@@ -1407,81 +1591,14 @@ static int process_attribute(char const* fn, int const line,
                        fr_strerror_printf("dict_init: %s[%d]: invalid length", fn, line);
                        return -1;
                }
+
+               flags.length = length;
        }
 
        /*
-        *      Only look up the vendor if the string
-        *      is non-empty.
+        *      Parse options.
         */
-       if (argc < 4) {
-               /*
-                *      Force "length" for data types of fixed length;
-                */
-               switch (type) {
-               case PW_TYPE_BYTE:
-                       length = 1;
-                       break;
-
-               case PW_TYPE_SHORT:
-                       length = 2;
-                       break;
-
-               case PW_TYPE_DATE:
-               case PW_TYPE_IPADDR:
-               case PW_TYPE_INTEGER:
-               case PW_TYPE_SIGNED:
-                       length = 4;
-                       break;
-
-               case PW_TYPE_INTEGER64:
-                       length = 8;
-                       break;
-
-               case PW_TYPE_ETHERNET:
-                       length = 6;
-                       break;
-
-               case PW_TYPE_IFID:
-                       length = 8;
-                       break;
-
-               case PW_TYPE_IPV6ADDR:
-                       length = 16;
-                       break;
-
-               case PW_TYPE_EXTENDED:
-                       if ((vendor != 0) || (value < 241)) {
-                               fr_strerror_printf("dict_init: %s[%d]: Attributes of type \"extended\" MUST be RFC attributes with value >= 241.", fn, line);
-                               return -1;
-                       }
-                       flags.extended = 1;
-                       break;
-
-               case PW_TYPE_LONG_EXTENDED:
-                       if ((vendor != 0) || (value < 241)) {
-                               fr_strerror_printf("dict_init: %s[%d]: Attributes of type \"long-extended\" MUST be RFC attributes with value >= 241.", fn, line);
-                               return -1;
-                       }
-                       flags.extended = 1;
-                       flags.long_extended = 1;
-                       break;
-
-               case PW_TYPE_EVS:
-                       flags.extended = 1;
-                       flags.evs = 1;
-                       if (value != PW_VENDOR_SPECIFIC) {
-                               fr_strerror_printf("dict_init: %s[%d]: Attributes of type \"evs\" MUST have attribute code 26.", fn, line);
-                               return -1;
-                       }
-                       break;
-
-               default:
-                       break;
-               }
-
-               flags.length = length;
-
-       } else {                /* argc == 4: we have options */
+       if (argc >= 4) {
                char *key, *next, *last;
 
                /*
@@ -1492,38 +1609,36 @@ static int process_attribute(char const* fn, int const line,
                        return -1;
                }
 
-               if (length != 0) {
-                       fr_strerror_printf("dict_init: %s[%d]: length cannot be used with options", fn, line);
-                       return -1;
-               }
-
                key = argv[3];
                do {
                        next = strchr(key, ',');
                        if (next) *(next++) = '\0';
 
-                       if (strcmp(key, "has_tag") == 0 ||
-                           strcmp(key, "has_tag=1") == 0) {
-                               /* Boolean flag, means this is a
-                                  tagged attribute */
+                       /*
+                        *      Boolean flag, means this is a tagged
+                        *      attribute.
+                        */
+                       if ((strcmp(key, "has_tag") == 0) || (strcmp(key, "has_tag=1") == 0)) {
                                flags.has_tag = 1;
 
+                       /*
+                        *      Encryption method, defaults to 0 (none).
+                        *      Currently valid is just type 2,
+                        *      Tunnel-Password style, which can only
+                        *      be applied to strings.
+                        */
                        } else if (strncmp(key, "encrypt=", 8) == 0) {
-                               /* Encryption method, defaults to 0 (none).
-                                  Currently valid is just type 2,
-                                  Tunnel-Password style, which can only
-                                  be applied to strings. */
                                flags.encrypt = strtol(key + 8, &last, 0);
                                if (*last) {
-                                       fr_strerror_printf( "dict_init: %s[%d] invalid option %s",
-                                                   fn, line, key);
+                                       fr_strerror_printf("dict_init: %s[%d] invalid option %s",
+                                                          fn, line, key);
                                        return -1;
                                }
 
                                if ((flags.encrypt == FLAG_ENCRYPT_ASCEND_SECRET) &&
                                    (type != PW_TYPE_STRING)) {
-                                       fr_strerror_printf( "dict_init: %s[%d] Only \"string\" types can have the \"encrypt=3\" flag set.",
-                                                           fn, line);
+                                       fr_strerror_printf("dict_init: %s[%d] Only \"string\" types can have the "
+                                                          "\"encrypt=3\" flag set", fn, line);
                                        return -1;
                                }
 
@@ -1531,42 +1646,61 @@ static int process_attribute(char const* fn, int const line,
                                flags.array = 1;
 
                                switch (type) {
-                                       case PW_TYPE_IPADDR:
-                                       case PW_TYPE_BYTE:
-                                       case PW_TYPE_SHORT:
-                                       case PW_TYPE_INTEGER:
-                                       case PW_TYPE_DATE:
-                                               break;
-
-                                       default:
-                                               fr_strerror_printf( "dict_init: %s[%d] Only IP addresses can have the \"array\" flag set.",
-                                                           fn, line);
-                                               return -1;
+                               case PW_TYPE_IPV4_ADDR:
+                               case PW_TYPE_IPV6_ADDR:
+                               case PW_TYPE_BYTE:
+                               case PW_TYPE_SHORT:
+                               case PW_TYPE_INTEGER:
+                               case PW_TYPE_DATE:
+                               case PW_TYPE_STRING:
+                                       break;
+
+                               default:
+                                       fr_strerror_printf("dict_init: %s[%d] \"%s\" type cannot have the "
+                                                          "\"array\" flag set",
+                                                          fn, line,
+                                                          fr_int2str(dict_attr_types, type, "<UNKNOWN>"));
+                                       return -1;
                                }
 
-                       } else if (strncmp(key, "concat", 6) == 0) {
+                       } else if (strncmp(key, "concat", 7) == 0) {
                                flags.concat = 1;
 
                                if (type != PW_TYPE_OCTETS) {
-                                               fr_strerror_printf( "dict_init: %s[%d] Only \"octets\" type can have the \"concat\" flag set.",
-                                                           fn, line);
-                                               return -1;
+                                       fr_strerror_printf("dict_init: %s[%d] Only \"octets\" type can have the "
+                                                          "\"concat\" flag set", fn, line);
+                                       return -1;
                                }
 
-                               /*
-                                *      The only thing is the vendor name,
-                                *      and it's a known name: allow it.
-                                */
+                       } else if (strncmp(key, "virtual", 8) == 0) {
+                               flags.virtual = 1;
+
+                               if (vendor != 0) {
+                                       fr_strerror_printf("dict_init: %s[%d] VSAs cannot have the \"virtual\" "
+                                                          "flag set", fn, line);
+                                       return -1;
+                               }
+
+                               if (value < 256) {
+                                       fr_strerror_printf("dict_init: %s[%d] Standard attributes cannot "
+                                                          "have the \"virtual\" flag set", fn, line);
+                                       return -1;
+                               }
+
+                       /*
+                        *      The only thing is the vendor name,
+                        *      and it's a known name: allow it.
+                        */
                        } else if ((key == argv[3]) && !next) {
                                if (oid) {
-                                       fr_strerror_printf( "dict_init: %s[%d] New-style attributes cannot use a vendor flag.",
-                                                           fn, line);
+                                       fr_strerror_printf("dict_init: %s[%d] New-style attributes cannot use "
+                                                          "a vendor flag", fn, line);
                                        return -1;
                                }
 
                                if (block_vendor) {
-                                       fr_strerror_printf( "dict_init: %s[%d] Vendor flag inside of \"BEGIN-VENDOR\" is not allowed.",
-                                                           fn, line);
+                                       fr_strerror_printf("dict_init: %s[%d] Vendor flag inside of \"BEGIN-VENDOR\" "
+                                                          "is not allowed", fn, line);
                                        return -1;
                                }
 
@@ -1576,8 +1710,7 @@ static int process_attribute(char const* fn, int const line,
 
                        } else {
                        unknown:
-                               fr_strerror_printf( "dict_init: %s[%d]: unknown option \"%s\"",
-                                           fn, line, key);
+                               fr_strerror_printf("dict_init: %s[%d]: unknown option \"%s\"", fn, line, key);
                                return -1;
                        }
 
@@ -1639,7 +1772,7 @@ static int process_attribute(char const* fn, int const line,
                }
 
                /*
-                *
+                *      Shift the value left.
                 */
                value <<= fr_attr_shift[tlv_depth];
                value |= block_tlv->attr;
@@ -1788,6 +1921,74 @@ static int process_value_alias(char const* fn, int const line, char **argv,
 }
 
 
+static int parse_format(char const *fn, int line, char const *format, int *pvalue, int *ptype, int *plength, bool *pcontinuation)
+{
+       char const *p;
+       int type, length;
+       bool continuation = false;
+
+       if (strncasecmp(format, "format=", 7) != 0) {
+               fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected \"format=\", got \"%s\"",
+                                  fn, line, format);
+               return -1;
+       }
+
+       p = format + 7;
+       if ((strlen(p) < 3) ||
+           !isdigit((int) p[0]) ||
+           (p[1] != ',') ||
+           !isdigit((int) p[2]) ||
+           (p[3] && (p[3] != ','))) {
+               fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
+                                  fn, line, p);
+               return -1;
+       }
+
+       type = (int) (p[0] - '0');
+       length = (int) (p[2] - '0');
+
+       if ((type != 1) && (type != 2) && (type != 4)) {
+               fr_strerror_printf("dict_init: %s[%d]: invalid type value %d for VENDOR",
+                                  fn, line, type);
+               return -1;
+       }
+
+       if ((length != 0) && (length != 1) && (length != 2)) {
+               fr_strerror_printf("dict_init: %s[%d]: invalid length value %d for VENDOR",
+                                  fn, line, length);
+               return -1;
+       }
+
+       if (p[3] == ',') {
+               if (!p[4]) {
+                       fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
+                                          fn, line, p);
+                       return -1;
+               }
+
+               if ((p[4] != 'c') ||
+                   (p[5] != '\0')) {
+                       fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
+                                          fn, line, p);
+                       return -1;
+               }
+               continuation = true;
+
+               if ((*pvalue != VENDORPEC_WIMAX) ||
+                   (type != 1) || (length != 1)) {
+                       fr_strerror_printf("dict_init: %s[%d]: Only WiMAX VSAs can have continuations",
+                                          fn, line);
+                       return -1;
+               }
+       }
+
+       *ptype = type;
+       *plength = length;
+       *pcontinuation = continuation;
+       return 0;
+}
+
+
 /*
  *     Process the VENDOR command
  */
@@ -1795,8 +1996,9 @@ static int process_vendor(char const* fn, int const line, char **argv,
                          int argc)
 {
        int             value;
+       int             type, length;
        bool            continuation = false;
-       char const      *format = NULL;
+       DICT_VENDOR     *dv;
 
        if ((argc < 2) || (argc > 3)) {
                fr_strerror_printf( "dict_init: %s[%d] invalid VENDOR entry",
@@ -1826,94 +2028,40 @@ static int process_vendor(char const* fn, int const line, char **argv,
        }
 
        /*
-        *      Look for a format statement
+        *      Look for a format statement.  Allow it to over-ride the hard-coded formats below.
         */
        if (argc == 3) {
-               format = argv[2];
+               if (parse_format(fn, line, argv[2], &value, &type, &length, &continuation) < 0) {
+                       return -1;
+               }
 
        } else if (value == VENDORPEC_USR) { /* catch dictionary screw-ups */
-               format = "format=4,0";
+               type = 4;
+               length = 0;
 
        } else if (value == VENDORPEC_LUCENT) {
-               format = "format=2,1";
+               type = 2;
+               length = 1;
 
        } else if (value == VENDORPEC_STARENT) {
-               format = "format=2,2";
+               type = 2;
+               length = 2;
 
-       } /* else no fixups to do */
-
-       if (format) {
-               int type, length;
-               char const *p;
-               DICT_VENDOR *dv;
-
-               if (strncasecmp(format, "format=", 7) != 0) {
-                       fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected \"format=\", got \"%s\"",
-                                  fn, line, format);
-                       return -1;
-               }
-
-               p = format + 7;
-               if ((strlen(p) < 3) ||
-                   !isdigit((int) p[0]) ||
-                   (p[1] != ',') ||
-                   !isdigit((int) p[2]) ||
-                   (p[3] && (p[3] != ','))) {
-                       fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
-                                  fn, line, p);
-                       return -1;
-               }
-
-               type = (int) (p[0] - '0');
-               length = (int) (p[2] - '0');
-
-               if (p[3] == ',') {
-                       if (!p[4]) {
-                               fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
-                                  fn, line, p);
-                               return -1;
-                       }
-
-                       if ((p[4] != 'c') ||
-                           (p[5] != '\0')) {
-                               fr_strerror_printf("dict_init: %s[%d]: Invalid format for VENDOR.  Expected text like \"1,1\", got \"%s\"",
-                                          fn, line, p);
-                               return -1;
-                       }
-                       continuation = true;
-
-                       if ((value != VENDORPEC_WIMAX) ||
-                           (type != 1) || (length != 1)) {
-                               fr_strerror_printf("dict_init: %s[%d]: Only WiMAX VSAs can have continuations",
-                                          fn, line);
-                               return -1;
-                       }
-               }
+       } else {
+               type = length = 1;
+       }
 
-               dv = dict_vendorbyvalue(value);
-               if (!dv) {
-                       fr_strerror_printf("dict_init: %s[%d]: Failed adding format for VENDOR",
+       dv = dict_vendorbyvalue(value);
+       if (!dv) {
+               fr_strerror_printf("dict_init: %s[%d]: Failed adding format for VENDOR",
                                   fn, line);
-                       return -1;
-               }
-
-               if ((type != 1) && (type != 2) && (type != 4)) {
-                       fr_strerror_printf("dict_init: %s[%d]: invalid type value %d for VENDOR",
-                                  fn, line, type);
-                       return -1;
-               }
-
-               if ((length != 0) && (length != 1) && (length != 2)) {
-                       fr_strerror_printf("dict_init: %s[%d]: invalid length value %d for VENDOR",
-                                  fn, line, length);
-                       return -1;
-               }
-
-               dv->type = type;
-               dv->length = length;
-               dv->flags = continuation;
+               return -1;
        }
 
+       dv->type = type;
+       dv->length = length;
+       dv->flags = continuation;
+
        return 0;
 }
 
@@ -2316,7 +2464,7 @@ static int my_dict_init(char const *parent, char const *filename,
                                 *      attribute into the upper 8
                                 *      bits of the vendor ID
                                 */
-                               block_vendor |= (da->attr & fr_attr_mask[0]) * FR_MAX_VENDOR;
+                               block_vendor |= da->vendor;
                        }
 
                        continue;
@@ -2541,16 +2689,17 @@ static size_t print_attr_oid(char *buffer, size_t size, unsigned int attr,
        size_t len;
 
        switch (dv_type) {
+       default:
+       case 1:
+               len = snprintf(buffer, size, "%u", attr & 0xff);
+               break;
+
        case 4:
                return snprintf(buffer, size, "%u", attr);
 
        case 2:
                return snprintf(buffer, size, "%u", attr & 0xffff);
 
-       default:
-       case 1:
-               len = snprintf(buffer, size, "%u", attr & 0xff);
-               break;
        }
 
        if ((attr >> 8) == 0) return len;
@@ -2592,79 +2741,36 @@ void dict_attr_free(DICT_ATTR const **da)
        }
 
        memcpy(&tmp, &da, sizeof(*tmp));
-       free(*tmp);
+       talloc_free(*tmp);
 
        *tmp = NULL;
 }
 
-/** Copies a dictionary attr
- *
- * If the attr is dynamically allocated (unknown attribute), then it will be
- * copied to a new attr.
- *
- * If the attr is known, a pointer to the da will be returned.
- *
- * @param da to copy.
- * @param vp_free if true, da will be freed at the same time as the
- *     VALUE_PAIR which contains it.
- * @return return a copy of the da.
- */
-DICT_ATTR const *dict_attr_copy(DICT_ATTR const *da, int vp_free)
-{
-       DICT_ATTR *copy;
-
-       if (!da) return NULL;
-
-       if (!da->flags.is_unknown) {
-               return da;
-       }
-
-       copy = malloc(DICT_ATTR_SIZE);
-       if (!copy) {
-               fr_strerror_printf("Out of memory");
-               return NULL;
-       }
-
-       memcpy(copy, da, DICT_ATTR_SIZE);
-       copy->flags.vp_free = (vp_free != 0);
-
-       return copy;
-}
 
-
-/** Allocs an dictionary attr for unknown attributes
- *
- * Allocates a dict attr for an unknown attribute/vendor/type
- * without adding it to dictionary pools/hashes.
+/** Initialises a dictionary attr for unknown attributes
  *
- * @note Must be freed with dict_attr_free if not used as part of a valuepair.
+ * Initialises a dict attr for an unknown attribute/vendor/type without adding
+ * it to dictionary pools/hashes.
  *
+ * @param[in,out] da struct to initialise, must be at least DICT_ATTR_SIZE bytes.
  * @param[in] attr number.
  * @param[in] vendor number.
- * @param[in] vp_free if > 0 DICT_ATTR will be freed on VALUE_PAIR free.
- * @return new dictionary attribute.
+ * @return 0 on success.
  */
-DICT_ATTR const *dict_attrunknown(unsigned int attr, unsigned int vendor,
-                                 int vp_free)
+int dict_unknown_from_fields(DICT_ATTR *da, unsigned int attr, unsigned int vendor)
 {
-       DICT_ATTR *da;
        char *p;
        int dv_type = 1;
        size_t len = 0;
        size_t bufsize = DICT_ATTR_MAX_NAME_LEN;
 
-       da = malloc(DICT_ATTR_SIZE);
-       if (!da) {
-               fr_strerror_printf("Out of memory");
-               return NULL;
-       }
        memset(da, 0, DICT_ATTR_SIZE);
 
        da->attr = attr;
        da->vendor = vendor;
        da->type = PW_TYPE_OCTETS;
        da->flags.is_unknown = true;
-       da->flags.vp_free = (vp_free != 0);
+       da->flags.is_pointer = true;
 
        /*
         *      Unknown attributes of the "WiMAX" vendor get marked up
@@ -2707,6 +2813,37 @@ DICT_ATTR const *dict_attrunknown(unsigned int attr, unsigned int vendor,
 
        print_attr_oid(p, bufsize , attr, dv_type);
 
+       return 0;
+}
+
+/** Allocs a dictionary attr for unknown attributes
+ *
+ * Allocs a dict attr for an unknown attribute/vendor/type without adding
+ * it to dictionary pools/hashes.
+ *
+ * @param[in] ctx to allocate DA in.
+ * @param[in] attr number.
+ * @param[in] vendor number.
+ * @return 0 on success.
+ */
+DICT_ATTR const *dict_unknown_afrom_fields(TALLOC_CTX *ctx, unsigned int attr, unsigned int vendor)
+{
+       uint8_t *p;
+       DICT_ATTR *da;
+
+       p = talloc_zero_array(ctx, uint8_t, DICT_ATTR_SIZE);
+       if (!p) {
+               fr_strerror_printf("Out of memory");
+               return NULL;
+       }
+       da = (DICT_ATTR *) p;
+       talloc_set_type(da, DICT_ATTR);
+
+       if (dict_unknown_from_fields(da, attr, vendor) < 0) {
+               talloc_free(p);
+               return NULL;
+       }
+
        return da;
 }
 
@@ -2718,22 +2855,22 @@ DICT_ATTR const *dict_attrunknown(unsigned int attr, unsigned int vendor,
  *  - Vendor-%d-Attr-%d
  *  - VendorName-Attr-%d
  *
- * @todo should check attr/vendor against dictionary and return the real da.
- *
- * @param[in] attribute name.
- * @param[in] vp_free if > 0 DICT_ATTR will be freed on VALUE_PAIR free.
- * @return new da or NULL on error.
+ * @param[in] da to initialise.
+ * @param[in] name of attribute.
+ * @return 0 on success -1 on failure.
  */
-DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
+int dict_unknown_from_str(DICT_ATTR *da, char const *name)
 {
        unsigned int    attr, vendor = 0;
        unsigned int    dv_type = 1;    /* The type of vendor field */
 
-       char const      *p = attribute;
+       char const      *p = name;
        char            *q;
 
        DICT_VENDOR     *dv;
-       DICT_ATTR const *da;
+       DICT_ATTR const *found;
+
+       if (dict_valid_name(name) < 0) return -1;
 
        /*
         *      Pull off vendor prefix first.
@@ -2742,10 +2879,9 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
                if (strncasecmp(p, "Vendor-", 7) == 0) {
                        vendor = (int) strtol(p + 7, &q, 10);
                        if ((vendor == 0) || (vendor > FR_MAX_VENDOR)) {
-                               fr_strerror_printf("Invalid vendor value in "
-                                                  "attribute name \"%s\"",
-                                                  attribute);
-                               return NULL;
+                               fr_strerror_printf("Invalid vendor value in attribute name \"%s\"", name);
+
+                               return -1;
                        }
 
                        p = q;
@@ -2757,17 +2893,14 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
                        q = strchr(p, '-');
 
                        if (!q) {
-                               fr_strerror_printf("Invalid vendor name in "
-                                                  "attribute name \"%s\"",
-                                                  attribute);
-                               return NULL;
+                               fr_strerror_printf("Invalid vendor name in attribute name \"%s\"", name);
+                               return -1;
                        }
 
                        if ((size_t) (q - p) >= sizeof(buffer)) {
-                               fr_strerror_printf("Vendor name too long "
-                                                  "in attribute name \"%s\"",
-                                                  attribute);
-                               return NULL;
+                               fr_strerror_printf("Vendor name too long in attribute name \"%s\"", name);
+
+                               return -1;
                        }
 
                        memcpy(buffer, p, (q - p));
@@ -2775,19 +2908,18 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
 
                        vendor = dict_vendorbyname(buffer);
                        if (!vendor) {
-                               fr_strerror_printf("Unknown attribute \"%s\"",
-                                                  attribute);
-                               return NULL;
+                               fr_strerror_printf("Unknown name \"%s\"", name);
+
+                               return -1;
                        }
 
                        p = q;
                }
 
                if (*p != '-') {
-                       fr_strerror_printf("Invalid text following vendor "
-                                          "definition in attribute name "
-                                          "\"%s\"", attribute);
-                       return NULL;
+                       fr_strerror_printf("Invalid text following vendor definition in attribute name \"%s\"", name);
+
+                       return -1;
                }
                p++;
        }
@@ -2796,20 +2928,20 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
         *      Attr-%d
         */
        if (strncasecmp(p, "Attr-", 5) != 0) {
-               fr_strerror_printf("Unknown attribute \"%s\"",
-                                  attribute);
-               return NULL;
+               fr_strerror_printf("Unknown attribute \"%s\"", name);
+
+               return -1;
        }
 
        attr = strtol(p + 5, &q, 10);
 
        /*
-        *      Invalid attribute.
+        *      Invalid name.
         */
        if (attr == 0) {
-               fr_strerror_printf("Invalid value in attribute name \"%s\"",
-                                  attribute);
-               return NULL;
+               fr_strerror_printf("Invalid value in attribute name \"%s\"", name);
+
+               return -1;
        }
 
        p = q;
@@ -2826,7 +2958,7 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
            ((vendor == 0) && *p && (*p != '.'))) {
        invalid:
                fr_strerror_printf("Invalid OID");
-               return NULL;
+               return -1;
        }
 
        /*
@@ -2836,43 +2968,42 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
         *
         *      This section parses the Vendor-Id portion of
         *      Attr-%d.%d.  where the first number is 26, *or* an
-        *      extended attribute of the "evs" data type.
+        *      extended name of the "evs" foundta type.
         */
        if (*p == '.') {
-               da = dict_attrbyvalue(attr, 0);
-               if (!da) {
-                       fr_strerror_printf("Cannot parse attributes without "
-                                          "dictionaries");
-                       return NULL;
+               found = dict_attrbyvalue(attr, 0);
+               if (!found) {
+                       fr_strerror_printf("Cannot parse names without dictionaries");
+
+                       return -1;
                }
 
                if ((attr != PW_VENDOR_SPECIFIC) &&
-                   !(da->flags.extended || da->flags.long_extended)) {
-                       fr_strerror_printf("Standard attributes cannot use "
-                                          "OIDs");
-                       return NULL;
+                   !(found->flags.extended || found->flags.long_extended)) {
+                       fr_strerror_printf("Standard attributes cannot use OIDs");
+
+                       return -1;
                }
 
-               if ((attr == PW_VENDOR_SPECIFIC) || da->flags.evs) {
+               if ((attr == PW_VENDOR_SPECIFIC) || found->flags.evs) {
                        vendor = strtol(p + 1, &q, 10);
                        if ((vendor == 0) || (vendor > FR_MAX_VENDOR)) {
                                fr_strerror_printf("Invalid vendor");
-                               return NULL;
+
+                               return -1;
                        }
 
                        if (*q != '.') goto invalid;
 
                        p = q;
 
-                       if (da->flags.evs) {
-                               vendor |= attr * FR_MAX_VENDOR;
-                       }
+                       if (found->flags.evs) vendor |= attr * FR_MAX_VENDOR;
                        attr = 0;
                } /* else the second number is a TLV number */
        }
 
        /*
-        *      Get the expected maximum size of the attribute.
+        *      Get the expected maximum size of the name.
         */
        if (vendor) {
                dv = dict_vendorbyvalue(vendor & (FR_MAX_VENDOR - 1));
@@ -2889,8 +3020,8 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
        if (*p == '.') {
                attr = strtol(p + 1, &q, 10);
                if (attr == 0) {
-                       fr_strerror_printf("Invalid attribute number");
-                       return NULL;
+                       fr_strerror_printf("Invalid name number");
+                       return -1;
                }
 
                if (*q) {
@@ -2909,15 +3040,103 @@ DICT_ATTR const *dict_attrunknownbyname(char const *attribute, int vp_free)
        /*
         *      Enforce a maximum value on the attribute number.
         */
-       if (attr >= (unsigned) (1 << (dv_type << 3))) goto invalid;
+       if ((vendor > 0) && (attr >= (unsigned) (1 << (dv_type << 3)))) goto invalid;
 
        if (*p == '.') {
                if (dict_str2oid(p + 1, &attr, &vendor, 1) < 0) {
-                       return NULL;
+                       return -1;
                }
        }
 
-       return dict_attrunknown(attr, vendor, vp_free);
+       /*
+        *      If the caller doesn't provide a DICT_ATTR
+        *      we can't call dict_unknown_from_fields.
+        */
+       if (!da) {
+               fr_strerror_printf("Unknown attributes disallowed");
+               return -1;
+       }
+
+       return dict_unknown_from_fields(da, attr, vendor);
+}
+
+/** Create a DICT_ATTR from an ASCII attribute and value
+ *
+ * Where the attribute name is in the form:
+ *  - Attr-%d
+ *  - Attr-%d.%d.%d...
+ *  - Vendor-%d-Attr-%d
+ *  - VendorName-Attr-%d
+ *
+ * @param[in] ctx to alloc new attribute in.
+ * @param[in] name of attribute.
+ * @return 0 on success -1 on failure.
+ */
+DICT_ATTR const *dict_unknown_afrom_str(TALLOC_CTX *ctx, char const *name)
+{
+       uint8_t *p;
+       DICT_ATTR *da;
+
+       p = talloc_zero_array(ctx, uint8_t, DICT_ATTR_SIZE);
+       if (!p) {
+               fr_strerror_printf("Out of memory");
+               return NULL;
+       }
+       da = (DICT_ATTR *) p;
+       talloc_set_type(da, DICT_ATTR);
+
+       if (dict_unknown_from_str(da, name) < 0) {
+               talloc_free(p);
+               return NULL;
+       }
+
+       return da;
+}
+
+/** Create a dictionary attribute by name embedded in another string
+ *
+ * Find the first invalid attribute name char in the string pointed
+ * to by name.
+ *
+ * Copy the characters between the start of the name string and the first
+ * none dict_attr_allowed_char to a buffer and initialise da as an
+ * unknown attribute.
+ *
+ * @param[out] da to initialise.
+ * @param[in,out] name string start.
+ * @return 0 on success or -1 on error;
+ */
+int dict_unknown_from_substr(DICT_ATTR *da, char const **name)
+{
+       char const *p;
+       size_t len;
+       char buffer[DICT_ATTR_MAX_NAME_LEN + 1];
+
+       if (!name || !*name) return -1;
+
+       /*
+        *      Advance p until we get something that's not part of
+        *      the dictionary attribute name.
+        */
+       for (p = *name; dict_attr_allowed_chars[(int) *p] || (*p == '.' ) || (*p == '-'); p++);
+
+       len = p - *name;
+       if (len > DICT_ATTR_MAX_NAME_LEN) {
+               fr_strerror_printf("Attribute name too long");
+
+               return -1;
+       }
+       if (len == 0) {
+               fr_strerror_printf("Invalid attribute name");
+               return -1;
+       }
+       strlcpy(buffer, *name, len + 1);
+
+       if (dict_unknown_from_str(da, buffer) < 0) return -1;
+
+       *name = p;
+
+       return 0;
 }
 
 /*
@@ -2936,10 +3155,9 @@ DICT_ATTR const *dict_attrbyvalue(unsigned int attr, unsigned int vendor)
 }
 
 
-/**
- * @brief Get an attribute by its numerical value. and data type
+/** Get an attribute by its numerical value and data type
  *
- *     Used only for COMBO_IP
+ * Used only for COMBO_IP
  *
  * @return The attribute, or NULL if not found
  */
@@ -2955,8 +3173,8 @@ DICT_ATTR const *dict_attrbytype(unsigned int attr, unsigned int vendor,
        return fr_hash_table_finddata(attributes_combo, &da);
 }
 
-/**
- * @brief Using a parent and attr/vendor, find a child attr/vendor
+/** Using a parent and attr/vendor, find a child attr/vendor
+ *
  */
 int dict_attr_child(DICT_ATTR const *parent,
                    unsigned int *pattr, unsigned int *pvendor)
@@ -3074,39 +3292,58 @@ DICT_ATTR const *dict_attrbyname(char const *name)
        return fr_hash_table_finddata(attributes_byname, da);
 }
 
-/*
- *     Get an attribute by its name, where the name might have a tag
- *     or something else after it.
+/** Look up a dictionary attribute by name embedded in another string
+ *
+ * Find the first invalid attribute name char in the string pointed
+ * to by name.
+ *
+ * Copy the characters between the start of the name string and the first
+ * none dict_attr_allowed_char to a buffer and perform a dictionary lookup
+ * using that value.
+ *
+ * If the attribute exists, advance the pointer pointed to by name
+ * to the first none dict_attr_allowed_char char, and return the DA.
+ *
+ * If the attribute does not exist, don't advance the pointer and return
+ * NULL.
+ *
+ * @param[in,out] name string start.
+ * @return NULL if no attributes matching the name could be found, else
  */
-DICT_ATTR const *dict_attrbytagged_name(char const *name)
+DICT_ATTR const *dict_attrbyname_substr(char const **name)
 {
-       DICT_ATTR *da;
-       char *p;
-       uint32_t buffer[(sizeof(*da) + DICT_ATTR_MAX_NAME_LEN + 3)/4];
+       DICT_ATTR *find;
+       DICT_ATTR const *da;
+       char const *p;
+       size_t len;
+       uint32_t buffer[(sizeof(*find) + DICT_ATTR_MAX_NAME_LEN + 3)/4];
 
-       if (!name) return NULL;
+       if (!name || !*name) return NULL;
 
-       da = (DICT_ATTR *) buffer;
-       strlcpy(da->name, name, DICT_ATTR_MAX_NAME_LEN + 1);
+       find = (DICT_ATTR *) buffer;
 
        /*
-        *      The name might have a tag or array reference.  That
-        *      isn't properly part of the name, and can be ignored on
-        *      lookup.
+        *      Advance p until we get something that's not part of
+        *      the dictionary attribute name.
         */
-       for (p = &da->name[0]; *p; p++) {
-               if (*p == ':') {
-                       *p = '\0';
-                       break;
-               }
+       for (p = *name; dict_attr_allowed_chars[(int) *p]; p++);
 
-               if (*p == '[') {
-                       *p = '\0';
-                       break;
-               }
+       len = p - *name;
+       if (len > DICT_ATTR_MAX_NAME_LEN) {
+               fr_strerror_printf("Attribute name too long");
+
+               return NULL;
        }
+       strlcpy(find->name, *name, len + 1);
 
-       return fr_hash_table_finddata(attributes_byname, da);
+       da = fr_hash_table_finddata(attributes_byname, find);
+       if (!da) {
+               fr_strerror_printf("Unknown attribute \"%s\"", find->name);
+               return NULL;
+       }
+       *name = p;
+
+       return da;
 }
 
 /*
@@ -3207,3 +3444,42 @@ DICT_VENDOR *dict_vendorbyvalue(int vendorpec)
 
        return fr_hash_table_finddata(vendors_byvalue, &dv);
 }
+
+/** Converts an unknown to a known by adding it to the internal dictionaries.
+ *
+ * Does not free old DICT_ATTR, that is left up to the caller.
+ *
+ * @param old unknown attribute to add.
+ * @return existing DICT_ATTR if old was found in a dictionary, else the new entry in the dictionary
+ *        representing old.
+ */
+DICT_ATTR const *dict_unknown_add(DICT_ATTR const *old)
+{
+       DICT_ATTR const *da, *parent;
+       ATTR_FLAGS flags;
+
+       if (!old) return NULL;
+
+       if (!old->flags.is_unknown) return old;
+
+       da = dict_attrbyvalue(old->attr, old->vendor);
+       if (da) return da;
+
+       memcpy(&flags, &old->flags, sizeof(flags));
+       flags.is_unknown = false;
+
+       parent = dict_parent(old->attr, old->vendor);
+       if (parent) {
+               if (parent->flags.has_tlv) flags.is_tlv = true;
+               flags.evs = parent->flags.evs;
+               flags.extended = parent->flags.extended;
+               flags.long_extended = parent->flags.long_extended;
+       }
+
+       if (dict_addattr(old->name, old->attr, old->vendor, old->type, flags) < 0) {
+               return NULL;
+       }
+
+       da = dict_attrbyvalue(old->attr, old->vendor);
+       return da;
+}