From: Arran Cudbard-Bell Date: Fri, 8 Oct 2021 21:23:23 +0000 (-0500) Subject: Add fr_pair_reinit_from_da X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ea4dc1b5ef2a8ec99426ea5ae9ab8172eccc8644;p=thirdparty%2Ffreeradius-server.git Add fr_pair_reinit_from_da This automatically converts existing values, and performs all the necessary checks and modifications to keep the fr_pair_list_t correct. --- diff --git a/src/lib/server/map.c b/src/lib/server/map.c index 09e58c129a2..1ed41cb3af0 100644 --- a/src/lib/server/map.c +++ b/src/lib/server/map.c @@ -1315,7 +1315,7 @@ int map_to_vp(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *request, map_t co * and operators */ for (; vp; vp = fr_dcursor_next(&from)) { - vp->da = tmpl_da(map->lhs); + fr_pair_reinit_from_da(&found, vp, tmpl_da(map->lhs)); vp->op = map->op; } fr_pair_list_append(out, &found); diff --git a/src/lib/server/snmp.c b/src/lib/server/snmp.c index 306aad8d844..a28684d4349 100644 --- a/src/lib/server/snmp.c +++ b/src/lib/server/snmp.c @@ -967,18 +967,12 @@ int fr_snmp_process(request_t *request) /* * Clear out any junk values */ - if (da->type == FR_TYPE_TLV) { - switch (vp->vp_type) { - case FR_TYPE_OCTETS: - case FR_TYPE_STRING: - talloc_free(vp->data.datum.ptr); - - FALL_THROUGH; - default: - memset(&vp->data, 0, sizeof(vp->data)); - } + if (da->type == FR_TYPE_TLV) fr_value_box_clear(&vp->data); + + if (fr_pair_reinit_from_da(&request->request_pairs, vp, da) < 0) { + RPWARN("Failed converting unknown attribute to known attribute"); + continue; } - vp->da = da; } for (vp = fr_dcursor_iter_by_ancestor_init(&request_cursor, &request->request_pairs, attr_snmp_root); diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c index 43d174927e9..2f3b05bc3b2 100644 --- a/src/lib/util/pair.c +++ b/src/lib/util/pair.c @@ -243,6 +243,38 @@ fr_pair_t *fr_pair_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) return vp; } +/** Re-initialise an attribute with a different da + * + * If the new da has a different type to the old da, we'll attempt to cast + * the current value in place. + */ +int fr_pair_reinit_from_da(fr_pair_list_t *list, fr_pair_t *vp, fr_dict_attr_t const *da) +{ + if (vp->da == da) return 0; + + if ((da->type != vp->da->type) && (fr_value_box_cast_in_place(vp, &vp->data, da->type, da) < 0)) return -1; + + /* + * Only frees unknown fr_dict_attr_t's + */ + fr_dict_unknown_free(&vp->da); + + /* + * Ensure we update the attr index in the parent + */ + if (list) { + fr_pair_remove(list, vp); + + vp->da = da; + + fr_pair_append(list, vp); + } else { + vp->da = da; + } + + return 0; +} + /** Create a new valuepair * * If attr and vendor match a dictionary entry then a VP with that #fr_dict_attr_t diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h index 58eb3cafd98..de653adca9e 100644 --- a/src/lib/util/pair.h +++ b/src/lib/util/pair.h @@ -90,10 +90,12 @@ static inline fr_dlist_head_t _CONST *fr_pair_list_order(fr_pair_list_t _CONST * * They also specify what behaviour should be used when the attribute is merged into a new list/tree. */ struct value_pair_s { - fr_dict_attr_t const *da; //!< Dictionary attribute defines the attribute + fr_dict_attr_t const * _CONST da; //!< Dictionary attribute defines the attribute //!< number, vendor and type of the pair. + ///< Note: This should not be modified outside + ///< of pair.c except via #fr_pair_reinit_from_da. - fr_dlist_t order_entry; //!< Entry to maintain relative order within a list + fr_dlist_t _CONST order_entry; //!< Entry to maintain relative order within a list ///< of pairs. This ensures pairs within the list ///< are encoded in the same order as they were ///< received or inserted. @@ -195,6 +197,9 @@ fr_pair_t *fr_pair_root_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) CC_H /** @hidecallergraph */ fr_pair_t *fr_pair_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) CC_HINT(warn_unused_result) CC_HINT(nonnull(2)); +int fr_pair_reinit_from_da(fr_pair_list_t *list, fr_pair_t *vp, fr_dict_attr_t const *da) + CC_HINT(nonnull(2, 3)); + fr_pair_t *fr_pair_afrom_child_num(TALLOC_CTX *ctx, fr_dict_attr_t const *parent, unsigned int attr) CC_HINT(warn_unused_result); fr_pair_t *fr_pair_copy(TALLOC_CTX *ctx, fr_pair_t const *vp) CC_HINT(warn_unused_result); diff --git a/src/lib/util/pair_legacy.c b/src/lib/util/pair_legacy.c index 60541f96449..a5da0901c99 100644 --- a/src/lib/util/pair_legacy.c +++ b/src/lib/util/pair_legacy.c @@ -119,10 +119,11 @@ static fr_pair_t *fr_pair_make_unknown(TALLOC_CTX *ctx, fr_dict_t const *dict, if ((fr_dict_unknown_afrom_oid_substr(vp, NULL, &n, fr_dict_root(dict), &sbuff, NULL) <= 0) || fr_sbuff_remaining(&sbuff)) { + error: talloc_free(vp); return NULL; } - vp->da = n; + if (fr_pair_reinit_from_da(NULL, vp, n) < 0) goto error; /* * No value, but ensure that we still set up vp->data properly. @@ -136,14 +137,10 @@ static fr_pair_t *fr_pair_make_unknown(TALLOC_CTX *ctx, fr_dict_t const *dict, */ fr_strerror_printf("Unknown attribute \"%s\" requires a hex " "string, not \"%s\"", attribute, value); - talloc_free(vp); - return NULL; + goto error; } - if (fr_pair_value_from_str(vp, value, -1, '"', false) < 0) { - talloc_free(vp); - return NULL; - } + if (fr_pair_value_from_str(vp, value, -1, '"', false) < 0) goto error; vp->op = (op == 0) ? T_OP_EQ : op; return vp; diff --git a/src/modules/rlm_detail/rlm_detail.c b/src/modules/rlm_detail/rlm_detail.c index baed4c485ba..96547f3b20d 100644 --- a/src/modules/rlm_detail/rlm_detail.c +++ b/src/modules/rlm_detail/rlm_detail.c @@ -289,18 +289,18 @@ static int detail_write(FILE *out, rlm_detail_t const *inst, request_t *request, switch (packet->socket.inet.src_ipaddr.af) { case AF_INET: - src_vp.da = attr_packet_src_ipv4_address; + fr_pair_reinit_from_da(NULL, &src_vp, attr_packet_src_ipv4_address); fr_value_box_shallow(&src_vp.data, &packet->socket.inet.src_ipaddr, true); - dst_vp.da = attr_packet_dst_ipv4_address; + fr_pair_reinit_from_da(NULL, &dst_vp, attr_packet_dst_ipv4_address); fr_value_box_shallow(&dst_vp.data, &packet->socket.inet.dst_ipaddr, true); break; case AF_INET6: - src_vp.da = attr_packet_src_ipv6_address; + fr_pair_reinit_from_da(NULL, &src_vp, attr_packet_src_ipv6_address); fr_value_box_shallow(&src_vp.data, &packet->socket.inet.src_ipaddr, true); - dst_vp.da = attr_packet_dst_ipv6_address; + fr_pair_reinit_from_da(NULL, &dst_vp, attr_packet_dst_ipv6_address); fr_value_box_shallow(&dst_vp.data, &packet->socket.inet.dst_ipaddr, true); break; @@ -311,10 +311,10 @@ static int detail_write(FILE *out, rlm_detail_t const *inst, request_t *request, detail_fr_pair_fprint(request, out, &src_vp); detail_fr_pair_fprint(request, out, &dst_vp); - src_vp.da = attr_packet_src_port; + fr_pair_reinit_from_da(NULL, &src_vp, attr_packet_src_port); fr_value_box_shallow(&src_vp.data, packet->socket.inet.src_port, true); - dst_vp.da = attr_packet_dst_port; + fr_pair_reinit_from_da(NULL, &dst_vp, attr_packet_dst_port); fr_value_box_shallow(&dst_vp.data, packet->socket.inet.dst_port, true); detail_fr_pair_fprint(request, out, &src_vp); diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c index 70eb8535255..b8ac86bb3d7 100644 --- a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c +++ b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c @@ -151,6 +151,7 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur SSL *ssl = decode_ctx; fr_dict_t const *dict_radius; fr_dict_attr_t const *attr_radius; + fr_dict_attr_t const *da; dict_radius = fr_dict_by_protocol_name("radius"); fr_assert(dict_radius != NULL); @@ -218,7 +219,11 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur goto error; } - MEM(vp->da = fr_dict_unknown_afrom_fields(vp, attr_vendor_specific, vendor, attr)); + MEM(da = fr_dict_unknown_afrom_fields(vp, attr_vendor_specific, vendor, attr)); + if (fr_pair_reinit_from_da(NULL, vp, da) < 0) { + talloc_free(vp); + goto error; + } goto do_value; } } else { @@ -228,14 +233,23 @@ static ssize_t eap_ttls_decode_pair(request_t *request, TALLOC_CTX *ctx, fr_dcur /* * Is the attribute known? */ - vp->da = fr_dict_attr_child_by_num(our_parent, attr); - if (!vp->da) { + da = fr_dict_attr_child_by_num(our_parent, attr); + if (da) { + if (fr_pair_reinit_from_da(NULL, vp, da) < 0) { + talloc_free(vp); + goto error; + } + } else { if (flags & FR_DIAMETER_AVP_FLAG_MANDATORY) { fr_strerror_printf("Mandatory bit set and no attribute %u defined for parent %s", attr, parent->name); talloc_free(vp); goto error; } - MEM(vp->da = fr_dict_unknown_attr_afrom_num(vp, parent, attr)); + MEM(da = fr_dict_unknown_attr_afrom_num(vp, parent, attr)); + if (fr_pair_reinit_from_da(NULL, vp, da) < 0) { + talloc_free(vp); + goto error; + } } do_value: diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index b14dda0a203..76c2f6e11c0 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -2771,13 +2771,14 @@ static unlang_action_t mod_enqueue(rlm_rcode_t *p_result, void **rctx_out, void talloc_set_destructor(r, _udp_result_free); #endif - MEM(u = talloc(treq, udp_request_t)); - *u = (udp_request_t){ - .code = request->packet->code, - .synchronous = inst->parent->synchronous, - .priority = request->async->priority, - .recv_time = request->async->recv_time - }; + /* + * Can't use compound literal - const issues. + */ + MEM(u = talloc_zero(treq, udp_request_t)); + u->code = request->packet->code; + u->synchronous = inst->parent->synchronous; + u->priority = request->async->priority; + u->recv_time = request->async->recv_time; fr_pair_list_init(&u->extra); r->rcode = RLM_MODULE_FAIL; diff --git a/src/protocols/dhcpv4/decode.c b/src/protocols/dhcpv4/decode.c index 1cdcf6376d9..8ae0514e518 100644 --- a/src/protocols/dhcpv4/decode.c +++ b/src/protocols/dhcpv4/decode.c @@ -109,7 +109,7 @@ static ssize_t decode_value_internal(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_di * caller to clean up the unknown da. */ if (da->flags.is_unknown) { - vp->da = fr_dict_unknown_attr_afrom_da(vp, da); + fr_pair_reinit_from_da(NULL, vp, fr_dict_unknown_attr_afrom_da(vp, da)); da = vp->da; }