From: Alan T. DeKok <aland@freeradius.org>
Date: Mon, 18 Dec 2023 00:09:32 +0000 (-0500)
Subject: Add more functionality.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=66f92b327ca0af301fa3f79f8f5a8ab9dc224bf0;p=thirdparty%2Ffreeradius-server.git

Add more functionality.

Allow inter-attribute comparisons in the "users" file.

Allow list references in the reply list.
---

diff --git a/src/lib/server/pairmove.c b/src/lib/server/pairmove.c
index ff21e73c9d0..4163b1a454a 100644
--- a/src/lib/server/pairmove.c
+++ b/src/lib/server/pairmove.c
@@ -318,6 +318,7 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 	fr_dict_attr_t const *da;
 	fr_pair_list_t *list;
 	TALLOC_CTX *ctx;
+	fr_value_box_t const *box;
 
 	/*
 	 *	Finds both the correct ctx and nested list.
@@ -327,7 +328,23 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 
 	da = tmpl_attr_tail_da(map->lhs);
 
-	fr_assert(tmpl_is_data(map->rhs));
+	if (tmpl_is_data(map->rhs)) {
+		box = tmpl_value(map->rhs);
+
+	} else if (tmpl_is_attr(map->rhs)) {
+		if (tmpl_find_vp(&vp, request, map->rhs) < 0) return -1;
+
+		if (vp->vp_type != da->type) {
+			fr_strerror_const("Incompatible data types");
+			return -1;
+		}
+
+		box = &vp->data;
+
+	} else {
+		fr_strerror_const("Unknown RHS");
+		return -1;
+	}
 
 	switch (map->op) {
 	case T_OP_CMP_FALSE:	/* delete all */
@@ -348,7 +365,7 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 		vp = fr_pair_afrom_da_nested(ctx, list, da);
 		if (!vp) return -1;
 
-		if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+		if (fr_value_box_copy(vp, &vp->data, box) < 0) {
 			talloc_free(vp);
 			return -1;
 		}
@@ -360,7 +377,7 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 		vp = fr_pair_afrom_da(ctx, da);
 		if (!vp) return -1;
 
-		if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+		if (fr_value_box_copy(vp, &vp->data, box) < 0) {
 			talloc_free(vp);
 			return -1;
 		}
@@ -374,7 +391,7 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 
 	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));
+		rcode = fr_value_box_cmp_op(T_OP_CMP_EQ, &vp->data, box);
 
 		if (rcode < 0) return -1;
 
@@ -395,11 +412,11 @@ int fr_pairmove_map(request_t *request, map_t const *map)
 		if (!vp) goto add;
 
 	redo_filter:
-		rcode = fr_value_box_cmp_op(map->op, &vp->data, tmpl_value(map->rhs));
+		rcode = fr_value_box_cmp_op(map->op, &vp->data, box);
 		if (rcode < 0) return -1;
 
 		if (rcode == 0) {
-			if (fr_value_box_copy(vp, &vp->data, tmpl_value(map->rhs)) < 0) {
+			if (fr_value_box_copy(vp, &vp->data, box) < 0) {
 				return -1;
 			}
 		}
diff --git a/src/lib/server/users_file.c b/src/lib/server/users_file.c
index 3dc9faee280..e5081337341 100644
--- a/src/lib/server/users_file.c
+++ b/src/lib/server/users_file.c
@@ -520,8 +520,14 @@ setup_reply:
 		 *	Setup the reply items.
 		 */
 		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_presence = TMPL_ATTR_LIST_REQUIRE;
+
 reply_item:
 		/*
 		 *	Reply items start with spaces.  If there's no
@@ -590,18 +596,9 @@ next_reply_item:
 		}
 
 		/*
-		 *	RHS can be NULL if it's a structural type.
+		 *	We no longer really care what the RHS is, or what the list is on the LHS.  The caller
+		 *	takes care of checking that for us.
 		 */
-		if (new_map->rhs) {
-			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 reply item",
-				      file, lineno, new_map->rhs->name);
-				goto fail_entry;
-			}
-		}
-
-		fr_assert(tmpl_list(new_map->lhs) == request_attr_reply);
 
 		if (!new_map->parent) map_list_insert_tail(&t->reply, new_map);
 
diff --git a/src/modules/rlm_attr_filter/rlm_attr_filter.c b/src/modules/rlm_attr_filter/rlm_attr_filter.c
index 1b05ceb998c..c832e99da79 100644
--- a/src/modules/rlm_attr_filter/rlm_attr_filter.c
+++ b/src/modules/rlm_attr_filter/rlm_attr_filter.c
@@ -131,6 +131,12 @@ static int attr_filter_getfile(TALLOC_CTX *ctx, module_inst_ctx_t const *mctx, c
 				return -1;
 			}
 
+			if (tmpl_list(map->rhs) != request_attr_reply) {
+				ERROR("%s[%d] Left side of filter %s is not in the reply list",
+				      filename, entry->lineno, map->lhs->name);
+				return -1;
+			}
+
 			/*
 			 *	Make sure that bad things don't happen.
 			 */
diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c
index 596e49f624e..b66e70877e9 100644
--- a/src/modules/rlm_files/rlm_files.c
+++ b/src/modules/rlm_files/rlm_files.c
@@ -175,14 +175,6 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 			 */
 			if ((map->op == T_OP_REG_EQ) || (map->op == T_OP_REG_NE)) {
 				fr_assert(tmpl_is_regex(map->rhs));
-
-			/*
-			 *	Disallow inter-attribute comparisons.
-			 */
-			} else 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;
 			}
 
 			/*
@@ -198,7 +190,24 @@ 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 */
@@ -239,6 +248,17 @@ static int getusersfile(TALLOC_CTX *ctx, char const *filename, fr_htrie_t **ptre
 				return -1;
 			}
 
+			/*
+			 *	Regex assignments aren't allowed.
+			 *
+			 *	Execs are being deprecated.
+			 */
+			if (tmpl_contains_regex(map->rhs) || tmpl_is_exec(map->rhs)) {
+				ERROR("%s[%d] Invalid right-hand side of assignment for attribute %s",
+				      entry->filename, entry->lineno, da->name);
+				return -1;
+			}
+
 			/*
 			 *	Check for Fall-Through in the reply list.  If so, delete it and set the flag
 			 *	in the entry.
diff --git a/src/tests/modules/files/attrref.attrs b/src/tests/modules/files/attrref.attrs
new file mode 100644
index 00000000000..77cdfdb7962
--- /dev/null
+++ b/src/tests/modules/files/attrref.attrs
@@ -0,0 +1,13 @@
+#
+#  Input packet
+#
+Packet-Type = Access-Request
+User-Name = "attrref"
+User-Password = "hopefully"
+Filter-Id = "hello"
+
+#
+#  Expected answer
+#
+Packet-Type == Access-Accept
+Reply-Message == 'hello'
diff --git a/src/tests/modules/files/attrref.unlang b/src/tests/modules/files/attrref.unlang
new file mode 100644
index 00000000000..e40420ede27
--- /dev/null
+++ b/src/tests/modules/files/attrref.unlang
@@ -0,0 +1,11 @@
+#
+#  Test inter-attribute comparisons
+#
+files
+
+#
+#  The "reply" items can now update any list.
+#
+if (&control.Filter-Id != "foo") {
+	test_fail
+}
diff --git a/src/tests/modules/files/authorize b/src/tests/modules/files/authorize
index fe159fa47c6..b5b43ae3f82 100644
--- a/src/tests/modules/files/authorize
+++ b/src/tests/modules/files/authorize
@@ -117,6 +117,13 @@ vendor	Password.Cleartext := "we_are_so_smart"
 	Cisco.AVPair := "1",
 	Cisco.AVPair += "2"
 
+#
+#  Compare attribute references
+#
+attrref	Password.Cleartext := "hopefully"
+	Reply-Message := &request.Filter-Id,
+	&control.Filter-Id := "foo"
+
 DEFAULT	User-Name == "cmp_eq",  Password.Cleartext := "hopping"
 	Reply-Message := "success-cmp_eq"