From: Alan T. DeKok Date: Mon, 18 Dec 2023 13:48:44 +0000 (-0500) Subject: hoist legacy map comparison code to pairmove.c X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7e5dc4efd2e4bb8d0cf5fbe15ec7fd0402f10c88;p=thirdparty%2Ffreeradius-server.git hoist legacy map comparison code to pairmove.c and add support for more functionality: * we call calc cmp functions, not value cmp functions, so that comparisons of different types can be automatically upcast * xlats are now supported again * inter-attribute comparisons are now supported --- diff --git a/src/lib/server/pairmove.c b/src/lib/server/pairmove.c index 3f038bf32bb..de1d36597a2 100644 --- a/src/lib/server/pairmove.c +++ b/src/lib/server/pairmove.c @@ -27,6 +27,7 @@ RCSID("$Id$") #include #include +#include #include #include @@ -308,10 +309,10 @@ void radius_pairmove(request_t *request, fr_pair_list_t *to, fr_pair_list_t *fro talloc_free(deleted); } -/** Move a map using the operators from the old pairmove API. +/** Move a map using the operators from the old pairmove functionality. * */ -int fr_pairmove_map(request_t *request, map_t const *map) +int radius_legacy_map_apply(request_t *request, map_t const *map) { int rcode; fr_pair_t *vp, *next; @@ -438,3 +439,73 @@ success: TALLOC_FREE(to_free); return 0; } + +int radius_legacy_map_cmp(request_t *request, map_t const *map) +{ + int rcode; + fr_pair_t *vp; + fr_value_box_t const *box; + fr_value_box_t *to_free = NULL; + fr_value_box_t dst; + + fr_assert(tmpl_is_attr(map->lhs)); + fr_assert(fr_comparison_op[map->op]); + + if (tmpl_find_vp(&vp, request, map->lhs) < 0) { + return 0; + } + + if (tmpl_is_data(map->rhs)) { + box = tmpl_value(map->rhs); + + } else if (tmpl_is_attr(map->rhs)) { + fr_pair_t *rhs; + + if (tmpl_find_vp(&rhs, request, map->rhs) < 0) return -1; + + box = &rhs->data; + + } else if (tmpl_contains_xlat(map->rhs)) { + if (tmpl_aexpand(request, &to_free, request, map->rhs, NULL, NULL) < 0) return -1; + + box = to_free; + + } else if (tmpl_is_regex(map->rhs)) { + /* + * @todo - why box it and parse it again, when we can just run the regex? + */ + box = fr_box_strvalue(map->rhs->name); + + } else { + fr_strerror_const("Unknown RHS"); + return -1; + } + + /* + * The RHS is a compiled regex, which we don't yet + * support. So just re-parse it at run time for + * programmer laziness. + */ + if ((map->op == T_OP_REG_EQ) || (map->op == T_OP_REG_NE)) { + if (box->type != FR_TYPE_STRING) { + fr_strerror_const("Invalid type for regular expression"); + return -1; + } + + rcode = fr_regex_cmp_op(map->op, &vp->data, box); + TALLOC_FREE(to_free); + if (rcode < 0) return rcode; + + return (rcode == 1); + } + + /* + * Let the calculation code do upcasting as necessary. + */ + rcode = fr_value_calc_binary_op(request, &dst, FR_TYPE_BOOL, &vp->data, map->op, box); + TALLOC_FREE(to_free); + + if (rcode < 0) return rcode; + + return dst.vb_bool; +} diff --git a/src/lib/server/pairmove.h b/src/lib/server/pairmove.h index 223411071f8..e13e20e3b92 100644 --- a/src/lib/server/pairmove.h +++ b/src/lib/server/pairmove.h @@ -35,7 +35,9 @@ extern "C" { void radius_pairmove(request_t *request, fr_pair_list_t *to, fr_pair_list_t *from) CC_HINT(nonnull); -int fr_pairmove_map(request_t *request, map_t const *map); +int radius_legacy_map_apply(request_t *request, map_t const *map) CC_HINT(nonnull); + +int radius_legacy_map_cmp(request_t *request, map_t const *map) CC_HINT(nonnull); #ifdef __cplusplus } diff --git a/src/lib/server/users_file.c b/src/lib/server/users_file.c index d8bb9e25e69..df315d70ce1 100644 --- a/src/lib/server/users_file.c +++ b/src/lib/server/users_file.c @@ -273,9 +273,9 @@ int pairlist_read(TALLOC_CTX *ctx, fr_dict_t const *dict, char const *file, PAIR lhs_rules = (tmpl_rules_t) { .attr = { .dict_def = dict, - .prefix = TMPL_ATTR_REF_PREFIX_NO, + .prefix = TMPL_ATTR_REF_PREFIX_AUTO, .list_def = request_attr_request, - .list_presence = TMPL_ATTR_LIST_FORBID, + .list_presence = TMPL_ATTR_LIST_ALLOW, /* * Otherwise the tmpl code returns 0 when asked @@ -292,7 +292,7 @@ int pairlist_read(TALLOC_CTX *ctx, fr_dict_t const *dict, char const *file, PAIR .dict_def = dict, .prefix = TMPL_ATTR_REF_PREFIX_YES, .list_def = request_attr_request, - .list_presence = TMPL_ATTR_LIST_FORBID, + .list_presence = TMPL_ATTR_LIST_ALLOW, } }; @@ -443,13 +443,6 @@ check_item: goto do_insert; } - if (!tmpl_is_data(new_map->rhs) && !tmpl_is_exec(new_map->rhs) && - !tmpl_contains_xlat(new_map->rhs)) { - ERROR("%s[%d]: Invalid RHS '%s' for check item", - file, lineno, new_map->rhs->name); - goto fail_entry; - } - do_insert: fr_assert(!new_map->parent); map_list_insert_tail(&t->check, new_map); @@ -521,13 +514,9 @@ setup_reply: */ lhs_rules.attr.list_def = request_attr_reply; - lhs_rules.attr.prefix = TMPL_ATTR_REF_PREFIX_AUTO; - lhs_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW; - comma = false; rhs_rules.attr.list_def = request_attr_request; - rhs_rules.attr.list_presence = TMPL_ATTR_LIST_ALLOW; reply_item: /* diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c index 2d62cecf8be..cd26c5ede16 100644 --- a/src/modules/rlm_files/rlm_files.c +++ b/src/modules/rlm_files/rlm_files.c @@ -185,24 +185,7 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre map = prev; break; - case T_OP_REG_EQ: - case T_OP_REG_NE: - if (tmpl_contains_xlat(map->rhs)) { - ERROR("%s[%d] Right side of check item %s is not a static value", - entry->filename, entry->lineno, map->lhs->name); - return -1; - } - break; - default: - /* - * Comparisons must be with data. - */ - if (!tmpl_is_data(map->rhs)) { - ERROR("%s[%d] Right side of check item %s is not a leaf value", - entry->filename, entry->lineno, map->lhs->name); - return -1; - } break; } } /* end of loop over check items */ @@ -439,30 +422,6 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) return 0; } -static bool files_eval_map(request_t *request, map_t *map) -{ - fr_pair_t *vp; - - fr_assert(tmpl_is_attr(map->lhs)); - fr_assert(fr_comparison_op[map->op]); - - if (tmpl_find_vp(&vp, request, map->lhs) < 0) return false; - - /* - * The RHS is a compiled regex, which we don't yet - * support. So just re-parse it at run time for - * programmer laziness. - */ - if ((map->op == T_OP_REG_EQ) || (map->op == T_OP_REG_NE)) { - return (fr_regex_cmp_op(map->op, &vp->data, fr_box_strvalue(map->rhs->name)) == 1); - } - - fr_assert(tmpl_is_data(map->rhs)); - - return (fr_value_box_cmp_op(map->op, &vp->data, tmpl_value(map->rhs)) == 1); -} - - /* * Common code called by everything below. */ @@ -551,6 +510,8 @@ redo: * Run the check items. */ while ((map = map_list_next(&pl->check, map))) { + int rcode; + RDEBUG3(" %s %s %s", map->lhs->name, fr_tokens[map->op], map->rhs->name); /* @@ -570,7 +531,14 @@ redo: * Evaluate the map, including regexes. */ default: - if (!files_eval_map(request, map)) { + rcode = radius_legacy_map_cmp(request, map); + if (rcode < 0) { + RPWARN("Failed parsing map for check item %s, skipping it", map->lhs->name); + match = false; + break; + } + + if (!rcode) { RDEBUG3(" failed match"); match = false; } @@ -601,7 +569,7 @@ redo: continue; } - if (fr_pairmove_map(request, map) < 0) { + if (radius_legacy_map_apply(request, map) < 0) { RPWARN("Failed parsing map for reply item %s, skipping it", map->lhs->name); break; } diff --git a/src/tests/modules/files/attrref_check.attrs b/src/tests/modules/files/attrref_check.attrs new file mode 100644 index 00000000000..37661671811 --- /dev/null +++ b/src/tests/modules/files/attrref_check.attrs @@ -0,0 +1,16 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = "attrref_check" +User-Password = "whoops" +Filter-Id = "magic" +NAS-Identifier = "magic" +NAS-IP-Address = 127.0.0.1 +Calling-Station-Id = "127.0.0.1" + +# +# Expected answer +# +Packet-Type == Access-Accept +Reply-Message == 'success' diff --git a/src/tests/modules/files/attrref_check.unlang b/src/tests/modules/files/attrref_check.unlang new file mode 100644 index 00000000000..027271b9b22 --- /dev/null +++ b/src/tests/modules/files/attrref_check.unlang @@ -0,0 +1 @@ +files diff --git a/src/tests/modules/files/authorize b/src/tests/modules/files/authorize index 0d46e72ffdc..e0d9c02631c 100644 --- a/src/tests/modules/files/authorize +++ b/src/tests/modules/files/authorize @@ -127,6 +127,12 @@ attrref Password.Cleartext := "hopefully" xlat Password.Cleartext := "open" Reply-Message := "Hello, %{User-Name}" +# +# Inter-attribute comparisons and xlats for comparisons +# +attrref_check Filter-Id == &NAS-Identifier, NAS-IP-Address == "%{Calling-Station-Id}", Password.Cleartext := "whoops" + Reply-Message := "success" + DEFAULT User-Name == "cmp_eq", Password.Cleartext := "hopping" Reply-Message := "success-cmp_eq"