From: Arran Cudbard-Bell Date: Tue, 11 May 2021 17:13:13 +0000 (+0200) Subject: Produce better errors when we can't resolve attributes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f69bf55fb52d0c64166aba9a985146bf6df4a9f0;p=thirdparty%2Ffreeradius-server.git Produce better errors when we can't resolve attributes Be more strict about only resolving attributes in the dictionary associated with the current virtual server --- diff --git a/src/lib/server/cond.h b/src/lib/server/cond.h index 245edbf7214..fb8a532bf04 100644 --- a/src/lib/server/cond.h +++ b/src/lib/server/cond.h @@ -95,13 +95,18 @@ struct fr_cond_s { fr_cond_t *next; }; +typedef struct { + fr_cond_t *cond; +} fr_cond_iter_t; + ssize_t fr_cond_tokenize(CONF_SECTION *cs, fr_cond_t **head, tmpl_rules_t const *rules, fr_sbuff_t *in) CC_HINT(nonnull(1,2,4)); int fr_cond_promote_types(fr_cond_t *c, fr_sbuff_t *in, fr_sbuff_marker_t *m_lhs, fr_sbuff_marker_t *m_rhs) CC_HINT(nonnull(1)); ssize_t cond_print(fr_sbuff_t *out, fr_cond_t const *c); -bool fr_cond_walk(fr_cond_t *head, bool (*callback)(fr_cond_t *cond, void *uctx), void *uctx); +fr_cond_t *fr_cond_iter_init(fr_cond_iter_t *iter, fr_cond_t *head); +fr_cond_t *fr_cond_iter_next(fr_cond_iter_t *iter); void fr_cond_async_update(fr_cond_t *cond); diff --git a/src/lib/server/cond_tokenize.c b/src/lib/server/cond_tokenize.c index 0697bf10436..2445b44ae38 100644 --- a/src/lib/server/cond_tokenize.c +++ b/src/lib/server/cond_tokenize.c @@ -1588,47 +1588,42 @@ ssize_t fr_cond_tokenize(CONF_SECTION *cs, fr_cond_t **head, tmpl_rules_t const return slen + diff; } -/* - * Walk in order. +/** Initialise a cond iterator + * + * Will return the first leaf condition node. + * + * @param[out] iter to initialise. + * @param[in] head the root of the condition structure. + * @return The first leaf condition node. */ -bool fr_cond_walk(fr_cond_t *c, bool (*callback)(fr_cond_t *cond, void *uctx), void *uctx) +fr_cond_t *fr_cond_iter_init(fr_cond_iter_t *iter, fr_cond_t *head) { - while (c) { - /* - * Process this one, exit on error. - */ - if (!callback(c, uctx)) return false; + fr_cond_t *c; - switch (c->type) { - case COND_TYPE_INVALID: - return false; + for (c = head; c->type == COND_TYPE_CHILD; c = c->data.child); /* Deepest condition */ - case COND_TYPE_RCODE: - case COND_TYPE_TMPL: - case COND_TYPE_MAP: - case COND_TYPE_AND: - case COND_TYPE_OR: - case COND_TYPE_TRUE: - case COND_TYPE_FALSE: - break; + return iter->cond = c; +} - case COND_TYPE_CHILD: - /* - * Walk over the child. - */ - if (!fr_cond_walk(c->data.child, callback, uctx)) { - return false; - } - break; - } +/** Get the next leaf condition node + * + * @param[in] iter to iterate over. + * @return The next leaf condition node. + */ +fr_cond_t *fr_cond_iter_next(fr_cond_iter_t *iter) +{ + fr_cond_t *c; - /* - * process the next sibling - */ - c = c->next; + /* + * Walk up the tree, maybe... + */ + for (c = iter->cond; c; c = c->parent) { + if (!c->next) continue; /* Done with this level */ + for (c = c->next; c->type == COND_TYPE_CHILD; c = c->data.child); /* down we go... */ + break; } - return true; + return iter->cond = c; } /** Update the condition with "is async required". diff --git a/src/lib/server/tmpl.h b/src/lib/server/tmpl.h index 5c1fde7f31d..8207356dbce 100644 --- a/src/lib/server/tmpl.h +++ b/src/lib/server/tmpl.h @@ -873,6 +873,8 @@ void tmpl_set_name_shallow(tmpl_t *vpt, fr_token_t quote, char const *name, ss void tmpl_set_name(tmpl_t *vpt, fr_token_t quote, char const *name, ssize_t len); +void tmpl_set_dict_def(tmpl_t *vpt, fr_dict_t const *dict); + int tmpl_afrom_value_box(TALLOC_CTX *ctx, tmpl_t **out, fr_value_box_t *data, bool steal); void tmpl_attr_ref_debug(const tmpl_attr_t *ar, int idx); diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index 1ffe1b97f76..6e19a5aec15 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -536,6 +536,16 @@ void tmpl_set_name(tmpl_t *vpt, fr_token_t quote, char const *name, ssize_t len) vpt->quote = quote; } +/** Change the default dictionary in the tmpl's resolution rules + * + * @param[in] vpt to alter. + * @param[in] dict to set. + */ +void tmpl_set_dict_def(tmpl_t *vpt, fr_dict_t const *dict) +{ + vpt->rules.dict_def = dict; +} + /** Initialise a tmpl using a format string to create the name * * @param[in] vpt to initialise. @@ -1437,12 +1447,13 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t slen = fr_dict_attr_search_by_qualified_name_substr(&dict_err, &da, t_rules->dict_def, name, p_rules ? p_rules->terminals : NULL, - !t_rules->disallow_internal); + !t_rules->disallow_internal, + t_rules->allow_foreign); /* * We can't know which dictionary the * attribute will be resolved in, so the - * only way of recording the parent is - * by looking at the da. + * only way of recording what the parent + * is by looking at the da. */ if (da) our_parent = da->parent; /* @@ -3054,7 +3065,8 @@ static inline CC_HINT(always_inline) int tmpl_attr_resolve(tmpl_t *vpt) &FR_SBUFF_IN(ar->ar_unresolved, talloc_array_length(ar->ar_unresolved) - 1), NULL, - !vpt->rules.disallow_internal); + !vpt->rules.disallow_internal, + vpt->rules.allow_foreign); if (!da) return -2; /* Can't resolve, maybe the caller can resolve later */ ar->ar_type = TMPL_ATTR_TYPE_NORMAL; diff --git a/src/lib/unlang/compile.c b/src/lib/unlang/compile.c index 04ccc192b70..e2fefa226cc 100644 --- a/src/lib/unlang/compile.c +++ b/src/lib/unlang/compile.c @@ -174,12 +174,18 @@ static inline CC_HINT(always_inline) int unlang_rules_verify(tmpl_rules_t const #define RULES_VERIFY(_rules) if (unlang_rules_verify(_rules) < 0) return NULL; -static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, CONF_ITEM const *ci, tmpl_t **vpt_p) +static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, tmpl_t **vpt_p, CONF_ITEM const *ci, fr_dict_t const *dict) { tmpl_t *vpt = *vpt_p; TMPL_VERIFY(vpt); + /* + * We may now know the correct dictionary + * where we didn't before... + */ + if (!vpt->rules.dict_def) tmpl_set_dict_def(vpt, dict); + /* * Convert virtual &Attr-Foo to "%{Attr-Foo}" */ @@ -205,9 +211,9 @@ static bool pass2_fixup_tmpl(TALLOC_CTX *ctx, CONF_ITEM const *ci, tmpl_t **vpt_ return true; } -static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) +static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci, fr_dict_t const *dict) { - tmpl_t *vpt; + tmpl_t *vpt; map_t *map; map = c->data.map; /* shorter */ @@ -237,11 +243,11 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) * Resolve the attribute references first */ if (tmpl_is_attr_unresolved(map->lhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) return false; + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) return false; } if (tmpl_is_attr_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false; + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; } c->pass2_fixup = PASS2_FIXUP_NONE; @@ -273,11 +279,11 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) * @todo - allow anything anywhere. */ if (!tmpl_is_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) { + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { return false; } } else { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) { + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { return false; } @@ -346,20 +352,20 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) * forbids this. */ if (tmpl_is_attr(map->lhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false; + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; } else { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false; + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) return false; } } if (tmpl_is_exec_unresolved(map->lhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) { + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, dict)) { return false; } } if (tmpl_is_exec_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) { + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) { return false; } } @@ -400,7 +406,7 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) #ifdef HAVE_REGEX if (tmpl_is_regex_xlat_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) { + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, dict)) { return false; } } @@ -470,50 +476,6 @@ static bool pass2_fixup_cond_map(fr_cond_t *c, CONF_ITEM *ci) return true; } -static bool pass2_cond_callback(fr_cond_t *c, void *uctx) -{ - CONF_SECTION *cs = talloc_get_type_abort(uctx, CONF_SECTION); - CONF_ITEM *ci = cf_section_to_item(cs); - - switch (c->type) { - /* - * These don't get optimized. - */ - case COND_TYPE_TRUE: - case COND_TYPE_FALSE: - case COND_TYPE_RCODE: - case COND_TYPE_AND: - case COND_TYPE_OR: - return true; - - /* - * Call children. - */ - case COND_TYPE_CHILD: - return pass2_cond_callback(c->data.child, uctx); - - /* - * Fix up the template. - */ - case COND_TYPE_TMPL: - fr_assert(!tmpl_is_regex_xlat_unresolved(c->data.vpt)); - return pass2_fixup_tmpl(c, ci, &c->data.vpt); - - /* - * Fixup the map - */ - case COND_TYPE_MAP: - return pass2_fixup_cond_map(c, uctx); - - /* - * Nothing else has pass2 fixups - */ - default: - fr_assert(0); - return false; - } -} - static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dict_attr_t const *parent) { RULES_VERIFY(rules); @@ -525,13 +487,13 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic * FIXME: compile to attribute && handle * the conversion in map_to_vp(). */ - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) { + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) { return false; } } if (tmpl_is_exec(map->lhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) { + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) { return false; } } @@ -540,7 +502,7 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic * Deal with undefined attributes now. */ if (tmpl_is_attr_unresolved(map->lhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->lhs)) return false; + if (!pass2_fixup_tmpl(map, &map->lhs, map->ci, rules->dict_def)) return false; } /* @@ -563,7 +525,7 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic * FIXME: compile to attribute && handle * the conversion in map_to_vp(). */ - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) { + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) { return false; } } @@ -571,11 +533,11 @@ static bool pass2_fixup_update_map(map_t *map, tmpl_rules_t const *rules, fr_dic fr_assert(!tmpl_is_regex_xlat_unresolved(map->rhs)); if (tmpl_is_attr_unresolved(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) return false; + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) return false; } if (tmpl_is_exec(map->rhs)) { - if (!pass2_fixup_tmpl(map, map->ci, &map->rhs)) { + if (!pass2_fixup_tmpl(map, &map->rhs, map->ci, rules->dict_def)) { return false; } } @@ -646,7 +608,8 @@ static bool pass2_fixup_map_rhs(unlang_group_t *g, tmpl_rules_t const *rules) */ if (!gext->vpt) return true; - return pass2_fixup_tmpl(fr_map_list_head(&gext->map)->ci, cf_section_to_item(g->cs), &gext->vpt); + return pass2_fixup_tmpl(fr_map_list_head(&gext->map)->ci, &gext->vpt, + cf_section_to_item(g->cs), rules->dict_def); } static void unlang_dump(unlang_t *instruction, int depth) @@ -1981,7 +1944,7 @@ static unlang_t *compile_switch(unlang_t *parent, unlang_compile_t *unlang_ctx, * This is so that compile_case() can do attribute type * checks / casts against us. */ - if (!pass2_fixup_tmpl(g, cf_section_to_item(cs), &gext->vpt)) { + if (!pass2_fixup_tmpl(g, &gext->vpt, cf_section_to_item(cs), unlang_ctx->rules->dict_def)) { talloc_free(g); return NULL; } @@ -2493,7 +2456,7 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan unlang_t *c; unlang_group_t *g; - unlang_cond_t *gext; + unlang_cond_t *gext; fr_cond_t *cond; @@ -2510,13 +2473,35 @@ static unlang_t *compile_if_subsection(unlang_t *parent, unlang_compile_t *unlan unlang_ops[ext->type].name); c = compile_empty(parent, unlang_ctx, cs, ext); } else { - /* - * The condition may refer to attributes, xlats, or - * Auth-Types which didn't exist when it was first - * parsed. Now that they are all defined, we need to fix - * them up. - */ - if (!fr_cond_walk(cond, pass2_cond_callback, cs)) return NULL; + fr_cond_iter_t iter; + fr_cond_t *leaf; + + for (leaf = fr_cond_iter_init(&iter, cond); + leaf; + leaf = fr_cond_iter_next(&iter)) { + switch (leaf->type) { + /* + * Fix up the template. + */ + case COND_TYPE_TMPL: + fr_assert(!tmpl_is_regex_xlat_unresolved(leaf->data.vpt)); + if (!pass2_fixup_tmpl(leaf, &leaf->data.vpt, cf_section_to_item(cs), + unlang_ctx->rules->dict_def)) return false; + break; + + /* + * Fixup the map + */ + case COND_TYPE_MAP: + if (!pass2_fixup_cond_map(leaf, cf_section_to_item(cs), + unlang_ctx->rules->dict_def)) return false; + break; + + default: + continue; + } + } + fr_cond_async_update(cond); c = compile_section(parent, unlang_ctx, cs, ext); } @@ -2744,7 +2729,7 @@ static unlang_t *compile_load_balance_subsection(unlang_t *parent, unlang_compil /* * Fixup the templates */ - if (!pass2_fixup_tmpl(g, cf_section_to_item(cs), &gext->vpt)) { + if (!pass2_fixup_tmpl(g, &gext->vpt, cf_section_to_item(cs), unlang_ctx->rules->dict_def)) { talloc_free(g); return NULL; } @@ -2857,7 +2842,7 @@ static unlang_t *compile_subrequest(unlang_t *parent, unlang_compile_t *unlang_c unlang_t *c; unlang_group_t *g; - unlang_subrequest_t *gext; + unlang_subrequest_t *gext; unlang_compile_t unlang_ctx2; diff --git a/src/lib/util/dict.h b/src/lib/util/dict.h index 38a9cdc4aa1..953d2e62296 100644 --- a/src/lib/util/dict.h +++ b/src/lib/util/dict.h @@ -502,29 +502,30 @@ fr_dict_attr_t const *fr_dict_vendor_da_by_num(fr_dict_attr_t const *vendor_root ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *name, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) CC_HINT(nonnull(2, 4)); ssize_t fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *name, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) CC_HINT(nonnull(2, 4)); ssize_t fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *in, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) CC_HINT(nonnull(2, 4)); fr_dict_attr_t const *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *err, - fr_dict_t const *dict_def, char const *attr, bool fallback) + fr_dict_t const *dict_def, char const *attr, + bool internal, bool foreign) CC_HINT(nonnull(3)); ssize_t fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *in, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) CC_HINT(nonnull(2, 4)); ssize_t fr_dict_attr_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c index 11909718ca8..7af2b06a818 100644 --- a/src/lib/util/dict_util.c +++ b/src/lib/util/dict_util.c @@ -1902,7 +1902,7 @@ ssize_t dict_by_protocol_substr(fr_dict_attr_err_t *err, if (!dict) { fr_strerror_printf("Unknown protocol '%s'", root.name); - *out = NULL; + memcpy(out, &dict_def, sizeof(*out)); return 0; } @@ -2160,12 +2160,25 @@ typedef ssize_t (*dict_attr_resolve_func_t)(fr_dict_attr_err_t *err, /** Internal function for searching for attributes in multiple dictionaries * + * @param[out] err Any errors that occurred searching. + * @param[out] out The attribute we found. + * @param[in] dict_def The default dictionary to search in. + * @param[in] in string to resolve to an attribute. + * @param[in] tt terminals that indicate the end of the string. + * @param[in] internal Resolve the attribute in the internal dictionary. + * @param[in] foreign Resolve attribute in a foreign dictionary, + * i.e. one other than dict_def. + * @param[in] func to use for resolution. + * @return + * - <=0 on error (the offset of the error). + * - >0 on success. */ static inline CC_HINT(always_inline) ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *in, fr_sbuff_term_t const *tt, - bool fallback, dict_attr_resolve_func_t func) + bool internal, bool foreign, + dict_attr_resolve_func_t func) { fr_dict_attr_err_t our_err; fr_hash_iter_t iter; @@ -2174,6 +2187,14 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, ssize_t slen; fr_sbuff_t our_in = FR_SBUFF_NO_ADVANCE(in); + /* + * Always going to fail... + */ + if (unlikely(!internal && !foreign && !dict_def)) { + if (err) *err = FR_DICT_ATTR_EINVAL; + return 0; + } + /* * dict_def search in the specified dictionary */ @@ -2184,7 +2205,7 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, return fr_sbuff_set(in, &our_in); case FR_DICT_ATTR_NOTFOUND: - if (!fallback) goto error; + if (!internal && !foreign) goto error; break; default: @@ -2195,17 +2216,19 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, /* * Next in the internal dictionary */ - slen = func(&our_err, out, fr_dict_root(dict_gctx->internal), &our_in, tt); - switch (our_err) { - case FR_DICT_ATTR_OK: - return fr_sbuff_set(in, &our_in); + if (internal) { + slen = func(&our_err, out, fr_dict_root(dict_gctx->internal), &our_in, tt); + switch (our_err) { + case FR_DICT_ATTR_OK: + return fr_sbuff_set(in, &our_in); - case FR_DICT_ATTR_NOTFOUND: - if (!fallback) goto error; - break; + case FR_DICT_ATTR_NOTFOUND: + if (!foreign) goto error; + break; - default: - goto error; + default: + goto error; + } } /* @@ -2231,6 +2254,44 @@ ssize_t dict_attr_search(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, } error: + if (our_err == FR_DICT_ATTR_NOTFOUND) { + fr_sbuff_marker_t start; + char *list = NULL; + + our_in = FR_SBUFF_NO_ADVANCE(in); + fr_sbuff_marker(&start, &our_in); + + list = talloc_strdup(NULL, ""); + + if (dict_def) { + list = talloc_strdup_append_buffer(list, fr_dict_root(dict_def)->name); + list = talloc_strdup_append_buffer(list, ", "); + } + if (internal) { + list = talloc_strdup_append_buffer(list, fr_dict_root(dict_gctx->internal)->name); + list = talloc_strdup_append_buffer(list, ", "); + } + + if (foreign) { + for (dict = fr_hash_table_iter_init(dict_gctx->protocol_by_num, &iter); + dict; + dict = fr_hash_table_iter_next(dict_gctx->protocol_by_num, &iter)) { + if (dict == dict_def) continue; + if (dict == dict_gctx->internal) continue; + + list = talloc_strdup_append_buffer(list, fr_dict_root(dict)->name); + list = talloc_strdup_append_buffer(list, ", "); + } + } + + fr_strerror_printf("Attribute '%pV' not found. Searched in: %pV", + fr_box_strvalue_len(fr_sbuff_current(&start), + fr_sbuff_adv_until(&our_in, SIZE_MAX, tt, '\0')), + fr_box_strvalue_len(list, talloc_array_length(list) - 3)); + + talloc_free(list); + } + if (err) *err = our_err; *out = NULL; @@ -2246,7 +2307,8 @@ static inline CC_HINT(always_inline) ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *in, fr_sbuff_term_t const *tt, - bool fallback, dict_attr_resolve_func_t func) + bool internal, bool foreign, + dict_attr_resolve_func_t func) { fr_sbuff_t our_in = FR_SBUFF_NO_ADVANCE(in); fr_dict_attr_err_t our_err; @@ -2277,10 +2339,10 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const return 0; } - fallback = false; + internal = foreign = false; } - slen = dict_attr_search(&our_err, out, initial, &our_in, tt, fallback, func); + slen = dict_attr_search(&our_err, out, initial, &our_in, tt, internal, foreign, func); if (our_err != FR_DICT_ATTR_OK) goto error; return fr_sbuff_set(in, &our_in); @@ -2303,7 +2365,8 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const * @param[in] dict_def Default dictionary for non-qualified dictionaries. * @param[in] name Dictionary/Attribute name. * @param[in] tt Terminal strings. - * @param[in] fallback If true, fallback to the internal dictionary. + * @param[in] internal If true, fallback to the internal dictionary. + * @param[in] foreign If true, fallback to foreign dictionaries. * @return * - <= 0 on failure. * - The number of bytes of name consumed on success. @@ -2311,9 +2374,10 @@ ssize_t dict_attr_search_qualified(fr_dict_attr_err_t *err, fr_dict_attr_t const ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *name, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) { - return dict_attr_search_qualified(err, out, dict_def, name, tt, fallback, fr_dict_attr_by_name_substr); + return dict_attr_search_qualified(err, out, dict_def, name, tt, + internal, foreign, fr_dict_attr_by_name_substr); } /** Locate a #fr_dict_attr_t by its name in the top level namespace of a dictionary @@ -2330,7 +2394,8 @@ ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr * @param[in] dict_def Default dictionary for non-qualified dictionaries. * @param[in] name Dictionary/Attribute name. * @param[in] tt Terminal strings. - * @param[in] fallback If true, fallback to the internal dictionary. + * @param[in] internal If true, fallback to the internal dictionary. + * @param[in] foreign If true, fallback to foreign dictionaries. * @return * - <= 0 on failure. * - The number of bytes of name consumed on success. @@ -2338,9 +2403,10 @@ ssize_t fr_dict_attr_search_by_qualified_name_substr(fr_dict_attr_err_t *err, fr ssize_t fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, fr_sbuff_t *name, fr_sbuff_term_t const *tt, - bool fallback) + bool internal, bool foreign) { - return dict_attr_search_qualified(err, out, dict_def, name, tt, fallback, fr_dict_attr_by_name_substr); + return dict_attr_search_qualified(err, out, dict_def, name, tt, + internal, foreign, fr_dict_attr_by_name_substr); } /** Locate a qualified #fr_dict_attr_t by a dictionary qualified OID string @@ -2357,16 +2423,19 @@ ssize_t fr_dict_attr_search_by_name_substr(fr_dict_attr_err_t *err, fr_dict_attr * @param[in] dict_def Default dictionary for non-qualified dictionaries. * @param[in] in Dictionary/Attribute name. * @param[in] tt Terminal strings. - * @param[in] fallback If true, fallback to the internal dictionary. + * @param[in] internal If true, fallback to the internal dictionary. + * @param[in] foreign If true, fallback to foreign dictionaries. * @return * - <= 0 on failure. * - The number of bytes of name consumed on success. */ ssize_t fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, - fr_sbuff_t *in, fr_sbuff_term_t const *tt, bool fallback) + fr_sbuff_t *in, fr_sbuff_term_t const *tt, + bool internal, bool foreign) { - return dict_attr_search_qualified(err, out, dict_def, in, tt, fallback, fr_dict_attr_by_oid_substr); + return dict_attr_search_qualified(err, out, dict_def, in, tt, + internal, foreign, fr_dict_attr_by_oid_substr); } /** Locate a qualified #fr_dict_attr_t by a dictionary using a non-qualified OID string @@ -2383,16 +2452,19 @@ ssize_t fr_dict_attr_search_by_qualified_oid_substr(fr_dict_attr_err_t *err, fr_ * @param[in] dict_def Default dictionary for non-qualified dictionaries. * @param[in] in Dictionary/Attribute name. * @param[in] tt Terminal strings. - * @param[in] fallback If true, fallback to the internal dictionary. + * @param[in] internal If true, fallback to the internal dictionary. + * @param[in] foreign If true, fallback to foreign dictionaries. * @return * - <= 0 on failure. * - The number of bytes of name consumed on success. */ ssize_t fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_t const **out, fr_dict_t const *dict_def, - fr_sbuff_t *in, fr_sbuff_term_t const *tt, bool fallback) + fr_sbuff_t *in, fr_sbuff_term_t const *tt, + bool internal, bool foreign) { - return dict_attr_search_qualified(err, out, dict_def, in, tt, fallback, fr_dict_attr_by_oid_substr); + return dict_attr_search_qualified(err, out, dict_def, in, tt, + internal, foreign, fr_dict_attr_by_oid_substr); } /** Locate a qualified #fr_dict_attr_t by its name and a dictionary qualifier @@ -2401,11 +2473,13 @@ ssize_t fr_dict_attr_search_by_oid_substr(fr_dict_attr_err_t *err, fr_dict_attr_ * @see fr_dict_attr_err_t. * @param[in] dict_def Default dictionary for non-qualified dictionaries. * @param[in] name Dictionary/Attribute name. - * @param[in] fallback If true, fallback to the internal dictionary. + * @param[in] internal If true, fallback to the internal dictionary. + * @param[in] foreign If true, fallback to foreign dictionaries. * @return an #fr_dict_attr_err_t value. */ fr_dict_attr_t const *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *err, fr_dict_t const *dict_def, - char const *name, bool fallback) + char const *name, + bool internal, bool foreign) { ssize_t slen; fr_sbuff_t our_name; @@ -2413,7 +2487,7 @@ fr_dict_attr_t const *fr_dict_attr_search_by_qualified_oid(fr_dict_attr_err_t *e fr_sbuff_init(&our_name, name, strlen(name) + 1); - slen = fr_dict_attr_search_by_qualified_oid_substr(err, &da, dict_def, &our_name, NULL, fallback); + slen = fr_dict_attr_search_by_qualified_oid_substr(err, &da, dict_def, &our_name, NULL, internal, foreign); if (slen <= 0) return NULL; if ((size_t)slen != fr_sbuff_len(&our_name)) { diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c index ecf2df65ae3..e9662ebc997 100644 --- a/src/lib/util/pair_legacy.c +++ b/src/lib/util/pair_legacy.c @@ -175,7 +175,7 @@ fr_pair_t *fr_pair_make(TALLOC_CTX *ctx, fr_dict_t const *dict, fr_pair_list_t * * It's not found in the dictionary, so we use * another method to create the attribute. */ - da = fr_dict_attr_search_by_qualified_oid(NULL, dict, attrname, true); + da = fr_dict_attr_search_by_qualified_oid(NULL, dict, attrname, true, true); if (!da) { vp = fr_pair_make_unknown(ctx, dict, attrname, value, op); if (!vp) return NULL; diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c index 28332168cd0..42b35361a8b 100644 --- a/src/lib/util/sbuff.c +++ b/src/lib/util/sbuff.c @@ -1624,12 +1624,12 @@ size_t fr_sbuff_adv_past_allowed(fr_sbuff_t *sbuff, size_t len, bool return total; } -/** Wind position until we hit a character in the until set +/** Wind position until we hit a character in the terminal set * * @param[in] sbuff sbuff to search in. * @param[in] len Maximum amount to advance by. Unconstrained if SIZE_MAX. * @param[in] tt Token terminals in the encompassing grammar. - * @param[in] escape_chr If not '\0', ignore characters in the until set when + * @param[in] escape_chr If not '\0', ignore characters in the tt set when * prefixed with this escape character. * @return how many bytes we advanced. */ diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c index 606d8352cab..7e4ef3c519a 100644 --- a/src/modules/rlm_detail/rlm_detail.c +++ b/src/modules/rlm_detail/rlm_detail.c @@ -174,7 +174,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf) attr = cf_pair_attr(cf_item_to_pair(ci)); if (!attr) continue; /* pair-anoia */ - da = fr_dict_attr_search_by_qualified_oid(NULL, dict_radius, attr, false); + da = fr_dict_attr_search_by_qualified_oid(NULL, dict_radius, attr, false, false); if (!da) { cf_log_perr(conf, "Failed resolving attribute"); return -1; diff --git a/src/modules/rlm_passwd/rlm_passwd.c b/src/modules/rlm_passwd/rlm_passwd.c index b024b8ed787..40bae43c4d3 100644 --- a/src/modules/rlm_passwd/rlm_passwd.c +++ b/src/modules/rlm_passwd/rlm_passwd.c @@ -480,7 +480,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf) } da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, - inst->pwd_fmt->field[key_field], true); + inst->pwd_fmt->field[key_field], true, true); if (!da) { PERROR("Unable to resolve attribute"); release_ht(inst->ht); diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 63717a1dec2..256eeaa0ec9 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -1069,7 +1069,8 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) goto error; } - inst->group_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, group_attribute, false); + inst->group_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, group_attribute, + false, false); if (!inst->group_da) { PERROR("Failed resolving group attribute"); goto error; diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index 70b3608645a..f689018e040 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -427,7 +427,7 @@ static int mod_instantiate(void *instance, CONF_SECTION *conf) } inst->allocated_address_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, - inst->allocated_address_attr, false); + inst->allocated_address_attr, false, false); if (!inst->allocated_address_da) { cf_log_perr(conf, "Failed resolving attribute"); return -1; diff --git a/src/tests/keywords/dhcpv4.conf b/src/tests/keywords/dhcpv4.conf new file mode 100644 index 00000000000..3e3f95a9979 --- /dev/null +++ b/src/tests/keywords/dhcpv4.conf @@ -0,0 +1,58 @@ + +modules { + $INCLUDE ${raddb}/mods-enabled/always + + $INCLUDE ${raddb}/mods-enabled/expr + + $INCLUDE ${raddb}/mods-enabled/escape + + dhcpv4 { + + } + + delay reschedule { + force_reschedule = yes + } + + delay delay_10s { + delay = 10 + } + + unpack { + + } + +} + +policy { +$INCLUDE policy.conf +} + +instantiate { + # + # Just check that this can be referred to as "virtual_instantiate.post-auth" + # + load-balance virtual_instantiate { + ok + ok + } +} + +# +# Virtual server for the DHCPv4 protocol. +# +server default { + namespace = dhcpv4 + + listen { + type = Discover + } + + recv Discover { + # + # Include the test file specified by the + # KEYWORD environment variable. + # + $INCLUDE ${keyword}/$ENV{KEYWORD} + } +} diff --git a/src/tests/keywords/policy.conf b/src/tests/keywords/policy.conf index 7351de904be..2fc6401b9f9 100644 --- a/src/tests/keywords/policy.conf +++ b/src/tests/keywords/policy.conf @@ -59,7 +59,7 @@ success { } } - if (&parent.request.User-Name && !&parent.reply.Result-Status) { + if (&parent.request && !&parent.reply.Result-Status) { update parent.reply { &Result-Status := "success" } @@ -73,7 +73,7 @@ test_fail { &Result-Status += "Failure in test at line %(interpreter:...line)" } - if (&parent.request.User-Name) { + if (&parent.request) { update parent.reply { &Result-Status += "Failure in test at line %(interpreter:...line)" } diff --git a/src/tests/keywords/xlat-dhcpv4 b/src/tests/keywords/xlat-dhcpv4 index b18496a41df..d87023000b1 100644 --- a/src/tests/keywords/xlat-dhcpv4 +++ b/src/tests/keywords/xlat-dhcpv4 @@ -1,5 +1,6 @@ # # PRE: update +# PROTOCOL: dhcpv4 # update control { diff --git a/src/tests/keywords/xlat-dhcpv4.attrs b/src/tests/keywords/xlat-dhcpv4.attrs new file mode 100644 index 00000000000..6065f635e6e --- /dev/null +++ b/src/tests/keywords/xlat-dhcpv4.attrs @@ -0,0 +1,4 @@ +Packet-Type = Discover + +Result-Status == success +Packet-Type == Offer