clean up attr_filter and map_to_vp()

only attr_filter passes comparison operators to map_to_vp()
we should arguably forbid comparison operators, as allowing them
could break other modules.

Also disallow comparison operators in map_to_request, as nothing
uses them.

diff --git a/src/lib/server/map.c b/src/lib/server/map.c
index faf3985b168ba76c62c002d552febec78d90abd3..c09e1e9529e0809191a40a7699fa765d3e3c7213 100644 (file)
--- a/src/lib/server/map.c
+++ b/src/lib/server/map.c
@@ -1581,21 +1581,6 @@ int map_to_vp(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *request, map_t co
 
        fr_assert(tmpl_is_attr(map->lhs));
 
-       /*
-        *      Special case for !*, we don't need to parse RHS as this is a unary operator.
-        */
-       if (map->op == T_OP_CMP_FALSE) return 0;
-
-       /*
-        *      Hoist this early, too.
-        */
-       if (map->op == T_OP_CMP_TRUE) {
-               MEM(n = fr_pair_afrom_da(ctx, tmpl_attr_tail_da(map->lhs)));
-               n->op = map->op;
-               fr_pair_append(out, n);
-               return 0;
-       }
-
        /*
         *      If there's no RHS, then it MUST be an attribute, and
         *      it MUST be structural.  And it MAY have children.
@@ -1880,6 +1865,14 @@ int map_to_request(request_t *request, map_t const *map, radius_map_getvalue_t f
        fr_assert(map->lhs != NULL);
        fr_assert(map->rhs != NULL);
 
+       /*
+        *      This function is called only when creating attributes.  It cannot be called for conditions.
+        */
+       if (fr_comparison_op[map->op]) {
+               REDEBUG("Invalid operator in %s %s ...", map->lhs->name, fr_tokens[map->op]);
+               return -1;
+       }
+
        tmp_ctx = talloc_pool(request, 1024);
 
        /*

diff --git a/src/modules/rlm_attr_filter/rlm_attr_filter.c b/src/modules/rlm_attr_filter/rlm_attr_filter.c
index 7c49e15b4634f7f08959001147ff8bee29ede012..5894af5856d95b125fd64d16e458a3231b475769 100644 (file)
--- a/src/modules/rlm_attr_filter/rlm_attr_filter.c
+++ b/src/modules/rlm_attr_filter/rlm_attr_filter.c
@@ -241,6 +241,34 @@ static unlang_action_t CC_HINT(nonnull) attr_filter_common(TALLOC_CTX *ctx, rlm_
                fr_pair_list_init(&check_list);
 
                while ((map = map_list_next(&pl->reply, map))) {
+                       /*
+                        *      Special case for !*
+                        */
+                       if (map->op == T_OP_CMP_FALSE) {
+                               RWDEBUG("Unsupported operator '!*'");
+                               continue;
+                       }
+
+                       /*
+                        *      Create an empty attribute.  This functionality is used by the attr_filter module.
+                        */
+                       if (map->op == T_OP_CMP_TRUE) {
+                               fr_pair_t *vp;
+
+                               MEM(vp = fr_pair_afrom_da(ctx, tmpl_attr_tail_da(map->lhs)));
+                               vp->op = T_OP_CMP_TRUE;
+                               fr_pair_append(&check_list, vp);
+                               continue;
+                       }
+
+                       /*
+                        *      @todo - this use of map_to_vp is completely wrong.  Nothing else uses
+                        *      comparison operators for map_to_vp().  This module doesn't handle nested
+                        *      pairs, and dumps all pairs in one list no matter their depth.  It requires
+                        *      that map_to_vp() appends the pair, rather than respecting the operator.
+                        *
+                        *      i.e. it really only works for RADIUS. :(
+                        */
                        if (map_to_vp(ctx, &tmp_list, request, map, NULL) < 0) {
                                RPWARN("Failed parsing map %s for check item, skipping it", map->lhs->name);
                                continue;

diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c
index 335a06c0ea03e68e74bc5efa5517a8dcd8c3bb65..3cd11d41d968a958cef0a005ee894af42823d256 100644 (file)
--- a/src/modules/rlm_cache/rlm_cache.c
+++ b/src/modules/rlm_cache/rlm_cache.c
@@ -1412,6 +1412,11 @@ static int cache_verify(map_t *map, void *uctx)
                return -1;
        }
 
+       if (!fr_assignment_op[map->op]) {
+               cf_log_err(map->ci, "Invalid operator '%s'", fr_tokens[map->op]);
+               return -1;
+       }
+
        return 0;
 }