From: Alan T. DeKok Date: Sat, 6 Dec 2025 20:10:19 +0000 (-0500) Subject: clean up set type and length X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3eeffaeffabe1d734cfedc598a39042a4a98372d;p=thirdparty%2Ffreeradius-server.git clean up set type and length so that it's done in the dict_attr_parent_init() function --- diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c index ee7fb7f4e7a..09f58992ff1 100644 --- a/src/lib/util/dict_tokenize.c +++ b/src/lib/util/dict_tokenize.c @@ -1837,49 +1837,6 @@ static int dict_read_process_begin_protocol(dict_tokenize_ctx_t *dctx, char **ar return dict_dctx_push(dctx, dctx->dict->root, NEST_PROTOCOL); } -/** Set the type size and length based on the parent. - * - */ -static void dict_flags_set(fr_dict_t const *dict, fr_dict_attr_t const *parent, fr_dict_vendor_t const *dv, fr_dict_attr_flags_t *flags) -{ - flags->type_size = dict->proto->default_type_size; - flags->length = dict->proto->default_type_length; - - /* - * Only RADIUS is crazy enough to have different type - * sizes for each vendor. - */ - if (dict->root->attr != FR_DICT_PROTO_RADIUS) return; - - /* - * If the parent exists, it will already have the correct flags set. - */ - if (parent->type != FR_TYPE_VSA) { - flags->type_size = parent->flags.type_size; - flags->length = parent->flags.length; - return; - } - - /* - * VSAs can exist inside Extended-Attribute-1, but they MUST be 1/1, and not any crazy - * vendor things. - */ - if (!parent->parent->flags.is_root) return; - - /* - * Only the top-level Vendor-Specific attribute can have different type sizes for each vendor. - * - * Unknown vendors default to 1/1. - */ - if (!dv) return; - - /* - * Otherwise we use the vendor-defined type size. - */ - flags->type_size = dv->type; - flags->length = dv->length; -} - static int dict_read_process_begin_vendor(dict_tokenize_ctx_t *dctx, char **argv, int argc, UNUSED fr_dict_attr_flags_t *base_flags) { @@ -1971,8 +1928,6 @@ static int dict_read_process_begin_vendor(dict_tokenize_ctx_t *dctx, char **argv if (!vendor_da) { fr_dict_attr_flags_t flags = {}; - dict_flags_set(dctx->dict, vsa_da, vendor, &flags); - new = dict_attr_alloc(dctx->dict->pool, vsa_da, argv[0], vendor->pen, FR_TYPE_VENDOR, &(dict_attr_args_t){ .flags = &flags }); diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c index a6ca55544d0..e7432bd1894 100644 --- a/src/lib/util/dict_util.c +++ b/src/lib/util/dict_util.c @@ -672,7 +672,9 @@ int dict_attr_type_init(fr_dict_attr_t **da_p, fr_type_t type) */ int dict_attr_parent_init(fr_dict_attr_t **da_p, fr_dict_attr_t const *parent) { - fr_dict_attr_t *da = *da_p; + fr_dict_attr_t *da = *da_p; + fr_dict_t const *dict = parent->dict; + fr_dict_attr_ext_vendor_t *ext; if (unlikely((*da_p)->type == FR_TYPE_NULL)) { fr_strerror_const("Attribute type must be set before initialising parent. Use dict_attr_type_init() first"); @@ -699,12 +701,34 @@ int dict_attr_parent_init(fr_dict_attr_t **da_p, fr_dict_attr_t const *parent) * Point to the vendor definition. Since ~90% of * attributes are VSAs, caching this pointer will help. */ + if (da->type == FR_TYPE_VENDOR) { + da->flags.type_size = dict->root->flags.type_size; + da->flags.length = dict->root->flags.type_size; + + if ((dict->root->attr == FR_DICT_PROTO_RADIUS) && (da->depth == 2)) { + fr_dict_vendor_t const *dv; + + dv = fr_dict_vendor_by_num(dict, da->attr); + if (dv) { + da->flags.type_size = dv->type; + da->flags.length = dv->length; + } + } + ext = NULL; + + } else if (da->type == FR_TYPE_TLV) { + da->flags.type_size = dict->root->flags.type_size; + da->flags.length = dict->root->flags.type_size; + } + if (parent->type == FR_TYPE_VENDOR) { - int ret = dict_attr_vendor_set(&da, parent); - *da_p = da; - if (ret < 0) return -1; + ext = dict_attr_ext_alloc(da_p, FR_DICT_ATTR_EXT_VENDOR); + if (unlikely(!ext)) return -1; + + ext->vendor = parent; + } else { - dict_attr_ext_copy(da_p, parent, FR_DICT_ATTR_EXT_VENDOR); /* Noop if no vendor extension */ + ext = dict_attr_ext_copy(da_p, parent, FR_DICT_ATTR_EXT_VENDOR); /* Noop if no vendor extension */ } /* @@ -713,6 +737,13 @@ int dict_attr_parent_init(fr_dict_attr_t **da_p, fr_dict_attr_t const *parent) */ dict_attr_da_stack_set(da_p); + da = *da_p; + + if (!ext || ((da->type != FR_TYPE_TLV) && (da->type != FR_TYPE_VENDOR))) return 0; + + da->flags.type_size = ext->vendor->flags.type_size; + da->flags.length = ext->vendor->flags.type_size; + return 0; } @@ -886,10 +917,16 @@ int _dict_attr_init(char const *filename, int line, char const *name, unsigned int attr, fr_type_t type, dict_attr_args_t const *args) { - if (unlikely(dict_attr_init_common(filename, line, da_p, parent, type, args) < 0)) return -1; - + /* + * We initialize the number first, as doing that doesn't have any other side effects. + */ if (unlikely(dict_attr_num_init(*da_p, attr) < 0)) return -1; + /* + * This function then checks the number, for things like VSAs. + */ + if (unlikely(dict_attr_init_common(filename, line, da_p, parent, type, args) < 0)) return -1; + if (unlikely(dict_attr_finalise(da_p, name) < 0)) return -1; return 0; diff --git a/src/lib/util/dict_validate.c b/src/lib/util/dict_validate.c index fe1b61af601..2b044372481 100644 --- a/src/lib/util/dict_validate.c +++ b/src/lib/util/dict_validate.c @@ -40,7 +40,6 @@ bool dict_attr_flags_valid(fr_dict_attr_t *da) uint32_t shift_array, shift_has_value; uint32_t shift_subtype, shift_extra; uint32_t shift_counter; - fr_dict_attr_t const *v; fr_dict_t *dict = da->dict; fr_dict_attr_t const *parent = da->parent; char const *name = da->name; @@ -369,6 +368,8 @@ bool dict_attr_flags_valid(fr_dict_attr_t *da) break; case FR_TYPE_VENDOR: + if (dict->string_based) break; + if (parent->type != FR_TYPE_VSA) { fr_strerror_printf("Attributes of type 'vendor' MUST have a parent of type 'vsa' " "instead of '%s'", @@ -376,74 +377,21 @@ bool dict_attr_flags_valid(fr_dict_attr_t *da) return false; } - if (flags->length) { - if ((flags->length != 1) && - (flags->length != 2) && - (flags->length != 4)) { - fr_strerror_const("The 'length' flag can only be used for attributes of type 'vendor' with lengths of 1,2 or 4"); - return false; - } - - break; - } - - /* - * Set the length / type_size of vendor attributes from the the dictionary defaults. If - * there's a vendor, set them from the vendor definition. - * - * But we only do this for RADIUS, because the other protocols aren't crazy enough to - * have different type sizes for different vendors. - */ - flags->type_size = dict->root->flags.type_size; - flags->length = dict->root->flags.type_size; - if ((attr > 0) && (dict->root->attr == FR_DICT_PROTO_RADIUS)) { - fr_dict_vendor_t const *dv; - - dv = fr_dict_vendor_by_num(dict, attr); - if (dv) { - flags->type_size = dv->type; - flags->length = dv->length; - } + if ((flags->length != 1) && + (flags->length != 2) && + (flags->length != 4)) { + fr_strerror_const("The 'length' flag can only be used for attributes of type 'vendor' with lengths of 1,2 or 4"); + return false; } break; case FR_TYPE_TLV: - if (flags->length) { - if ((flags->length != 1) && - (flags->length != 2) && - (flags->length != 4)) { - fr_strerror_const("The 'length' flag can only be used for attributes of type 'tlv' with lengths of 1,2 or 4"); - return false; - } - - break; - } - - /* - * Find an appropriate parent to copy the - * TLV-specific fields from. - */ - for (v = parent; v != NULL; v = v->parent) { - if ((v->type == FR_TYPE_TLV) || (v->type == FR_TYPE_VENDOR)) { - break; - } - } - - /* - * root is always FR_TYPE_TLV, so we're OK. - */ - if (!v) { - fr_strerror_printf("Attributes of type '%s' require a parent attribute", - fr_type_to_str(type)); + if ((flags->length != 1) && + (flags->length != 2) && + (flags->length != 4)) { + fr_strerror_const("The 'length' flag can only be used for attributes of type 'tlv' with lengths of 1,2 or 4"); return false; } - - /* - * Over-ride whatever was there before, so we - * don't have multiple formats of VSAs. - */ - flags->type_size = v->flags.type_size; - flags->length = v->flags.length; break; /* diff --git a/src/protocols/der/base.c b/src/protocols/der/base.c index 562bebad6b3..0ced861f798 100644 --- a/src/protocols/der/base.c +++ b/src/protocols/der/base.c @@ -819,6 +819,15 @@ static bool attr_valid(fr_dict_attr_t *da) fr_der_attr_flags_t *flags = fr_dict_attr_ext(da, FR_DICT_ATTR_EXT_PROTOCOL_SPECIFIC); fr_der_attr_flags_t *parent; + /* + * We might have created the attribute of type "tlv", and then later swapped it to "group". + * Ensure that the main validation routines are happy with the result. + */ + if (da->type == FR_TYPE_GROUP) { + da->flags.type_size = 0; + da->flags.length = 0; + } + /* * sequence_of=oid_and_value has to have a reference to the OID tree. *