reduce scope of variable. Found by PVS-Studio
[freeradius.git] / src / main / exec.c
index 0990334..d5b3a9a 100644 (file)
@@ -32,7 +32,6 @@ RCSID("$Id$")
 
 #include <fcntl.h>
 #include <ctype.h>
-#include <signal.h>
 
 #ifdef HAVE_SYS_WAIT_H
 #      include <sys/wait.h>
@@ -59,7 +58,7 @@ static void tv_sub(struct timeval *end, struct timeval *start,
        }
        elapsed->tv_usec += end->tv_usec;
        elapsed->tv_usec -= start->tv_usec;
-       
+
        if (elapsed->tv_usec >= USEC) {
                elapsed->tv_usec -= USEC;
                elapsed->tv_sec++;
@@ -79,46 +78,48 @@ static void tv_sub(struct timeval *end, struct timeval *start,
  *     descriptor. Set to NULL and child will have /dev/null on stdout.
  * @param input_pairs list of value pairs - these will be put into
  *     the environment variables of the child.
- * @param shell_escape
+ * @param shell_escape values before passing them as arguments.
  * @return PID of the child process, -1 on error.
  */
-pid_t radius_start_program(char const *cmd, REQUEST *request,
-                       int exec_wait,
-                       int *input_fd,
-                       int *output_fd,
-                       VALUE_PAIR *input_pairs,
-                       int shell_escape)
+pid_t radius_start_program(char const *cmd, REQUEST *request, bool exec_wait,
+                          int *input_fd, int *output_fd,
+                          VALUE_PAIR *input_pairs, bool shell_escape)
 {
 #ifndef __MINGW32__
-       char *p;
        VALUE_PAIR *vp;
        int n;
        int to_child[2] = {-1, -1};
        int from_child[2] = {-1, -1};
        pid_t pid;
 #endif
-       int argc;
-       int i;
-       char const *argv[MAX_ARGV];
-       char **argv_p;
-       char argv_buf[4096];
+       int             argc;
+       int             i;
+       char const      **argv_p;
+       char            *argv[MAX_ARGV], **argv_start = argv;
+       char            argv_buf[4096];
 #define MAX_ENVP 1024
        char *envp[MAX_ENVP];
-       
-       memcpy(&argv_p, &argv, sizeof(argv_p));
+       int envlen = 0;
 
-       argc = rad_expand_xlat(request, cmd, MAX_ARGV, argv, 1, sizeof(argv_buf), argv_buf);
+       /*
+        *      Stupid array decomposition...
+        *
+        *      If we do memcpy(&argv_p, &argv, sizeof(argv_p)) src ends up being a char **
+        *      pointing to the value of the first element.
+        */
+       memcpy(&argv_p, &argv_start, sizeof(argv_p));
+       argc = rad_expand_xlat(request, cmd, MAX_ARGV, argv_p, true, sizeof(argv_buf), argv_buf);
        if (argc <= 0) {
-               RDEBUG("invalid command line '%s'.", cmd);
+               DEBUG("invalid command line '%s'.", cmd);
                return -1;
        }
 
 
 #ifndef NDEBUG
-       if (debug_flag > 2) {
-               RDEBUG3("executing cmd %s", cmd);
+       if (rad_debug_lvl > 2) {
+               DEBUG3("executing cmd %s", cmd);
                for (i = 0; i < argc; i++) {
-                       RDEBUG3("\t[%d] %s", i, argv[i]);
+                       DEBUG3("\t[%d] %s", i, argv[i]);
                }
        }
 #endif
@@ -130,13 +131,13 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
        if (exec_wait) {
                if (input_fd) {
                        if (pipe(to_child) != 0) {
-                               RDEBUG("Couldn't open pipe to child: %s", strerror(errno));
+                               DEBUG("Couldn't open pipe to child: %s", fr_syserror(errno));
                                return -1;
                        }
                }
                if (output_fd) {
                        if (pipe(from_child) != 0) {
-                               RDEBUG("Couldn't open pipe from child: %s", strerror(errno));
+                               DEBUG("Couldn't open pipe from child: %s", fr_syserror(errno));
                                /* safe because these either need closing or are == -1 */
                                close(to_child[0]);
                                close(to_child[1]);
@@ -148,7 +149,7 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
        envp[0] = NULL;
 
        if (input_pairs) {
-               int envlen;
+               vp_cursor_t cursor;
                char buffer[1024];
 
                /*
@@ -157,9 +158,9 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 *      hold mutexes.  They might be locked when we fork,
                 *      and will remain locked in the child.
                 */
-               envlen = 0;
-
-               for (vp = input_pairs; vp != NULL; vp = vp->next) {
+               for (vp = fr_cursor_init(&cursor, &input_pairs);
+                    vp;
+                    vp = fr_cursor_next(&cursor)) {
                        /*
                         *      Hmm... maybe we shouldn't pass the
                         *      user's password in an environment
@@ -167,6 +168,8 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                         */
                        snprintf(buffer, sizeof(buffer), "%s=", vp->da->name);
                        if (shell_escape) {
+                               char *p;
+
                                for (p = buffer; *p != '='; p++) {
                                        if (*p == '-') {
                                                *p = '_';
@@ -177,7 +180,7 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                        }
 
                        n = strlen(buffer);
-                       vp_prints_value(buffer+n, sizeof(buffer) - n, vp, shell_escape);
+                       vp_prints_value(buffer + n, sizeof(buffer) - n, vp, shell_escape ? '"' : 0);
 
                        envp[envlen++] = strdup(buffer);
 
@@ -185,8 +188,12 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                         *      Don't add too many attributes.
                         */
                        if (envlen == (MAX_ENVP - 1)) break;
+
+                       /*
+                        *      NULL terminate for execve
+                        */
+                       envp[envlen] = NULL;
                }
-               envp[envlen] = NULL;
        }
 
        if (exec_wait) {
@@ -210,9 +217,15 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 */
                devnull = open("/dev/null", O_RDWR);
                if (devnull < 0) {
-                       RDEBUG("Failed opening /dev/null: %s\n", strerror(errno));
-                       
-                       exit(1);
+                       DEBUG("Failed opening /dev/null: %s\n", fr_syserror(errno));
+
+                       /*
+                        *      Where the status code is interpreted as a module rcode
+                        *      one is subtracted from it, to allow 0 to equal success
+                        *
+                        *      2 is RLM_MODULE_FAIL + 1
+                        */
+                       exit(2);
                }
 
                /*
@@ -220,7 +233,6 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 *      has created them.
                 */
                if (exec_wait) {
-
                        if (input_fd) {
                                close(to_child[1]);
                                dup2(to_child[0], STDIN_FILENO);
@@ -248,7 +260,7 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 *      If we are debugging, then we want the error
                 *      messages to go to the STDERR of the server.
                 */
-               if (debug_flag == 0) {
+               if (rad_debug_lvl == 0) {
                        dup2(devnull, STDERR_FILENO);
                }
                close(devnull);
@@ -260,15 +272,30 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 */
                closefrom(3);
 
-               execve(argv_p[0], argv_p, envp);
-               RDEBUGW("Failed to execute %s: %s", argv[0], strerror(errno));
-               exit(1);
+               /*
+                *      I swear the signature for execve is wrong and should
+                *      take 'char const * const argv[]'.
+                *
+                *      Note: execve(), unlike system(), treats all the space
+                *      delimited arguments as literals, so there's no need
+                *      to perform additional escaping.
+                */
+               execve(argv[0], argv, envp);
+               printf("Failed to execute \"%s\": %s", argv[0], fr_syserror(errno)); /* fork output will be captured */
+
+               /*
+                *      Where the status code is interpreted as a module rcode
+                *      one is subtracted from it, to allow 0 to equal success
+                *
+                *      2 is RLM_MODULE_FAIL + 1
+                */
+               exit(2);
        }
 
        /*
         *      Free child environment variables
         */
-       for (i = 0; envp[i] != NULL; i++) {
+       for (i = 0; i < envlen; i++) {
                free(envp[i]);
        }
 
@@ -276,13 +303,13 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
         *      Parent process.
         */
        if (pid < 0) {
-               RDEBUG("Couldn't fork %s: %s", argv[0], strerror(errno));
+               DEBUG("Couldn't fork %s: %s", argv[0], fr_syserror(errno));
                if (exec_wait) {
                        /* safe because these either need closing or are == -1 */
                        close(to_child[0]);
                        close(to_child[1]);
                        close(from_child[0]);
-                       close(from_child[0]);
+                       close(from_child[1]);
                }
                return -1;
        }
@@ -309,10 +336,10 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
        return pid;
 #else
        if (exec_wait) {
-               RDEBUG("Wait is not supported");
+               DEBUG("Wait is not supported");
                return -1;
        }
-       
+
        {
                /*
                 *      The _spawn and _exec families of functions are
@@ -327,7 +354,7 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 *      to close your program (_P_OVERLAY), to wait
                 *      until the new process is finished (_P_WAIT) or
                 *      for the two to run concurrently (_P_NOWAIT).
-               
+
                 *      _spawn and _exec are useful for instances in
                 *      which you have simple requirements for running
                 *      the program, don't want the overhead of the
@@ -347,7 +374,6 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
 
 /** Read from the child process.
  *
- * @param request The current request.
  * @param fd file descriptor to read from.
  * @param pid pid of child, will be reaped if it dies.
  * @param timeout amount of time to wait, in seconds.
@@ -355,7 +381,7 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
  * @param left length of buffer.
  * @return -1 on error, or length of output.
  */
-int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
+int radius_readfrom_program(int fd, pid_t pid, int timeout,
                            char *answer, int left)
 {
        int done = 0;
@@ -363,7 +389,7 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
        int status;
        struct timeval start;
 #ifdef O_NONBLOCK
-       int nonblock = true;
+       bool nonblock = true;
 #endif
 
 #ifdef O_NONBLOCK
@@ -372,12 +398,12 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
         */
        do {
                int flags;
-               
+
                if ((flags = fcntl(fd, F_GETFL, NULL)) < 0)  {
                        nonblock = false;
                        break;
                }
-               
+
                flags |= O_NONBLOCK;
                if( fcntl(fd, F_SETFL, flags) < 0) {
                        nonblock = false;
@@ -403,7 +429,7 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
                gettimeofday(&when, NULL);
                tv_sub(&when, &start, &elapsed);
                if (elapsed.tv_sec >= timeout) goto too_long;
-               
+
                when.tv_sec = timeout;
                when.tv_usec = 0;
                tv_sub(&when, &elapsed, &wake);
@@ -411,7 +437,7 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
                rcode = select(fd + 1, &fds, NULL, NULL, &wake);
                if (rcode == 0) {
                too_long:
-                       RDEBUG("Child PID %u is taking too much time: forcing failure and killing child.", pid);
+                       DEBUG("Child PID %u is taking too much time: forcing failure and killing child.", (unsigned int) pid);
                        kill(pid, SIGTERM);
                        close(fd); /* should give SIGPIPE to child, too */
 
@@ -471,64 +497,73 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
                if (left <= 0) break;
        }
 #endif /* __MINGW32__ */
+
+       /* Strip trailing new lines */
+       while ((done > 0) && (answer[done - 1] == '\n')) {
+               answer[--done] = '\0';
+       }
+
        return done;
 }
 
 /** Execute a program.
  *
- * @param cmd Command to execute. This is parsed into argv[] parts,
- *     then each individual argv part is xlat'ed.
- * @param request current request.
- * @param exec_wait set to 1 if you want to read from or write to child
- * @param user_msg buffer to append plaintext (non valuepair) output.
- * @param msg_len length of user_msg buffer.
- * @param input_pairs list of value pairs - these will be put into
- *     the environment variables of the child.
- * @param[out] output_pairs list of value pairs - child stdout will be
- *     parsed and added into this list of value pairs.
- * @param shell_escape
+ * @param[in,out] ctx to allocate new VALUE_PAIR (s) in.
+ * @param[out] out buffer to append plaintext (non valuepair) output.
+ * @param[in] outlen length of out buffer.
+ * @param[out] output_pairs list of value pairs - child stdout will be parsed and added into this list
+ *     of value pairs.
+ * @param[in] request Current request (may be NULL).
+ * @param[in] cmd Command to execute. This is parsed into argv[] parts, then each individual argv part
+ *     is xlat'ed.
+ * @param[in] input_pairs list of value pairs - these will be available in the environment of the child.
+ * @param[in] exec_wait set to 1 if you want to read from or write to child.
+ * @param[in] shell_escape values before passing them as arguments.
+ * @param[in] timeout amount of time to wait, in seconds.
+
  * @return 0 if exec_wait==0, exit code if exec_wait!=0, -1 on error.
  */
-int radius_exec_program(char const *cmd, REQUEST *request,
-                       int exec_wait,
-                       char *user_msg, int msg_len,
-                       VALUE_PAIR *input_pairs,
-                       VALUE_PAIR **output_pairs,
-                       int shell_escape)
+int radius_exec_program(TALLOC_CTX *ctx, char *out, size_t outlen, VALUE_PAIR **output_pairs,
+                       REQUEST *request, char const *cmd, VALUE_PAIR *input_pairs,
+                       bool exec_wait, bool shell_escape, int timeout)
+
 {
        pid_t pid;
        int from_child;
 #ifndef __MINGW32__
-       VALUE_PAIR *vp;
        char *p;
        pid_t child_pid;
        int comma = 0;
-       int status;
-       int n, done;
+       int status, ret = 0;
+       ssize_t len;
        char answer[4096];
 #endif
 
+       RDEBUG2("Executing: %s:", cmd);
+
+       if (out) *out = '\0';
+
        pid = radius_start_program(cmd, request, exec_wait, NULL, &from_child, input_pairs, shell_escape);
        if (pid < 0) {
                return -1;
        }
 
-       if (!exec_wait)
+       if (!exec_wait) {
                return 0;
+       }
 
 #ifndef __MINGW32__
-       done = radius_readfrom_program(request, from_child, pid, 10, answer, sizeof(answer));
-       if (done < 0) {
+       len = radius_readfrom_program(from_child, pid, timeout, answer, sizeof(answer));
+       if (len < 0) {
                /*
-                * failure - radius_readfrom_program will
-                * have called close(from_child) for us
+                *      Failure - radius_readfrom_program will
+                *      have called close(from_child) for us
                 */
-               DEBUG("Failed to read from child output");
-               return 1;
+               RERROR("Failed to read from child output");
+               return -1;
 
        }
-       answer[done] = 0;
-
+       answer[len] = '\0';
 
        /*
         *      Make sure that the writer can't block while writing to
@@ -536,193 +571,80 @@ int radius_exec_program(char const *cmd, REQUEST *request,
         */
        close(from_child);
 
-       RDEBUG2("Program output is %s", answer);
+       if (len == 0) {
+               goto wait;
+       }
 
        /*
         *      Parse the output, if any.
         */
-       if (done) {
-               n = T_OP_INVALID;
-               if (output_pairs) {
-                       /*
-                        *      For backwards compatibility, first check
-                        *      for plain text (user_msg).
-                        */
-                       vp = NULL;
-                       n = userparse(request, answer, &vp);
-                       if (vp) {
-                               pairfree(&vp);
-                       }
-               }
-
-               if (n == T_OP_INVALID) {
-                       if (user_msg) {
-                               strlcpy(user_msg, answer, msg_len);
+       if (output_pairs) {
+               /*
+                *      HACK: Replace '\n' with ',' so that
+                *      fr_pair_list_afrom_str() can parse the buffer in
+                *      one go (the proper way would be to
+                *      fix fr_pair_list_afrom_str(), but oh well).
+                */
+               for (p = answer; *p; p++) {
+                       if (*p == '\n') {
+                               *p = comma ? ' ' : ',';
+                               p++;
+                               comma = 0;
                        }
-               } else {
-                       /*
-                        *      HACK: Replace '\n' with ',' so that
-                        *      userparse() can parse the buffer in
-                        *      one go (the proper way would be to
-                        *      fix userparse(), but oh well).
-                        */
-                       for (p = answer; *p; p++) {
-                               if (*p == '\n') {
-                                       *p = comma ? ' ' : ',';
-                                       p++;
-                                       comma = 0;
-                               }
-                               if (*p == ',') comma++;
+                       if (*p == ',') {
+                               comma++;
                        }
+               }
 
-                       /*
-                        *      Replace any trailing comma by a NUL.
-                        */
-                       if (answer[strlen(answer) - 1] == ',') {
-                               answer[strlen(answer) - 1] = '\0';
-                       }
+               /*
+                *      Replace any trailing comma by a NUL.
+                */
+               if (answer[len - 1] == ',') {
+                       answer[--len] = '\0';
+               }
 
-                       if (userparse(request, answer, &vp) == T_OP_INVALID) {
-                               RDEBUGE("Unparsable reply from '%s'", cmd);
+               if (fr_pair_list_afrom_str(ctx, answer, output_pairs) == T_INVALID) {
+                       RERROR("Failed parsing output from: %s: %s", cmd, fr_strerror());
+                       strlcpy(out, answer, len);
+                       ret = -1;
+               }
+       /*
+        *      We've not been told to extract output pairs,
+        *      just copy the programs output to the out
+        *      buffer.
+        */
 
-                       } else {
-                               /*
-                                *      Tell the caller about the value
-                                *      pairs.
-                                */
-                               *output_pairs = vp;
-                       }
-               } /* else the answer was a set of VP's, not a text message */
-       } /* else we didn't read anything from the child */
+       } else if (out) {
+               strlcpy(out, answer, outlen);
+       }
 
        /*
         *      Call rad_waitpid (should map to waitpid on non-threaded
         *      or single-server systems).
         */
+wait:
        child_pid = rad_waitpid(pid, &status);
        if (child_pid == 0) {
-               RDEBUGE("Timeout waiting for child");
-               return 2;
+               RERROR("Timeout waiting for child");
+
+               return -2;
        }
 
        if (child_pid == pid) {
                if (WIFEXITED(status)) {
                        status = WEXITSTATUS(status);
-                       if (status != 0) {
-                               RDEBUGE("Child returned error %d", status);
-                               return status;
+                       if ((status != 0) || (ret < 0)) {
+                               RERROR("Program returned code (%d) and output '%s'", status, answer);
+                       } else {
+                               RDEBUG2("Program returned code (%d) and output '%s'", status, answer);
                        }
-                       
-                       RDEBUG("Child executed successfully");
-                       return 0;
-               }
-       }
 
-       RDEBUGE("Abnormal child exit: %s", strerror(errno));
-#endif /* __MINGW32__ */
-
-       return 1;
-}
-
-static void time_free(void *data)
-{
-       free(data);
-}
-
-void exec_trigger(REQUEST *request, CONF_SECTION *cs, char const *name, int quench)
-{
-       CONF_SECTION *subcs;
-       CONF_ITEM *ci;
-       CONF_PAIR *cp;
-       char const *attr;
-       char const *value;
-       VALUE_PAIR *vp;
-
-       /*
-        *      Use global "trigger" section if no local config is given.
-        */
-       if (!cs) {
-               cs = mainconfig.config;
-               attr = name;
-       } else {
-               /*
-                *      Try to use pair name, rather than reference.
-                */
-               attr = strrchr(name, '.');
-               if (attr) {
-                       attr++;
-               } else {
-                       attr = name;
+                       return ret < 0 ? ret : status;
                }
        }
 
-       /*
-        *      Find local "trigger" subsection.  If it isn't found,
-        *      try using the global "trigger" section, and reset the
-        *      reference to the full path, rather than the sub-path.
-        */
-       subcs = cf_section_sub_find(cs, "trigger");
-       if (!subcs && (cs != mainconfig.config)) {
-               subcs = cf_section_sub_find(mainconfig.config, "trigger");
-               attr = name;
-       }
-
-       if (!subcs) return;
-
-       ci = cf_reference_item(subcs, mainconfig.config, attr);
-       if (!ci) {
-               RDEBUG3("No such item in trigger section: %s", attr);
-               return;
-       }
-
-       if (!cf_item_is_pair(ci)) {
-               RDEBUG2("Trigger is not a configuration variable: %s", attr);
-               return;
-       }
-
-       cp = cf_itemtopair(ci);
-       if (!cp) return;
-
-       value = cf_pair_value(cp);
-       if (!value) {
-               RDEBUG2("Trigger has no value: %s", name);
-               return;
-       }
-
-       /*
-        *      May be called for Status-Server packets.
-        */
-       vp = NULL;
-       if (request && request->packet) vp = request->packet->vps;
-
-       /*
-        *      Perform periodic quenching.
-        */
-       if (quench) {
-               time_t *last_time;
-
-               last_time = cf_data_find(cs, value);
-               if (!last_time) {
-                       last_time = rad_malloc(sizeof(*last_time));
-                       *last_time = 0;
-
-                       if (cf_data_add(cs, value, last_time, time_free) < 0) {
-                               free(last_time);
-                               last_time = NULL;
-                       }
-               }
-
-               /*
-                *      Send the quenched traps at most once per second.
-                */
-               if (last_time) {
-                       time_t now = time(NULL);
-                       if (*last_time == now) return;
-
-                       *last_time = now;
-               }
-       }
+       RERROR("Abnormal child exit: %s", fr_syserror(errno));
+#endif /* __MINGW32__ */
 
-       RDEBUG("Trigger %s -> %s", name, value);
-       radius_exec_program(value, request, 0, NULL, 0, vp, NULL, 1);
+       return -1;
 }