From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 17 Feb 2023 03:02:06 +0000 (-0500)
Subject: allow for different parent attribute when decoding arguments
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bc4776d1be64d70e4b174abe8787994924a9f3ff;p=thirdparty%2Ffreeradius-server.git

allow for different parent attribute when decoding arguments

in preparation for allowing client-specific vendors.
---

diff --git a/src/protocols/tacacs/decode.c b/src/protocols/tacacs/decode.c
index e97b7f665ad..75426dc33b3 100644
--- a/src/protocols/tacacs/decode.c
+++ b/src/protocols/tacacs/decode.c
@@ -227,63 +227,80 @@ static int tacacs_decode_args(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr
 		 */
 		if (!name_end) goto next;
 
-		da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_tacacs), (char *) buffer);
-		if (!da) {
-		raw:
+		/*
+		 *	Dupe the whole thing so that we have:
+		 *
+		 *	Argument-List += "name=value"
+		 */
+		if (parent) {
+			da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_tacacs), (char *) buffer);
+			if (!da) goto raw;
+
 			/*
-			 *	Dupe the whole thing so that we have:
+			 *      If it's OCTETS or STRING type, then just copy the value verbatim, as the
+			 *      contents are (should be?) binary-safe.  But if it's zero length, then don't need to
+			 *      copy anything.
 			 *
-			 *	Argument-List += "name=value"
+			 *      Note that we copy things manually here because
+			 *      we don't want the OCTETS type to be parsed as
+			 *      hex.  And, we don't want the string type to be
+			 *      unescaped.
 			 */
-			da = parent;
-			value = p;
-			arg_end = p + argv[i];
-		}
+			if (da->type == FR_TYPE_OCTETS) {
+				if ((arg_end > value) &&
+				    (fr_pair_value_memdup(vp, value, arg_end - value, true) < 0)) {
+					talloc_free(vp);
+					return -1;
+				}
+
+			} else if (da->type == FR_TYPE_STRING) {
+				if ((arg_end > value) &&
+				    (fr_pair_value_bstrndup(vp, (char const *) value, arg_end - value, true) < 0)) {
+					talloc_free(vp);
+					return -1;
+				}
+
+			} else if (arg_end == value) {
+				/*
+				 *	Any other leaf type MUST have non-zero contents.
+				 */
+				goto raw;
 
-		vp = fr_pair_afrom_da(ctx, da);
-		if (!vp) {
-			fr_strerror_const("Out of Memory");
-			return -1;
+			} else {
 
-		}
+				/*
+				 *      Parse the string, and try to convert it to the
+				 *      underlying data type.  If it can't be
+				 *      converted as a data type, just convert it as
+				 *      Argument-List.
+				 *
+				 *      And if that fails, just ignore it completely.
+				 */
+				if (fr_pair_value_from_str(vp, (char const *) value, arg_end - value, NULL, true) < 0) {
+					talloc_free(vp);
+					goto raw;
+				}
 
-		/*
-		 *	If it's OCTETS or STRING type, then just copy
-		 *	the value verbatim.  But if it's zero length,
-		 *	then don't do anything.
-		 *
-		 *	Note that we copy things manually here because
-		 *	we don't want the OCTETS type to be parsed as
-		 *	hex.  And, we don't want the string type to be
-		 *	unescaped.
-		 */
-		if (da->type == FR_TYPE_OCTETS) {
-			if ((arg_end > value) &&
-			    (fr_pair_value_memdup(vp, value, arg_end - value, true) < 0)) {
-				goto fail;
+				/*
+				 *	Else it parsed fine, append it to the output list.
+				 */
 			}
 
-		} else if (da->type == FR_TYPE_STRING) {
-			if ((arg_end > value) &&
-			    (fr_pair_value_bstrndup(vp, (char const *) value, arg_end - value, true) < 0)) {
-				goto fail;
+		} else {
+		raw:
+			vp = fr_pair_afrom_da(ctx, attr_tacacs_argument_list);
+			if (!vp) {
+				fr_strerror_const("Out of Memory");
+				return -1;
 			}
 
-		} else {
-			/*
-			 *	Parse the string, and try to convert it to the
-			 *	underlying data type.  If it can't be
-			 *	converted as a data type, just convert it as
-			 *	Argument-List.
-			 *
-			 *	And if that fails, just ignore it completely.
-			 */
-			if (fr_pair_value_from_str(vp, (char const *) value, arg_end - value, NULL, true) < 0) {
-			fail:
-				talloc_free(vp);
-				if (da != parent) goto raw;
+			value = buffer;
+			arg_end = buffer + argv[i];
 
-				goto next;
+			if ((arg_end > value) &&
+			    (fr_pair_value_bstrndup(vp, (char const *) value, arg_end - value, true) < 0)) {
+				talloc_free(vp);
+				return -1;
 			}
 		}
 
@@ -793,7 +810,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			/*
 			 *	Decode 'arg_N' arguments (horrible format)
 			 */
-			if (tacacs_decode_args(ctx, out, attr_tacacs_argument_list,
+			if (tacacs_decode_args(ctx, out, NULL,
 					       pkt->author_req.arg_cnt, argv, attrs, end) < 0) goto fail;
 
 		} else if (packet_is_author_reply(pkt)) {
@@ -846,7 +863,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			/*
 			 *	Decode 'arg_N' arguments (horrible format)
 			 */
-			if (tacacs_decode_args(ctx, out, attr_tacacs_argument_list,
+			if (tacacs_decode_args(ctx, out, NULL,
 					pkt->author_reply.arg_cnt, argv, attrs, end) < 0) goto fail;
 
 		} else {
@@ -915,7 +932,7 @@ ssize_t fr_tacacs_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *bu
 			/*
 			 *	Decode 'arg_N' arguments (horrible format)
 			 */
-			if (tacacs_decode_args(ctx, out, attr_tacacs_argument_list,
+			if (tacacs_decode_args(ctx, out, NULL,
 					pkt->acct_req.arg_cnt, argv, attrs, end) < 0) goto fail;
 
 		} else if (packet_is_acct_reply(pkt)) {
diff --git a/src/tests/unit/protocols/tacacs/typed.txt b/src/tests/unit/protocols/tacacs/typed.txt.ignore
similarity index 100%
rename from src/tests/unit/protocols/tacacs/typed.txt
rename to src/tests/unit/protocols/tacacs/typed.txt.ignore