]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
use new functions to decode IP addresses and prefixes
authorAlan T. DeKok <aland@freeradius.org>
Thu, 25 Dec 2025 19:46:31 +0000 (14:46 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Fri, 26 Dec 2025 11:25:34 +0000 (06:25 -0500)
add test from fuzzer, and update RADIUS IPv6 prefix decode test.

src/protocols/dhcpv4/decode.c
src/protocols/dhcpv6/decode.c
src/protocols/dns/decode.c
src/protocols/radius/decode.c
src/tests/unit/protocols/radius/foreign.txt
src/tests/unit/protocols/radius/packet_rfc3162.txt

index 51bff07e4e7017fa211b51a9d3a8cc719fdef338..b1ea167299a685e0a27776663cbedc1f91b58187 100644 (file)
@@ -161,32 +161,32 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t
                break;
 
        /*
-        *      Doesn't include scope, whereas the generic format can
+        *      Doesn't include scope, whereas the generic format can.
         */
        case FR_TYPE_IPV6_ADDR:
-               if ((size_t) (end - p) < sizeof(vp->vp_ipv6addr)) goto raw;
+               slen = fr_value_box_ipaddr_from_network(&vp->data, da->type, da,
+                                                       128, p, (size_t) (end - p),
+                                                       exact, true);
+               if (slen < 0) goto raw;
+               fr_assert(slen == sizeof(vp->vp_ipv6addr));
 
-               if (exact && ((size_t) (end - p) > sizeof(vp->vp_ipv6addr))) goto raw;
-
-               memcpy(&vp->vp_ipv6addr, p, sizeof(vp->vp_ipv6addr));
-               vp->vp_ip.af = AF_INET6;
-               vp->vp_ip.scope_id = 0;
-               vp->vp_ip.prefix = 128;
-               vp->vp_tainted = true;
                p += sizeof(vp->vp_ipv6addr);
                break;
 
        case FR_TYPE_IPV6_PREFIX:
-               if ((size_t) (end - (p + 1)) < sizeof(vp->vp_ipv6addr)) goto raw;
+               /*
+                *      Not enough room for the prefix length, that's an issue.
+                *
+                *      Note that there's actually no standard for IPv6 prefixes inside of DHCPv4.
+                */
+               if ((end - p) < 1) goto raw;
 
-               if (exact && ((size_t) (end - p) > sizeof(vp->vp_ipv6addr))) goto raw;
+               slen = fr_value_box_ipaddr_from_network(&vp->data, da->type, da,
+                                                       p[0], p + 1, ((size_t) (end - p)) - 1,
+                                                       exact, true);
+               if (slen < 0) goto raw;
 
-               memcpy(&vp->vp_ipv6addr, p + 1, sizeof(vp->vp_ipv6addr));
-               vp->vp_ip.af = AF_INET6;
-               vp->vp_ip.scope_id = 0;
-               vp->vp_ip.prefix = p[0];
-               vp->vp_tainted = true;
-               p += sizeof(vp->vp_ipv6addr) + 1;
+               p += slen + 1;
                break;
 
        case FR_TYPE_STRUCTURAL:
index 4f2d97dcadfece4b184ce7abe585d1e846e23fc3..046738f83460ed03b8d0c27582387190bf17c710 100644 (file)
@@ -72,7 +72,6 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 {
        ssize_t                 slen;
        fr_pair_t               *vp = NULL;
-       uint8_t                 prefix_len;
        fr_dict_attr_t const    *ref;
 
        FR_PROTO_HEX_DUMP(data, data_len, "decode_value");
@@ -115,61 +114,16 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
                }
 
-               /*
-                *      Structs used fixed length IPv6 addressews.
-                */
-               if (parent->parent->type == FR_TYPE_STRUCT) {
-                       if (data_len != (1 + sizeof(vp->vp_ipv6addr))) {
-                               goto raw;
-                       }
-
-                       vp = fr_pair_afrom_da(ctx, parent);
-                       if (!vp) return PAIR_DECODE_OOM;
-                       PAIR_ALLOCED(vp);
-
-                       vp->vp_ip.af = AF_INET6;
-                       vp->vp_ip.scope_id = 0;
-                       vp->vp_ip.prefix = data[0];
-                       memcpy(&vp->vp_ipv6addr, data + 1, sizeof(vp->vp_ipv6addr));
-                       slen = 1 + sizeof(vp->vp_ipv6addr);
-                       break;
-               }
-
-               /*
-                *      No address, the prefix length MUST be zero.
-                */
-               if (data_len == 1) {
-                       if (data[0] != 0) goto raw;
-
-                       vp = fr_pair_afrom_da(ctx, parent);
-                       if (!vp) return PAIR_DECODE_OOM;
-                       PAIR_ALLOCED(vp);
-
-                       vp->vp_ip.af = AF_INET6;
-                       slen = 1;
-                       break;
-               }
-
-               prefix_len = data[0];
-
-               /*
-                *      If we have a /64 prefix but only 7 bytes of
-                *      address, that's an error.
-                */
-               slen = fr_bytes_from_bits(prefix_len);
-               if ((size_t) slen > (data_len - 1)) {
-                       goto raw;
-               }
-
                vp = fr_pair_afrom_da(ctx, parent);
                if (!vp) return PAIR_DECODE_OOM;
                PAIR_ALLOCED(vp);
 
-               vp->vp_ip.af = AF_INET6;
-               vp->vp_ip.prefix = prefix_len;
-               memcpy(&vp->vp_ipv6addr, data + 1, slen);
+               slen = fr_value_box_ipaddr_from_network(&vp->data, parent->type, parent,
+                                                       data[0], data + 1, data_len - 1,
+                                                       (parent->parent->type == FR_TYPE_STRUCT), true);
+               if (slen < 0) goto raw_free;
 
-               slen++;
+               slen++;         /* account for the prefix */
                break;
 
        /*
@@ -258,13 +212,9 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
                if (!vp) return PAIR_DECODE_OOM;
                PAIR_ALLOCED(vp);
 
-               /*
-                *      Limit the IPv6 address to 16 octets, with no scope.
-                */
-               if (data_len < sizeof(vp->vp_ipv6addr)) goto raw;
-
-               slen = fr_value_box_from_network(vp, &vp->data, vp->vp_type, vp->da,
-                                                &FR_DBUFF_TMP(data,  sizeof(vp->vp_ipv6addr)),  sizeof(vp->vp_ipv6addr), true);
+               slen = fr_value_box_ipaddr_from_network(&vp->data, parent->type, parent,
+                                                       128, data, data_len,
+                                                       true, true);
                if (slen < 0) goto raw_free;
                break;
 
index 0b24100822406e98f59b724605c705cd9d6b7419..be0edec5fd80aa70667f35ecf116d1778d057b2a 100644 (file)
@@ -69,7 +69,6 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 {
        ssize_t                 slen;
        fr_pair_t               *vp;
-       uint8_t                 prefix_len;
 
        FR_PROTO_HEX_DUMP(data, data_len, "decode_value");
 
@@ -78,57 +77,24 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      Address MAY be shorter than 16 bytes.
         */
        case FR_TYPE_IPV6_PREFIX:
-               if ((data_len == 0) || (data_len > (1 + sizeof(vp->vp_ipv6addr)))) {
+               if (data_len == 0) {
                raw:
                        return fr_pair_raw_from_network(ctx, out, parent, data, data_len);
-
-               }
-
-               /*
-                *      Structs used fixed length fields
-                */
-               if (parent->parent->type == FR_TYPE_STRUCT) {
-                       if (data_len != (1 + sizeof(vp->vp_ipv6addr))) goto raw;
-
-                       vp = fr_pair_afrom_da(ctx, parent);
-                       if (!vp) return PAIR_DECODE_OOM;
-                       PAIR_ALLOCED(vp);
-
-                       vp->vp_ip.af = AF_INET6;
-                       vp->vp_ip.prefix = data[0];
-                       memcpy(&vp->vp_ipv6addr, data + 1, data_len - 1);
-                       break;
-               }
-
-               /*
-                *      No address, the prefix length MUST be zero.
-                */
-               if (data_len == 1) {
-                       if (data[0] != 0) goto raw;
-
-                       vp = fr_pair_afrom_da(ctx, parent);
-                       if (!vp) return PAIR_DECODE_OOM;
-                       PAIR_ALLOCED(vp);
-
-                       vp->vp_ip.af = AF_INET6;
-                       break;
                }
 
-               prefix_len = data[0];
-
-               /*
-                *      If we have a /64 prefix but only 7 bytes of
-                *      address, that's an error.
-                */
-               if (fr_bytes_from_bits(prefix_len) > (data_len - 1)) goto raw;
-
                vp = fr_pair_afrom_da(ctx, parent);
                if (!vp) return PAIR_DECODE_OOM;
                PAIR_ALLOCED(vp);
 
-               vp->vp_ip.af = AF_INET6;
-               vp->vp_ip.prefix = prefix_len;
-               memcpy(&vp->vp_ipv6addr, data + 1, data_len - 1);
+               /*
+                *      Check values of prefix length, data lengths, etc.
+                */
+               if (fr_value_box_ipaddr_from_network(&vp->data, parent->type, parent,
+                                                    data[0], data + 1, data_len - 1,
+                                                    (parent->parent->type == FR_TYPE_STRUCT), true) < 0) {
+                       talloc_free(vp);
+                       goto raw;
+               }
                break;
 
        /*
index f6822a57533cf216c362c26b06727ea96dad6e1c..0d7ae46287627fc3fce105bcd71bc836ce7c8a6c 100644 (file)
@@ -1852,7 +1852,7 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
 
        switch (parent->type) {
        /*
-        *  Magic RADIUS format IPv4 prefix
+        *  RFC8044 IPv4 prefix
         *
         *  0                   1                   2                   3
         *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ -1862,22 +1862,20 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      ... Prefix                 |
         * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         *
-        * RFC does not require non-masked bits to be zero.
+        *  The bits outside of the prefix mask MUST be zero.
         */
        case FR_TYPE_IPV4_PREFIX:
                if (data_len != 6) goto raw;
                if (p[0] != 0) goto raw;
-               if ((p[1] & 0x3f) > 32) goto raw;
 
-               vp->vp_ip.af = AF_INET;
-               vp->vp_ip.scope_id = 0;
-               vp->vp_ip.prefix = p[1] & 0x3f;
-               memcpy((uint8_t *)&vp->vp_ipv4addr, p + 2, data_len - 2);
-               fr_ipaddr_mask(&vp->vp_ip, p[1] & 0x3f);
+               if (fr_value_box_ipaddr_from_network(&vp->data, parent->type, parent,
+                                                    p[1], p + 2, 4, true, true) < 0) {
+                       goto raw;
+               }
                break;
 
        /*
-        *  Magic RADIUS format IPv6 prefix
+        *  RFC8044 IPv6 prefix
         *
         *   0                   1                   2                   3
         *   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
@@ -1893,36 +1891,19 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *                               Prefix                             |
         *  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
         *
-        *  RFC says non-masked bits MUST be zero.
+        *  The bits outside of the prefix mask MUST be zero.
         */
        case FR_TYPE_IPV6_PREFIX:
        {
                if (data_len > 18) goto raw;
                if (data_len < 2) goto raw;
                if (p[0] != 0) goto raw;        /* First byte is always 0 */
-               if (p[1] > 128) goto raw;
-
-               /*
-                *      Convert prefix bits to bytes to check that
-                *      we have sufficient data.
-                *
-                *      If Prefix has more data than Prefix-Length
-                *      indicates, we just ignore the rest.
-                */
-               if (fr_bytes_from_bits(p[1]) > (data_len - 2)) goto raw;
 
-               vp->vp_ip.af = AF_INET6;
-               vp->vp_ip.scope_id = 0;
-               vp->vp_ip.prefix = p[1];
-
-               memcpy((uint8_t *)&vp->vp_ipv6addr, p + 2, data_len - 2);
-               fr_ipaddr_mask(&vp->vp_ip, p[1]);
+               if (fr_value_box_ipaddr_from_network(&vp->data, parent->type, parent,
+                                                    p[1], p + 2, data_len - 2, false, true) < 0) {
+                       goto raw;
+               }
 
-               /*
-                *      Check the prefix data is the same before
-                *      and after casting (it should be).
-                */
-               if (memcmp(p + 2, (uint8_t *)&vp->vp_ipv6addr, data_len - 2) != 0) goto raw;
        }
                break;
 
index 8fa171003f0b4aadecd70e23eed643d366606c79..409e4b02e22cbc6720f93a4bd3f71df4b915b1b5 100644 (file)
@@ -78,5 +78,9 @@ match Vendor-Specific = { Nokia-SR = { ToServer-Dhcp-Options = { Message-Type =
 decode-proto 05 ff 00 1e 00 2a 3e 00 c7 00 00 18 13 00 2f 00 00 20 00 00 1a 0a 00 00 19 7f 66 04 21 00
 match Packet-Type = ::Accounting-Response, Packet-Authentication-Vector = 0x002a3e00c700001813002f0000200000, Vendor-Specific = { Nokia-SR = { ToServer-Dhcp-Options = { } } }
 
+decode-proto 20 20 00 b5 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 ff f5 a1 03 20 20 20 00 00 20 20 00 00 20 20 00 01 20 20 20 00 00 00 5b 00 21 ff 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 00 00 20 20 00 5b 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 00 00
+
+match Packet-Type = ::Password-Expired, Packet-Authentication-Vector = 0x202020202020202020202020202020ff, Extended-Attribute-5 = { DHCPv6-Options = { raw.8224 = 0x, raw.8224 = 0x, raw.8224 = 0x20, raw.8224 = 0x, raw.S46-Default-Mapping-Rule = 0xff2020202020202020202020202020202020202020202020202020202020202020, raw.8224 = 0x, raw.8224 = 0x20202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020202020, raw.8224 = 0x } }
+
 count
-match 31
+match 33
index ede549ef2a7be2e8fa1bb6b559a63afcffdfc665..d0817b4da1db4b862aa3bc2e732aeb9a84d41ebf 100644 (file)
@@ -40,8 +40,11 @@ match Framed-IPv6-Prefix = 2001:db8:a0b:12f0::/64
 #
 #  trailing bytes outside of the prefix exist, but are not zero
 #
+#  This violates RFC 8044, but we don't care.  We just mash the extra
+#  bytes to zeroes.
+#
 decode-pair 61 14 00 40 20 01 0d b8 0a 0b 12 f0 00 00 00 00 00 00 00 01
-match raw.Framed-IPv6-Prefix = 0x004020010db80a0b12f00000000000000001
+match Framed-IPv6-Prefix = 2001:db8:a0b:12f0::/64
 
 #
 #  1.