From: Alan T. DeKok <aland@freeradius.org>
Date: Thu, 8 Apr 2021 20:51:46 +0000 (-0400)
Subject: allow encoding of nested Vendor-Specific / Vendor types
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=284a0398083772abc3a2e540ebbba3be51a432da;p=thirdparty%2Ffreeradius-server.git

allow encoding of nested Vendor-Specific / Vendor types

There are still things to be done:

* we really want to have a flag which encodes one VSA at a time
* right now it encodes multiple VSAs into one Vendor-Specific,
  and then fails if there's an overflow.
* instead we want to catch the case of an overflow, encode a
  new Vendor-Specific / vendor header, and then continue
* we don't need any of this for VSAs in the extended space,
  because RFC 6929 Section 4 defines that each extended VSA
  starts off with one byte of vendor-type.  So we don't need
  any loops there.
---

diff --git a/src/protocols/radius/encode.c b/src/protocols/radius/encode.c
index da19f65235..a9f59b5f0d 100644
--- a/src/protocols/radius/encode.c
+++ b/src/protocols/radius/encode.c
@@ -1037,6 +1037,7 @@ static ssize_t encode_vendor_attr_hdr(fr_dbuff_t *dbuff,
 	}
 
 	da = da_stack->da[depth];
+	fr_assert(da != NULL);
 
 	if ((da->type != FR_TYPE_TLV) && (dv->flags.type_size == 1) && (dv->flags.length == 1)) {
 		return encode_rfc_hdr_internal(dbuff, da_stack, depth, cursor, encode_ctx);
@@ -1199,9 +1200,9 @@ static ssize_t encode_vendor_hdr(fr_dbuff_t *dbuff,
 				 fr_dcursor_t *cursor, void *encode_ctx)
 {
 	fr_dbuff_marker_t	hdr;
-	fr_dbuff_t		work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff);
+	fr_dbuff_t		work_dbuff = FR_DBUFF_MAX_NO_ADVANCE(dbuff, 253);
 	fr_dict_attr_t const	*da = da_stack->da[depth];
-	ssize_t			len;
+	ssize_t			slen;
 
 	fr_dbuff_marker(&hdr, &work_dbuff);
 
@@ -1232,8 +1233,32 @@ static ssize_t encode_vendor_hdr(fr_dbuff_t *dbuff,
 
 	FR_DBUFF_IN_RETURN(&work_dbuff, (uint32_t)da->attr);	/* Copy in the 32bit vendor ID */
 
-	len = encode_vendor_attr_hdr(&FR_DBUFF_MAX(&work_dbuff, 255 - 6), da_stack, depth, cursor, encode_ctx);
-	if (len < 0) return len;
+	if (da_stack->da[depth + 1]) {
+		slen = encode_vendor_attr_hdr(&work_dbuff, da_stack, depth, cursor, encode_ctx);
+		if (slen < 0) return slen;
+	} else {
+		fr_pair_t *vp;
+		fr_dcursor_t child_cursor;
+
+		vp = fr_dcursor_current(cursor);
+		fr_assert(vp->da == da);
+
+		fr_dcursor_init(&child_cursor, &vp->vp_group);
+		while ((vp = fr_dcursor_current(&child_cursor)) != NULL) {
+			fr_proto_da_stack_build(da_stack, vp->da);
+
+			/*
+			 *	@todo - if we run out of room, re-do
+			 *	the header, redo the work buffer, and
+			 *	continue.
+			 */
+			slen = encode_vendor_attr_hdr(&work_dbuff, da_stack, depth, &child_cursor, encode_ctx);
+			if (slen < 0) return slen;
+		}
+
+		vp = fr_dcursor_next(cursor);
+		fr_proto_da_stack_build(da_stack, vp ? vp->da : NULL);
+	}
 
 	fr_dbuff_advance(&hdr, 1);
 	fr_dbuff_in_bytes(&hdr, (uint8_t) fr_dbuff_used(&work_dbuff));
@@ -1296,8 +1321,7 @@ static ssize_t encode_vsa_hdr(fr_dbuff_t *dbuff,
 	 *	attribute.
 	 */
 	fr_dcursor_init(&child_cursor, &vp->vp_group);
-	vp = fr_dcursor_current(&child_cursor);
-	while (vp) {
+	while ((vp = fr_dcursor_current(&child_cursor)) != NULL) {
 		fr_proto_da_stack_build(da_stack, vp->da);
 
 		fr_assert(da_stack->da[depth + 1]->type == FR_TYPE_VENDOR);
@@ -1313,8 +1337,6 @@ static ssize_t encode_vsa_hdr(fr_dbuff_t *dbuff,
 			slen = encode_vendor_hdr(&FR_DBUFF_MAX(&work_dbuff, 253), da_stack, depth, &child_cursor, encode_ctx);
 		}
 		if (slen <= 0) return slen;
-
-		vp = fr_dcursor_current(&child_cursor);
 	}
 
 	/*
diff --git a/src/tests/unit/protocols/radius/vendor.txt b/src/tests/unit/protocols/radius/vendor.txt
index bc4606d812..25db811866 100644
--- a/src/tests/unit/protocols/radius/vendor.txt
+++ b/src/tests/unit/protocols/radius/vendor.txt
@@ -1,6 +1,9 @@
 proto radius
 proto-dictionary radius
 
+encode-pair Vendor-Specific = { Starent = { VPN-Name = "foo" } }
+match 1a 0d 00 00 1f e4 00 02 00 07 66 6f 6f
+
 encode-pair Vendor-Specific.Starent.VPN-Name = "foo"
 match 1a 0d 00 00 1f e4 00 02 00 07 66 6f 6f
 
@@ -34,8 +37,8 @@ match Vendor-Specific.3com.User-Access-Level = Visitor, Vendor-Specific.3com.Ip-
 #
 #  Vendor-Specific is of type "vsa", and therefore cannot be assigned values.
 #
-encode-pair Vendor-Specific = 0xabcdef
-match Attributes of type 'vsa' are not yet supported
+encode-pair raw.Vendor-Specific = 0xabcdef
+match 1a 05 ab cd ef
 
 encode-pair raw.26 = 0x00000009abcdef
 match 1a 09 00 00 00 09 ab cd ef
@@ -85,4 +88,4 @@ decode-pair 1a 39 00 00 01 37 01 33 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78
 match raw.Vendor-Specific.Microsoft.CHAP-Response = 0x78787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878
 
 count
-match 40
+match 42