Reorder arguments to radius_exec_program so they make more sense
authorArran Cudbard-Bell <a.cudbardb@freeradius.org>
Tue, 2 Jul 2013 11:00:56 +0000 (12:00 +0100)
committerArran Cudbard-Bell <a.cudbardb@freeradius.org>
Tue, 2 Jul 2013 11:03:17 +0000 (12:03 +0100)
Use a single function to deal with status codes and module failure messages

Have fork return 2 (translates to RLM_MODULE_FAIL)

src/include/radiusd.h
src/main/evaluate.c
src/main/exec.c
src/main/modcall.c
src/main/threads.c
src/main/tls.c
src/main/valuepair.c
src/modules/rlm_exec/rlm_exec.c
src/modules/rlm_mschap/rlm_mschap.c

index 0187c3a..be4a27b 100644 (file)
@@ -476,7 +476,7 @@ extern char const   *radius_libdir;
 extern uint32_t                expiration_seconds;
 extern int             log_stripped_names;
 extern int             log_auth_detail;
-extern char const      *radiusd_version;
+extern char const      *radiusd_version;
 void                   radius_signal_self(int flag);
 
 #define RADIUS_SIGNAL_SELF_NONE                (0)
@@ -584,11 +584,9 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                        int shell_escape);
 int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
                            char *answer, int left);
-int            radius_exec_program(char const *,  REQUEST *, int,
-                                   char *user_msg, int msg_len,
-                                   VALUE_PAIR *input_pairs,
-                                   VALUE_PAIR **output_pairs,
-                                       int shell_escape);
+int radius_exec_program(REQUEST *request, char const *cmd, bool exec_wait, bool shell_escape,
+                       char *user_msg, size_t msg_len,
+                       VALUE_PAIR *input_pairs, VALUE_PAIR **output_pairs);
 void exec_trigger(REQUEST *request, CONF_SECTION *cs, char const *name, int quench);
 
 /* valuepair.c */
index a57d758..ce10187 100644 (file)
@@ -100,8 +100,7 @@ static char *radius_expand_tmpl(REQUEST *request, value_pair_tmpl_t const *vpt)
        case VPT_TYPE_EXEC:
                EVAL_DEBUG("TMPL EXEC");
                buffer = talloc_array(request, char, 1024);
-               if (radius_exec_program(vpt->name, request, 1,
-                                       buffer, 1024, NULL, NULL, 0) != 0) {
+               if (radius_exec_program(request, vpt->name, true, false, buffer, 1024, NULL, NULL) != 0) {
                        talloc_free(buffer);
                        return NULL;
                }
index 5791bb0..b5d2ecc 100644 (file)
@@ -209,8 +209,14 @@ 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);
+       
+                       /* 
+                        *      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);        
                }
 
                /*
@@ -263,7 +269,14 @@ pid_t radius_start_program(char const *cmd, REQUEST *request,
                 */
                execve(argv[0], argv, envp);
                printf("Failed to execute \"%s\": %s", argv[0], strerror(errno)); /* fork output will be captured */
-               exit(1);
+               
+               /* 
+                *      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);
        }
 
        /*
@@ -477,24 +490,22 @@ int radius_readfrom_program(REQUEST *request, int fd, pid_t pid, int timeout,
 
 /** Execute a program.
  *
- * @param cmd Command to execute. This is parsed into argv[] parts, then each individual argv part
- *     is xlat'ed.
  * @param[in] request Current request.
+ * @param[in] cmd Command to execute. This is parsed into argv[] parts, then each individual argv part
+ *     is xlat'ed.
  * @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] user_msg buffer to append plaintext (non valuepair) output.
  * @param[in] msg_len length of user_msg buffer.
  * @param[in] input_pairs list of value pairs - these will be available in the environment 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 values before passing them as arguments.
  * @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(REQUEST *request, char const *cmd, bool exec_wait, bool shell_escape,
+                       char *user_msg, size_t msg_len,
+                       VALUE_PAIR *input_pairs, VALUE_PAIR **output_pairs)
+
 {
        pid_t pid;
        int from_child;
@@ -528,10 +539,10 @@ int radius_exec_program(char const *cmd, REQUEST *request,
                 * have called close(from_child) for us
                 */
                DEBUG("Failed to read from child output");
-               return 1;
+               return -1;
 
        }
-       answer[done] = 0;
+       answer[done] = '\0';
 
        /*
         *      Make sure that the writer can't block while writing to
@@ -585,6 +596,8 @@ int radius_exec_program(char const *cmd, REQUEST *request,
 
                        if (userparse(request, answer, &vp) == T_OP_INVALID) {
                                REDEBUG("Unparsable reply from '%s'", cmd);
+                               
+                               return -1;
                        } else {
                                /*
                                 *      Tell the caller about the value
@@ -602,25 +615,22 @@ int radius_exec_program(char const *cmd, REQUEST *request,
        child_pid = rad_waitpid(pid, &status);
        if (child_pid == 0) {
                REDEBUG("Timeout waiting for child");
-               return 2;
+               
+               return -2;
        }
 
        if (child_pid == pid) {
                if (WIFEXITED(status)) {
                        status = WEXITSTATUS(status);
-                       if (status != 0) {
-                               REDEBUG("Program returned error code(%d): %s", status, answer);
-                               return status;
-                       }
                        
-                       RDEBUG("Program executed successfully");
-                       RDEBUG2("Program output is \"%s\"", answer);
-                       return 0;
+                       RDEBUG("Program returned code (%d): %s", status, answer);
+
+                       return status;
                }
        }
 
        REDEBUG("Abnormal child exit: %s", strerror(errno));
 #endif /* __MINGW32__ */
 
-       return 1;
+       return -1;
 }
index 1cd53c2..3d4fca7 100644 (file)
@@ -632,10 +632,8 @@ int modcall(int component, modcallable *c, REQUEST *request)
                                radius_xlat(buffer, sizeof(buffer), request, mx->xlat_name, NULL, NULL);
                        } else {
                                RDEBUG("`%s`", mx->xlat_name);
-                               radius_exec_program(mx->xlat_name, request,
-                                                   0, NULL, 0,
-                                                   request->packet->vps,
-                                                   NULL, 1);
+                               radius_exec_program(request, mx->xlat_name, false, true,
+                                                   NULL, 0, request->packet->vps, NULL);
                        }
                                            
                        goto skip; /* don't change anything on the stack */
index 187705e..b733951 100644 (file)
@@ -1468,5 +1468,5 @@ void exec_trigger(REQUEST *request, CONF_SECTION *cs, char const *name, int quen
        }
 
        RDEBUG("Trigger %s -> %s", name, value);
-       radius_exec_program(value, request, 0, NULL, 0, vp, NULL, 1);
+       radius_exec_program(request, value, false, true, NULL, 0, vp, NULL);
 }
index 269306d..cea6985 100644 (file)
@@ -1786,12 +1786,9 @@ int cbtls_verify(int ok, X509_STORE_CTX *ctx)
                                goto do_unlink;
                        }
 
-                       RDEBUG("Verifying client certificate: %s",
-                              conf->verify_client_cert_cmd);
-                       if (radius_exec_program(conf->verify_client_cert_cmd,
-                                               request, 1, NULL, 0,
-                                               request->packet->vps,
-                                               NULL, 1) != 0) {
+                       RDEBUG("Verifying client certificate: %s", conf->verify_client_cert_cmd);
+                       if (radius_exec_program(request, conf->verify_client_cert_cmd, true, true, NULL, 0,
+                                               request->packet->vps, NULL) != 0) {
                                AUTH("rlm_eap_tls: Certificate CN (%s) fails external verification!", common_name);
                                my_ok = 0;
                        } else {
index 59b3e51..4eb0ab5 100644 (file)
@@ -1285,6 +1285,10 @@ void module_failure_msg(REQUEST *request, char const *fmt, ...)
        char *p;
        VALUE_PAIR *vp;
 
+       if (!fmt) {
+               return;
+       }
+       
        va_start(ap, fmt);
        vp = paircreate(request->packet, PW_MODULE_FAILURE_MESSAGE, 0);
        if (!vp) {
index 049bff0..cb6f3b1 100644 (file)
@@ -64,12 +64,96 @@ static const CONF_PARSER module_config[] = {
        { NULL, -1, 0, NULL, NULL }             /* end the list */
 };
 
+static char const special[] = "\\'\"`<>|; \t\r\n()[]?#$^&*=";
+
+/*
+ *     Escape special characters
+ */
+static size_t rlm_exec_shell_escape(UNUSED REQUEST *request, char *out, size_t outlen, char const *in,
+                                   UNUSED void *inst)
+{
+       char *q, *end;
+       char const *p;
+
+       q = out;
+       end = out + outlen;
+       p = in;
+
+       while (*p) {
+               if ((q + 3) >= end) break;
+
+               if (strchr(special, *p) != NULL) {
+                       *(q++) = '\\';
+               }
+               *(q++) = *(p++);
+       }
+
+       *q = '\0';
+       return q - out;
+}
+
+/** Process the exit code returned by one of the exec functions
+ *
+ * @param request Current request.
+ * @param answer Output string from exec call.
+ * @param len length of data in answer.
+ * @param status code returned by exec call.
+ * @return One of the RLM_MODULE_* values.
+ */
+static rlm_rcode_t rlm_exec_status2rcode(REQUEST *request, char *answer, size_t len, int status)
+{
+       if (status < 0) {
+               return RLM_MODULE_FAIL;
+       }
+       
+       /*
+        *      Exec'd programs are meant to return exit statuses that correspond 
+        *      to the standard RLM_MODULE_* + 1. 
+        *
+        *      This frees up 0, for success where it'd normally be reject.
+        */             
+       if (status == 0) {
+               RDEBUG("Program executed successfully");
+
+               return RLM_MODULE_OK;   
+       }
+
+       if (status > RLM_MODULE_NUMCODES) {
+               REDEBUG("Program returned invalid code (greater than max rcode) (%i > %i): %s",
+                       status, RLM_MODULE_NUMCODES, answer);
+               goto fail;
+
+               return RLM_MODULE_FAIL;
+       }
+
+       status--;       /* Lets hope no one ever re-enumerates RLM_MODULE_* */
+
+       if (status == RLM_MODULE_FAIL) {
+               fail:
+       
+               if (len > 0) {
+                       char *p = &answer[len - 1];
+               
+                       /*
+                        *      Trim off trailing returns
+                        */
+                       while((p > answer) && ((*p == '\r') || (*p == '\n'))) {
+                               *p-- = '\0';
+                       }
+       
+                       module_failure_msg(request, answer);
+               }
+
+               return RLM_MODULE_FAIL;
+       }
+
+       return status;          
+}
 
 /*
  *     Do xlat of strings.
  */
-static size_t exec_xlat(void *instance, REQUEST *request,
-                       char const *fmt, char *out, size_t outlen)
+static size_t exec_xlat(void *instance, REQUEST *request, char const *fmt, char *out, size_t outlen)
 {
        int             result;
        rlm_exec_t      *inst = instance;
@@ -94,9 +178,8 @@ static size_t exec_xlat(void *instance, REQUEST *request,
        /*
         *      FIXME: Do xlat of program name?
         */
-       result = radius_exec_program(fmt, request,
-                                    inst->wait, out, outlen,
-                                    input_pairs ? *input_pairs : NULL, NULL, inst->shell_escape);
+       result = radius_exec_program(request, fmt, inst->wait, inst->shell_escape,
+                                    out, outlen, input_pairs ? *input_pairs : NULL, NULL);
        if (result != 0) {
                out[0] = '\0';
                return 0;
@@ -109,34 +192,6 @@ static size_t exec_xlat(void *instance, REQUEST *request,
        return strlen(out);
 }
 
-static char const special[] = "\\'\"`<>|; \t\r\n()[]?#$^&*=";
-
-/*
- *     Escape special characters
- */
-static size_t shell_escape(UNUSED REQUEST *request, char *out, size_t outlen, char const *in, UNUSED void *inst)
-{
-       char *q, *end;
-       char const *p;
-
-       q = out;
-       end = out + outlen;
-       p = in;
-
-       while (*p) {
-               if ((q + 3) >= end) break;
-
-               if (strchr(special, *p) != NULL) {
-                       *(q++) = '\\';
-               }
-               *(q++) = *(p++);
-       }
-
-       *q = '\0';
-       return q - out;
-}
-
-
 /*
  *     Do any per-module initialization that is separate to each
  *     configured instance of the module.  e.g. set up connections
@@ -158,7 +213,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
                inst->bare = 1;
        }
 
-       xlat_register(inst->xlat_name, exec_xlat, shell_escape, inst);
+       xlat_register(inst->xlat_name, exec_xlat, rlm_exec_shell_escape, inst);
        
        /*
         *      Check whether program actually exists
@@ -188,8 +243,7 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
         */
        if (!inst->wait &&
            (inst->output != NULL)) {
-               cf_log_err_cs(conf, "Cannot read output pairs if "
-                             "wait=no");
+               cf_log_err_cs(conf, "Cannot read output pairs if wait = no");
                return -1;
        }
 
@@ -219,12 +273,13 @@ static int mod_instantiate(CONF_SECTION *conf, void *instance)
  */
 static rlm_rcode_t exec_dispatch(void *instance, REQUEST *request)
 {
-       rlm_exec_t *inst = (rlm_exec_t *)instance;
-       int result;
+       rlm_exec_t      *inst = (rlm_exec_t *)instance;
+       rlm_rcode_t     rcode;
+       int             status;
+
        VALUE_PAIR      **input_pairs = NULL, **output_pairs = NULL;
        VALUE_PAIR      *answer = NULL;
        char            out[1024];
-       size_t          len;
 
        /*
         *      We need a program to execute.
@@ -245,6 +300,7 @@ static rlm_rcode_t exec_dispatch(void *instance, REQUEST *request)
 #endif
                    )) {
                RDEBUG2("Packet type is not %s. Not executing.", inst->packet_type);
+               
                return RLM_MODULE_NOOP;
        }
 
@@ -276,25 +332,10 @@ static rlm_rcode_t exec_dispatch(void *instance, REQUEST *request)
         *      exec program function xlat's it's string value
         *      into something else.
         */
-       result = radius_exec_program(inst->program, request,
-                                    inst->wait, out, sizeof(out),
-                                    input_pairs ? *input_pairs : NULL, &answer, inst->shell_escape);
-       /*
-        *      Write any exec output to module failure message
-        */
-       if (*out) {
-               /* Trim off returns and newlines */
-               len = strlen(out);
-               if (out[len - 1] == '\n' || out[len - 1] == '\r') {
-                       out[len - 1] = '\0';
-               }
-       }
-       
-       if (result < 0) {
-               module_failure_msg(request, out);
-
-               return RLM_MODULE_FAIL;
-       }
+       status = radius_exec_program(request, inst->program, inst->wait, inst->shell_escape,
+                                    out, sizeof(out),
+                                    input_pairs ? *input_pairs : NULL, &answer);
+       rcode = rlm_exec_status2rcode(request, out, strlen(out), status);
 
        /*
         *      Move the answer over to the output pairs.
@@ -304,19 +345,9 @@ static rlm_rcode_t exec_dispatch(void *instance, REQUEST *request)
        if (output_pairs) {
                pairmove(request, output_pairs, &answer);
        }
-       
        pairfree(&answer);
-
-       if (result == 0) {
-               return RLM_MODULE_OK;
-       }
-       if (result > RLM_MODULE_NUMCODES) {
-               module_failure_msg(request, out);
-
-               return RLM_MODULE_FAIL;
-       }
        
-       return result - 1;
+       return rcode;
 }
 
 
@@ -327,53 +358,53 @@ static rlm_rcode_t exec_dispatch(void *instance, REQUEST *request)
  */
 static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request)
 {
-       int result;
-       int exec_wait = 0;
-       VALUE_PAIR *vp, *tmp;
-       rlm_exec_t *inst = (rlm_exec_t *) instance;
+       rlm_exec_t      *inst = (rlm_exec_t *) instance;
+       rlm_rcode_t     rcode;
+       int             status;
+
+       char            out[1024];
+       bool            wait = false;
+       VALUE_PAIR      *vp, *tmp;
 
        vp = pairfind(request->reply->vps, PW_EXEC_PROGRAM, 0, TAG_ANY);
        if (vp) {
-               exec_wait = 0;
-
+               wait = false;
        } else if ((vp = pairfind(request->reply->vps, PW_EXEC_PROGRAM_WAIT, 0, TAG_ANY)) != NULL) {
-               exec_wait = 1;
+               wait = true;
        }
        if (!vp) {
-               if (!inst->program) return RLM_MODULE_NOOP;
+               if (!inst->program) {
+                       return RLM_MODULE_NOOP;
+               }
                
-               return exec_dispatch(instance, request);
+               rcode = exec_dispatch(instance, request);
+               goto finish;
        }
 
        tmp = NULL;
-       result = radius_exec_program(vp->vp_strvalue, request, exec_wait, NULL, 0, request->packet->vps, &tmp,
-                                    inst->shell_escape);
+       status = radius_exec_program(request, vp->vp_strvalue, wait, inst->shell_escape,
+                                    out, sizeof(out),
+                                    request->packet->vps, &tmp);
+       rcode = rlm_exec_status2rcode(request, out, strlen(out), status);
 
        /*
         *      Always add the value-pairs to the reply.
         */
        pairmove(request->reply, &request->reply->vps, &tmp);
        pairfree(&tmp);
-
-       if (result < 0) {
-               REDEBUG("Login incorrect (external check failed)");
-
-               request->reply->code = PW_AUTHENTICATION_REJECT;
-               return RLM_MODULE_REJECT;
-       }
-       if (result > 0) {
-               /*
-                *      Reject. radius_exec_program() returns >0
-                *      if the exec'ed program had a non-zero
-                *      exit status.
-                */
-               request->reply->code = PW_AUTHENTICATION_REJECT;
-               
-               REDEBUG("Login incorrect (external check said so)");
-               return RLM_MODULE_REJECT;
+       
+       finish:
+       switch (rcode) {
+               case RLM_MODULE_FAIL:
+               case RLM_MODULE_INVALID:
+               case RLM_MODULE_REJECT:
+                       request->reply->code = PW_AUTHENTICATION_REJECT;
+                       break;
+               default:
+                       break;
        }
 
-       return RLM_MODULE_OK;
+       return rcode;
 }
 
 /*
@@ -383,10 +414,12 @@ static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request)
  */
 static  rlm_rcode_t mod_accounting(void *instance, REQUEST *request)
 {
-       int result;
-       int exec_wait = 0;
-       VALUE_PAIR *vp;
-       rlm_exec_t *inst = (rlm_exec_t *) instance;
+       rlm_exec_t      *inst = (rlm_exec_t *) instance;
+       int             status;
+       
+       char            out[1024];
+       bool            wait = false;
+       VALUE_PAIR      *vp;
 
        /*
         *      The "bare" exec module takes care of handling
@@ -398,22 +431,18 @@ static  rlm_rcode_t mod_accounting(void *instance, REQUEST *request)
        
        vp = pairfind(request->reply->vps, PW_EXEC_PROGRAM, 0, TAG_ANY);
        if (vp) {
-               exec_wait = 0;
+               wait = true;
        } else if ((vp = pairfind(request->reply->vps, PW_EXEC_PROGRAM_WAIT, 0, TAG_ANY)) != NULL) {
-               exec_wait = 1;
+               wait = false;
        }
        if (!vp) {
                return RLM_MODULE_NOOP;
        }
        
-       result = radius_exec_program(vp->vp_strvalue, request, exec_wait,
-                                    NULL, 0, request->packet->vps, NULL,
-                                    inst->shell_escape);
-       if (result != 0) {
-               return RLM_MODULE_REJECT;
-       }
-
-       return RLM_MODULE_OK;
+       status = radius_exec_program(request, vp->vp_strvalue, wait, inst->shell_escape,
+                                    out, sizeof(out),
+                                    request->packet->vps, NULL);
+       return rlm_exec_status2rcode(request, out, strlen(out), status);
 }
 
 /*
index ddc717e..38e90f8 100644 (file)
@@ -1053,10 +1053,9 @@ static int do_mschap(rlm_mschap_t *inst,
                /*
                 *      Run the program, and expect that we get 16
                 */
-               result = radius_exec_program(inst->ntlm_auth, request,
-                                            true, /* wait */
+               result = radius_exec_program(request, inst->ntlm_auth, true, true,
                                             buffer, sizeof(buffer),
-                                            NULL, NULL, 1);
+                                            NULL, NULL);
                if (result != 0) {
                        char *p;