From: Alan T. DeKok Date: Tue, 23 Dec 2025 10:58:55 +0000 (+0100) Subject: minor tweaks to tmpl_afrom_attr_substr() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9cc7fdf8dd58927546534ab9102991c84e7fc51c;p=thirdparty%2Ffreeradius-server.git minor tweaks to tmpl_afrom_attr_substr() and disallow raw local attributes --- diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index c77fd0b4cd1..75d05c98348 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -2291,55 +2291,10 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, &our_name, p_rules, at_rules, 0); if (ret < 0) goto error; - /* - * Check to see if the user wants the leaf - * attribute to be raw. - * - * We can only do the conversion now _if_ - * the complete hierarchy has been resolved - * otherwise we'll need to do the conversion - * later. - */ - if (tmpl_is_attr(vpt) && is_raw) { - ret = attr_to_raw(vpt, tmpl_attr_list_tail(tmpl_attr(vpt))); - if (ret < 0) goto error; - } - - /* - * Local variables cannot be given a list modifier. - */ - if (tmpl_is_attr(vpt) && tmpl_attr_tail_da(vpt)) { - tmpl_attr_t *ar = tmpl_attr_list_head(tmpl_attr(vpt)); - bool is_local = ar->ar_da->flags.local; - - for (; ar != NULL; - ar = tmpl_attr_list_next(tmpl_attr(vpt), ar)) { - if (!ar->ar_da->flags.local || - (ar->ar_da->flags.local && is_local)) continue; - - fr_strerror_printf("Local attributes cannot be used in any list"); - if (err) *err = TMPL_ATTR_ERROR_FOREIGN_NOT_ALLOWED; - fr_sbuff_set(&our_name, &m_l); - goto error; - } - - /* - * That being said, local variables are named "foo", but are always put into the local list. - */ - if (is_local && (at_rules->list_presence != TMPL_ATTR_LIST_FORBID)) { - MEM(ar = talloc(vpt, tmpl_attr_t)); - *ar = (tmpl_attr_t){ - .ar_type = TMPL_ATTR_TYPE_NORMAL, - .ar_da = request_attr_local, - .ar_parent = fr_dict_root(fr_dict_internal()) - }; - - /* - * Prepend the local list ref so it gets evaluated - * first. - */ - tmpl_attr_list_insert_head(tmpl_attr(vpt), ar); - } + if (!tmpl_substr_terminal_check(&our_name, p_rules)) { + fr_strerror_const("Unexpected text after attribute reference"); + if (err) *err = TMPL_ATTR_ERROR_MISSING_TERMINATOR; + goto error; } /* @@ -2366,8 +2321,88 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, break; } + tmpl_set_name(vpt, T_BARE_WORD, fr_sbuff_start(&our_name), fr_sbuff_used(&our_name)); + vpt->rules = *t_rules; /* Record the rules */ + + /* + * Check to see if the user wants the leaf + * attribute to be raw. + * + * We can only do the conversion now _if_ + * the complete hierarchy has been resolved + * otherwise we'll need to do the conversion + * later. + */ + if (tmpl_is_attr(vpt)) { + if (is_raw) { + ret = attr_to_raw(vpt, tmpl_attr_list_tail(tmpl_attr(vpt))); + if (ret < 0) goto error; + } + + /* + * Local variables cannot be given a list modifier. + */ + if (tmpl_attr_tail_da(vpt)) { + tmpl_attr_t *ar = tmpl_attr_list_head(tmpl_attr(vpt)); + bool is_local = ar->ar_da->flags.local; + bool allow_local = is_local; + + if (is_local && is_raw) { + fr_strerror_printf("Local attributes cannot be 'raw'"); + if (err) *err = TMPL_ATTR_ERROR_UNKNOWN_NOT_ALLOWED; + fr_sbuff_set(&our_name, &m_l); + goto error; + } + + /* + * We can transition from local to non-local, but not the other way around. + */ + for (; + ar != NULL; + ar = tmpl_attr_list_next(tmpl_attr(vpt), ar)) { + if (ar->ar_da->flags.local == allow_local) continue; + + if (!ar->ar_da->flags.local && allow_local) { + allow_local = false; + continue; + } + + if (ar->ar_da->flags.local) { + fr_strerror_printf("Local attributes cannot be used in any list"); + if (err) *err = TMPL_ATTR_ERROR_FOREIGN_NOT_ALLOWED; + fr_sbuff_set(&our_name, &m_l); + goto error; + } + } + + /* + * Local variables are named "foo", but are always put into the local list. + * + * We add the list after checking for non-local -> local transition, as + * request_attr_local isn't a local attribute. + * + * When the list is forbidden, we're creating a local attribute inside of a local + * TLV. + */ + if (is_local && (at_rules->list_presence != TMPL_ATTR_LIST_FORBID)) { + MEM(ar = talloc(vpt, tmpl_attr_t)); + *ar = (tmpl_attr_t){ + .ar_type = TMPL_ATTR_TYPE_NORMAL, + .ar_da = request_attr_local, + .ar_parent = fr_dict_root(fr_dict_internal()) + }; + + /* + * Prepend the local list ref so it gets evaluated + * first. + */ + tmpl_attr_list_insert_head(tmpl_attr(vpt), ar); + } + } + } + /* - * If we're using lists, ensure that the default list is specified. + * Ensure that the default list is specified. */ if (!tmpl_attr_is_list_attr(tmpl_attr_list_head(tmpl_attr(vpt)))) { tmpl_attr_t *ar; @@ -2388,30 +2423,23 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, tmpl_attr_list_insert_head(tmpl_attr(vpt), ar); } - tmpl_set_name(vpt, T_BARE_WORD, fr_sbuff_start(&our_name), fr_sbuff_used(&our_name)); - vpt->rules = *t_rules; /* Record the rules */ - /* - * If there are actual requests, duplicate them - * and move them into the list. + * If there is a default request (parent, outer, etc.), duplicate them and move them into the + * list. * - * A NULL request_def pointer is equivalent to the - * current request. + * A NULL request_def pointer is equivalent to the current request. */ if (t_rules->attr.request_def) { tmpl_request_ref_list_acopy(vpt, &vpt->rules.attr.request_def, t_rules->attr.request_def); } + /* + * Now that all of the lists are set correctly, do some final validation and updates on the + * attribute. + */ if (tmpl_is_attr(vpt)) { tmpl_attr_t *ar; - /* - * Suppress useless casts. - */ - if (tmpl_attr_tail_is_normal(vpt) && (tmpl_attr_tail_da(vpt)->type == tmpl_rules_cast(vpt))) { - vpt->rules.cast = FR_TYPE_NULL; - } - /* * Ensure that the list is set correctly, so that * the returned vpt just doesn't just match the @@ -2421,25 +2449,27 @@ ssize_t tmpl_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t *err, fr_assert(ar != NULL); if (tmpl_attr_is_list_attr(ar)) vpt->rules.attr.list_def = ar->ar_da; - } - if (!tmpl_substr_terminal_check(&our_name, p_rules)) { - fr_strerror_const("Unexpected text after attribute reference"); - if (err) *err = TMPL_ATTR_ERROR_MISSING_TERMINATOR; - goto error; - } + if (tmpl_attr_tail_is_normal(vpt)) { + /* + * Suppress useless casts. + */ + if (tmpl_attr_tail_da(vpt)->type == tmpl_rules_cast(vpt)) { + vpt->rules.cast = FR_TYPE_NULL; + } - /* - * If everything was resolved correctly - * we now need to check the cast type. - */ - if (!tmpl_needs_resolving(vpt) && !fr_type_is_null(t_rules->cast) && - !fr_type_cast(t_rules->cast, tmpl_attr_tail_da(vpt)->type)) { - fr_strerror_printf("Cannot cast type '%s' to '%s'", - fr_type_to_str(tmpl_attr_tail_da(vpt)->type), fr_type_to_str(t_rules->cast)); - if (err) *err = TMPL_ATTR_ERROR_BAD_CAST; - fr_sbuff_set_to_start(&our_name); - goto error; + /* + * Check if the cast is allowed. This lets us give better errors at compile time. + */ + if ((tmpl_rules_cast(vpt)!= FR_TYPE_NULL) && + !fr_type_cast(tmpl_rules_cast(vpt), tmpl_attr_tail_da(vpt)->type)) { + fr_strerror_printf("Cannot cast type '%s' to '%s'", + fr_type_to_str(tmpl_attr_tail_da(vpt)->type), fr_type_to_str(t_rules->cast)); + if (err) *err = TMPL_ATTR_ERROR_BAD_CAST; + fr_sbuff_set_to_start(&our_name); + goto error; + } + } } TMPL_VERIFY(vpt); /* Because we want to ensure we produced something sane */ diff --git a/src/lib/util/dict_unknown.c b/src/lib/util/dict_unknown.c index 744ae7d3a52..06124fe02b3 100644 --- a/src/lib/util/dict_unknown.c +++ b/src/lib/util/dict_unknown.c @@ -28,6 +28,12 @@ static int dict_attr_unknown_init(fr_dict_attr_t const *parent, UNUSED fr_dict_a { flags->is_unknown = true; + if (parent->flags.local) { + fr_strerror_printf("Cannot create 'raw' attribute of data type '%s' which is a local variable", + fr_type_to_str(type)); + return -1; + } + if (parent->flags.internal) { fr_strerror_printf("Cannot create 'raw' attribute of data type '%s' which is 'internal'", fr_type_to_str(type)); diff --git a/src/lib/util/dict_validate.c b/src/lib/util/dict_validate.c index 2b044372481..2268c0bdb4a 100644 --- a/src/lib/util/dict_validate.c +++ b/src/lib/util/dict_validate.c @@ -97,6 +97,11 @@ bool dict_attr_flags_valid(fr_dict_attr_t *da) return false; } + if (flags->local && (flags->is_unknown || flags->is_raw)) { + fr_strerror_const("Local variables cannot be 'raw' or unknown"); + return false; + } + /* * Only some data types can be in arrays, because we need * to be able to decode the various array members. diff --git a/src/tests/keywords/local-raw-error b/src/tests/keywords/local-raw-error new file mode 100644 index 00000000000..69ef25e5fdd --- /dev/null +++ b/src/tests/keywords/local-raw-error @@ -0,0 +1,4 @@ +uint32 foo +octets bar + +bar := raw.foo # ERROR