From: Alan T. DeKok Date: Sun, 17 Apr 2022 05:22:10 +0000 (-0400) Subject: move string/octet length encoding/decode to value.c X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a98fb2bce87083a3d03880f75abb237070bcacd;p=thirdparty%2Ffreeradius-server.git move string/octet length encoding/decode to value.c The code already enforced some lengths (e.g. fixed-length fields, with either truncation or zero-fill). Since the dictionaries now support da_is_length_field() and flags.array, we might as well support that in the functions fr_value_box_to_network() and fr_value_box_from_network() doing this allows a whole set of additional optimizations in the rest of the code. encode/decode array becomes a simple loop around encode_value(), which means it's protocol agnostic. And the dhcpv4/dhcpv6 option_len functions aren't needed, and can just go away. --- diff --git a/src/lib/util/value.c b/src/lib/util/value.c index 988b7be3d59..35fca884527 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -1290,17 +1290,39 @@ ssize_t fr_value_box_to_network(fr_dbuff_t *dbuff, fr_value_box_t const *value) * Sometimes variable length *inside* the server * has maximum length on the wire. */ - if (value->enumv && value->enumv->flags.length) { - if (max < value->enumv->flags.length) { - FR_DBUFF_IN_MEMCPY_RETURN(&work_dbuff, (uint8_t const *)value->datum.ptr, max); - FR_DBUFF_MEMSET_RETURN(&work_dbuff, 0, value->enumv->flags.length - max); - return fr_dbuff_set(dbuff, &work_dbuff); - } + if (value->enumv) { + if (value->enumv->flags.length) { + /* + * The field is fixed size, and the data is smaller than that, We zero-pad the field. + */ + if (max < value->enumv->flags.length) { + FR_DBUFF_IN_MEMCPY_RETURN(&work_dbuff, (uint8_t const *)value->datum.ptr, max); + FR_DBUFF_MEMSET_RETURN(&work_dbuff, 0, value->enumv->flags.length - max); + return fr_dbuff_set(dbuff, &work_dbuff); + + } else if (max > value->enumv->flags.length) { + /* + * Truncate the input to the maximum allowed length. + */ + max = value->enumv->flags.length; + } - /* - * Truncate the input to the maximum allowed length. - */ - max = value->enumv->flags.length; + } else if (da_is_length_field(value->enumv)) { + /* + * Truncate the output to the max allowed for this field and encode the length. + */ + if (value->enumv->flags.subtype == FLAG_LENGTH_UINT8) { + if (max > 255) max = 255; + fr_dbuff_in(&work_dbuff, (uint8_t) max); + + } else if (value->enumv->flags.subtype == FLAG_LENGTH_UINT16) { + if (max > 65536) max = 65535; + fr_dbuff_in(&work_dbuff, (uint16_t) max); + + } else { + return -1; + } + } } FR_DBUFF_IN_MEMCPY_RETURN(&work_dbuff, (uint8_t const *)value->datum.ptr, max); @@ -1601,28 +1623,82 @@ ssize_t fr_value_box_from_network(TALLOC_CTX *ctx, min, len); return -(len); } + + /* + * For array entries, we only decode one value at a time. + */ if (len > max) { - fr_strerror_printf("Found trailing garbage parsing type \"%s\". " - "Expected length <= %zu bytes, got %zu bytes", - fr_type_to_str(type), + if (!enumv->flags.array) { + fr_strerror_printf("Found trailing garbage parsing type \"%s\". " + "Expected length <= %zu bytes, got %zu bytes", + fr_type_to_str(type), max, len); - return -(max); + return -(max); + } + + len = max; } - switch (type) { - case FR_TYPE_STRING: - if (fr_value_box_bstrndup_dbuff(ctx, dst, enumv, &work_dbuff, len, tainted) < 0) { - return FR_VALUE_BOX_NET_OOM; + /* + * String / octets are special. + */ + if (fr_type_is_variable_size(type)) { + size_t newlen = len; + size_t offset = 0; + + /* + * Decode fixed-width fields. + */ + if (enumv->flags.length) { + newlen = enumv->flags.length; + + } else if (da_is_length_field(enumv)) { + /* + * Or fields with a length prefix. + */ + if (enumv->flags.subtype == FLAG_LENGTH_UINT8) { + uint8_t num; + + FR_DBUFF_OUT_RETURN(&num, &work_dbuff); + newlen = num; + offset = 1; + + } else if (enumv->flags.subtype == FLAG_LENGTH_UINT16) { + uint16_t num; + + FR_DBUFF_OUT_RETURN(&num, &work_dbuff); + newlen = num; + offset = 2; + + } else { + return -1; + } } - return fr_dbuff_set(dbuff, &work_dbuff); - case FR_TYPE_OCTETS: - if (fr_value_box_memdup_dbuff(ctx, dst, enumv, &work_dbuff, len, tainted) < 0) { - return FR_VALUE_BOX_NET_OOM; + /* + * If we need more data than exists, that's an error. + * + * Otherwise, bound the decoding to the count we found. + */ + if (newlen > len) return -(newlen + offset); + len = newlen; + + switch (type) { + case FR_TYPE_STRING: + if (fr_value_box_bstrndup_dbuff(ctx, dst, enumv, &work_dbuff, len, tainted) < 0) { + return FR_VALUE_BOX_NET_OOM; + } + return fr_dbuff_set(dbuff, &work_dbuff); + + case FR_TYPE_OCTETS: + if (fr_value_box_memdup_dbuff(ctx, dst, enumv, &work_dbuff, len, tainted) < 0) { + return FR_VALUE_BOX_NET_OOM; + } + return fr_dbuff_set(dbuff, &work_dbuff); + + default: + return -1; } - return fr_dbuff_set(dbuff, &work_dbuff); - default: - break; } /* diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c index 04dee0b1507..be80f6a7694 100644 --- a/src/protocols/dhcpv4/decode.c +++ b/src/protocols/dhcpv4/decode.c @@ -43,7 +43,7 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t uint8_t const *data, size_t 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 data_len, void *decode_ctx, bool exact); + 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, @@ -67,18 +67,20 @@ static ssize_t decode_value_trampoline(TALLOC_CTX *ctx, fr_pair_list_t *out, if (parent->flags.array) return decode_array(ctx, out, parent, data, data_len, decode_ctx); - return decode_value(ctx, out, parent, data, data_len, decode_ctx, true); + return decode_value(ctx, out, parent, data, data_len, decode_ctx); } /* * Decode ONE value into a VP */ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t const *da, - uint8_t const *data, size_t data_len, void *decode_ctx, bool exact) + uint8_t const *data, size_t data_len, void *decode_ctx) { + ssize_t slen; fr_pair_t *vp; uint8_t const *p = data; uint8_t const *end = data + data_len; + bool exact = !da->flags.array; FR_PROTO_TRACE("%s called to parse %zu bytes", __FUNCTION__, data_len); FR_PROTO_HEX_DUMP(data, data_len, NULL); @@ -87,8 +89,6 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t * Structs create their own VP wrapper. */ if (da->type == FR_TYPE_STRUCT) { - ssize_t slen; - slen = fr_struct_from_network(ctx, out, da, data, data_len, true, decode_ctx, decode_value_trampoline, decode_tlv); if (slen < 0) return slen; @@ -179,7 +179,7 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t if (da_is_split_prefix(da)) { uint32_t ipaddr, mask; - if (data_len != 8) goto raw; + if (data_len < 8) goto raw; ipaddr = fr_nbo_to_uint32(p); mask = fr_nbo_to_uint32(p + 4); @@ -261,12 +261,9 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t FALL_THROUGH; default: - { - ssize_t ret; - - ret = fr_value_box_from_network(vp, &vp->data, vp->da->type, da, + slen = fr_value_box_from_network(vp, &vp->data, vp->da->type, da, &FR_DBUFF_TMP(p, end - p), end - p, true); - if (ret < 0) { + if (slen < 0) { raw: FR_PROTO_TRACE("decoding as unknown type"); if (fr_pair_to_unknown(vp) < 0) return -1; @@ -275,13 +272,13 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t break; } - if (exact && (ret != (end - p))) { + if (exact && (slen != (end - p))) { talloc_free(vp); goto raw; } - p += (size_t) ret; - } + p += (size_t) slen; + break; } finish: @@ -456,7 +453,6 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t { uint8_t const *p = data, *end = p + data_len; ssize_t slen; - size_t element_len; FR_PROTO_HEX_DUMP(data, data_len, "decode_array"); @@ -464,85 +460,17 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_attr_t "%s: Internal sanity check failed, attribute \"%s\" does not have array bit set", __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR; - /* - * Fixed-size fields get decoded with a simple decoder. - */ - element_len = fr_dhcpv4_attr_sizes[parent->type][0]; - if (!element_len) element_len = parent->flags.length; - - /* - * Fix up insane nonsense. - */ - if (da_is_split_prefix(parent)) element_len = 8; - - /* - * Array of structs. - */ - if (parent->type == FR_TYPE_STRUCT) { - while (p < end) { - slen = decode_value(ctx, out, parent, p, end - p, decode_ctx, false); - if (slen < 0) return slen - (p - data); - p += slen; - } - - return data_len; - } - - if (element_len > 0) { - size_t num_elements = (end - p) / element_len; - - FR_PROTO_TRACE("decode_array %zu input expected %zd total (%zu elements * %zu size)", - (size_t) (end - p), num_elements * element_len, num_elements, element_len); - - if ((num_elements * element_len) != (size_t) (end - p)) { - raw: - slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx); - if (slen < 0) return slen; - return data_len; - } - - while (p < end) { - /* - * Not enough room for one more element, - * decode the last bit as raw data. - */ - if ((size_t) (end - p) < element_len) goto raw; - - slen = decode_value(ctx, out, parent, p, element_len, decode_ctx, false); - if (slen < 0) return slen - (p - data); - if (!fr_cond_assert((size_t) slen == element_len)) return -(p - data); - - p += slen; - } - - /* - * We MUST have decoded the entire input. If - * not, we ignore the extra bits. - */ - return data_len; - } - - /* - * If the data is variable length i.e. strings or octets - * there is a length field before each element. - * - * Note that we don't bother checking if the data type is - * string or octets. There will only be issues if - * someone edited the dictionaries and broke them. - */ while (p < end) { - if ((end - p) < 1) goto raw; - - element_len = *p; - - if ((size_t) (end - p) < (((size_t) element_len) + 1)) goto raw; - - p += 1; - slen = decode_value(ctx, out, parent, p, element_len, decode_ctx, false); + slen = decode_value(ctx, out, parent, p, (end - p), decode_ctx); if (slen < 0) return slen - (p - data); + p += slen; } + /* + * We MUST have decoded the entire input. If + * not, we ignore the extra bits. + */ return data_len; } @@ -783,7 +711,7 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, slen = decode_tlv(ctx, out, da, data + 2, len, decode_ctx); } else { - slen = decode_value(ctx, out, da, data + 2, len, decode_ctx, true); + slen = decode_value(ctx, out, da, data + 2, len, decode_ctx); } if (slen < 0) { @@ -892,7 +820,7 @@ ssize_t fr_dhcpv4_decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, slen = decode_array(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx); } else { - slen = decode_value(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx, true); + slen = decode_value(ctx, out, da, packet_ctx->buffer, q - packet_ctx->buffer, packet_ctx); } if (slen <= 0) return slen; diff --git a/src/protocols/dhcpv4/encode.c b/src/protocols/dhcpv4/encode.c index 3bb6f2bc2aa..05d96b910b5 100644 --- a/src/protocols/dhcpv4/encode.c +++ b/src/protocols/dhcpv4/encode.c @@ -204,65 +204,18 @@ static ssize_t encode_array(fr_dbuff_t *dbuff, fr_pair_t *vp; fr_dict_attr_t const *da = da_stack->da[depth]; + FR_PROTO_STACK_PRINT(da_stack, depth); + if (!fr_cond_assert_msg(da->flags.array, "%s: Internal sanity check failed, attribute \"%s\" does not have array bit set", __FUNCTION__, da->name)) return PAIR_ENCODE_FATAL_ERROR; - /* - * DNS labels have internalized length, so we don't need - * length headers. - */ - if ((da->type == FR_TYPE_STRING) && da_is_dns_label(da)) { - while (fr_dbuff_extend(&work_dbuff)) { - vp = fr_dcursor_current(cursor); - - /* - * DNS labels get a special encoder. DNS labels are generally not compressed in - * DHCPv4. But sometimes are compressed. Based on whatever the RFC author - * decided was a good idea that day. - */ - slen = fr_dns_label_from_value_box_dbuff(&work_dbuff, false, &vp->data, NULL); - if (slen <= 0) return PAIR_ENCODE_FATAL_ERROR; - - vp = fr_dcursor_next(cursor); - if (!vp || (vp->da != da)) break; /* Stop if we have an attribute of a different type */ - } - - return fr_dbuff_set(dbuff, &work_dbuff); - } - while (fr_dbuff_extend(&work_dbuff)) { - bool len_field = false; fr_dbuff_t element_dbuff = FR_DBUFF(&work_dbuff); - /* - * If the data is variable length i.e. strings or octets - * we need to include an 8-bit length field before each element. - * - * The struct encoder adds it's own length field. - */ - if (da_is_length_field(da) && (da->type != FR_TYPE_STRUCT)) { - len_field = true; - FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint8_t)); /* Make room for the length field */ - } - slen = encode_value(&element_dbuff, da_stack, depth, cursor, encode_ctx); if (slen < 0) return slen; - if (slen > UINT8_MAX) return PAIR_ENCODE_FATAL_ERROR; - /* - * Ensure we always create elements of the correct length. - * This is mainly for fixed length octets type attributes - * containing one or more keys. - */ - if (da->flags.length && (size_t)slen < da->flags.length) { - FR_DBUFF_MEMSET_RETURN(&element_dbuff, 0, da->flags.length - slen); - } - - /* - * Populate the length field - */ - else if (len_field) fr_dbuff_in(&work_dbuff, (uint8_t) slen); fr_dbuff_set(&work_dbuff, &element_dbuff); vp = fr_dcursor_current(cursor); diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c index 5308bdd201f..9b096289174 100644 --- a/src/protocols/dhcpv6/decode.c +++ b/src/protocols/dhcpv6/decode.c @@ -138,17 +138,19 @@ 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 decode_raw(ctx, out, parent, data, data_len, decode_ctx); }; /* - * Structs used fixed length fields + * Structs used fixed length IPv6 addressews. */ if (parent->parent->type == FR_TYPE_STRUCT) { - if (data_len != (1 + sizeof(vp->vp_ipv6addr))) goto raw; + if (data_len != (1 + sizeof(vp->vp_ipv6addr))) { + goto raw; + } vp = fr_pair_afrom_da(ctx, parent); if (!vp) return PAIR_DECODE_OOM; @@ -156,7 +158,8 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, vp->vp_ip.af = AF_INET6; vp->vp_ip.scope_id = 0; vp->vp_ip.prefix = data[0]; - memcpy(&vp->vp_ipv6addr, data + 1, data_len - 1); + memcpy(&vp->vp_ipv6addr, data + 1, sizeof(vp->vp_ipv6addr)); + slen = 1 + sizeof(vp->vp_ipv6addr); break; } @@ -179,14 +182,19 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, * 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; + 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; vp->vp_ip.af = AF_INET6; vp->vp_ip.prefix = prefix_len; - memcpy(&vp->vp_ipv6addr, data + 1, data_len - 1); + memcpy(&vp->vp_ipv6addr, data + 1, slen); + + slen++; break; /* @@ -212,8 +220,9 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, vp = fr_pair_afrom_da(ctx, parent); if (!vp) return PAIR_DECODE_OOM; - if (fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da, - &FR_DBUFF_TMP(data, data_len), data_len, true) < 0) { + slen = fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da, + &FR_DBUFF_TMP(data, data_len), data_len, true); + if (slen < 0) { talloc_free(vp); goto raw; } @@ -223,7 +232,9 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, case FR_TYPE_STRUCT: slen = fr_struct_from_network(ctx, out, parent, data, data_len, false, decode_ctx, decode_value_trampoline, decode_tlv_trampoline); - if (slen < 0) return slen; + if (slen < 0) goto raw; + + if (parent->flags.array) return slen; return data_len; case FR_TYPE_GROUP: @@ -239,6 +250,24 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, * here. */ slen = decode_tlvs(vp, &vp->vp_group, fr_dict_root(dict_dhcpv6), data, data_len, decode_ctx); + if (slen < 0) { + talloc_free(vp); + return slen; + goto raw; + } + break; + + case FR_TYPE_IPV6_ADDR: + vp = fr_pair_afrom_da(ctx, parent); + if (!vp) return PAIR_DECODE_OOM; + + /* + * 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->da->type, vp->da, + &FR_DBUFF_TMP(data, sizeof(vp->vp_ipv6addr)), sizeof(vp->vp_ipv6addr), true); if (slen < 0) { talloc_free(vp); goto raw; @@ -249,16 +278,28 @@ static ssize_t decode_value(TALLOC_CTX *ctx, fr_pair_list_t *out, vp = fr_pair_afrom_da(ctx, parent); if (!vp) return PAIR_DECODE_OOM; - if (fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da, - &FR_DBUFF_TMP(data, data_len), data_len, true) < 0) { + slen = fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da, + &FR_DBUFF_TMP(data, data_len), data_len, true); + if (slen < 0) { talloc_free(vp); goto raw; } break; } + /* + * The input is larger than the decoded value, re-do it as a raw attribute. + */ + if (!parent->flags.array && ((size_t) slen < data_len)) { + talloc_free(vp); + goto raw; + } + vp->vp_tainted = true; fr_pair_append(out, vp); + + if (parent->flags.array) return slen; + return data_len; } @@ -269,7 +310,6 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out, { uint8_t const *p = data, *end = p + data_len; ssize_t slen; - size_t element_len; FR_PROTO_HEX_DUMP(data, data_len, "decode_array"); @@ -277,71 +317,17 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out, "%s: Internal sanity check failed, attribute \"%s\" does not have array bit set", __FUNCTION__, parent->name)) return PAIR_DECODE_FATAL_ERROR; - /* - * Fixed-size fields get decoded with a simple decoder. - */ - element_len = fr_dhcpv6_attr_sizes[parent->type][0]; - if (!element_len) element_len = parent->flags.length; - - if (element_len > 0) { - size_t num_elements = (end - p) / element_len; - - FR_PROTO_TRACE("decode_array %zu input expected %zd total (%zu elements * %zu size)", - (size_t) (end - p), num_elements * element_len, num_elements, element_len); - - if ((num_elements * element_len) != (size_t) (end - p)) { - raw: - slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx); - if (slen < 0) return slen; - return data_len; - } - - while (p < end) { - /* - * Not enough room for one more element, - * decode the last bit as raw data. - */ - if ((size_t) (end - p) < element_len) goto raw; - - slen = decode_value(ctx, out, parent, p, element_len, decode_ctx); - if (slen < 0) return slen; - if (!fr_cond_assert((size_t) slen == element_len)) return -(p - data); - - p += slen; - } - - /* - * We MUST have decoded the entire input. If - * not, we ignore the extra bits. - */ - return data_len; - } - - /* - * If the data is variable length i.e. strings or octets - * there is a length field before each element. - * - * Note that we don't bother checking if the data type is - * string or octets. There will only be issues if - * someone edited the dictionaries and broke them. - * - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+ - * | text-len | String | - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+ - */ while (p < end) { - if ((end - p) < 2) goto raw; - - element_len = fr_nbo_to_uint16(p); - - if ((size_t) (end - p) < (((size_t) element_len) + 2)) goto raw; - - p += 2; - slen = decode_value(ctx, out, parent, p, element_len, decode_ctx); + slen = decode_value(ctx, out, parent, p, (end - p), decode_ctx); if (slen < 0) return slen; + p += slen; } + /* + * We MUST have decoded the entire input. If + * not, we ignore the extra bits. + */ return data_len; } @@ -530,7 +516,6 @@ static ssize_t decode_option(TALLOC_CTX *ctx, fr_pair_list_t *out, option = DHCPV6_GET_OPTION_NUM(data); len = DHCPV6_GET_OPTION_LEN(data); if (len > (data_len - 4)) { - fprintf(stderr, "FAIL %d\n", __LINE__); fr_strerror_printf("%s: Option overflows input. " "Optional length must be less than %zu bytes, got %zu bytes", __FUNCTION__, data_len - 4, len); diff --git a/src/protocols/dhcpv6/encode.c b/src/protocols/dhcpv6/encode.c index f7acd815c35..31b3a0ac8c0 100644 --- a/src/protocols/dhcpv6/encode.c +++ b/src/protocols/dhcpv6/encode.c @@ -401,65 +401,12 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff, "%s: Internal sanity check failed, attribute \"%s\" does not have array bit set", __FUNCTION__, da->name)) return PAIR_ENCODE_FATAL_ERROR; - /* - * DNS labels have internalized length, so we don't need - * length headers. - */ - if ((da->type == FR_TYPE_STRING) && !da->flags.extra && da->flags.subtype){ - while (fr_dbuff_extend(&work_dbuff)) { - vp = fr_dcursor_current(cursor); - - /* - * DNS labels get a special encoder. DNS labels - * MUST NOT be compressed in DHCP. - * - * https://tools.ietf.org/html/rfc8415#section-10 - */ - slen = fr_dns_label_from_value_box_dbuff(&work_dbuff, false, &vp->data, NULL); - if (slen <= 0) return PAIR_ENCODE_FATAL_ERROR; - - vp = fr_dcursor_next(cursor); - if (!vp || (vp->da != da)) break; /* Stop if we have an attribute of a different type */ - } - - return fr_dbuff_set(dbuff, &work_dbuff); - } - while (fr_dbuff_extend(&work_dbuff)) { - bool len_field = false; fr_dbuff_t element_dbuff = FR_DBUFF(&work_dbuff); - /* - * If the data is variable length i.e. strings or octets - * we need to include a length field before each element. - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+ - * | text-len | String | - * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+ - * - * The struct encoder already adds the length field. - */ - if (da_is_length_field(da) && (da->type != FR_TYPE_STRUCT)) { - len_field = true; - FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint16_t)); /* Make room for the length field */ - } - slen = encode_value(&element_dbuff, da_stack, depth, cursor, encode_ctx); if (slen < 0) return slen; - if (slen > UINT16_MAX) return PAIR_ENCODE_FATAL_ERROR; - /* - * Ensure we always create elements of the correct length. - * This is mainly for fixed length octets type attributes - * containing one or more keys. - */ - if (da->flags.length && (size_t)slen < da->flags.length) { - FR_DBUFF_MEMSET_RETURN(&element_dbuff, 0, da->flags.length - slen); - } - - /* - * Populate the length field - */ - else if (len_field) fr_dbuff_in(&work_dbuff, (uint16_t) slen); fr_dbuff_set(&work_dbuff, &element_dbuff); vp = fr_dcursor_current(cursor); diff --git a/src/tests/unit/protocols/dhcpv4/error.txt b/src/tests/unit/protocols/dhcpv4/error.txt index 5a8a186dba2..c1186602cb8 100644 --- a/src/tests/unit/protocols/dhcpv4/error.txt +++ b/src/tests/unit/protocols/dhcpv4/error.txt @@ -39,7 +39,7 @@ match 03 08 7f 00 00 01 7f 00 00 02 # But if we don't have enough data, the whole thing is corrupt. # decode-pair 03 07 7f 00 00 01 7f 00 00 -match raw.Router-Address = 0x7f0000017f0000 +match Router-Address = 127.0.0.1, raw.Router-Address = 0x7f0000 count match 21 diff --git a/src/tests/unit/protocols/dhcpv4/rfc3004.txt b/src/tests/unit/protocols/dhcpv4/rfc3004.txt index 1214adae0bc..8b0274c0e5a 100644 --- a/src/tests/unit/protocols/dhcpv4/rfc3004.txt +++ b/src/tests/unit/protocols/dhcpv4/rfc3004.txt @@ -24,10 +24,9 @@ match 4d ff ff 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 decode-pair - match User-Class = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxy" -# too long results in "not encoded". -# @todo - do we want to just truncate? +# too long results just get truncated encode-pair User-Class = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx123" -match +match 4d ff ff 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 78 4d 01 78 # Many longer ones encode-pair User-Class = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", User-Class = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", User-Class = "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"