From: Alan T. DeKok Date: Tue, 19 Apr 2022 01:09:01 +0000 (-0400) Subject: add fr_pair_dns_labels_from_network() X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c8a1f6a0d5d713744728374c8f73b478418180ef;p=thirdparty%2Ffreeradius-server.git add fr_pair_dns_labels_from_network() and make various protocols call it. --- diff --git a/src/lib/util/decode.c b/src/lib/util/decode.c index eaed8cf87ae..dc9d505f136 100644 --- a/src/lib/util/decode.c +++ b/src/lib/util/decode.c @@ -35,6 +35,8 @@ * @param[in] data_len of data to parse. * @param[in] decode_ctx passed to decode_value * @param[in] decode_value function to decode one value. + * <0 on error - decode error, or OOM + * data_len on success */ 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, @@ -66,6 +68,8 @@ ssize_t fr_pair_array_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict * @param[in] parent dictionary entry * @param[in] data to parse. * @param[in] data_len of data to parse. + * <0 on error - decode error, or OOM + * data_len on success */ 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) @@ -79,6 +83,8 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a if (!parent->parent) return -1; /* stupid static analyzers */ #endif + FR_PROTO_HEX_DUMP(data, data_len, "fr_pair_raw_from_network"); + /* * Build an unknown attr of the entire data. */ @@ -115,9 +121,11 @@ ssize_t fr_pair_raw_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a * @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_ctx passed to decode_tlv * @param[in] decode_tlv function to decode one attribute / option / tlv * @param[in] nested whether or not we create nested VPs. + * <0 on error - decode error, or OOM + * data_len on success * * The decode_tlv function should return an error if the option is * malformed. In that case, the entire list of pairs is thrown away, @@ -177,3 +185,113 @@ ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, return data_len; } + + +/** Decode a DNS label or a list of DNS labels 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] start of the DNS labels to decode + * @param[in] data to parse. + * @param[in] data_len of data to parse. + * @param[in] lb struct to help with decoding packets. + * @param[in] exact whether the labels should entirely fill the buffer. + * @return + * <0 on error - decode error, or OOM + * data_len on success + * + * DNS labels exist in many protocols, and we also have src/lib/dns.c, so we might + * as well put a common function here, too. + * + * This function assumes that the DNS label or labels take up all of the + * input. If they do not, then the decoded DNS labels are freed, and + * a raw attribute is returned instead. + */ +ssize_t fr_pair_dns_labels_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, uint8_t const *start, + uint8_t const *data, size_t const data_len, fr_dns_labels_t *lb, bool exact) +{ + ssize_t slen; + size_t total, labels_len; + fr_pair_t *vp; + uint8_t const *next = data; + fr_pair_list_t tmp; + + FR_PROTO_HEX_DUMP(data, data_len, "fr_pair_dns_labels_from_network"); + + fr_pair_list_init(&tmp); + + /* + * This function handles both single-valued and array + * types. It's just easier that way. + */ + if (!parent->flags.array) { + /* + * Decode starting at "NEXT", but allowing decodes from the start of the packet. + */ + slen = fr_dns_label_uncompressed_length(start, data, data_len, &next, lb); + if (slen <= 0) { + FR_PROTO_TRACE("ERROR - uncompressed length failed"); + goto raw; + } + + labels_len = next - data; /* decode only what we've found */ + } else { + /* + * Get the length of the entire set of labels, up + * to (and including) the final 0x00. + * + * If any of the labels point outside of this + * area, OR they are otherwise invalid, then that's an error. + */ + slen = fr_dns_labels_network_verify(start, data, data_len, data, lb); + if (slen <= 0) { + FR_PROTO_TRACE("ERROR - network verify failed"); + goto raw; + } + + labels_len = slen; + } + + /* + * The labels MUST fill the entire buffer. + */ + if (exact && (labels_len != data_len)) { + FR_PROTO_TRACE("ERROR - labels_len %zu != data_len %zu", labels_len, data_len); + raw: + return fr_pair_raw_from_network(ctx, out, parent, data, data_len); + } + + /* + * Loop over the input buffer, decoding the labels one by + * one. + * + * @todo - put the labels into a child cursor, and then + * merge them only if it succeeds. That doesn't seem to + * work for some reason, and I don't have time to debug + * it right now. So... let's leave it. + */ + for (total = 0; total < labels_len; total += slen) { + vp = fr_pair_afrom_da(ctx, parent); + if (!vp) return PAIR_DECODE_OOM; + + /* + * Having verified the input above, this next + * function should never fail unless there's a + * bug in the code. + */ + slen = fr_dns_label_to_value_box(vp, &vp->data, data, labels_len, data + total, true, lb); + if (slen <= 0) { + FR_PROTO_TRACE("ERROR - failed decoding DNS label at with %zu error %zd", total, slen); + talloc_free(vp); + fr_pair_list_free(&tmp); + goto raw; + } + + fr_pair_append(&tmp, vp); + } + + fr_pair_list_append(out, &tmp); + return labels_len; +} diff --git a/src/lib/util/decode.h b/src/lib/util/decode.h index 0caf380c053..166963545f5 100644 --- a/src/lib/util/decode.h +++ b/src/lib/util/decode.h @@ -28,6 +28,7 @@ extern "C" { #endif #include +#include /** Decode a value from the network into an output #fr_pair_list_t * @@ -59,6 +60,10 @@ ssize_t fr_pair_tlvs_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, 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)); +ssize_t fr_pair_dns_labels_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out, + fr_dict_attr_t const *parent, uint8_t const *start, + uint8_t const *data, size_t const data_len, fr_dns_labels_t *lb, bool exact); + #ifdef __cplusplus } #endif diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c index 3eb564e3cc6..a2a0f8a3441 100644 --- a/src/protocols/dhcpv4/decode.c +++ b/src/protocols/dhcpv4/decode.c @@ -46,10 +46,6 @@ static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, 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); -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); - /** Handle arrays of DNS labels for fr_struct_from_network() * */ @@ -63,7 +59,7 @@ static ssize_t decode_value_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, * @todo - we might need to limit this to only one DNS label. */ if ((parent->type == FR_TYPE_STRING) && da_is_dns_label(parent)) { - return decode_dns_labels(ctx, out, parent, data, data_len, decode_ctx); + return fr_pair_dns_labels_from_network(ctx, out, parent, data, data, data_len, NULL, false); } return decode_value(ctx, out, parent, data, data_len, decode_ctx); @@ -314,74 +310,6 @@ finish: * Vendor-Specific-Information with raw octets contents. */ -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) -{ - ssize_t slen; - size_t total, labels_len; - fr_pair_t *vp; - uint8_t const *next = data; - - FR_PROTO_HEX_DUMP(data, data_len, "decode_dns_labels"); - - /* - * This function handles both single-valued and array - * types. It's just easier that way. - */ - if (!parent->flags.array) { - slen = fr_dns_label_uncompressed_length(data, data, data_len, &next, NULL); - if (slen <= 0) goto raw; - - /* - * If the DNS label doesn't exactly fill the option, it's an error. - * - * @todo - we may want to remove this check. - */ - if (next != (data + data_len)) goto raw; - - labels_len = next - data; /* decode only what we've found */ - } else { - /* - * Get the length of the entire set of labels, up - * to (and including) the final 0x00. - * - * If any of the labels point outside of this - * area, OR they are otherwise invalid, then that's an error. - */ - slen = fr_dns_labels_network_verify(data, data, data_len, data, NULL); - if (slen < 0) { - raw: - return fr_pair_raw_from_network(ctx, out, parent, data, data_len); - } - - labels_len = slen; - } - - /* - * Loop over the input buffer, decoding the labels one by - * one. - */ - for (total = 0; total < labels_len; total += slen) { - vp = fr_pair_afrom_da(ctx, parent); - if (!vp) return PAIR_DECODE_OOM; - - /* - * Having verified the input above, this next - * function should never fail unless there's a - * bug in the code. - */ - slen = fr_dns_label_to_value_box(vp, &vp->data, data, labels_len, data + total, true, NULL); - if (slen <= 0) { - talloc_free(vp); - goto raw; - } - - fr_pair_append(out, vp); - } - - return labels_len; -} /* * One VSA option may contain multiple vendors, each vendor @@ -536,25 +464,8 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, FR_PROTO_TRACE("decode context changed %s -> %s",da->parent->name, da->name); if ((da->type == FR_TYPE_STRING) && da_is_dns_label(da)) { - fr_pair_list_t tmp; - - fr_pair_list_init(&tmp); - - slen = decode_dns_labels(ctx, &tmp, da, data + 2, len, decode_ctx); - if (slen < 0) { - raw: - fr_pair_list_free(&tmp); - slen = fr_pair_raw_from_network(ctx, out, da, data + 2, len); - if (slen < 0) return slen; - } else { - /* - * The DNS labels may only partially fill the - * option. If so, then it's a decode error. - */ - if ((size_t) slen != len) goto raw; - - fr_pair_list_append(out, &tmp); - } + slen = fr_pair_dns_labels_from_network(ctx, out, da, data + 2, data + 2, len, NULL, true); + if (slen < 0) return slen; } else if (da->flags.array) { slen = fr_pair_array_from_network(ctx, out, da, data + 2, len, decode_ctx, decode_value); diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c index 2ab1eafcf14..4ca4a0e2a4f 100644 --- a/src/protocols/dhcpv6/decode.c +++ b/src/protocols/dhcpv6/decode.c @@ -50,9 +50,6 @@ static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, 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_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); /** Handle arrays of DNS labels for fr_struct_from_network() * @@ -62,7 +59,7 @@ static ssize_t decode_value_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *data, size_t const data_len, void *decode_ctx) { if ((parent->type == FR_TYPE_STRING) && da_is_dns_label(parent)) { - return decode_dns_labels(ctx, out, parent, data, data_len, decode_ctx); + return fr_pair_dns_labels_from_network(ctx, out, parent, data, data, data_len, NULL, false); } return decode_value(ctx, out, parent, data, data_len, decode_ctx); @@ -251,76 +248,6 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, } -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) -{ - ssize_t slen; - size_t total, labels_len; - fr_pair_t *vp; - uint8_t const *next = data; - - FR_PROTO_HEX_DUMP(data, data_len, "decode_dns_labels"); - - /* - * This function handles both single-valued and array - * types. It's just easier that way. - */ - if (!parent->flags.array) { - slen = fr_dns_label_uncompressed_length(data, data, data_len, &next, NULL); - if (slen <= 0) goto raw; - - /* - * If the DNS label doesn't exactly fill the option, it's an error. - * - * @todo - we may want to remove this check. - */ - if (next != (data + data_len)) goto raw; - - labels_len = next - data; /* decode only what we've found */ - } else { - /* - * Get the length of the entire set of labels, up - * to (and including) the final 0x00. - * - * If any of the labels point outside of this - * area, OR they are otherwise invalid, then that's an error. - */ - slen = fr_dns_labels_network_verify(data, data, data_len, data, NULL); - if (slen < 0) { - raw: - return fr_pair_raw_from_network(ctx, out, parent, data, data_len); - } - - labels_len = slen; - } - - /* - * Loop over the input buffer, decoding the labels one by - * one. - */ - for (total = 0; total < labels_len; total += slen) { - vp = fr_pair_afrom_da(ctx, parent); - if (!vp) return PAIR_DECODE_OOM; - - /* - * Having verified the input above, this next - * function should never fail unless there's a - * bug in the code. - */ - slen = fr_dns_label_to_value_box(vp, &vp->data, data, labels_len, data + total, true, NULL); - if (slen <= 0) { - talloc_free(vp); - goto raw; - } - - fr_pair_append(out, vp); - } - - return labels_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) @@ -425,23 +352,8 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_pair_append(out, vp); } else if ((da->type == FR_TYPE_STRING) && da_is_dns_label(da)) { - fr_pair_list_t tmp; - - fr_pair_list_init(&tmp); - - slen = decode_dns_labels(ctx, &tmp, da, data + 4, len, decode_ctx); - if (slen < 0) goto raw; - - /* - * The DNS labels may only partially fill the - * option. If so, then it's a decode error. - */ - if ((size_t) slen != len) { - fr_pair_list_free(&tmp); - goto raw; - } - - fr_pair_list_append(out, &tmp); + slen = fr_pair_dns_labels_from_network(ctx, out, da, data + 4, data + 4, len, NULL, true); + if (slen < 0) return slen; } else if (da->flags.array) { slen = fr_pair_array_from_network(ctx, out, da, data + 4, len, decode_ctx, decode_value); diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c index 6bcba4f37e7..4c12f2cae0e 100644 --- a/src/protocols/dns/decode.c +++ b/src/protocols/dns/decode.c @@ -49,20 +49,14 @@ static ssize_t decode_tlv_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, } -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); - -/** Handle arrays of DNS labels for fr_struct_from_network() - * - */ static ssize_t decode_value_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) { if ((parent->type == FR_TYPE_STRING) && !parent->flags.extra && parent->flags.subtype) { - FR_PROTO_TRACE("decode DNS labels"); - return decode_dns_labels(ctx, out, parent, data, data_len, decode_ctx); + fr_dns_ctx_t *packet_ctx = decode_ctx; + + return fr_pair_dns_labels_from_network(ctx, out, parent, packet_ctx->packet, data, data_len, packet_ctx->lb, false); } return decode_value(ctx, out, parent, data, data_len, decode_ctx); @@ -174,83 +168,6 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, } -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) -{ - ssize_t slen; - size_t total, labels_len; - fr_pair_t *vp; - uint8_t const *next = data; - fr_dns_ctx_t *packet_ctx = decode_ctx; - - FR_PROTO_HEX_DUMP(data, data_len, "decode_dns_labels"); - - /* - * This function handles both single-valued and array - * types. It's just easier that way. - */ - if (!parent->flags.array) { - /* - * Decode starting at "NEXT", but allowing decodes from the start of the packet. - */ - slen = fr_dns_label_uncompressed_length(packet_ctx->packet, data, data_len, &next, packet_ctx->lb); - if (slen <= 0) { - FR_PROTO_TRACE("length failed at %zd - %s", -slen, fr_strerror()); - return slen; - } - - labels_len = next - data; /* decode only what we've found */ - } else { - /* - * Get the length of the entire set of labels, up - * to (and including) the final 0x00. - * - * If any of the labels point outside of this - * area, OR they are otherwise invalid, then that's an error. - */ - slen = fr_dns_labels_network_verify(packet_ctx->packet, data, data_len, data, packet_ctx->lb); - if (slen <= 0) { - FR_PROTO_TRACE("verify failed"); - return slen; - } - - labels_len = slen; - } - - /* - * Loop over the input buffer, decoding the labels one by - * one. - * - * @todo - put the labels into a child cursor, and then - * merge them only if it succeeds. That doesn't seem to - * work for some reason, and I don't have time to debug - * it right now. So... let's leave it. - */ - for (total = 0; total < labels_len; total += slen) { - vp = fr_pair_afrom_da(ctx, parent); - if (!vp) return PAIR_DECODE_OOM; - - /* - * Having verified the input above, this next - * function should never fail unless there's a - * bug in the code. - */ - slen = fr_dns_label_to_value_box(vp, &vp->data, data, labels_len, data + total, true, packet_ctx->lb); - if (slen <= 0) { - FR_PROTO_TRACE("Failed decoding label at %zd", slen); - talloc_free(vp); - return -1; - } - - fr_pair_append(out, vp); - } - - FR_PROTO_TRACE("decode_dns_labels - %zu", labels_len); - return labels_len; -} - - #define DNS_GET_OPTION_NUM(_x) fr_nbo_to_uint16(_x) #define DNS_GET_OPTION_LEN(_x) fr_nbo_to_uint16((_x) + 2) @@ -295,22 +212,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, FR_PROTO_TRACE("decode context changed %s -> %s",da->parent->name, da->name); if ((da->type == FR_TYPE_STRING) && !da->flags.extra && da->flags.subtype) { - slen = decode_dns_labels(ctx, out, da, data + 4, len, decode_ctx); - if (slen < 0) { - fr_dict_unknown_free(&da); - return slen; - } - - /* - * The DNS labels may only partially fill the - * option. If so, that's an error. Point to the - * byte which caused the error, accounting for - * the option header. - */ - if ((size_t) slen != len) { - fr_dict_unknown_free(&da); - return -(4 + slen); - } + slen = fr_pair_dns_labels_from_network(ctx, out, da, packet_ctx->packet, data + 4, len, packet_ctx->lb, true); } else if (da->flags.array) { slen = fr_pair_array_from_network(ctx, out, da, data + 4, len, decode_ctx, decode_value);