From: Alan T. DeKok Date: Mon, 18 Nov 2019 23:44:12 +0000 (-0500) Subject: add DNS label decoding to structs, and update the tests X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4611ed00a39903cdd2a1fcf859cff3b547b28a5c;p=thirdparty%2Ffreeradius-server.git add DNS label decoding to structs, and update the tests --- diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c index 988fb34331e..07a603a2b2e 100644 --- a/src/lib/util/struct.c +++ b/src/lib/util/struct.c @@ -62,7 +62,8 @@ VALUE_PAIR *fr_unknown_from_network(TALLOC_CTX *ctx, fr_dict_attr_t const *paren */ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len, - fr_dict_attr_t const **child_p) + fr_dict_attr_t const **child_p, + fr_decode_value_t decode_value, void *decoder_ctx) { unsigned int child_num; uint8_t const *p = data, *end = data + data_len; @@ -185,6 +186,23 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_cursor_t *cursor, if (!child_length) child_length = (end - p); + /* + * Magic values get the callback called. + * + * Note that if this is an *array* of DNS labels, + * the callback should deal with this. + */ + if (decode_value && !child->flags.extra && child->flags.subtype) { + ssize_t slen; + + slen = decode_value(ctx, &child_cursor, NULL, child, p, child_length, decoder_ctx); + if (slen < 0) return slen - (p - data); + + p += slen; /* not always the same as child->flags.length */ + child_num++; /* go to the next child */ + continue; + } + /* * We only allow a limited number of data types * inside of a struct. @@ -281,7 +299,8 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_cursor_t *cursor, } if (child->type == FR_TYPE_STRUCT) { - slen = fr_struct_from_network(ctx, &child_cursor, child, p, end - p, child_p); + slen = fr_struct_from_network(ctx, &child_cursor, child, p, end - p, child_p, + decode_value, decoder_ctx); if (slen < 0) goto unknown_child; p += slen; diff --git a/src/lib/util/struct.h b/src/lib/util/struct.h index 1c4b016b75c..99e57e15ad2 100644 --- a/src/lib/util/struct.h +++ b/src/lib/util/struct.h @@ -31,9 +31,14 @@ RCSIDH(struct_h, "$Id$") extern "C" { #endif +typedef ssize_t (*fr_decode_value_t)(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx); + ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_attr_t const *parent, uint8_t const *data, size_t data_len, - fr_dict_attr_t const **child) CC_HINT(nonnull(2,3,4)); + fr_dict_attr_t const **child, + fr_decode_value_t decode_value, void *decoder_ctx) CC_HINT(nonnull(2,3,4)); typedef ssize_t (*fr_encode_value_t)(uint8_t *out, size_t outlen, fr_dict_attr_t const **tlv_stack, unsigned int depth, fr_cursor_t *cursor, void *encoder_ctx); diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c index 79cfcfdd14e..d5bb07ab35b 100644 --- a/src/protocols/dhcpv6/decode.c +++ b/src/protocols/dhcpv6/decode.c @@ -39,8 +39,8 @@ #include "attrs.h" static ssize_t decode_option(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, - fr_dict_attr_t const *parent, - uint8_t const *data, size_t const data_len, void *decoder_ctx); + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx); static ssize_t decode_raw(TALLOC_CTX *ctx, fr_cursor_t *cursor, UNUSED fr_dict_t const *dict, fr_dict_attr_t const *parent, @@ -83,6 +83,35 @@ static ssize_t decode_raw(TALLOC_CTX *ctx, fr_cursor_t *cursor, UNUSED fr_dict_t return data_len; } +static ssize_t decode_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx); +static ssize_t decode_array(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx); +static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx); + +/** Handle arrays of DNS lavels for fr_struct_from_network() + * + */ +static ssize_t decode_value_trampoline(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t const data_len, void *decoder_ctx) +{ + if (parent->flags.array) return decode_array(ctx, cursor, dict, parent, data, data_len, decoder_ctx); + + /* + * @todo - we might need to limit this to only one DNS label. + */ + if (!parent->flags.extra && (parent->type == FR_TYPE_STRING) && parent->flags.subtype) { + return decode_dns_labels(ctx, cursor, dict, parent, data, data_len, decoder_ctx); + } + + return decode_value(ctx, cursor, dict, parent, data, data_len, decoder_ctx); +} + static ssize_t decode_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, fr_dict_attr_t const *parent, @@ -168,7 +197,8 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t cons break; case FR_TYPE_STRUCT: - rcode = fr_struct_from_network(ctx, cursor, parent, data, data_len, &tlv); + rcode = fr_struct_from_network(ctx, cursor, parent, data, data_len, &tlv, + decode_value_trampoline, decoder_ctx); if (rcode < 0) return rcode; if (tlv) { @@ -312,7 +342,7 @@ static ssize_t decode_dns_labels(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t * bug in the code. */ rcode = fr_dns_label_to_value_box(vp, &vp->data, data, data_len, data + total, true); - if (rcode < 0) { + if (rcode <= 0) { fr_pair_list_free(&vp); goto raw; } diff --git a/src/protocols/radius/decode.c b/src/protocols/radius/decode.c index eb9aeffc2e4..d4c426efd4a 100644 --- a/src/protocols/radius/decode.c +++ b/src/protocols/radius/decode.c @@ -974,6 +974,21 @@ create_attrs: return total; } +/** Wrapper called by fr_struct_from_network() + * + * Because extended attributes can continue across the current value. + * So that function needs to know both the value length, *and* the + * packet length. But when we're decoding values inside of a struct, + * we're not using extended attributes. + */ +static ssize_t decode_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dict_attr_t const *parent, + uint8_t const *data, size_t data_len, void *decoder_ctx) +{ + return fr_radius_decode_pair_value(ctx, cursor, dict, parent, data, data_len, data_len, decoder_ctx); +} + + /** Create any kind of VP from the attribute contents * * "length" is AT LEAST the length of this attribute, as we @@ -1351,7 +1366,8 @@ ssize_t fr_radius_decode_pair_value(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dic * attribute, OR it's already been grouped * into a contiguous memory buffer. */ - rcode = fr_struct_from_network(ctx, cursor, parent, p, attr_len, &child); + rcode = fr_struct_from_network(ctx, cursor, parent, p, attr_len, &child, + decode_value, decoder_ctx); if (rcode < 0) goto raw; /* diff --git a/src/tests/unit/protocols/dhcpv6/rfc4704.txt b/src/tests/unit/protocols/dhcpv6/rfc4704.txt index 464e2621d3b..72ab6496a4a 100644 --- a/src/tests/unit/protocols/dhcpv6/rfc4704.txt +++ b/src/tests/unit/protocols/dhcpv6/rfc4704.txt @@ -45,7 +45,7 @@ match 00 27 00 0b 01 09 74 61 70 69 6f 63 61 30 31 # fields like 'string encode=dns_label'. e.g: Client-FQDN-Domain-Name # decode-pair - -no match Client-FQDN-Flags = Server-Update, Client-FQDN-Domain-Name = "tapioca01" +match Client-FQDN-Flags = Server-Update, Client-FQDN-Domain-Name = "tapioca01" count match 6