From: James Jones Date: Thu, 21 Jan 2021 13:41:50 +0000 (-0600) Subject: Move internal decode to use dbuffs internally. (#3836) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f31ec2ddf25c9b4b626d0290c6bf9491ae5b055f;p=thirdparty%2Ffreeradius-server.git Move internal decode to use dbuffs internally. (#3836) --- diff --git a/src/protocols/internal/decode.c b/src/protocols/internal/decode.c index 549c378c39a..84fc2bd8ca6 100644 --- a/src/protocols/internal/decode.c +++ b/src/protocols/internal/decode.c @@ -34,21 +34,23 @@ #include #include #include +#include #include static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, - uint8_t const *start, uint8_t const *end, void *decoder_ctx); + fr_dbuff_t *dbuff, void *decoder_ctx); /** Decodes the value of an attribute, potentially producing a pair (added to the cursor) * */ -static ssize_t internal_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, - uint8_t const *start, uint8_t const *end, - bool tainted, UNUSED void *decoder_ctx) +static ssize_t internal_decode_pair_value_dbuff(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, + fr_dbuff_t *dbuff, + bool tainted, UNUSED void *decoder_ctx) { fr_pair_t *vp; ssize_t slen; + fr_dbuff_t work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff); vp = fr_pair_afrom_da(ctx, parent_da); if (!vp) return PAIR_DECODE_OOM; @@ -56,42 +58,42 @@ static ssize_t internal_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *head, /* * Zero length is fine here */ - slen = fr_value_box_from_network(vp, &vp->data, vp->da->type, vp->da, start, end - start, tainted); + slen = fr_value_box_from_network_dbuff(vp, &vp->data, vp->da->type, vp->da, + &work_dbuff, fr_dbuff_len(&work_dbuff), tainted); if (slen < 0) { talloc_free(vp); return slen; } fr_pair_add(head, vp); - return slen; + return fr_dbuff_set(dbuff, &work_dbuff); } /** Decode a TLV as a group type attribute * */ static ssize_t internal_decode_tlv(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, - uint8_t const *start, uint8_t const *end, void *decoder_ctx) + fr_dbuff_t *dbuff, void *decoder_ctx) { ssize_t slen; fr_pair_list_t children; fr_cursor_t cursor; - uint8_t const *p = start; + fr_dbuff_t work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff); - FR_PROTO_TRACE("Decoding TLV - %s (%zu bytes)", parent_da->name, end - start); + FR_PROTO_TRACE("Decoding TLV - %s (%zu bytes)", parent_da->name, fr_dbuff_len(&work_dbuff)); fr_pair_list_init(&children); /* * Decode all the children of this TLV */ - while (p < end) { - FR_PROTO_HEX_MARKER(start, end - start, p - start, "Decoding child"); + while (fr_dbuff_extend(&work_dbuff)) { + FR_PROTO_HEX_MARKER(fr_dbuff_start(&work_dbuff), fr_dbuff_len(&work_dbuff), + fr_dbuff_remaining(&work_dbuff), "Decoding child"); - slen = internal_decode_pair(ctx, &children, parent_da, p, end, decoder_ctx); + slen = internal_decode_pair(ctx, &children, parent_da, &work_dbuff, decoder_ctx); if (slen <= 0) return slen; - - p += slen; } /* @@ -116,18 +118,18 @@ static ssize_t internal_decode_tlv(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dic fr_pair_add(head, fr_cursor_head(&cursor)); } - return p - start; + return fr_dbuff_set(dbuff, &work_dbuff); } /** Decode a group * */ static ssize_t internal_decode_group(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, - uint8_t const *start, uint8_t const *end, void *decoder_ctx) + fr_dbuff_t *dbuff, void *decoder_ctx) { fr_pair_t *vp; ssize_t slen; - uint8_t const *p = start; + fr_dbuff_t work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff); FR_PROTO_TRACE("Decoding group - %s", parent_da->name); @@ -137,33 +139,32 @@ static ssize_t internal_decode_group(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_d /* * Decode all the children of this group */ - while (p < end) { - FR_PROTO_HEX_MARKER(start, end - start, p - start, "Decoding child"); + while (fr_dbuff_extend(&work_dbuff)) { + FR_PROTO_HEX_MARKER(fr_dbuff_current(dbuff), fr_dbuff_remaining(dbuff), fr_dbuff_used(&work_dbuff), + "Decoding child"); - slen = internal_decode_pair(vp, &vp->vp_group, parent_da, p, end, decoder_ctx); + slen = internal_decode_pair(vp, &vp->vp_group, parent_da, &work_dbuff, decoder_ctx); if (slen <= 0) { talloc_free(vp); return slen; } - - p += slen; } fr_pair_add(head, vp); - return p - start; + return fr_dbuff_set(dbuff, &work_dbuff); } static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_dict_attr_t const *parent_da, - uint8_t const *start, uint8_t const *end, void *decoder_ctx) + fr_dbuff_t *dbuff, void *decoder_ctx) { ssize_t slen = 0; fr_dict_attr_t const *da; - uint8_t type_field_size, len_field_size; - uint8_t const *len_field = NULL, *enc_field = NULL, *ext_field = NULL; + uint8_t enc_byte = 0, ext_byte = 0, type_field_size, len_field_size; + fr_dbuff_marker_t len_field, enc_field, ext_field; uint64_t type = 0; - size_t len = 0; + size_t len = 0, remaining, needed; bool tainted, extended, unknown = false; - uint8_t const *p = start; + fr_dbuff_t work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff); /* * The first byte of each attribute describes the encoding format. @@ -176,32 +177,36 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di * field (allows for future extensions). * * 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 + * 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 1kk * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * |tlen |llen |t|e| Type (min) | Length (min) | value... * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ * */ - if ((p + 3) > end) { + remaining = fr_dbuff_extend_lowat(NULL, &work_dbuff, 3); + if (remaining < 3) { fr_strerror_printf("%s: Insufficient data. Need %zu additional byte(s)", - __FUNCTION__, 3 - (end - p)); - return -(end - start); + __FUNCTION__, 3 - remaining); + return -fr_dbuff_len(&work_dbuff); } - enc_field = p; - type_field_size = ((enc_field[0] & FR_INTERNAL_MASK_TYPE) >> 5) + 1; /* bits 0-2 */ - len_field_size = ((enc_field[0] & FR_INTERNAL_MASK_LEN) >> 2) + 1; /* bits 3-5 */ + fr_dbuff_marker(&enc_field, &work_dbuff); + fr_dbuff_marker(&ext_field, &work_dbuff); /* Placed here to make static analysis happy */ + FR_DBUFF_OUT_RETURN(&enc_byte, &work_dbuff); + type_field_size = ((enc_byte & FR_INTERNAL_MASK_TYPE) >> 5) + 1; /* bits 0-2 */ + len_field_size = ((enc_byte & FR_INTERNAL_MASK_LEN) >> 2) + 1; /* bits 3-5 */ - tainted = (enc_field[0] & FR_INTERNAL_FLAG_TAINTED) != 0; /* bit 6 */ - extended = (enc_field[0] & FR_INTERNAL_FLAG_EXTENDED) != 0; /* bit 7 */ + tainted = (enc_byte & FR_INTERNAL_FLAG_TAINTED) != 0; /* bit 6 */ + extended = (enc_byte & FR_INTERNAL_FLAG_EXTENDED) != 0; /* bit 7 */ - p++; /* Processed first encoding byte */ + /* Processed first encoding byte */ - if ((p + (type_field_size + len_field_size + extended)) > end) { + needed = type_field_size + len_field_size + extended; + remaining = fr_dbuff_extend_lowat(NULL, &work_dbuff, needed); + if (remaining < needed) { fr_strerror_printf("%s: Encoding byte invalid, fields overrun input data. " "%zu byte(s) remaining, need %zu byte(s)", - __FUNCTION__, end - p, - (size_t)type_field_size + (size_t)len_field_size + (size_t)extended); + __FUNCTION__, remaining, needed); return 0; } @@ -224,31 +229,30 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di * */ if (extended) { - ext_field = p; - unknown = (ext_field[0] & 0x80) != 0; - if (ext_field[0] & 0x01) { + fr_dbuff_set(&ext_field, &work_dbuff); + FR_DBUFF_OUT_RETURN(&ext_byte, &work_dbuff); + unknown = (ext_byte & 0x80) != 0; + if (ext_byte & 0x01) { fr_strerror_printf("%s: Third extension byte not in use", __FUNCTION__); return PAIR_DECODE_FATAL_ERROR; } - p++; } - type = fr_net_to_uint64v(p, type_field_size); - p += type_field_size; + FR_DBUFF_OUT_UINT64V_RETURN(&type, &work_dbuff, type_field_size); /* * This is the length of the start *after* the flags and * type/length fields. */ - len_field = p; - len = fr_net_to_uint64v(len_field, len_field_size); - p += len_field_size; + fr_dbuff_marker(&len_field, &work_dbuff); + FR_DBUFF_OUT_UINT64V_RETURN(&len, &work_dbuff, len_field_size); - if ((p + len) > end) { + remaining = fr_dbuff_extend_lowat(NULL, &work_dbuff, len); + if (remaining < len) { fr_strerror_printf("%s: Length field value overruns input data. " "%zu byte(s) remaining, need %zu byte(s)", - __FUNCTION__, end - p, len); - return -(len_field - start); + __FUNCTION__, remaining, len); + return -(fr_dbuff_current(&len_field) - fr_dbuff_start(&work_dbuff)); } if (unknown || parent_da->flags.is_unknown) { @@ -263,6 +267,7 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di } } + FR_PROTO_TRACE("decode context changed %s -> %s", da->parent->name, da->name); switch (da->type) { @@ -274,7 +279,7 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di if (unlikely(unknown)) { fr_strerror_printf("%s: %s can't be marked as unknown", __FUNCTION__, fr_table_str_by_value(fr_value_box_type_table, da->type, "?Unknown?")); - p = ext_field; + fr_dbuff_set(&work_dbuff, &ext_field); goto error; } FALL_THROUGH; @@ -284,16 +289,16 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di bad_tainted: fr_strerror_printf("%s: %s can't be marked as tainted", __FUNCTION__, fr_table_str_by_value(fr_value_box_type_table, da->type, "?Unknown?")); - p = enc_field; + fr_dbuff_set(&work_dbuff, &enc_field); error: if (unknown) fr_dict_unknown_free(&da); - return fr_pair_decode_slen(slen, start, p); + return fr_pair_decode_slen(slen, fr_dbuff_start(&work_dbuff), fr_dbuff_current(&work_dbuff)); } FR_PROTO_TRACE("Decoding %s - %s", da->name, fr_table_str_by_value(fr_value_box_type_table, da->type, "?Unknown?")); - slen = internal_decode_pair(ctx, head, parent_da, p, p + len, decoder_ctx); + slen = internal_decode_pair(ctx, head, parent_da, &work_dbuff, decoder_ctx); if (slen <= 0) goto error; break; @@ -303,12 +308,12 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di case FR_TYPE_TLV: if (unlikely(tainted)) goto bad_tainted; - slen = internal_decode_tlv(ctx, head, da, p, p + len, decoder_ctx); + slen = internal_decode_tlv(ctx, head, da, &work_dbuff, decoder_ctx); if (slen <= 0) goto error; break; case FR_TYPE_GROUP: - slen = internal_decode_group(ctx, head, da, p, p + len, decoder_ctx); + slen = internal_decode_group(ctx, head, da, &work_dbuff, decoder_ctx); if (slen <= 0) goto error; break; @@ -317,11 +322,11 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di * It's ok for this function to return 0 * we can have zero length strings. */ - slen = internal_decode_pair_value(ctx, head, da, p, p + len, tainted, decoder_ctx); + slen = internal_decode_pair_value_dbuff(ctx, head, da, &work_dbuff, tainted, decoder_ctx); if (slen < 0) goto error; } - return (p - start) + slen; + return fr_dbuff_set(dbuff, &work_dbuff); } /** Create a single fr_pair_t and all its nesting @@ -329,20 +334,27 @@ static ssize_t internal_decode_pair(TALLOC_CTX *ctx, fr_pair_list_t *head, fr_di */ ssize_t fr_internal_decode_pair(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, uint8_t const *data, size_t data_len, void *decoder_ctx) +{ + return fr_internal_decode_pair_dbuff(ctx, cursor, dict, &FR_DBUFF_TMP(data, data_len), decoder_ctx); +} + +ssize_t fr_internal_decode_pair_dbuff(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dbuff_t *dbuff, void *decoder_ctx) { fr_pair_list_t list; fr_cursor_t tmp_cursor; ssize_t slen; + fr_dbuff_t work_dbuff = FR_DBUFF_NO_ADVANCE(dbuff); fr_pair_list_init(&list); - slen = internal_decode_pair(ctx, &list, fr_dict_root(dict), data, data + data_len, decoder_ctx); + slen = internal_decode_pair(ctx, &list, fr_dict_root(dict), &work_dbuff, decoder_ctx); if (slen <= 0) return slen; fr_cursor_init(&tmp_cursor, &list); fr_cursor_merge(cursor, &tmp_cursor); - return slen; + return fr_dbuff_set(dbuff, &work_dbuff); } /* diff --git a/src/protocols/internal/internal.h b/src/protocols/internal/internal.h index cf9a1861080..a762221deb2 100644 --- a/src/protocols/internal/internal.h +++ b/src/protocols/internal/internal.h @@ -46,3 +46,6 @@ ssize_t fr_internal_encode_pair(fr_dbuff_t *dbuff, fr_cursor_t *cursor, void *en ssize_t fr_internal_decode_pair(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, uint8_t const *data, size_t data_len, void *decoder_ctx); + +ssize_t fr_internal_decode_pair_dbuff(TALLOC_CTX *ctx, fr_cursor_t *cursor, fr_dict_t const *dict, + fr_dbuff_t *dbuff, void *decoder_ctx);