From: Alan T. DeKok <aland@freeradius.org>
Date: Sun, 17 Dec 2023 22:38:02 +0000 (-0500)
Subject: start of new pairmove API
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7687d4062a7b642d3665d0e31f87732a54907ed6;p=thirdparty%2Ffreeradius-server.git

start of new pairmove API

which will (eventually) take LHS / op / RHS, so that it can be
called from SQL with fields.

Update rlm_files to call the new API, so that it no longer creates
an intermediate VP, and just passes the map to the new API.

Update rlm_files to move the "update control" attributes to the
reply list.  The new API will look at the tmpl to find the correct
request and list context.
---

diff --git a/src/lib/server/pairmove.c b/src/lib/server/pairmove.c
index d0a575ec960..ca0f0b34356 100644
--- a/src/lib/server/pairmove.c
+++ b/src/lib/server/pairmove.c
@@ -307,3 +307,109 @@ void radius_pairmove(request_t *request, fr_pair_list_t *to, fr_pair_list_t *fro
 	talloc_free(edited);
 	talloc_free(deleted);
 }
+
+/** Move a map using the operators from the old pairmove API.
+ *
+ */
+int fr_pairmove_map(request_t *request, map_t const *map)
+{
+	int rcode;
+	fr_pair_t *vp, *next;
+	fr_dict_attr_t const *da;
+	fr_pair_list_t *list;
+	TALLOC_CTX *ctx;
+
+	/*
+	 *	Finds both the correct ctx and nested list.
+	 */
+	tmpl_pair_list_and_ctx(ctx, list, request, tmpl_request(map->lhs), tmpl_list(map->lhs));
+	if (!ctx) return -1;
+
+	da = tmpl_attr_tail_da(map->lhs);
+
+	fr_assert(tmpl_is_data(map->rhs));
+
+	switch (map->op) {
+	case T_OP_CMP_FALSE:	/* delete all */
+		fr_pair_delete_by_da(list, da);
+		break;
+
+	case T_OP_EQ:		/* set only if not already exist */
+		vp = fr_pair_find_by_da(list, NULL, da);
+		if (vp) return 0;
+		goto add;
+
+	case T_OP_SET:		/* delete all and set one */
+		fr_pair_delete_by_da(list, da);
+		FALL_THROUGH;
+
+	case T_OP_ADD_EQ:	/* append one */
+	add:
+		vp = fr_pair_afrom_da(ctx, da);
+		if (!vp) return -1;
+
+		if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+			talloc_free(vp);
+			return -1;
+		}
+
+		fr_pair_append(list, vp);
+		break;
+
+	case T_OP_PREPEND:	/* prepend one */
+		vp = fr_pair_afrom_da(ctx, da);
+		if (!vp) return -1;
+
+		if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+			talloc_free(vp);
+			return -1;
+		}
+
+		fr_pair_prepend(list, vp);
+		break;
+
+	case T_OP_SUB_EQ:		/* delete if match */
+		vp = fr_pair_find_by_da(list, NULL, da);
+		if (!vp) break;
+
+	redo_sub:
+		next = fr_pair_find_by_da(list, vp, da);
+		rcode = fr_value_box_cmp_op(T_OP_CMP_EQ, &vp->data, tmpl_value(map->rhs));
+
+		if (rcode < 0) return -1;
+
+		if (rcode == 1) {
+			fr_pair_delete(list, vp);
+		}
+
+		if (!next) break;
+		vp = next;
+		goto redo_sub;
+
+	case T_OP_CMP_EQ:      	/* replace if not == */
+	case T_OP_LE:		/* replace if not <= */
+	case T_OP_GE:		/* replace if not >= */
+		vp = fr_pair_find_by_da(list, NULL, da);
+		if (!vp) goto add;
+
+	redo_filter:
+		rcode = fr_value_box_cmp_op(map->op, &vp->data, tmpl_value(map->rhs));
+		if (rcode < 0) return -1;
+
+		if (rcode == 0) {
+			if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+				return -1;
+			}
+		}
+
+		vp = fr_pair_find_by_da(list, vp, da);
+		if (vp) goto redo_filter;
+		break;
+
+	default:
+		fr_assert(0);
+		break;
+	}
+
+	return 0;
+}
diff --git a/src/lib/server/pairmove.h b/src/lib/server/pairmove.h
index 40993bc1710..223411071f8 100644
--- a/src/lib/server/pairmove.h
+++ b/src/lib/server/pairmove.h
@@ -27,6 +27,7 @@
 RCSIDH(pairmove_h, "$Id$")
 
 #include <freeradius-devel/server/request.h>
+#include <freeradius-devel/server/map.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -34,6 +35,8 @@ 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);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c
index d6187fd0f6d..c3a1e730e8a 100644
--- a/src/modules/rlm_files/rlm_files.c
+++ b/src/modules/rlm_files/rlm_files.c
@@ -128,6 +128,7 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 	fr_htrie_t *tree;
 	fr_htrie_type_t htype;
 	fr_value_box_t *box;
+	map_t		*reply_head;
 
 	if (!filename) {
 		*ptree = NULL;
@@ -148,8 +149,11 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 	entry = NULL;
 	while ((entry = fr_dlist_next(&users.head, entry))) {
 		map_t *map = NULL;
+		map_t *prev;
 		fr_dict_attr_t const *da;
 
+		reply_head = NULL;
+
 		/*
 		 *	Look for improper use of '=' in the
 		 *	check items.  They should be using
@@ -181,26 +185,20 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 			}
 
 			/*
-			 *	Ignore attributes which are set
-			 *	properly.
+			 *	Move assignment operations to the reply list.
 			 */
-			if (map->op != T_OP_EQ) {
-				continue;
-			}
+			switch (map->op) {
+			case T_OP_EQ:
+			case T_OP_SET:
+			case T_OP_ADD_EQ:
+				prev = map_list_remove(&entry->check, map);
+				map_list_insert_after(&entry->reply, reply_head, map);
+				reply_head = map;
+				map = prev;
+				break;
 
-			/*
-			 *	If it's a vendor attribute,
-			 *	or it's a wire protocol,
-			 *	ensure it has '=='.
-			 */
-			if ((fr_dict_vendor_num_by_da(da) != 0) ||
-			    (da->attr < 0x100)) {
-				WARN("%s[%d] Changing '%s =' to '%s =='\n\tfor comparing RADIUS attribute in check item list for key %s",
-				     entry->filename, entry->lineno,
-				     da->name, da->name,
-				     entry->name);
-				map->op = T_OP_CMP_EQ;
-				continue;
+			default:
+				break;
 			}
 		} /* end of loop over check items */
 
@@ -220,6 +218,12 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 			}
 			da = tmpl_attr_tail_da(map->lhs);
 
+			if (map->op == T_OP_CMP_FALSE) {
+				ERROR("%s[%d] Invalid operator '!*' for reply item %s",
+				      entry->filename, entry->lineno, map->lhs->name);
+				return -1;
+			}
+
 			if ((htype != FR_HTRIE_TRIE) && (da == attr_next_shortest_prefix)) {
 				ERROR("%s[%d] Cannot use %s when key is not an IP / IP prefix",
 				      entry->filename, entry->lineno, da->name);
@@ -235,8 +239,6 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 			 *	free all subsequent maps.
 			 */
 			if (da == attr_fall_through) {
-				map_t *prev;
-
 				if (tmpl_is_data(map->rhs) && (tmpl_value_type(map->rhs) == FR_TYPE_BOOL)) {
 					entry->fall_through = tmpl_value(map->rhs)->vb_bool;
 				}
@@ -246,15 +248,6 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 				map = prev;
 				continue;
 			}
-
-			/*
-			 *	If we allow list qualifiers in
-			 *	users_file.c, then this module also
-			 *	needs to be updated.  Ensure via an
-			 *	assertion that they do not get out of
-			 *	sync.
-			 */
-			fr_assert(tmpl_list(map->lhs) == request_attr_reply);
 		}
 	}
 
@@ -475,10 +468,8 @@ redo:
 	 *	Find the entry for the user.
 	 */
 	while (user_pl || default_pl) {
-		fr_pair_t *vp;
 		map_t *map = NULL;
 		PAIR_LIST const *pl;
-		fr_pair_list_t list;
 		bool next_shortest_prefix;
 		bool match = true;
 
@@ -508,14 +499,10 @@ redo:
 			default_pl = fr_dlist_next(&default_list->head, default_pl);
 		}
 
-		fr_pair_list_init(&list);
-
 		/*
-		 *	Realize the map to a list of VPs
+		 *	Run the check items.
 		 */
 		while ((map = map_list_next(&pl->check, map))) {
-			fr_pair_list_t tmp_list;
-
 			RDEBUG3("    %s %s %s", map->lhs->name, fr_tokens[map->op], map->rhs->name);
 
 			/*
@@ -528,23 +515,15 @@ redo:
 			case T_OP_EQ:
 			case T_OP_SET:
 			case T_OP_ADD_EQ:
-				fr_pair_list_init(&tmp_list);
-				if (map_to_vp(request->control_ctx, &tmp_list, request, map, NULL) < 0) {
-					fr_pair_list_free(&list);
-					RPWARN("Failed parsing check item, skipping entry");
-					match = false;
-					break;
-				}
-
-				fr_pair_list_append(&list, &tmp_list);
-				break;
+				fr_assert(0);
+				return -1;
 
 				/*
 				 *	Evaluate the map, including regexes.
 				 */
 			default:
 				if (!files_eval_map(request, map)) {
-					RDEBUG3("    failed match - %s", fr_strerror());
+					RDEBUG3("    failed match");
 					match = false;
 				}
 				break;
@@ -553,46 +532,31 @@ redo:
 			if (!match) break;
 		}
 
-		if (!match) {
-			fr_pair_list_free(&list);
-			continue;
-		}
+		if (!match) continue;
 
 		RDEBUG2("Found match \"%s\" on line %d of %s", pl->name, pl->lineno, pl->filename);
 		found = true;
 		next_shortest_prefix = false;
 
-		/*
-		 *	Move the control items over, too.
-		 */
-		radius_pairmove(request, &request->control_pairs, &list);
-
 		/* ctx may be reply */
 		if (!map_list_empty(&pl->reply)) {
 			map = NULL;
-			while ((map = map_list_next(&pl->reply, map))) {
-				fr_pair_list_t tmp_list;
-				fr_pair_list_init(&tmp_list);
-				if (map->op == T_OP_CMP_FALSE) continue;
-
-				if (map_to_vp(request->reply_ctx, &tmp_list, request, map, NULL) < 0) {
-					RPWARN("Failed parsing map for reply item %s, skipping it", map->lhs->name);
-					break;
-				}
 
+			while ((map = map_list_next(&pl->reply, map))) {
 				/*
-				 *	And do the same checks for prefix tries.
+				 *	Do Fall-Through style checks for prefix tries.
 				 */
-				if (trie && (keylen > 0)) {
-					vp = fr_pair_list_head(&tmp_list);
-					if (vp->da == attr_next_shortest_prefix) {
-						next_shortest_prefix = vp->vp_bool;
-						fr_pair_list_free(&tmp_list);
-						continue;
-					}
+				if (trie && (keylen > 0) && (tmpl_attr_tail_da(map->lhs) == attr_next_shortest_prefix)) {
+					fr_assert(tmpl_is_data(map->rhs));
+
+					next_shortest_prefix = tmpl_value(map->rhs)->vb_bool;
+					continue;
 				}
 
-				radius_pairmove(request, &request->reply_pairs, &tmp_list);
+				if (fr_pairmove_map(request, map) < 0) {
+					RPWARN("Failed parsing map for reply item %s, skipping it", map->lhs->name);
+					break;
+				}
 			}
 		}