always decode RADIUS VSAs as nested

not sure why this wasn't done before. But now we're happy.
FreeRADIUS · Sep 14, 2023 · e060170 · e060170
1 parent 221ae4e
commit e060170
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 34 deletions.
diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c
@@ -1268,12 +1268,13 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 {
 	size_t			total;
 	ssize_t			ret;
-	uint32_t		vendor;
+	uint32_t		vendor_pen;
 	fr_dict_vendor_t const	*dv;
 	fr_pair_list_t		head;
 	fr_dict_vendor_t	my_dv;
 	fr_dict_attr_t const	*vendor_da;
 	fr_pair_list_t		tlv_tmp;
+	fr_pair_t		*vsa, *vendor;
 
 	fr_pair_list_init(&head);
 
@@ -1292,8 +1293,8 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 
 	FR_PROTO_TRACE("Decoding VSA");
 
-	memcpy(&vendor, data, 4);
-	vendor = ntohl(vendor);
+	memcpy(&vendor_pen, data, 4);
+	vendor_pen = ntohl(vendor_pen);
 
 	/*
 	 *	Verify that the parent (which should be a VSA)
@@ -1303,7 +1304,7 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 	 *	(unlike DHCP) we know vendor attributes have a
 	 *	standard format, so we can decode the data anyway.
 	 */
-	vendor_da = fr_dict_attr_child_by_num(parent, vendor);
+	vendor_da = fr_dict_attr_child_by_num(parent, vendor_pen);
 	if (!vendor_da) {
 		fr_dict_attr_t *n;
 		/*
@@ -1314,16 +1315,18 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 			return -1;
 		}
 
-		n = fr_dict_unknown_vendor_afrom_num(packet_ctx->tmp_ctx, parent, vendor);
+		n = fr_dict_unknown_vendor_afrom_num(packet_ctx->tmp_ctx, parent, vendor_pen);
 		if (!n) return -1;
 		vendor_da = n;
 
+		fr_assert(vendor_da->flags.type_size == 1);
+
 		/*
 		 *	Create an unknown DV too...
 		 */
 		memset(&my_dv, 0, sizeof(my_dv));
 
-		my_dv.pen = vendor;
+		my_dv.pen = vendor_pen;
 		my_dv.type = 1;
 		my_dv.length = 1;
 
@@ -1336,7 +1339,7 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 	 *	We found an attribute representing the vendor
 	 *	so it *MUST* exist in the vendor tree.
 	 */
-	dv = fr_dict_vendor_by_num(dict_radius, vendor);
+	dv = fr_dict_vendor_by_num(dict_radius, vendor_pen);
 	if (!fr_cond_assert(dv)) return -1;
 	FR_PROTO_TRACE("decode context %s -> %s", parent->name, vendor_da->name);
 
@@ -1361,6 +1364,10 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 	 *	Vendor-Specific.  If so, loop over them all.
 	 */
 create_attrs:
+	if (fr_pair_find_or_append_by_da(ctx, &vsa, out, parent) < 0) return -1;
+
+	if (fr_pair_find_or_append_by_da(vsa, &vendor, &vsa->vp_group, vendor_da) < 0) return -1;
+
 	data += 4;
 	attr_len -= 4;
 	total = 4;
@@ -1372,7 +1379,7 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 		/*
 		 *	Vendor attributes can have subattributes (if you hadn't guessed)
 		 */
-		vsa_len = decode_vsa_internal(ctx, &tlv_tmp,
+		vsa_len = decode_vsa_internal(vendor, &tlv_tmp,
 					      vendor_da, data, attr_len, packet_ctx, dv);
 		if (vsa_len < 0) {
 			FR_PROTO_TRACE("TLV decode failed: %s", fr_strerror());
@@ -1385,7 +1392,7 @@ static ssize_t  CC_HINT(nonnull) decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out
 		attr_len -= vsa_len;
 		total += vsa_len;
 	}
-	fr_pair_list_append(out, &tlv_tmp);
+	fr_pair_list_append(&vendor->vp_group, &tlv_tmp);
 
 	/*
 	 *	When the unknown attributes were created by

diff --git a/src/tests/digest/config/digest.conf b/src/tests/digest/config/digest.conf
@@ -29,6 +29,10 @@ security {
 	allow_vulnerable_openssl = yes
 }
 
+migrate {
+	tmpl_tokenize_all_nested = true
+}
+
 policy {
 	files.authorize {
 		if (&User-Name == "bob") {

diff --git a/src/tests/unit/protocols/radius/ascend.txt b/src/tests/unit/protocols/radius/ascend.txt
@@ -6,7 +6,7 @@ encode-pair Vendor-Specific.Ascend.Data-Filter = "ip in drop tcp dstport > 1023"
 match 1a 28 00 00 02 11 f2 22 01 00 01 00 00 00 00 00 00 00 00 00 00 00 06 00 00 00 03 ff 00 03 00 00 00 00 00 00 00 00 00 00
 
 decode-pair -
-match Vendor-Specific.Ascend.Data-Filter = "ip in drop tcp dstport > 1023"
+match Vendor-Specific = { Ascend = { Data-Filter = "ip in drop tcp dstport > 1023" } }
 
 #
 #  We no longer support setting Vendor-Specific.Ascend.Data-Filters as hex strings
@@ -18,28 +18,27 @@ encode-pair Vendor-Specific.Ascend.Data-Filter = "ip in forward srcip 1.2.3.4/5
 match 1a 28 00 00 02 11 f2 22 01 01 01 00 01 02 03 04 0a 0b 0c 0d 05 20 06 00 00 05 04 d2 02 02 00 00 00 00 00 00 00 00 00 00
 
 decode-pair 1a2800000211f22201010100010203040a0b0c0d05200600000504d2020200000000000000000000
-match Vendor-Specific.Ascend.Data-Filter = "ip in forward srcip 1.2.3.4/5 dstip 10.11.12.13/32 tcp srcport = 5 dstport = 1234"
-
+match Vendor-Specific = { Ascend = { Data-Filter = "ip in forward srcip 1.2.3.4/5 dstip 10.11.12.13/32 tcp srcport = 5 dstport = 1234" } }
 
 encode-pair Vendor-Specific.Ascend.Data-Filter = "ipv6 in forward srcip ::1 dstip ::1 tcp srcport = 5 dstport = 1234"
 match 1a 38 00 00 02 11 f2 32 03 01 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 80 80 06 00 00 05 04 d2 02 02 00 00
 
 decode-pair -
-match Vendor-Specific.Ascend.Data-Filter = "ipv6 in forward srcip ::1/128 dstip ::1/128 tcp srcport = 5 dstport = 1234"
+match Vendor-Specific = { Ascend = { Data-Filter = "ipv6 in forward srcip ::1/128 dstip ::1/128 tcp srcport = 5 dstport = 1234" } }
 
 # length < 16 gets padded with zeros
 encode-pair Vendor-Specific.Ascend.Send-Secret = "foo"
 match 1a 18 00 00 02 11 d6 12 ce 8d bb 09 a0 cd c2 9c ca f1 bd cb 25 41 f7 70
 
 decode-pair -
-match Vendor-Specific.Ascend.Send-Secret = "foo"
+match Vendor-Specific = { Ascend = { Send-Secret = "foo" } }
 
 # length > 16 gets chopped
 encode-pair Vendor-Specific.Ascend.Send-Secret = "foo 56789abcdef012"
 match 1a 18 00 00 02 11 d6 12 ce 8d bb 29 95 fb f5 a4 f3 90 df a8 41 24 91 40
 
 decode-pair -
-match Vendor-Specific.Ascend.Send-Secret = "foo 56789abcdef0"
+match Vendor-Specific = { Ascend = { Send-Secret = "foo 56789abcdef0" } }
 
 count
 match 23
diff --git a/src/tests/unit/protocols/radius/eapol_msg.txt b/src/tests/unit/protocols/radius/eapol_msg.txt
@@ -7,7 +7,26 @@ fuzzer-out radius
 #  Snip of packet extracted from eapol_test (GET_RADIUS_REPLY -RAW)
 #
 decode-proto 020200eb8b7a26bee11f1ca308233d49733187720506000030391217506f776572656420627920467265655241444955531a0c000004d23806deadbeef1a0c000000141e06cafecafe1a0c000000141e06cadecade1a1200000be1130c6d792070726f66696c651a0c00000be11006000000051a0c000001370706000000011a2a0000013711248701b3e481d72fa1333b9838a3cd448837eaed62a843295f1c9dd153c6866e499f201a2a0000013710249385f7dc0fd758b02dd0dc43f68266508ec93c678a5fa38525749016edede8eeea0e4f0603fc0004501200e9e565eb053138254850edb41fc013
-match Packet-Type = Access-Accept, Packet-Authentication-Vector = 0x8b7a26bee11f1ca308233d4973318772, NAS-Port = 12345, Reply-Message = "Powered by FreeRADIUS", raw.Vendor-Specific.1234.56 = 0xdeadbeef, raw.Vendor-Specific.20.30 = 0xcafecafe, raw.Vendor-Specific.20.30 = 0xcadecade, Vendor-Specific.Alcatel.FR-Direct-Profile = "my profile", Vendor-Specific.Alcatel.Home-Agent-UDP-Port = 5, Vendor-Specific.Microsoft.MPPE-Encryption-Policy = Encryption-Allowed, raw.Vendor-Specific.Microsoft.MPPE-Recv-Key = 0x8701b3e481d72fa1333b9838a3cd448837eaed62a843295f1c9dd153c6866e499f20, raw.Vendor-Specific.Microsoft.MPPE-Send-Key = 0x9385f7dc0fd758b02dd0dc43f68266508ec93c678a5fa38525749016edede8eeea0e, EAP-Message = 0x03fc0004, Message-Authenticator = 0x00e9e565eb053138254850edb41fc013
+match Packet-Type = Access-Accept, Packet-Authentication-Vector = 0x8b7a26bee11f1ca308233d4973318772, NAS-Port = 12345, Reply-Message = "Powered by FreeRADIUS", Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef }, raw.20 = { raw.30 = 0xcafecafe }, raw.20 = { raw.30 = 0xcadecade }, Alcatel = { FR-Direct-Profile = "my profile", Home-Agent-UDP-Port = 5 }, Microsoft = { MPPE-Encryption-Policy = Encryption-Allowed, raw.MPPE-Recv-Key = 0x8701b3e481d72fa1333b9838a3cd448837eaed62a843295f1c9dd153c6866e499f20, raw.MPPE-Send-Key = 0x9385f7dc0fd758b02dd0dc43f68266508ec93c678a5fa38525749016edede8eeea0e } }, EAP-Message = 0x03fc0004, Message-Authenticator = 0x00e9e565eb053138254850edb41fc013
+
+encode-pair Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef } }
+match 1a 0c 00 00 04 d2 38 06 de ad be ef
+
+decode-pair -
+match Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef } }
+
+encode-pair Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef }, raw.20 = { raw.30 = 0xcafecafe }, raw.20 = { raw.30 = 0xcadecade }, Alcatel = { FR-Direct-Profile = "my profile", Home-Agent-UDP-Port = 5 } }
+match 1a 0c 00 00 04 d2 38 06 de ad be ef 1a 0c 00 00 00 14 1e 06 ca fe ca fe 1a 0c 00 00 00 14 1e 06 ca de ca de 1a 12 00 00 0b e1 13 0c 6d 79 20 70 72 6f 66 69 6c 65 1a 0c 00 00 0b e1 10 06 00 00 00 05
+
+encode-pair NAS-Port = 12345, Reply-Message = "Powered by FreeRADIUS", Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef }, raw.20 = { raw.30 = 0xcafecafe }, raw.20 = { raw.30 = 0xcadecade }, Alcatel = { FR-Direct-Profile = "my profile", Home-Agent-UDP-Port = 5 } }
+
+match 05 06 00 00 30 39 12 17 50 6f 77 65 72 65 64 20 62 79 20 46 72 65 65 52 41 44 49 55 53 1a 0c 00 00 04 d2 38 06 de ad be ef 1a 0c 00 00 00 14 1e 06 ca fe ca fe 1a 0c 00 00 00 14 1e 06 ca de ca de 1a 12 00 00 0b e1 13 0c 6d 79 20 70 72 6f 66 69 6c 65 1a 0c 00 00 0b e1 10 06 00 00 00 05
+
+#
+#  This seems very very wrong.
+#
+decode-pair -
+match  NAS-Port = 12345, Reply-Message = "Powered by FreeRADIUS", Vendor-Specific = { raw.1234 = { raw.56 = 0xdeadbeef }, raw.20 = { raw.30 = 0xcafecafe }, raw.20 = { raw.30 = 0xcadecade }, Alcatel = { FR-Direct-Profile = "my profile", Home-Agent-UDP-Port = 5 } }
 
 count
-match 5
+match 15
diff --git a/src/tests/unit/protocols/radius/lucent.txt b/src/tests/unit/protocols/radius/lucent.txt
@@ -6,10 +6,10 @@ encode-pair Vendor-Specific.Lucent.Max-Shared-Users = 1
 match 1a 0d 00 00 12 ee 00 02 07 00 00 00 01
 
 decode-pair -
-match Vendor-Specific.Lucent.Max-Shared-Users = 1
+match Vendor-Specific = { Lucent = { Max-Shared-Users = 1 } }
 
 decode-pair 1a 0d 00 00 12 ee ff 02 07 00 00 00 01
-match raw.Vendor-Specific.Lucent.65282 = 0x00000001
+match Vendor-Specific = { Lucent = { raw.65282 = 0x00000001 } }
 
 encode-pair -
 match 1a 0d 00 00 12 ee ff 02 07 00 00 00 01

diff --git a/src/tests/unit/protocols/radius/vendor.txt b/src/tests/unit/protocols/radius/vendor.txt
@@ -11,31 +11,31 @@ encode-pair Vendor-Specific.Starent.VPN-Name = "foo"
 match 1a 0d 00 00 1f e4 00 02 00 07 66 6f 6f
 
 decode-pair -
-match Vendor-Specific.Starent.VPN-Name = "foo"
+match Vendor-Specific = { Starent = { VPN-Name = "foo" } }
 
 encode-pair Vendor-Specific.USR.Event-Id = 1234
 match 1a 0e 00 00 01 ad 00 00 bf be 00 00 04 d2
 
 decode-pair -
-match Vendor-Specific.USR.Event-Id = 1234
+match Vendor-Specific = { USR = { Event-Id = 1234 } }
 
 decode-pair 1a 15 00 00 4e 20 01 0f 6c 69 74 68 69 61 73 70 72 69 6e 67 73
-match raw.Vendor-Specific.20000.1 = 0x6c6974686961737072696e6773
+match  Vendor-Specific = { raw.20000 = { raw.1 = 0x6c6974686961737072696e6773 } }
 
 encode-pair Vendor-Specific.3com.User-Access-Level = Visitor, Vendor-Specific.3com.Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00"
 match 1a 0c 00 00 00 2b 01 06 00 00 00 00 1a 26 00 00 00 2b 3c 20 31 35 35 2e 34 2e 31 32 2e 31 30 30 20 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30
 
 decode-pair -
-match Vendor-Specific.3com.User-Access-Level = Visitor, Vendor-Specific.3com.Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00"
+match Vendor-Specific = { 3com = { User-Access-Level = Visitor, Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00" } }
 
 #
-#  The VSAs could also be packet into one Vendor-Specific
+#  The VSAs could also be packed into one Vendor-Specific
 #
 decode-pair 1a 2e 00 00 00 2b 1c 02 01 06 00 00 00 00 3c 20 31 35 35 2e 34 2e 31 32 2e 31 30 30 20 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30
-match Vendor-Specific.3com.User-Access-Level = Visitor, Vendor-Specific.3com.Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00"
+match Vendor-Specific = { 3com = { User-Access-Level = Visitor, Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00" } }
 
 decode-pair 1a 2c 00 00 00 2b 01 06 00 00 00 00 3c 20 31 35 35 2e 34 2e 31 32 2e 31 30 30 20 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30 3a 30 30
-match Vendor-Specific.3com.User-Access-Level = Visitor, Vendor-Specific.3com.Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00"
+match Vendor-Specific = { 3com = { User-Access-Level = Visitor, Ip-Host-Addr = "155.4.12.100 00:00:00:00:00:00" } }
 
 #
 #  Vendor-Specific is of type "vsa", and therefore cannot be assigned values.
@@ -46,7 +46,7 @@ match 1a 05 ab cd ef
 encode-pair raw.26 = 0x00000009abcdef
 match 1a 09 00 00 00 09 ab cd ef
 
-decode-pair -
+decode-pair 1a 09 00 00 00 09 ab cd ef
 match raw.Vendor-Specific = 0x00000009abcdef
 
 #
@@ -60,7 +60,7 @@ encode-pair Vendor-Specific.Unisphere.LI-Action = off
 match 1a 1a 00 00 13 0a 3a 14 86 2a d5 3d ac 81 44 14 10 34 bc 97 47 38 9a 18 25 e5
 
 decode-pair -
-match Vendor-Specific.Unisphere.LI-Action = off
+match Vendor-Specific = { Unisphere = { LI-Action = off } }
 
 #packet coa_request
 #original null
@@ -72,7 +72,7 @@ match Vendor-Specific.Unisphere.LI-Action = off
 #  Unknown vendors get decoded as TLVs with type=1,length=1
 #
 decode-pair 1a12000080000105abcdef02076161616161
-match raw.Vendor-Specific.32768.1 = 0xabcdef, raw.Vendor-Specific.32768.2 = 0x6161616161
+match Vendor-Specific = { raw.32768 = { raw.1 = 0xabcdef, raw.2 = 0x6161616161 } }
 
 #
 #  This MUST be 50 octets of data
@@ -81,14 +81,14 @@ encode-pair Vendor-Specific.Microsoft.CHAP-Response = "xxxxxxxxxxxxxxxxxxxxxxxxx
 match 1a 3a 00 00 01 37 01 34 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 58
 
 decode-pair -
-match Vendor-Specific.Microsoft.CHAP-Response = 0x7878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787858
+match Vendor-Specific = { Microsoft = { CHAP-Response = 0x7878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787858 } }
 
 #
 #  And now something which *isn't* 50 octets, but instead 49.
 #  Therefore it's an "invalid attribute", as defined in RFC 6929 Section 2.8.
 #
 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 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78
-match raw.Vendor-Specific.Microsoft.CHAP-Response = 0x78787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878
+match Vendor-Specific = { Microsoft = { raw.CHAP-Response = 0x78787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878787878 } }
 
 #
 #  And an ALIAS.  First we see how the VSA should be defined,
@@ -116,7 +116,7 @@ encode-pair raw.26.6809.1 = 0xabcdef
 match 1a 0b 00 00 1a 99 01 05 ab cd ef
 
 decode-pair -
-match raw.Vendor-Specific.6809.1 = 0xabcdef
+match Vendor-Specific = { raw.6809 = { raw.1 = 0xabcdef } }
 
 pair raw.26.6809.1.2 = 0xabcdef
 match Vendor-Specific = { raw.6809 = { raw.1 = { raw.2 = 0xabcdef } } }
@@ -131,13 +131,13 @@ encode-pair raw.26.6809.1.2 = 0xabcdef
 match 1a 0d 00 00 1a 99 01 07 02 05 ab cd ef
 
 decode-pair -
-match raw.Vendor-Specific.6809.1 = { raw.2 = 0xabcdef }
+match Vendor-Specific = { raw.6809 = { raw.1 = { raw.2 = 0xabcdef } } }
 
 encode-pair raw.26.6809.1.2.3 = 0xabcdef
 match 1a 0f 00 00 1a 99 01 09 02 07 03 05 ab cd ef
 
 decode-pair -
-match raw.Vendor-Specific.6809.1 = { raw.2 = { raw.3 = 0xabcdef } }
+match Vendor-Specific = { raw.6809 = { raw.1 = { raw.2 = { raw.3 = 0xabcdef } } } }
 
 count
 match 66