From: Arran Cudbard-Bell Date: Sun, 3 Nov 2024 05:43:22 +0000 (+0200) Subject: Use common parsing functions for all references. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e10a85fb8b67ffdb2fe812344ebaf823fffd2b1f;p=thirdparty%2Ffreeradius-server.git Use common parsing functions for all references. Add support for '@', so that '..' works as one would expect. Stop clones adding a pre-cloned version of the attribute to the dictionary. Allow clones, and enum references to reference foreign attributes, but only when they share the same base protocol. Use the file and line numbers from refs, instead of from the fixup structs. --- diff --git a/doc/antora/modules/reference/pages/dictionary/attribute.adoc b/doc/antora/modules/reference/pages/dictionary/attribute.adoc index 2e51cb0ef3e..e17291e80b9 100644 --- a/doc/antora/modules/reference/pages/dictionary/attribute.adoc +++ b/doc/antora/modules/reference/pages/dictionary/attribute.adoc @@ -60,9 +60,13 @@ Common flags and meanings | `secret` | The server will not print `secret` values in debug mode, and in many other situations. |===== -The `` field in the examples above is an attribute references such as `Foo`, or `Foo.Bar`, or `dhcpv4.foo.bar`. +The `` field in the examples above is an attribute references such as `Foo`, or `Foo.Bar`, or `@dhcpv4.foo.bar`. All `` fields use the same syntax. -The `enum` and `clone` flags are similar, in that they copy "children" of the attribute. However, the `enum` flag copies values, and `clone` copies attributes. +If the ref begins with `@` the ref will be treated as a "foreign" reference to another protocol. The first component will be used as the protocol name e.g. `ref=@RADIUS`. + +If the ref begins with `.` the ref will be treated as relative to the current attribute, else resolution will begin from the root of the current dictionary. + +Multiple consecutive `.` can be used to traverse up the attribute hierarchy from the current attribute. .Examples ---- diff --git a/share/dictionary/dhcpv6/dictionary.rfc7037 b/share/dictionary/dhcpv6/dictionary.rfc7037 index 9f5d435c2f0..ed67c01543b 100644 --- a/share/dictionary/dhcpv6/dictionary.rfc7037 +++ b/share/dictionary/dhcpv6/dictionary.rfc7037 @@ -10,4 +10,4 @@ # ############################################################################## -ATTRIBUTE RADIUS 81 group ref=..RADIUS +ATTRIBUTE RADIUS 81 group ref=@RADIUS diff --git a/share/dictionary/radius/dictionary.huawei b/share/dictionary/radius/dictionary.huawei index 57ea7a6ec02..02a3e3d2385 100644 --- a/share/dictionary/radius/dictionary.huawei +++ b/share/dictionary/radius/dictionary.huawei @@ -191,7 +191,7 @@ VALUE Port-Mirror Enable 3 ATTRIBUTE Account-Info 184 string ATTRIBUTE Service-Info 185 string -ATTRIBUTE DHCP-Option 187 group ref=..DHCPv4 +ATTRIBUTE DHCP-Option 187 group ref=@DHCPv4 ATTRIBUTE AVpair 188 string ATTRIBUTE Delegated-IPv6-Prefix-Pool 191 string ATTRIBUTE IPv6-Prefix-Lease 192 struct diff --git a/share/dictionary/radius/dictionary.nokia.sr b/share/dictionary/radius/dictionary.nokia.sr index 89c62e3e5ff..d477de36a32 100644 --- a/share/dictionary/radius/dictionary.nokia.sr +++ b/share/dictionary/radius/dictionary.nokia.sr @@ -299,8 +299,8 @@ ATTRIBUTE Force-Nak 98 string # CoA ATTRIBUTE Ipv6-Address 99 ipv6addr ATTRIBUTE Serv-Id 100 integer ATTRIBUTE Interface 101 string -ATTRIBUTE ToServer-Dhcp-Options 102 group ref=..DHCPv4 -ATTRIBUTE ToClient-Dhcp-Options 103 group ref=..DHCPv4 +ATTRIBUTE ToServer-Dhcp-Options 102 group ref=@DHCPv4 +ATTRIBUTE ToClient-Dhcp-Options 103 group ref=@DHCPv4 ATTRIBUTE Tunnel-Serv-Id 104 integer ATTRIBUTE Ipv6-Primary-Dns 105 ipv6addr ATTRIBUTE Ipv6-Secondary-Dns 106 ipv6addr @@ -533,8 +533,8 @@ ATTRIBUTE WPP-ErrorCode 183 integer ATTRIBUTE Onetime-Http-Redirect-Reactivate 185 string # DHCP6 attributes -ATTRIBUTE ToServer-Dhcp6-Options 191 group ref=..DHCPv6 -ATTRIBUTE ToClient-Dhcp6-Options 192 group ref=..DHCPv6 +ATTRIBUTE ToServer-Dhcp6-Options 191 group ref=@DHCPv6 +ATTRIBUTE ToClient-Dhcp6-Options 192 group ref=@DHCPv6 # # MUST have renew time <= rebind time <= preferred lifetime <= valid lifetime diff --git a/share/dictionary/radius/dictionary.rfc9445 b/share/dictionary/radius/dictionary.rfc9445 index efd3ecd3697..f9eb7afac3f 100644 --- a/share/dictionary/radius/dictionary.rfc9445 +++ b/share/dictionary/radius/dictionary.rfc9445 @@ -8,5 +8,5 @@ # # $Id$ # -ATTRIBUTE DHCPv6-Options 245.3 group ref=..DHCPv6 -ATTRIBUTE DHCPv4-Options 245.4 group ref=..DHCPv4 +ATTRIBUTE DHCPv6-Options 245.3 group ref=@DHCPv6 +ATTRIBUTE DHCPv4-Options 245.4 group ref=@DHCPv4 diff --git a/share/dictionary/radius/dictionary.starent b/share/dictionary/radius/dictionary.starent index 4401463dbf3..78745836168 100644 --- a/share/dictionary/radius/dictionary.starent +++ b/share/dictionary/radius/dictionary.starent @@ -288,7 +288,7 @@ ATTRIBUTE Fast-Reauth-Username 304 octets ATTRIBUTE Pseudonym-Username 305 octets ATTRIBUTE WiMAX-Auth-Only 306 integer ATTRIBUTE TrafficSelector-Class 307 integer -ATTRIBUTE DHCP-Options 309 group ref=..DHCPv4 +ATTRIBUTE DHCP-Options 309 group ref=@DHCPv4 ATTRIBUTE Handoff-Indicator 310 integer ATTRIBUTE User-Privilege 313 integer ATTRIBUTE IPv6-Alloc-Method 314 integer diff --git a/share/dictionary/radius/dictionary.unisphere b/share/dictionary/radius/dictionary.unisphere index d70651cd800..4a3b4ecf00c 100644 --- a/share/dictionary/radius/dictionary.unisphere +++ b/share/dictionary/radius/dictionary.unisphere @@ -108,7 +108,7 @@ ATTRIBUTE Sdx-Tunnel-Disconnect-Cause-Info 51 string ATTRIBUTE Radius-Client-Address 52 ipaddr ATTRIBUTE Service-Description 53 string ATTRIBUTE L2tp-Recv-Window-Size 54 integer -ATTRIBUTE DHCP-Options 55 group ref=..DHCPv4 +ATTRIBUTE DHCP-Options 55 group ref=@DHCPv4 ATTRIBUTE DHCP-Mac-Addr 56 string ATTRIBUTE DHCP-Gi-Address 57 ipaddr ATTRIBUTE LI-Action 58 integer encrypt=Tunnel-Password @@ -262,7 +262,7 @@ ATTRIBUTE IPv6-Output-Service-Filter 203 string ATTRIBUTE Adv-Pcef-Profile-Name 204 string ATTRIBUTE Adv-Pcef-Rule-Name 205 string ATTRIBUTE Re-Authentication-Catalyst 206 integer -ATTRIBUTE DHCPv6-Options 207 group ref=..DHCPv4 +ATTRIBUTE DHCPv6-Options 207 group ref=@DHCPv6 ATTRIBUTE DHCP-Header 208 octets ATTRIBUTE DHCPv6-Header 209 octets ATTRIBUTE Acct-Request-Reason 210 integer diff --git a/src/lib/util/dict_fixup.c b/src/lib/util/dict_fixup.c index 7aff72d1e0c..d18ca1f40f0 100644 --- a/src/lib/util/dict_fixup.c +++ b/src/lib/util/dict_fixup.c @@ -23,8 +23,14 @@ */ RCSID("$Id$") -#include +#include #include +#include +#include +#include +#include +#include + #include "dict_fixup_priv.h" /** Common fields for every fixup structure @@ -80,8 +86,7 @@ typedef struct { typedef struct { dict_fixup_common_t common; //!< Common fields. - fr_dict_attr_t *parent; //!< parent where we add the clone - fr_dict_attr_t *da; //!< FR_TYPE_TLV to clone + fr_dict_attr_t *da; //!< to populate with cloned information. char *ref; //!< the target attribute to clone } dict_fixup_clone_t; @@ -116,12 +121,14 @@ typedef struct { static inline CC_HINT(always_inline) int dict_fixup_common(char const *filename, int line, fr_dlist_head_t *fixup_list, dict_fixup_common_t *common) { - common->filename = talloc_strdup(common, filename); - if (!common->filename) { - fr_strerror_const("Out of memory"); - return -1; + if (filename) { + common->filename = talloc_strdup(common, filename); + if (!common->filename) { + fr_strerror_const("Out of memory"); + return -1; + } + common->line = line; } - common->line = line; fr_dlist_insert_tail(fixup_list, common); @@ -220,16 +227,13 @@ static inline CC_HINT(always_inline) int dict_fixup_enumv_apply(UNUSED dict_fixu * hasn't been loaded yet. * * @param[in] fctx Holds current dictionary parsing information. - * @param[in] filename this fixup relates to. - * @param[in] line this fixup relates to. * @param[in] da The group dictionary attribute. * @param[in] ref OID string representing what the group references. * @return * - 0 on success. * - -1 on out of memory. */ -int dict_fixup_group(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *da, char const *ref) +int dict_fixup_group(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref) { dict_fixup_group_t *fixup; @@ -243,99 +247,91 @@ int dict_fixup_group(dict_fixup_ctx_t *fctx, char const *filename, int line, .ref = talloc_strdup(fixup, ref), }; - return dict_fixup_common(filename, line, &fctx->group, &fixup->common); + return dict_fixup_common(NULL, 0, &fctx->group, &fixup->common); } -static fr_dict_attr_t const *dict_protocol_reference(fr_dict_t **dict_def, char const *ref, char const *filename, int line) +/** Resolve a ref= or copy= value to a dictionary */ +static fr_dict_attr_t const *dict_protocol_reference(fr_dict_attr_t const *root, char const *ref) { - fr_dict_t *dict; - fr_dict_attr_t const *da; - char const *name; - ssize_t slen; - char protocol[64]; - - fr_assert((ref[0] == '.') && (ref[1] == '.')); - name = ref + 2; /* already checked when we inserted it */ + fr_dict_t *dict = fr_dict_unconst(root->dict); + fr_dict_attr_t const *da = root, *found; + ssize_t slen; + fr_sbuff_t sbuff = FR_SBUFF_IN(ref, strlen(ref)); /* - * Reference to foreign protocol. Get the protocol name. + * Are we resolving a foreign reference? */ - slen = dict_by_protocol_substr(NULL, &dict, &FR_SBUFF_IN(name, strlen(name)), NULL); - if (slen <= 0) { - char *p; - char const *q; - - /* - * The filenames are lowercase. The names in the dictionaries are case-insensitive. So - * we mash the name to all lowercase. - */ - p = protocol; - q = name; - while (*q && (*q != '.')) { - *(p++) = tolower((int) *(q++)); - - if ((size_t) (p - protocol) >= sizeof(protocol)) { - invalid: - fr_strerror_printf("Invalid attribute reference '%s' at %s[%d]", ref, - fr_cwd_strip(filename), line); + if (fr_sbuff_next_if_char(&sbuff, '@')) { + char proto_name[FR_DICT_ATTR_MAX_NAME_LEN + 1]; + + slen = dict_by_protocol_substr(NULL, &dict, &sbuff, NULL); + /* Need to load it... */ + if (slen <= 0) { + /* Fixme, probably want to limit allowed chars */ + if (fr_sbuff_out_bstrncpy_until(&FR_SBUFF_OUT(proto_name, sizeof(proto_name)), &sbuff, SIZE_MAX, + &FR_SBUFF_TERMS(L(""), L(".")), NULL) <= 0) { + invalid_name: + fr_strerror_const("Invalid protocol name"); return NULL; } - } - *p = '\0'; - /* - * Load the new dictionary, and mark it as loaded from our dictionary. - */ - if (fr_dict_protocol_afrom_file(&dict, protocol, NULL, (*dict_def)->root->name) < 0) { - return NULL; - } + /* + * The filenames are lowercase. The names in the dictionaries are case-insensitive. So + * we mash the name to all lowercase. + */ + fr_tolower(proto_name); + + /* + * Catch this early, so people don't do stupid things + * like put slashes in the references and then claim + * it's a security issue. + */ + if (fr_dict_valid_oid_str(proto_name, -1) < 0) goto invalid_name; + + /* + * Load the new dictionary, and mark it as loaded from our dictionary. + */ + if (fr_dict_protocol_afrom_file(&dict, proto_name, NULL, (root->dict)->root->name) < 0) { + return NULL; + } - if (!fr_hash_table_insert((*dict_def)->autoref, dict)) { - fr_strerror_const("Failed inserting into internal autoref table"); - return NULL; + if (!fr_hash_table_insert((root->dict)->autoref, dict)) { + fr_strerror_const("Failed inserting into internal autoref table"); + return NULL; + } } /* - * The reference is to the root of the foreign protocol, we're done. + * Didn't stop at an attribute ref... we're done */ - if (!*q) { - *dict_def = dict; + if (!fr_sbuff_next_if_char(&sbuff, '.')) { return dict->root; } - name = p + 1; + da = dict->root; + } + /* + * If we don't have a '.' to make it relative, we're starting from the dictionary root + */ + if (fr_sbuff_next_if_char(&sbuff, '.')) { + do { + if (!da->parent) { + fr_strerror_const("Reference attempted to navigate above dictionary root"); + return NULL; + } + da = da->parent; + } while (fr_sbuff_next_if_char(&sbuff, '.')); } else { - /* - * The foreign dictionary was loaded by someone - * else, try to resolve the attribute. - */ - name += slen; - - if (!*name) { - /* - * The reference is to the root of the foreign protocol, we're done. - */ - *dict_def = dict; - return dict->root; - } - - if (*name != '.') goto invalid; - name++; + da = dict->root; } /* * Look up the attribute. */ - da = fr_dict_attr_by_oid(NULL, fr_dict_root(dict), name); - if (!da) { - fr_strerror_printf("No such attribute '%s' in protocol '%s' at %s[%d]", - name, dict->root->name, fr_cwd_strip(filename), line); - return NULL; - } + if (fr_dict_attr_by_oid_substr(NULL, &found, da, &sbuff, NULL) <= 0) return NULL; - *dict_def = dict; - return da; + return found; } /** Resolve a group reference @@ -349,36 +345,23 @@ static fr_dict_attr_t const *dict_protocol_reference(fr_dict_t **dict_def, char static inline CC_HINT(always_inline) int dict_fixup_group_apply(UNUSED dict_fixup_ctx_t *fctx, dict_fixup_group_t *fixup) { fr_dict_attr_t const *da; - fr_dict_t *dict = fr_dict_unconst(fr_dict_by_da(fixup->da)); - /* - * Ref to attribute in existing dictionary. The dictionary MUST be loaded by $INCLUDEs. - */ - if (fixup->ref[0] != '.') { - da = fr_dict_attr_by_oid(NULL, fr_dict_root(dict), fixup->ref); - if (!da) { - fr_strerror_printf("Invalid attribute reference '%s' at %s[%d]", fixup->ref, - fr_cwd_strip(fixup->common.filename), fixup->common.line); - return -1; - } - } else { - /* - * Load a foreign protocol, which may include - * loops. - */ - da = dict_protocol_reference(&dict, fixup->ref, fixup->common.filename, fixup->common.line); - if (!da) return -1; + da = dict_protocol_reference(fixup->da, fixup->ref); + if (!da) { + fr_strerror_printf_push("Failed resolving reference for attribute at %s[%u]", + fr_cwd_strip(fixup->da->filename), fixup->da->line); + return -1; } if (da->type != FR_TYPE_TLV) { fr_strerror_printf("References MUST be to attributes of type 'tlv' at %s[%d]", - fr_cwd_strip(fixup->common.filename), fixup->common.line); + fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } if (fr_dict_attr_ref(da)) { fr_strerror_printf("References MUST NOT refer to an ATTRIBUTE which also has 'ref=...' at %s[%d]", - fr_cwd_strip(fixup->common.filename), fixup->common.line); + fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } if (unlikely(dict_attr_ref_resolve(fixup->da, da) < 0)) return -1; @@ -392,18 +375,13 @@ static inline CC_HINT(always_inline) int dict_fixup_group_apply(UNUSED dict_fixu * attribute by the time it has been cloned. * * @param[in] fctx Holds current dictionary parsing information. - * @param[in] filename this fixup relates to. - * @param[in] line this fixup relates to. - * @param[in] parent for the cloned attribute. * @param[in] da The group dictionary attribute. * @param[in] ref OID string representing what the group references.. * @return * - 0 on success. * - -1 on out of memory. */ -int dict_fixup_clone(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *parent, fr_dict_attr_t *da, - char const *ref) +int dict_fixup_clone(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref) { dict_fixup_clone_t *fixup; @@ -419,12 +397,11 @@ int dict_fixup_clone(dict_fixup_ctx_t *fctx, char const *filename, int line, return -1; } *fixup = (dict_fixup_clone_t) { - .parent = parent, .da = da, .ref = talloc_typed_strdup(fixup, ref) }; - return dict_fixup_common(filename, line, &fctx->clone, &fixup->common); + return dict_fixup_common(NULL, 0, &fctx->clone, &fixup->common); } /** Clone one are of a tree into another @@ -437,42 +414,20 @@ int dict_fixup_clone(dict_fixup_ctx_t *fctx, char const *filename, int line, */ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixup_ctx_t *fctx, dict_fixup_clone_t *fixup) { - fr_dict_attr_t const *da, *root; + fr_dict_t *dict = fr_dict_unconst(fixup->da->dict); + fr_dict_attr_t const *src; fr_dict_attr_t *cloned; - fr_dict_t *dict = fr_dict_unconst(fr_dict_by_da(fixup->da)); - char const *ref = fixup->ref; - - /* - * Allow relative attribute references. - */ - if (ref[0] == '.') { - root = fixup->da->parent; - ref++; - - while (ref[0] == '.') { - /* - * @todo - allow references to other protocols. - */ - if (root->flags.is_root) { - fr_strerror_printf("Too many '.' in 'clone=%s' at %s[%d]", - fixup->ref, fr_cwd_strip(fixup->common.filename), fixup->common.line); - return -1; - } - root = root->parent; - ref++; - } - } else { - root = fr_dict_root(dict); + src = dict_protocol_reference(fixup->da, fixup->ref); + if (!src) { + fr_strerror_printf_push("Failed resolving reference for attribute at %s[%u]", + fr_cwd_strip(fixup->da->filename), fixup->da->line); + return -1; } - /* - * Find the reference - */ - da = fr_dict_attr_by_oid(NULL, root, ref); - if (!da) { - fr_strerror_printf("Unknown attribute reference 'clone=%s' at parent %s %s[%d]", - fixup->ref, root->name, fr_cwd_strip(fixup->common.filename), fixup->common.line); + if (src->dict->proto != fixup->da->dict->proto) { + fr_strerror_printf("Incompatible protocols. Referenced '%s', referencing '%s'. Defined at %s[%u]", + src->dict->proto->name, fixup->da->dict->proto->name, fixup->da->filename, fixup->da->line); return -1; } @@ -480,21 +435,21 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * The referenced DA is higher than the one we're * creating. Ensure it's not a parent. */ - if (da->depth < fixup->da->depth) { + if (src->depth < fixup->da->depth) { fr_dict_attr_t const *parent; for (parent = fixup->da->parent; !parent->flags.is_root; parent = parent->parent) { - if (parent == da) { + if (parent == src) { fr_strerror_printf("References MUST NOT refer to a parent attribute %s at %s[%d]", - parent->name, fr_cwd_strip(fixup->common.filename), fixup->common.line); + parent->name, fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } } } - if (fr_dict_attr_ref(da)) { + if (fr_dict_attr_ref(src)) { fr_strerror_printf("References MUST NOT refer to an ATTRIBUTE which itself has a 'ref=...' at %s[%d]", - fr_cwd_strip(fixup->common.filename), fixup->common.line); + fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } @@ -510,7 +465,7 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * children. */ if (!fr_type_is_non_leaf(fixup->da->type) && - ((da->type != fixup->da->type) || !dict_attr_children(da))) { + ((src->type != fixup->da->type) || !dict_attr_children(src))) { int copied; /* @@ -520,11 +475,11 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * different types. But only if they don't have * children (i.e. key fields). */ - if (fr_type_is_non_leaf(da->type) || fr_type_is_non_leaf(fixup->da->type) || - dict_attr_children(da) || dict_attr_children(fixup->da)) { + if (fr_type_is_non_leaf(src->type) || fr_type_is_non_leaf(fixup->da->type) || + dict_attr_children(src) || dict_attr_children(fixup->da)) { fr_strerror_printf("Reference MUST be to a simple data type of type '%s' at %s[%d]", fr_type_to_str(fixup->da->type), - fr_cwd_strip(fixup->common.filename), fixup->common.line); + fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } @@ -537,13 +492,13 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * copies the VALUE with the *source* data type, * where we need the *destination* data type. */ - copied = dict_attr_acopy_enumv(fixup->da, da); + copied = dict_attr_acopy_enumv(fixup->da, src); if (copied < 0) return -1; if (!copied) { fr_strerror_printf("Reference copied no VALUEs from type type '%s' at %s[%d]", fr_type_to_str(fixup->da->type), - fr_cwd_strip(fixup->common.filename), fixup->common.line); + fr_cwd_strip(fixup->da->filename), fixup->da->line); return -1; } @@ -557,8 +512,8 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu /* * Can't clone KEY fields directly, you MUST clone the parent struct. */ - if (!fr_type_is_non_leaf(da->type) || fr_dict_attr_is_key_field(da) || fr_dict_attr_is_key_field(fixup->da)) { - fr_strerror_printf("Invalid reference from '%s' to %s", fixup->ref, da->name); + if (!fr_type_is_non_leaf(src->type) || fr_dict_attr_is_key_field(src) || fr_dict_attr_is_key_field(fixup->da)) { + fr_strerror_printf("Invalid reference from '%s' to %s", fixup->ref, src->name); return -1; } @@ -566,14 +521,14 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * Copy the source attribute, but with a * new name and a new attribute number. */ - cloned = dict_attr_acopy(dict->pool, da, fixup->da->name); + cloned = dict_attr_acopy(dict->pool, src, fixup->da->name); if (!cloned) { - fr_strerror_printf("Failed copying attribute '%s' to %s", da->name, fixup->ref); + fr_strerror_printf("Failed copying attribute '%s' to %s", src->name, fixup->ref); return -1; } cloned->attr = fixup->da->attr; - cloned->parent = fixup->parent; /* we need to re-parent this attribute */ + cloned->parent = fixup->da->parent; /* we need to re-parent this attribute */ cloned->depth = cloned->parent->depth + 1; /* @@ -581,7 +536,7 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu */ if (dict_attr_children(fixup->da)) { if (dict_attr_acopy_children(dict, cloned, fixup->da) < 0) { - fr_strerror_printf("Failed copying attribute '%s' from children of %s", da->name, fixup->ref); + fr_strerror_printf("Failed copying attribute '%s' from children of %s", src->name, fixup->ref); return -1; } } @@ -589,20 +544,117 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu /* * Copy children of the DA we're cloning. */ - if (dict_attr_children(da)) { - if (dict_attr_acopy_children(dict, cloned, da) < 0) { - fr_strerror_printf("Failed copying attribute '%s' from children of %s", da->name, fixup->ref); + if (dict_attr_children(src)) { + if (dict_attr_acopy_children(dict, cloned, src) < 0) { + fr_strerror_printf("Failed copying attribute '%s' from children of %s", src->name, fixup->ref); return -1; } + } - if (dict_attr_child_add(fr_dict_attr_unconst(fixup->parent), cloned) < 0) { - fr_strerror_printf("Failed adding attribute %s", da->name); - talloc_free(cloned); - return -1; - } + if (fr_dict_attr_add_initialised(cloned) < 0) { + talloc_free(cloned); + return -1; + } + + return 0; +} + +/** Clone enumeration values from one attribute to another + * + * These must be processed later to ensure that we've finished building an + * attribute by the time it has been cloned. + * + * @param[in] fctx Holds current dictionary parsing information. + * @param[in] da The group dictionary attribute. + * @param[in] ref OID string representing what the group references.. + * @return + * - 0 on success. + * - -1 on out of memory. + */ +int dict_fixup_clone_enum(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref) +{ + dict_fixup_clone_t *fixup; + + /* + * Delay type checks until we've loaded all of the + * dictionaries. This means that errors are produced + * later, but that shouldn't matter for the default + * dictionaries. They're supposed to work. + */ + fixup = talloc(fctx->pool, dict_fixup_clone_t); + if (!fixup) { + fr_strerror_const("Out of memory"); + return -1; + } + *fixup = (dict_fixup_clone_t) { + .da = da, + .ref = talloc_typed_strdup(fixup, ref) + }; + + return dict_fixup_common(NULL, 0, &fctx->clone, &fixup->common); +} + +/** Clone one are of a tree into another + * + * @param[in] fctx Holds current dictionary parsing information. + * @param[in] fixup Containing source/destination of the clone. + * @return + * - 0 on success. + * - -1 on failure. + */ +static inline CC_HINT(always_inline) int dict_fixup_clone_enum_apply(UNUSED dict_fixup_ctx_t *fctx, dict_fixup_clone_t *fixup) +{ + fr_dict_attr_t const *src; + int copied; + + src = dict_protocol_reference(fixup->da, fixup->ref); + if (!src) { + fr_strerror_printf_push("Failed resolving reference for attribute at %s[%u]", + fr_cwd_strip(fixup->da->filename), fixup->da->line); + return -1; + } + + if (src->dict->proto != fixup->da->dict->proto) { + fr_strerror_printf("Incompatible protocols. Referenced '%s', referencing '%s'. Defined at %s[%u]", + src->dict->proto->name, fixup->da->dict->proto->name, fixup->da->filename, fixup->da->line); + return -1; } - if (dict_attr_add_to_namespace(fixup->parent, cloned) < 0) return -1; + if (fr_dict_attr_ref(src)) { + fr_strerror_printf("References MUST NOT refer to an ATTRIBUTE which itself has a 'ref=...' at %s[%d]", + fr_cwd_strip(fixup->da->filename), fixup->da->line); + return -1; + } + + if (!fr_type_is_non_leaf(fixup->da->type)) { + fr_strerror_printf("enum copy can only be applied to leaf types, not %s", fr_type_to_str(fixup->da->type)); + return -1; + } + + if (src->type != fixup->da->type) { + fr_strerror_printf("enum copy type mismatch. src '%s', dst '%s'", + fr_type_to_str(src->type), fr_type_to_str(fixup->da->type)); + return -1; + } + + /* + * We copy all of the VALUEs over from the source + * da by hand, by casting them. + * + * We have to do this work manually because we + * can't call dict_attr_acopy(), as that function + * copies the VALUE with the *source* data type, + * where we need the *destination* data type. + */ + copied = dict_attr_acopy_enumv(fixup->da, src); + if (copied < 0) return -1; + + if (!copied) { + fr_strerror_printf("Reference copied no VALUEs from type type '%s' at %s[%d]", + fr_type_to_str(fixup->da->type), + fr_cwd_strip(fixup->da->filename), fixup->da->line); + return -1; + } return 0; } @@ -616,15 +668,12 @@ static inline CC_HINT(always_inline) int dict_fixup_clone_apply(UNUSED dict_fixu * do these lookups at run-time. * * @param[in] fctx Holds current dictionary parsing information. - * @param[in] filename this fixup relates to. - * @param[in] line this fixup relates to. * @param[in] da The group dictionary attribute. * @return * - 0 on success. * - -1 on out of memory. */ -int dict_fixup_vsa(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *da) +int dict_fixup_vsa(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da) { dict_fixup_vsa_t *fixup; @@ -637,7 +686,7 @@ int dict_fixup_vsa(dict_fixup_ctx_t *fctx, char const *filename, int line, .da = da, }; - return dict_fixup_common(filename, line, &fctx->vsa, &fixup->common); + return dict_fixup_common(NULL, 0, &fctx->vsa, &fixup->common); } /** Run VSA fixups @@ -771,14 +820,17 @@ do { \ * - Group references next, as group attributes may be cloned. * - Clones last as all other references and additions should * be applied before cloning. + * - Clone enum clones the enumeration values from a dedicated + * enum, or another attribute with enumerations. * - VSAs * - Aliases last as all attributes need to be defined. */ - APPLY_FIXUP(fctx, enumv, dict_fixup_enumv_apply, dict_fixup_enumv_t); - APPLY_FIXUP(fctx, group, dict_fixup_group_apply, dict_fixup_group_t); - APPLY_FIXUP(fctx, clone, dict_fixup_clone_apply, dict_fixup_clone_t); - APPLY_FIXUP(fctx, vsa, dict_fixup_vsa_apply, dict_fixup_vsa_t); - APPLY_FIXUP(fctx, alias, dict_fixup_alias_apply, dict_fixup_alias_t); + APPLY_FIXUP(fctx, enumv, dict_fixup_enumv_apply, dict_fixup_enumv_t); + APPLY_FIXUP(fctx, group, dict_fixup_group_apply, dict_fixup_group_t); + APPLY_FIXUP(fctx, clone, dict_fixup_clone_apply, dict_fixup_clone_t); + APPLY_FIXUP(fctx, clone_enum, dict_fixup_clone_enum_apply, dict_fixup_clone_t); + APPLY_FIXUP(fctx, vsa, dict_fixup_vsa_apply, dict_fixup_vsa_t); + APPLY_FIXUP(fctx, alias, dict_fixup_alias_apply, dict_fixup_alias_t); TALLOC_FREE(fctx->pool); diff --git a/src/lib/util/dict_fixup_priv.h b/src/lib/util/dict_fixup_priv.h index 68d3499c425..54dd9df4ec0 100644 --- a/src/lib/util/dict_fixup_priv.h +++ b/src/lib/util/dict_fixup_priv.h @@ -35,6 +35,7 @@ typedef struct { fr_dlist_head_t enumv; //!< Raw enumeration values to add. fr_dlist_head_t group; //!< Group references to resolve. fr_dlist_head_t clone; //!< Clone operation to apply. + fr_dlist_head_t clone_enum; //!< Clone enum operation to apply. fr_dlist_head_t vsa; //!< VSAs to add vendors for fr_dlist_head_t alias; //!< Aliases that can't be resolved immediately. } dict_fixup_ctx_t; @@ -45,15 +46,13 @@ int dict_fixup_enumv(dict_fixup_ctx_t *fctx, char const *filename, int line, char const *value, size_t value_len, fr_dict_attr_t const *parent); -int dict_fixup_group(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *da, char const *ref); +int dict_fixup_group(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref); -int dict_fixup_clone(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *parent, fr_dict_attr_t *da, - char const *ref); +int dict_fixup_clone(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref); -int dict_fixup_vsa(dict_fixup_ctx_t *fctx, char const *filename, int line, - fr_dict_attr_t *da); +int dict_fixup_clone_enum(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da, char const *ref); + +int dict_fixup_vsa(dict_fixup_ctx_t *fctx, fr_dict_attr_t *da); int dict_fixup_alias(dict_fixup_ctx_t *fctx, char const *filename, int line, fr_dict_attr_t *alias_parent, char const *alias, diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c index b777ed29034..45c341f0a92 100644 --- a/src/lib/util/dict_tokenize.c +++ b/src/lib/util/dict_tokenize.c @@ -26,6 +26,7 @@ RCSID("$Id$") #include #include #include +#include #include #include #include @@ -736,7 +737,7 @@ void dict_attr_location_set(dict_tokenize_ctx_t *dctx, fr_dict_attr_t *da) da->line = dctx->stack[dctx->stack_depth].line; } -static int dict_attr_add_fixups(dict_fixup_ctx_t *fixup, fr_dict_attr_t *da) +static int dict_attr_add_or_fixup(dict_fixup_ctx_t *fixup, fr_dict_attr_t *da) { fr_dict_attr_ext_ref_t *ref; @@ -751,22 +752,32 @@ static int dict_attr_add_fixups(dict_fixup_ctx_t *fixup, fr_dict_attr_t *da) if (ref && fr_dict_attr_ref_is_unresolved(ref->type)) { switch (fr_dict_attr_ref_type(ref->type)) { case FR_DICT_ATTR_REF_ALIAS: - if (dict_fixup_group(fixup, da->filename, da->line, - da, ref->unresolved) < 0) return -1; + if (fr_dict_attr_add_initialised(da) < 0) { + error: + talloc_free(da); + return -1; + } + + if (dict_fixup_group(fixup, da, ref->unresolved) < 0) return -1; + break; - case FR_DICT_ATTR_REF_CLONE: case FR_DICT_ATTR_REF_ENUM: - if (dict_fixup_clone(fixup, da->filename, da->line, - fr_dict_attr_unconst(da->parent), da, - ref->unresolved) < 0) return -1; + if (fr_dict_attr_add_initialised(da) < 0) goto error; + + if (dict_fixup_clone_enum(fixup, da, ref->unresolved) < 0) return -1; + break; + case FR_DICT_ATTR_REF_CLONE: + if (dict_fixup_clone(fixup, da, ref->unresolved) < 0) return -1; break; default: fr_strerror_const("Unknown reference type"); return -1; } + } else { + if (fr_dict_attr_add_initialised(da) < 0) goto error; } return 0; @@ -991,9 +1002,7 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *ctx, char **argv, in /* * Add the attribute we allocated earlier */ - if (fr_dict_attr_add_initialised(da) < 0) goto error; - - if (unlikely(dict_attr_add_fixups(&ctx->fixup, da) < 0)) return -1; + if (dict_attr_add_or_fixup(&ctx->fixup, da) < 0) goto error; /* * Dynamically define where VSAs go. Note that we CANNOT @@ -1002,8 +1011,7 @@ static int dict_read_process_attribute(dict_tokenize_ctx_t *ctx, char **argv, in if (da->type == FR_TYPE_VSA) { if (parent->flags.is_root) ctx->dict->vsa_parent = attr; - if (dict_fixup_vsa(&ctx->fixup, CURRENT_FRAME(ctx)->filename, CURRENT_FRAME(ctx)->line, - UNCONST(fr_dict_attr_t *, da)) < 0) { + if (dict_fixup_vsa(&ctx->fixup, UNCONST(fr_dict_attr_t *, da)) < 0) { return -1; /* Leaves attr added */ } } @@ -1146,9 +1154,7 @@ static int dict_read_process_define(dict_tokenize_ctx_t *ctx, char **argv, int a /* * Add the attribute we allocated earlier */ - if (fr_dict_attr_add_initialised(da) < 0) goto error; - - if (unlikely(dict_attr_add_fixups(&ctx->fixup, da) < 0)) return -1; + if (unlikely(dict_attr_add_or_fixup(&ctx->fixup, da) < 0)) return -1; /* * Adding an attribute of type 'struct' is an implicit @@ -1257,9 +1263,7 @@ static int dict_read_process_enum(dict_tokenize_ctx_t *ctx, char **argv, int arg /* * Add the attribute we allocated earlier */ - if (fr_dict_attr_add_initialised(da) < 0) goto error; - - if (unlikely(dict_attr_add_fixups(&ctx->fixup, da) < 0)) return -1; + if (unlikely(dict_attr_add_or_fixup(&ctx->fixup, da) < 0)) return -1; memcpy(&ctx->value_attr, &da, sizeof(da)); @@ -1500,17 +1504,7 @@ static int dict_read_process_member(dict_tokenize_ctx_t *ctx, char **argv, int a goto error; } - if (unlikely(fr_dict_attr_add_initialised(da) < 0)) goto error; - if (unlikely(dict_attr_add_fixups(&ctx->fixup, da) < 0)) return -1; - - /* - * If we need to set the previous attribute, we have to - * look it up by number. This lets us set the - * *canonical* previous attribute, and not any potential - * duplicate which was just added. - */ - da = dict_attr_child_by_num(ctx->stack[ctx->stack_depth].da, ctx->stack[ctx->stack_depth].member_num); - fr_assert(da != NULL); + if (unlikely(dict_attr_add_or_fixup(&ctx->fixup, da) < 0)) return -1; /* * A 'struct' can have a MEMBER of type 'tlv', but ONLY @@ -1809,8 +1803,7 @@ static int dict_read_process_struct(dict_tokenize_ctx_t *ctx, char **argv, int a /* * Add the keyed STRUCT to the global namespace, and as a child of "parent". */ - if (unlikely(fr_dict_attr_add_initialised(da) < 0)) goto error; - if (unlikely(dict_attr_add_fixups(&ctx->fixup, da) < 0)) return -1; + if (unlikely(dict_attr_add_or_fixup(&ctx->fixup, da) < 0)) goto error; da = dict_attr_by_name(NULL, parent, name); if (!da) return -1;