From b4b9c0ca4c88dbddc700ce7faa8cc049f5d60bbc Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Sun, 1 Jun 2025 11:36:02 -0600 Subject: [PATCH] Audit more uses of unlang_function_push --- src/modules/rlm_ldap/groups.c | 53 +++++++++++++++++++++++---------- src/modules/rlm_ldap/profile.c | 8 +++-- src/modules/rlm_ldap/rlm_ldap.c | 10 +++++-- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/src/modules/rlm_ldap/groups.c b/src/modules/rlm_ldap/groups.c index 79ae5f9aa8..f7ccfefdf0 100644 --- a/src/modules/rlm_ldap/groups.c +++ b/src/modules/rlm_ldap/groups.c @@ -396,7 +396,7 @@ static unlang_action_t ldap_cacheable_userobj_resolve(unlang_result_t *p_result, */ if (*group_ctx->dn) { if (unlang_function_repeat_set(request, ldap_cacheable_userobj_resolve) < 0) RETURN_UNLANG_FAIL; - if (unlang_function_push(p_result, request, + if (unlang_function_push(/* both start and resume provide an rcode */p_result, request, ldap_group_dn2name_start, ldap_group_dn2name_resume, ldap_group_userobj_cancel, ~FR_SIGNAL_CANCEL, @@ -409,7 +409,7 @@ static unlang_action_t ldap_cacheable_userobj_resolve(unlang_result_t *p_result, */ if (*group_ctx->group_name) { if (unlang_function_repeat_set(request, ldap_cacheable_userobj_resolve) < 0) RETURN_UNLANG_FAIL; - if (unlang_function_push(p_result, request, + if (unlang_function_push(/* both start and resume provide an rcode */p_result, request, ldap_group_name2dn_start, ldap_group_name2dn_resume, ldap_group_userobj_cancel, ~FR_SIGNAL_CANCEL, @@ -544,10 +544,12 @@ unlang_action_t rlm_ldap_cacheable_userobj(unlang_result_t *p_result, request_t */ if ((name_p != group_ctx->group_name) || (dn_p != group_ctx->group_dn)) { group_ctx->attrs[0] = inst->group.obj_name_attr; - if (unlang_function_push(NULL, request, + if (unlang_function_push(p_result, request, ldap_cacheable_userobj_resolve, NULL, - ldap_group_userobj_cancel, ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx) < 0) { + ldap_group_userobj_cancel, + ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, + group_ctx) < 0) { talloc_free(group_ctx); RETURN_UNLANG_FAIL; } @@ -712,8 +714,13 @@ unlang_action_t rlm_ldap_cacheable_groupobj(unlang_result_t *p_result, request_t group_ctx->base_dn = &autz_ctx->call_env->group_base; fr_value_box_list_init(&group_ctx->expanded_filter); - if (unlang_function_push(NULL, request, ldap_cacheable_groupobj_start, ldap_cacheable_groupobj_resume, - ldap_group_groupobj_cancel, ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx) < 0) { + if (unlang_function_push(p_result, + request, + ldap_cacheable_groupobj_start, + ldap_cacheable_groupobj_resume, + ldap_group_groupobj_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, + group_ctx) < 0) { error: talloc_free(group_ctx); RETURN_UNLANG_FAIL; @@ -834,9 +841,13 @@ unlang_action_t rlm_ldap_check_groupobj_dynamic(unlang_result_t *p_result, reque group_ctx->base_dn = &xlat_ctx->env_data->group_base; } - if (unlang_function_push(NULL, request, ldap_cacheable_groupobj_start, ldap_check_groupobj_resume, + if (unlang_function_push(p_result, + request, + ldap_cacheable_groupobj_start, + ldap_check_groupobj_resume, ldap_group_groupobj_cancel, ~FR_SIGNAL_CANCEL, - UNLANG_SUB_FRAME, group_ctx) < 0) { + UNLANG_SUB_FRAME, + group_ctx) < 0) { error: talloc_free(group_ctx); RETURN_UNLANG_FAIL; @@ -902,7 +913,7 @@ static unlang_action_t ldap_check_userobj_resume(UNUSED unlang_result_t *p_resul request_t *request, void *uctx) { ldap_group_userobj_dyn_ctx_t *group_ctx = talloc_get_type_abort(uctx, ldap_group_userobj_dyn_ctx_t); - ldap_group_xlat_ctx_t *xlat_ctx = talloc_get_type_abort(group_ctx->xlat_ctx, ldap_group_xlat_ctx_t); + ldap_group_xlat_ctx_t *xlat_ctx = talloc_get_type_abort(group_ctx->xlat_ctx, ldap_group_xlat_ctx_t); rlm_ldap_t const *inst = xlat_ctx->inst; fr_ldap_query_t *query = xlat_ctx->query; LDAPMessage *entry; @@ -1056,8 +1067,12 @@ static unlang_action_t ldap_check_userobj_resume(UNUSED unlang_result_t *p_resul if (unlang_function_repeat_set(request, ldap_check_userobj_resume) < 0) RETURN_UNLANG_FAIL; - return unlang_function_push(NULL, request, ldap_dn2name_start, NULL, ldap_dn2name_cancel, - ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx); + return unlang_function_push(/* discard, ldap_check_userobj_resume looks at the query result */ NULL, + request, + ldap_dn2name_start, + NULL, + ldap_dn2name_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, group_ctx); } if (((talloc_array_length(group_ctx->group_name) - 1) == value->bv_len) && @@ -1081,8 +1096,12 @@ static unlang_action_t ldap_check_userobj_resume(UNUSED unlang_result_t *p_resul if (unlang_function_repeat_set(request, ldap_check_userobj_resume) < 0) RETURN_UNLANG_FAIL; - return unlang_function_push(NULL, request, ldap_dn2name_start, NULL, ldap_dn2name_cancel, - ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx); + return unlang_function_push(/* discard, ldap_check_userobj_resume gets result from group_ctx */ NULL, + request, + ldap_dn2name_start, + NULL, + ldap_dn2name_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, group_ctx); } fr_assert(0); @@ -1131,8 +1150,12 @@ unlang_action_t rlm_ldap_check_userobj_dynamic(unlang_result_t *p_result, reques * can be checked. * If not then a query is needed to retrieve the user object. */ - if (unlang_function_push(NULL, request, xlat_ctx->query ? NULL : ldap_check_userobj_start, ldap_check_userobj_resume, - ldap_group_userobj_cancel, ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, group_ctx) < 0) { + if (unlang_function_push(/* ldap_check_userobj_resume provides an rcode result */p_result, + request, + xlat_ctx->query ? NULL : ldap_check_userobj_start, + ldap_check_userobj_resume, + ldap_group_userobj_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, group_ctx) < 0) { talloc_free(group_ctx); RETURN_UNLANG_FAIL; } diff --git a/src/modules/rlm_ldap/profile.c b/src/modules/rlm_ldap/profile.c index b50ff72345..558565540c 100644 --- a/src/modules/rlm_ldap/profile.c +++ b/src/modules/rlm_ldap/profile.c @@ -228,8 +228,12 @@ unlang_action_t rlm_ldap_map_profile(fr_ldap_result_code_t *ret, int *applied, }; if (ret) *ret = LDAP_RESULT_ERROR; - if (unlang_function_push(NULL, request, NULL, ldap_map_profile_resume, ldap_map_profile_cancel, - ~FR_SIGNAL_CANCEL, UNLANG_SUB_FRAME, profile_ctx) < 0) { + if (unlang_function_push(/* discard, ldap_map_profile_resume doesn't appear to return an rcode */ NULL, + request, + NULL, + ldap_map_profile_resume, + ldap_map_profile_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, profile_ctx) < 0) { talloc_free(profile_ctx); return UNLANG_ACTION_FAIL; } diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c index 715e55eddf..dfe26f08ee 100644 --- a/src/modules/rlm_ldap/rlm_ldap.c +++ b/src/modules/rlm_ldap/rlm_ldap.c @@ -1118,9 +1118,13 @@ static xlat_action_t ldap_group_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_ct if (unlang_xlat_yield(request, ldap_group_xlat_resume, NULL, 0, xlat_ctx) != XLAT_ACTION_YIELD) goto error; - if (unlang_function_push(NULL, request, xlat_ctx->dn ? NULL : ldap_group_xlat_user_find, - ldap_group_xlat_results, ldap_group_xlat_cancel, ~FR_SIGNAL_CANCEL, - UNLANG_SUB_FRAME, xlat_ctx) < 0) goto error; + if (unlang_function_push(/* discard, ldap_group_xlat_resume just looks at xlat_ctx to see if things succeeded */ NULL, + request, + xlat_ctx->dn ? NULL : ldap_group_xlat_user_find, + ldap_group_xlat_results, + ldap_group_xlat_cancel, ~FR_SIGNAL_CANCEL, + UNLANG_SUB_FRAME, + xlat_ctx) < 0) goto error; return XLAT_ACTION_PUSH_UNLANG; } -- 2.47.3