Add back env_escape to rlm_exec, and add env_inherit

diff --git a/raddb/mods-available/exec b/raddb/mods-available/exec
index bbbce9d8a4fc0e17a4db0410b733a8f5cd066c6c..27c7bce036da037c76d988908a7a4509fd6fc20f 100644 (file)
--- 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 997c6dd118d38ac35c1e73564ba1bef361c5ba2d..bcd64aeac915b059146eb83ec5a798a85ee93fa7 100644 (file)
--- 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 f3c88869a763e5231f513086c7c5242a7a391a9a..e22c2152299dc1b1c1143d3fb604d192f517163d 100644 (file)
--- 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 bac070e9aff7e39cc545616b54937cf75ed6bc1d..a785407012c1202e2b4cf36a54e08766fe0219cd 100644 (file)
--- 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 5061680fac7877bcb68907095a6acda4183e50d3..0d0d8c3ebddb8f07ee135047200d45457a5d0d77 100644 (file)
--- 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 d820d4617501b70d699ebf02ef72f93991c03aca..703643699deeaabaf12d6c962737abee7ca7b863 100644 (file)
--- 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;
        }