From: Arran Cudbard-Bell Date: Wed, 15 Sep 2021 14:58:24 +0000 (-0500) Subject: Add back env_escape to rlm_exec, and add env_inherit X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a6a033b2f05ca83cc8eec2b3df8803ede4115c60;p=thirdparty%2Ffreeradius-server.git Add back env_escape to rlm_exec, and add env_inherit --- diff --git a/raddb/mods-available/exec b/raddb/mods-available/exec index bbbce9d8a4f..27c7bce036d 100644 --- a/raddb/mods-available/exec +++ b/raddb/mods-available/exec @@ -116,8 +116,19 @@ exec { # # User-Name=BobUser => USER_NAME="BobUser" # + # Note that this escaping only applies to environmental variables + # created from the request list. For environmental variables inherited + # from the main radiusd process no escaping is applied. + # shell_escape = yes + # + # env_inherit:: Inherit the environment of the current radiusd process. + # + # Any `input_pairs` will be merged with these environmental variables. + # +# env_inherit = no + # # timeout:: Set a time wait for the program to finish. # diff --git a/src/lib/server/exec.c b/src/lib/server/exec.c index 997c6dd118d..bcd64aeac91 100644 --- a/src/lib/server/exec.c +++ b/src/lib/server/exec.c @@ -111,16 +111,18 @@ static int exec_value_box_list_to_argv(TALLOC_CTX *ctx, char ***argv_p, fr_value * a '\0'. * @param[in] request Will look for &control.Exec-Export items to convert to * env vars. - * @param[in] input_pairs Other items to convert to environmental variables. + * @param[in] env_pairs Other items to convert to environmental variables. * The dictionary attribute name will be converted to * uppercase, and all '-' converted to '_' and will form * the variable name. + * @param[in] env_escape Wrap string values in double quotes, and apply doublequote + * escaping to all environmental variable values. * @return * - The number of environmental variables created. * - -1 on failure. */ static int exec_pair_to_env(char **env_p, size_t env_len, fr_sbuff_t *env_sbuff, - request_t *request, fr_pair_list_t *input_pairs) + request_t *request, fr_pair_list_t *env_pairs, bool env_escape) { char *p; size_t i, j; @@ -135,9 +137,9 @@ static int exec_pair_to_env(char **env_p, size_t env_len, fr_sbuff_t *env_sbuff, * hold mutexes. They might be locked when we fork, * and will remain locked in the child. */ - for (vp = fr_pair_list_head(input_pairs), i = 0; + for (vp = fr_pair_list_head(env_pairs), i = 0; vp && (i < env_len - 1); - vp = fr_pair_list_next(input_pairs, vp), i++) { + vp = fr_pair_list_next(env_pairs, vp), i++) { fr_sbuff_marker(&env_m[i], &sbuff); if (fr_sbuff_in_strcpy(&sbuff, vp->da->name) <= 0) { @@ -165,29 +167,35 @@ static int exec_pair_to_env(char **env_p, size_t env_len, fr_sbuff_t *env_sbuff, return -1; } - /* - * This can be zero length for empty strings - * - * Note we don't do double quote escaping here, - * we just escape unprintable chars. - * - * Environmental variable values are not - * restricted we likely wouldn't need to do - * any escaping if we weren't dealing with C - * strings. - * - * If we end up passing binary data through - * then the user can unescape the octal - * sequences on the other side. - * - * We unfortunately still need to escape '\' - * because of this. - */ - if (fr_value_box_print(&sbuff, &vp->data, &fr_value_escape_unprintables) < 0) { - RPEDEBUG("Out of buffer space adding attribute value for %pV", &vp->data); - return -1; + if (env_escape) { + if (fr_value_box_print_quoted(&sbuff, &vp->data, T_DOUBLE_QUOTED_STRING) < 0) { + RPEDEBUG("Out of buffer space adding attribute value for %pV", &vp->data); + return -1; + } + } else { + /* + * This can be zero length for empty strings + * + * Note we don't do double quote escaping here, + * we just escape unprintable chars. + * + * Environmental variable values are not + * restricted we likely wouldn't need to do + * any escaping if we weren't dealing with C + * strings. + * + * If we end up passing binary data through + * then the user can unescape the octal + * sequences on the other side. + * + * We unfortunately still need to escape '\' + * because of this. + */ + if (fr_value_box_print(&sbuff, &vp->data, &fr_value_escape_unprintables) < 0) { + RPEDEBUG("Out of buffer space adding attribute value for %pV", &vp->data); + return -1; + } } - if (fr_sbuff_in_char(&sbuff, '\0') <= 0) { REDEBUG("Out of buffer space"); return -1; @@ -336,6 +344,8 @@ static NEVER_RETURNS void exec_child(request_t *request, char **argv, char **env * @param[in] request the request * @param[in] args as returned by xlat_frame_eval() * @param[in] env_pairs env_pairs to put into into the environment. May be NULL. + * @param[in] env_escape Wrap string values in double quotes, and apply doublequote + * escaping to all environmental variable values. * @param[in] env_inherit Inherit the environment from the current process. * This will be merged with any variables from env_pairs. * @return @@ -347,7 +357,7 @@ static NEVER_RETURNS void exec_child(request_t *request, char **argv, char **env * the environment. */ int fr_exec_fork_nowait(request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit) + fr_pair_list_t *env_pairs, bool env_escape, bool env_inherit) { int argc; @@ -381,7 +391,7 @@ int fr_exec_fork_nowait(request_t *request, fr_value_box_list_t *args, if (exec_pair_to_env(env_p, env_end - env_p, &FR_SBUFF_OUT(env_buff, sizeof(env_buff)), - request, env_pairs) < 0) return -1; + request, env_pairs, env_escape) < 0) return -1; env = env_arr; } else if (env_inherit) { @@ -452,6 +462,8 @@ int fr_exec_fork_nowait(request_t *request, fr_value_box_list_t *args, * @param[in] request the request * @param[in] args as returned by xlat_frame_eval() * @param[in] env_pairs env_pairs to put into into the environment. May be NULL. + * @param[in] env_escape Wrap string values in double quotes, and apply doublequote + * escaping to all environmental variable values. * @param[in] env_inherit Inherit the environment from the current process. * This will be merged with any variables from env_pairs. * @return @@ -464,7 +476,7 @@ int fr_exec_fork_nowait(request_t *request, fr_value_box_list_t *args, */ int fr_exec_fork_wait(pid_t *pid_p, int *stdin_fd, int *stdout_fd, int *stderr_fd, request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit) + fr_pair_list_t *env_pairs, bool env_escape, bool env_inherit) { int argc; char **env; @@ -500,7 +512,7 @@ int fr_exec_fork_wait(pid_t *pid_p, int *stdin_fd, int *stdout_fd, int *stderr_f if (exec_pair_to_env(env_p, env_end - env_p, &FR_SBUFF_OUT(env_buff, sizeof(env_buff)), - request, env_pairs) < 0) return -1; + request, env_pairs, env_escape) < 0) return -1; env = env_arr; } else if (env_inherit) { env = environ; /* Our current environment */ @@ -899,6 +911,8 @@ static void exec_stdout_read(UNUSED fr_event_list_t *el, int fd, int flags, void * be the first box and arguments in the subsequent boxes. * @param[in] env_pairs list of pairs to be presented as evironment variables * to the child. + * @param[in] env_escape Wrap string values in double quotes, and apply doublequote + * escaping to all environmental variable values. * @param[in] env_inherit Inherit the environment from the current process. * This will be merged with any variables from env_pairs. * @param[in] need_stdin If true, allocate a pipe that will allow us to send data to the @@ -913,7 +927,7 @@ static void exec_stdout_read(UNUSED fr_event_list_t *el, int fd, int flags, void */ int fr_exec_start(TALLOC_CTX *ctx, fr_exec_state_t *exec, request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit, + fr_pair_list_t *env_pairs, bool env_escape, bool env_inherit, bool need_stdin, bool store_stdout, TALLOC_CTX *stdout_ctx, fr_time_delta_t timeout) @@ -935,7 +949,8 @@ int fr_exec_start(TALLOC_CTX *ctx, fr_exec_state_t *exec, request_t *request, }; if (fr_exec_fork_wait(&exec->pid, exec->stdin_used ? &exec->stdin_fd : NULL, - stdout_fd, &exec->stderr_fd, request, args, exec->env_pairs, env_inherit) < 0) { + stdout_fd, &exec->stderr_fd, request, args, + exec->env_pairs, env_escape, env_inherit) < 0) { RPEDEBUG("Failed executing program"); fail: /* diff --git a/src/lib/server/exec.h b/src/lib/server/exec.h index f3c88869a76..e22c2152299 100644 --- a/src/lib/server/exec.h +++ b/src/lib/server/exec.h @@ -82,15 +82,15 @@ typedef struct { void fr_exec_cleanup(fr_exec_state_t *exec, int signal); int fr_exec_fork_nowait(request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit); + fr_pair_list_t *env_pairs, bool env_escape, bool env_inherit); int fr_exec_fork_wait(pid_t *pid_p, int *stdin_fd, int *stdout_fd, int *stderr_fd, request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit); + fr_pair_list_t *env_pairs, bool env_escape, bool env_inherit); int fr_exec_start(TALLOC_CTX *ctx, fr_exec_state_t *exec, request_t *request, fr_value_box_list_t *args, - fr_pair_list_t *env_pairs, bool env_inherit, + fr_pair_list_t *env_pairs, bool env_inherit, bool env_escape, bool need_stdin, bool store_stdout, TALLOC_CTX *stdout_ctx, fr_time_delta_t timeout); diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c index bac070e9aff..a785407012c 100644 --- a/src/lib/server/trigger.c +++ b/src/lib/server/trigger.c @@ -223,7 +223,7 @@ static unlang_action_t trigger_resume(rlm_rcode_t *p_result, UNUSED int *priorit */ if (fr_exec_start(request, &trigger->exec, request, &trigger->args, - NULL, true, + NULL, false, true, false, false, NULL, trigger->timeout) < 0) { fail: diff --git a/src/lib/unlang/tmpl.c b/src/lib/unlang/tmpl.c index 5061680fac7..0d0d8c3ebdd 100644 --- a/src/lib/unlang/tmpl.c +++ b/src/lib/unlang/tmpl.c @@ -168,7 +168,7 @@ static unlang_action_t unlang_tmpl_exec_wait_resume(rlm_rcode_t *p_result, reque if (fr_exec_start(state->ctx, &state->exec, request, &state->box, - state->args.exec.env, false, + state->args.exec.env, false, false, false, (state->out != NULL), state, state->args.exec.timeout) < 0) { @@ -190,7 +190,7 @@ static unlang_action_t unlang_tmpl_exec_nowait_resume(rlm_rcode_t *p_result, req { unlang_frame_state_tmpl_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_tmpl_t); - if (fr_exec_fork_nowait(request, &state->box, state->exec.env_pairs, false) < 0) { + if (fr_exec_fork_nowait(request, &state->box, state->exec.env_pairs, false, false) < 0) { RPEDEBUG("Failed executing program"); *p_result = RLM_MODULE_FAIL; diff --git a/src/modules/rlm_exec/rlm_exec.c b/src/modules/rlm_exec/rlm_exec.c index d820d461750..703643699de 100644 --- a/src/modules/rlm_exec/rlm_exec.c +++ b/src/modules/rlm_exec/rlm_exec.c @@ -44,6 +44,7 @@ typedef struct { tmpl_pair_list_t input_list; tmpl_pair_list_t output_list; bool shell_escape; + bool env_inherit; fr_time_delta_t timeout; bool timeout_is_set; @@ -56,6 +57,7 @@ static const CONF_PARSER module_config[] = { { FR_CONF_OFFSET("input_pairs", FR_TYPE_STRING, rlm_exec_t, input) }, { FR_CONF_OFFSET("output_pairs", FR_TYPE_STRING, rlm_exec_t, output) }, { FR_CONF_OFFSET("shell_escape", FR_TYPE_BOOL, rlm_exec_t, shell_escape), .dflt = "yes" }, + { FR_CONF_OFFSET("env_inherit", FR_TYPE_BOOL, rlm_exec_t, env_inherit), .dflt = "no" }, { FR_CONF_OFFSET_IS_SET("timeout", FR_TYPE_TIME_DELTA, rlm_exec_t, timeout) }, CONF_PARSER_TERMINATOR }; @@ -114,15 +116,10 @@ static xlat_action_t exec_xlat(TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, reques void const *xlat_inst, UNUSED void *xlat_thread_inst, fr_value_box_list_t *in) { - rlm_exec_t const *inst; - void *instance; + rlm_exec_t const *inst = talloc_get_type_abort_const(*UNCONST(void **, xlat_inst), rlm_exec_t); fr_pair_list_t *env_pairs = NULL; fr_exec_state_t *exec; - memcpy(&instance, xlat_inst, sizeof(instance)); - - inst = talloc_get_type_abort(instance, rlm_exec_t); - if (inst->input_list) { env_pairs = tmpl_list_head(request, inst->input_list); if (!env_pairs) { @@ -133,7 +130,7 @@ static xlat_action_t exec_xlat(TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, reques if (!inst->wait) { /* Not waiting for the response */ - fr_exec_fork_nowait(request, in, env_pairs, false); + fr_exec_fork_nowait(request, in, env_pairs, inst->shell_escape, false); return XLAT_ACTION_DONE; } @@ -141,7 +138,7 @@ static xlat_action_t exec_xlat(TALLOC_CTX *ctx, UNUSED fr_dcursor_t *out, reques if (fr_exec_start(exec, exec, request, in, - env_pairs, false, + env_pairs, inst->shell_escape, inst->env_inherit, false, inst->wait, ctx, inst->timeout) < 0) { @@ -299,7 +296,7 @@ static unlang_action_t mod_exec_nowait_resume(rlm_rcode_t *p_result, module_ctx_ } } - if (fr_exec_fork_nowait(request, box, env_pairs, false) < 0) { + if (fr_exec_fork_nowait(request, box, env_pairs, inst->shell_escape, inst->env_inherit) < 0) { RPEDEBUG("Failed executing program"); RETURN_MODULE_FAIL; }