From: Alan T. DeKok Date: Thu, 25 Dec 2025 19:46:31 +0000 (-0500) Subject: use new functions to decode IP addresses and prefixes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ffc8152a0ceb1de0553526caa313ec240e851f2b;p=thirdparty%2Ffreeradius-server.git use new functions to decode IP addresses and prefixes add test from fuzzer, and update RADIUS IPv6 prefix decode test. --- diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c index 51bff07e4e7..b1ea167299a 100644 --- a/src/protocols/dhcpv4/decode.c +++ b/src/protocols/dhcpv4/decode.c @@ -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: diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c index 4f2d97dcadf..046738f8346 100644 --- a/src/protocols/dhcpv6/decode.c +++ b/src/protocols/dhcpv6/decode.c @@ -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; diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c index 0b241008224..be0edec5fd8 100644 --- a/src/protocols/dns/decode.c +++ b/src/protocols/dns/decode.c @@ -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; /* diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c index f6822a57533..0d7ae462876 100644 --- a/src/protocols/radius/decode.c +++ b/src/protocols/radius/decode.c @@ -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; diff --git a/src/tests/unit/protocols/radius/foreign.txt b/src/tests/unit/protocols/radius/foreign.txt index 8fa171003f0..409e4b02e22 100644 --- a/src/tests/unit/protocols/radius/foreign.txt +++ b/src/tests/unit/protocols/radius/foreign.txt @@ -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 diff --git a/src/tests/unit/protocols/radius/packet_rfc3162.txt b/src/tests/unit/protocols/radius/packet_rfc3162.txt index ede549ef2a7..d0817b4da1d 100644 --- a/src/tests/unit/protocols/radius/packet_rfc3162.txt +++ b/src/tests/unit/protocols/radius/packet_rfc3162.txt @@ -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.