From: Alan T. DeKok Date: Mon, 13 Apr 2020 14:43:36 +0000 (-0400) Subject: turn the output buffer into an output value box X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=931fc7524d6b436b5ceafbbae8bd82c81c672baa;p=thirdparty%2Ffreeradius-server.git turn the output buffer into an output value box and allow exec-wait without an output FD --- diff --git a/src/lib/server/exec.c b/src/lib/server/exec.c index 88ac5f685cd..18d16148c1b 100644 --- a/src/lib/server/exec.c +++ b/src/lib/server/exec.c @@ -751,22 +751,23 @@ int fr_exec_nowait(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs) * @param vb as returned by xlat_frame_eval() * @param env_pairs VPs to put into into the environment. May be NULL. * @param[out] pid_p The PID of the child + * @param[out] fd_p The stdout FD of the child * @return * - <0 on error - * - >= 0 on success, with FD to read program output + * - 0 on success * * @todo - maybe take an fr_cursor_t instead of env_pairs? That * would allow finer-grained control over the attributes to put into * the environment. */ -int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs, pid_t *pid_p) +int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs, pid_t *pid_p, int *fd_p) { int argc; char **envp; char **argv; pid_t pid; fr_value_box_t *first; - int from_child[2]; + int from_child[2] = {-1, -1}; /* * Clean up any previous child processes. @@ -797,9 +798,11 @@ int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pai argc = fr_value_box_list_flatten_argv(request, &argv, vb); if (argc < 0) return -1; - if (pipe(from_child) < 0) { - fr_strerror_printf("Failed opening pipe to read from child - %s", fr_strerror()); - return -1; + if (fd_p) { + if (pipe(from_child) < 0) { + fr_strerror_printf("Failed opening pipe to read from child - %s", fr_strerror()); + return -1; + } } pid = fork(); @@ -808,20 +811,19 @@ int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pai * The child never returns from calling fr_exec_child(); */ if (pid == 0) { - int unused[2]; + int unused[2] = {-1, -1}; - fr_exec_child(request, argv, envp, false, NULL, NULL, unused, from_child); + fr_exec_child(request, argv, envp, false, NULL, fd_p, unused, from_child); } /* * Parent process. Do all necessary cleanups. */ talloc_free(envp); - close(from_child[1]); if (pid < 0) { ERROR("Couldn't fork %s: %s", argv[0], fr_syserror(errno)); - close(from_child[0]); + if (fd_p) close(*fd_p); talloc_free(argv); return -1; } @@ -831,8 +833,10 @@ int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pai * from. */ *pid_p = pid; - fr_nonblock(from_child[0]); - return from_child[0]; + + if (fd_p) fr_nonblock(*fd_p); + + return 0; } /** Remember child processes. diff --git a/src/lib/server/exec.h b/src/lib/server/exec.h index 5c31181ae06..b52328cbae9 100644 --- a/src/lib/server/exec.h +++ b/src/lib/server/exec.h @@ -61,7 +61,7 @@ int radius_exec_program(TALLOC_CTX *ctx, char *out, size_t outlen, VALUE_PAIR ** int fr_exec_nowait(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs); -int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs, pid_t *pid_p); +int fr_exec_wait_start(REQUEST *request, fr_value_box_t *vb, VALUE_PAIR *env_pairs, pid_t *pid_p, int *fd_p); void fr_exec_waitpid(pid_t pid); diff --git a/src/lib/unlang/tmpl.c b/src/lib/unlang/tmpl.c index c40da286f07..8be54bdff18 100644 --- a/src/lib/unlang/tmpl.c +++ b/src/lib/unlang/tmpl.c @@ -265,6 +265,24 @@ static unlang_action_t unlang_tmpl_exec_wait_final(REQUEST *request, rlm_rcode_t return UNLANG_ACTION_CALCULATE_RESULT; } + /* + * We might want to just get the status of the program, + * and not care about the output. + * + * If we do care about the output, it's unquoted, and tainted. + */ + if (state->out) { + fr_type_t type = FR_TYPE_STRING; + + MEM(state->box = fr_value_box_alloc(state->ctx, FR_TYPE_STRING, NULL, true)); + if (fr_value_box_from_str(state->box, state->box, &type, NULL, + state->buffer, state->ptr - state->buffer, 0, true) < 0) { + TALLOC_FREE(state->box); + *presult = RLM_MODULE_FAIL; + return UNLANG_ACTION_CALCULATE_RESULT; + } + } + /* * Ensure that the callers resume function is called. */ @@ -282,21 +300,28 @@ static unlang_action_t unlang_tmpl_exec_wait_resume(REQUEST *request, rlm_rcode_ unlang_stack_frame_t *frame = &stack->frame[stack->depth]; unlang_frame_state_tmpl_t *state = talloc_get_type_abort(frame->state, unlang_frame_state_tmpl_t); - int fd; + int fd = -1; pid_t pid; + int *fd_p = NULL; + + /* + * @todo - if there's no state->out, then we don't need + * to have an FD, and we don't need to have a buffer. + */ + if (state->out) fd_p = &fd; - fd = fr_exec_wait_start(request, state->box, NULL, &pid); - if (fd < 0) { + if (fr_exec_wait_start(request, state->box, NULL, &pid, fd_p) < 0) { REDEBUG("Failed executing program - %s", fr_strerror()); fail: *presult = RLM_MODULE_FAIL; return UNLANG_ACTION_CALCULATE_RESULT; } + TALLOC_FREE(state->box); /* this is the xlat expansion, and not the output string we want */ + state->fd = fd; state->pid = pid; state->status = 1; /* default to program didn't work */ - MEM(state->buffer = talloc_array(state, char, 024)); /* * Kill the child process after a period of time. @@ -308,10 +333,17 @@ static unlang_action_t unlang_tmpl_exec_wait_resume(REQUEST *request, rlm_rcode_ goto fail; } - if (fr_event_fd_insert(state->ctx, request->el, state->fd, unlang_tmpl_exec_read, NULL, NULL, request) < 0) { - REDEBUG("Failed adding event - %s", fr_strerror()); - unlang_tmpl_exec_cleanup(request); - goto fail; + /* + * If the caller doesn't want the output box, we can just skip reading from the FD. + */ + if (state->fd >= 0) { + if (fr_event_fd_insert(state->ctx, request->el, state->fd, unlang_tmpl_exec_read, NULL, NULL, request) < 0) { + REDEBUG("Failed adding event - %s", fr_strerror()); + unlang_tmpl_exec_cleanup(request); + goto fail; + } + + MEM(state->buffer = talloc_array(state, char, 1024)); } frame->interpret = unlang_tmpl_exec_wait_final;