From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 24 Oct 2019 13:05:49 +0000 (-0400)
Subject: fix encoding of keyed child structures
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6fbd5899d549153536b4c9bf293e76a73ef57bea;p=thirdparty%2Ffreeradius-server.git

fix encoding of keyed child structures
---

diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 3d7e5748063..cc7d6bd9a91 100644
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -300,46 +300,23 @@ ssize_t fr_struct_to_network(uint8_t *out, size_t outlen,
 	if (!vp || !outlen) return p - out;
 
 	/*
-	 *	Encode the key field based on the value of the next
-	 *	attribute.  Note that there isn't much point in
-	 *	converting key_da->attr into a value_box_t, and then
-	 *	calling fr_value_box_to_network() to do the work.  The
-	 *	code below isn't much larger in the source, but is
-	 *	rather substantially simpler over all.
+	 *	If our parent is a struct, AND it's parent is
+	 *	the key_da, then we have a keyed struct for
+	 *	the child.  Go encode it.
 	 */
-	if (key_da && (vp->da->parent == key_da)) {
-		switch (key_da->type) {
-		case FR_TYPE_UINT8:
-			*key_data = key_da->attr;
-			break;
-
-		case FR_TYPE_UINT16:
-			if ((p - key_data) < 2) return p - out;
-
-			key_data[0] = (key_da->attr >> 8) & 0xff;
-			key_data[1] = key_da->attr & 0xff;
-			break;
-
-		case FR_TYPE_UINT32:
-			if ((p - key_data) < 4) return p - out;
-
-			key_data[0] = (key_da->attr >> 24) & 0xff;
-			key_data[1] = (key_da->attr >> 16) & 0xff;
-			key_data[2] = (key_da->attr >> 8) & 0xff;
-			key_data[3] = key_da->attr & 0xff;
-			break;
-
-		default:
-			return p - out;
-		}
-
-		/*
-		 *	We don't need to recurse.  the caller will see
-		 *	that the next attribute is of type 'struct',
-		 *	and will call this function again to encode
-		 *	it.
-		 */
+	if (key_da &&
+	    (vp->da->parent->type == FR_TYPE_STRUCT) &&
+	    (vp->da->parent->parent == key_da)) {
+		len = fr_struct_to_network(p, outlen,
+					   vp->da->parent, cursor);
+		if (len < 0) return len;
+		return (p - out) + len;
 	}
+	/*
+	 *	Else we have a key_da with no child struct.
+	 *	Oh well.  Assume that the caller knows WTF
+	 *	he's doing, and encode things as best we can.
+	 */
 
 	return p - out;
 }
diff --git a/src/tests/unit/radius_struct.txt b/src/tests/unit/radius_struct.txt
index 31c580241b7..d9809770c7d 100644
--- a/src/tests/unit/radius_struct.txt
+++ b/src/tests/unit/radius_struct.txt
@@ -57,8 +57,8 @@ match Unit-Struct4-Int1 = 1, Unit-Struct4-Int2 = 2, Unit-Struct4-Short = 4
 decode-pair ff 09 01 00 00 1a 99 05 06
 match Key-Field = Sub-Struct, Filler = 6809, Nested-Sub1 = 5, Nested-Sub2 = 6
 
-encode-pair Key-Field = Sub-Struct, Filler = 6809, Nested-Sub1 = 5, Nested-Sub2 = 6
-match ff 0b 01 00 00 1a 99 01 04 05 06
+encode-pair -
+match ff 09 01 00 00 1a 99 05 06
 
 count
 match 28