From: Alan T. DeKok Date: Mon, 18 Apr 2022 15:46:11 +0000 (-0400) Subject: move decode_tlvs to common API X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=56d48f89ee7d63ca468027f026f6bd1d51f614dc;p=thirdparty%2Ffreeradius-server.git move decode_tlvs to common API with a flag to see if we decode the TLVs as nested (or not) --- diff --git a/src/lib/util/decode.c b/src/lib/util/decode.c index a6e7fba0ff5..eaed8cf87ae 100644 --- a/src/lib/util/decode.c +++ b/src/lib/util/decode.c @@ -37,7 +37,8 @@ * @param[in] decode_value function to decode one value. */ ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, - uint8_t const *data, size_t data_len, void *decode_ctx, fr_pair_decode_value_t decode_value) + uint8_t const *data, size_t data_len, + void *decode_ctx, fr_pair_decode_value_t decode_value) { uint8_t const *p = data, *end = p + data_len; ssize_t slen; @@ -66,7 +67,8 @@ ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict * @param[in] data to parse. * @param[in] data_len of data to parse. */ -ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len) +ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, + uint8_t const *data, size_t data_len) { ssize_t slen; fr_pair_t *vp; @@ -104,3 +106,74 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a return data_len; } + + +/** Decode a list of pairs from the network + * + * @param[in] ctx context to alloc new attributes in. + * @param[out] out Where to write the decoded #fr_pair_t + * @param[in] parent dictionary entry, must have parent->flags.array set + * @param[in] data to parse. + * @param[in] data_len of data to parse. + * @param[in] decode_ctx passed to decode_value + * @param[in] decode_tlv function to decode one attribute / option / tlv + * @param[in] nested whether or not we create nested VPs. + * + * The decode_tlv function should return an error if the option is + * malformed. In that case, the entire list of pairs is thrown away, + * and a "raw" attribute is created which contains the entire + * data_len. + * + * If the value is malformed, then the decode_tlv function should call + * fr_pair_raw_from_network() on the value, and return a positive value. + */ +ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, + void *decode_ctx, fr_pair_decode_value_t decode_tlv, bool nested) +{ + uint8_t const *p, *end; + fr_pair_list_t tlvs, *list; + fr_pair_t *vp = NULL; + TALLOC_CTX *child_ctx; + + FR_PROTO_HEX_DUMP(data, data_len, "fr_pair_tlvs_from_network"); + + if (!fr_cond_assert_msg((parent->type == FR_TYPE_TLV || (parent->type == FR_TYPE_VENDOR)), + "%s: Internal sanity check failed, attribute \"%s\" is not of type 'tlv'", + __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR; + p = data; + end = data + data_len; + + if (!nested) { + fr_pair_list_init(&tlvs); + list = &tlvs; + child_ctx = ctx; + } else { + vp = fr_pair_afrom_da(ctx, parent); + if (!vp) return PAIR_DECODE_OOM; + list = &vp->vp_group; + child_ctx = vp; + } + + while (p < end) { + ssize_t slen; + + slen = decode_tlv(child_ctx, list, parent, p, (end - p), decode_ctx); + if (slen <= 0) { + fr_pair_list_free(list); + talloc_free(vp); + return fr_pair_raw_from_network(ctx, out, parent, data, data_len); + } + + p += slen; + } + + if (!nested) { + fr_pair_list_append(out, &tlvs); + } else { + fr_pair_append(out, vp); + } + + return data_len; +} diff --git a/src/lib/util/decode.h b/src/lib/util/decode.h index f3ac4205fda..0caf380c053 100644 --- a/src/lib/util/decode.h +++ b/src/lib/util/decode.h @@ -49,11 +49,16 @@ typedef ssize_t (*fr_pair_decode_value_t)(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *data, size_t const data_len, void *decode_ctx) ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, - uint8_t const *data, size_t data_len, void *decode_ctx, fr_pair_decode_value_t decode_value) CC_HINT(nonnull(1,2,3,4,6)); + uint8_t const *data, size_t data_len, void *decode_ctx, fr_pair_decode_value_t decode_value) CC_HINT(nonnull(1,2,3,4,7)); ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len) CC_HINT(nonnull); +ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, + void *decode_ctx, fr_pair_decode_value_t decode_tlv, bool nested) CC_HINT(nonnull(1,2,3,4,7)); + #ifdef __cplusplus } #endif diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c index 9a43a6c79b4..3eb564e3cc6 100644 --- a/src/protocols/dhcpv4/decode.c +++ b/src/protocols/dhcpv4/decode.c @@ -36,8 +36,12 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx); -static ssize_t decode_tlv(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, - uint8_t const *data, size_t data_len, void *decode_ctx); +static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decode_ctx) +{ + return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, false); +} static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len, void *decode_ctx); @@ -85,7 +89,7 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t */ if (da->type == FR_TYPE_STRUCT) { slen = fr_struct_from_network(ctx, out, da, data, data_len, true, - decode_ctx, decode_value_trampoline, decode_tlv); + decode_ctx, decode_value_trampoline, decode_tlv_trampoline); if (slen < 0) return slen; if (!exact) return slen; @@ -310,88 +314,6 @@ finish: * Vendor-Specific-Information with raw octets contents. */ -/** Decode DHCP suboptions - * - * @param[in] ctx context to alloc new attributes in. - * @param[out] out Where to write the decoded options. - * @param[in] parent of sub TLVs. - * @param[in] data to parse. - * @param[in] data_len of the data to parse - * @return - * <= 0 on error - * data_len on success. - */ -static ssize_t decode_tlv(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, - uint8_t const *data, size_t data_len, void *decode_ctx) -{ - uint8_t const *p = data; - uint8_t const *end = data + data_len; - - /* - * Type, length, data. If that doesn't exist, we decode - * the data as "raw" in the parents context/ - */ - if (data_len < 3) { - return fr_pair_raw_from_network(ctx, out, parent, data, data_len); - } - - FR_PROTO_TRACE("%s called to parse %zu byte(s)", __FUNCTION__, data_len); - FR_PROTO_HEX_DUMP(data, data_len, NULL); - - /* - * Each TLV may contain multiple children - */ - while (p < end) { - ssize_t slen; - - /* - * RFC 3046 is very specific about not allowing termination - * with a 255 sub-option. But it's required for decoding - * option 43, and vendors will probably screw it up - * anyway. - * - * Similarly, option 0 is sometimes treated as - * "end of options". - */ - if ((p[0] == 0) || (p[0] == 255)) { - if ((p + 1) == end) return data_len; - - /* - * There's stuff after the "end of - * options" option. Return it as random crap. - */ - raw: - /* - * @todo - the "goto raw" can leave partially decoded VPs in the output. we'll - * need a temporary cursor / pair_list to fix that. - */ - slen = fr_pair_raw_from_network(ctx, out, parent, p, end - p); - if (slen < 0) return slen - (p - data); - - return data_len; - } - - /* - * Everything else should be real options - */ - if ((end - p) < 2) goto raw; - - if ((p[1] + 2) > (end - p)) goto raw; - - slen = decode_option(ctx, out, parent, p, p[1] + 2, decode_ctx); - if (slen <= 0) { - return slen - (p - data); - } - fr_assert(slen <= (2 + p[1])); - - p += 2 + p[1]; - FR_PROTO_TRACE("remaining TLV data %zu byte(s)" , end - p); - } - FR_PROTO_TRACE("tlv parsing complete, returning %zu byte(s)", p - data); - - return data_len; -} - static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, UNUSED void *decode_ctx) @@ -545,7 +467,7 @@ next: } p++; - len = decode_tlv(ctx, out, vendor, p, option_len, decode_ctx); + len = fr_pair_tlvs_from_network(ctx, out, vendor, p, option_len, decode_ctx, decode_option, false); if (len <= 0) return len; p += len; @@ -574,6 +496,21 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_assert(parent != NULL); + /* + * RFC 3046 is very specific about not allowing termination + * with a 255 sub-option. But it's required for decoding + * option 43, and vendors will probably screw it up + * anyway. + * + * Similarly, option 0 is sometimes treated as + * "end of options". + * + * @todo - this check is likely correct only when at the + * dhcpv4 root, OR inside of option 43. It could be + * argued that it's wrong for all other TLVs. + */ + if ((data_len == 1) && ((data[0] == 0) || (data[1] == 255))) return data_len; + /* * Must have at least an option header. */ @@ -626,7 +563,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, slen = decode_vsa(ctx, out, da, data + 2, len, decode_ctx); } else if (da->type == FR_TYPE_TLV) { - slen = decode_tlv(ctx, out, da, data + 2, len, decode_ctx); + slen = fr_pair_tlvs_from_network(ctx, out, da, data + 2, len, decode_ctx, decode_option, false); } else { slen = decode_value(ctx, out, da, data + 2, len, decode_ctx); @@ -732,7 +669,7 @@ ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, slen = decode_vsa(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx); } else if (da->type == FR_TYPE_TLV) { - slen = decode_tlv(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx); + slen = fr_pair_tlvs_from_network(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx, decode_option, false); } else if (da->flags.array) { slen = fr_pair_array_from_network(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx, decode_value); diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c index ee5673f3d14..2ab1eafcf14 100644 --- a/src/protocols/dhcpv6/decode.c +++ b/src/protocols/dhcpv6/decode.c @@ -39,18 +39,14 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx); -static ssize_t decode_tlvs(TALLOC_CTX *ctx, fr_pair_list_t *out, - fr_dict_attr_t const *parent, - uint8_t const *data, size_t const data_len, void *decode_ctx); static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx) { - return decode_tlvs(ctx, out, parent, data, data_len, decode_ctx); + return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, true); } - static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx); @@ -78,7 +74,7 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *data, size_t const data_len, void *decode_ctx) { ssize_t slen; - fr_pair_t *vp; + fr_pair_t *vp = NULL; uint8_t prefix_len; FR_PROTO_HEX_DUMP(data, data_len, "decode_value"); @@ -199,11 +195,10 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, * header, as we're just decoding the values * here. */ - slen = decode_tlvs(vp, &vp->vp_group, fr_dict_root(dict_dhcpv6), data, data_len, decode_ctx); + slen = fr_pair_tlvs_from_network(vp, &vp->vp_group, fr_dict_root(dict_dhcpv6), data, data_len, decode_ctx, decode_option, false); if (slen < 0) { talloc_free(vp); return slen; - goto raw; } break; @@ -245,6 +240,8 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, goto raw; } + fr_assert(vp != NULL); + vp->vp_tainted = true; fr_pair_append(out, vp); @@ -324,52 +321,6 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_pair_list_t *out, } -/** Like decode_option(), but decodes *all* of the options. - * - * If decoding an option here fails, then we *don't* return a raw - * attribute, as we're only decoding the *values*. We rely on the - * calling function to return a raw attribute of the correct parent - * attribute. - * - * This behavior is because when we decode #FR_TYPE_GROUP, we get - * passed the *root* attribute. But if the group is malformed, we - * need to create a "raw" attribute from the *parent* - * (i.e. container) attribute, not from the root. - */ -static ssize_t decode_tlvs(TALLOC_CTX *ctx, fr_pair_list_t *out, - fr_dict_attr_t const *parent, - uint8_t const *data, size_t const data_len, void *decode_ctx) -{ - uint8_t const *p, *end; - fr_pair_list_t tlvs; - - FR_PROTO_HEX_DUMP(data, data_len, "decode_tlvs"); - - if (!fr_cond_assert_msg((parent->type == FR_TYPE_TLV || (parent->type == FR_TYPE_VENDOR)), - "%s: Internal sanity check failed, attribute \"%s\" is not of type 'tlv'", - __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR; - p = data; - end = data + data_len; - - fr_pair_list_init(&tlvs); - - while (p < end) { - ssize_t slen; - - slen = decode_option(ctx, &tlvs, parent, p, (end - p), decode_ctx); - if (slen <= 0) { - fr_pair_list_free(&tlvs); - return slen; - } - - p += slen; - } - - fr_pair_list_append(out, &tlvs); - return data_len; -} - - static ssize_t decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx) @@ -411,7 +362,7 @@ static ssize_t decode_vsa(TALLOC_CTX *ctx, fr_pair_list_t *out, FR_PROTO_TRACE("decode context %s -> %s", parent->name, da->name); - return decode_tlvs(ctx, out, da, data + 4, data_len - 4, decode_ctx); + return fr_pair_tlvs_from_network(ctx, out, da, data + 4, data_len - 4, decode_ctx, decode_option, false); } static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, @@ -499,7 +450,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, slen = decode_vsa(ctx, out, da, data + 4, len, decode_ctx); } else if (da->type == FR_TYPE_TLV) { - slen = decode_tlvs(ctx, out, da, data + 4, len, decode_ctx); + slen = fr_pair_tlvs_from_network(ctx, out, da, data + 4, len, decode_ctx, decode_option, false); } else { slen = decode_value(ctx, out, da, data + 4, len, decode_ctx); diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c index 4eb3633f5e8..6bcba4f37e7 100644 --- a/src/protocols/dns/decode.c +++ b/src/protocols/dns/decode.c @@ -33,10 +33,22 @@ #include "dns.h" #include "attrs.h" +static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decode_ctx); + static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx); +static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decode_ctx) +{ + return fr_pair_tlvs_from_network(ctx, out, parent, data, data_len, decode_ctx, decode_option, true); +} + + static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *parent, uint8_t const *data, size_t const data_len, void *decode_ctx); @@ -313,43 +325,6 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, return len + 4; } -/** Only for OPT 41 - * - */ -static ssize_t decode_tlvs(TALLOC_CTX *ctx, fr_pair_list_t *out, - fr_dict_attr_t const *parent, - uint8_t const *data, size_t const data_len, void *decode_ctx) -{ - uint8_t const *p, *end; - fr_pair_t *vp; - - FR_PROTO_HEX_DUMP(data, data_len, "decode_tlvs"); - - if (!fr_cond_assert_msg((parent->type == FR_TYPE_TLV || (parent->type == FR_TYPE_VENDOR)), - "%s: Internal sanity check failed, attribute \"%s\" is not of type 'tlv'", - __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR; - p = data; - end = data + data_len; - - vp = fr_pair_afrom_da(ctx, parent); - if (!vp) return PAIR_DECODE_OOM; - - while (p < end) { - ssize_t slen; - - slen = decode_option(vp, &vp->vp_group, parent, p, (end - p), decode_ctx); - if (slen <= 0) { - slen = fr_pair_raw_from_network(vp, &vp->vp_group, parent, p, (end - p)); - if (slen <= 0) return slen - (p - data); - } - - p += slen; - } - - fr_pair_append(out, vp); - return data_len; -} - static ssize_t decode_record(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *attr, uint8_t const *rr, uint8_t const *end, fr_dns_ctx_t *packet_ctx, uint8_t const *counter) @@ -370,7 +345,7 @@ static ssize_t decode_record(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_ } slen = fr_struct_from_network(ctx, out, attr, p, end - p, true, - packet_ctx, decode_value_trampoline, decode_tlvs); + packet_ctx, decode_value_trampoline, decode_tlv_trampoline); if (slen < 0) return slen; if (!slen) break; p += slen; @@ -482,7 +457,7 @@ static ssize_t decode_rr(TALLOC_CTX *ctx, fr_pair_list_t *out, UNUSED fr_dict_at } slen = fr_struct_from_network(ctx, out, attr_dns_rr, data, data_len, true, - decode_ctx, decode_value_trampoline, decode_tlvs); + decode_ctx, decode_value_trampoline, decode_tlv_trampoline); if (slen < 0) return slen; FR_PROTO_TRACE("decoding option complete, returning %zd byte(s)", slen);