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 3f038bf32bb62adff5316214c88a242b93a487f2..de1d36597a2ff4eb6f47012f2074369eabc300c0 100644 (file)
--- a/src/lib/server/pairmove.c
+++ b/src/lib/server/pairmove.c
@@ -27,6 +27,7 @@ RCSID("$Id$")
 
 #include <freeradius-devel/server/base.h>
 #include <freeradius-devel/util/debug.h>
+#include <freeradius-devel/util/calc.h>
 #include <freeradius-devel/server/pairmove.h>
 
 #include <freeradius-devel/protocol/radius/rfc2865.h>
@@ -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 223411071f85354309ced5a4b615dc23bea08050..e13e20e3b92e18e765e3e623a4dfb2d4edadbaf9 100644 (file)
--- 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 d8bb9e25e69eaa4442a77be215da590c6f9ab3b6..df315d70ce18ec61ad812cc1b7ed01e1b576f13e 100644 (file)
--- 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 2d62cecf8beb5ac2feeb2a91bfed3f9f9afbaae2..cd26c5ede162008d59280783d8189164096c741f 100644 (file)
--- 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 (file)
index 0000000..3766167
--- /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 (file)
index 0000000..027271b
--- /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 0d46e72ffdc4b61370d4d0351121ee094ed3aa62..e0d9c02631cbc2060d41abd0a7948f3401652819 100644 (file)
--- 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"