Treat TID req as error if a response is not sent
authorJennifer Richards <jennifer@painless-security.com>
Mon, 7 May 2018 17:24:19 +0000 (13:24 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Mon, 7 May 2018 17:28:51 +0000 (13:28 -0400)
  * Return an error code from tr_gss_handle_connection()
  * When TID process terminates, send "OK" or "ERR" over the pipe
  * Refactor handling of the TID fork() and messaging

common/tr_gss.c
include/tr_gss.h
mon/mons.c
tid/tids.c

index e7902ee..d576449 100644 (file)
@@ -214,13 +214,13 @@ static int tr_gss_write_resp(int conn, gss_ctx_id_t gssctx, const char *resp)
  * @param req_cb callback to handle the request and produce the response
  * @param req_cookie cookie for the req_cb
  */
-void tr_gss_handle_connection(int conn,
-                              const char *acceptor_service,
-                              const char *acceptor_hostname,
-                              TR_GSS_AUTH_FN auth_cb,
-                              void *auth_cookie,
-                              TR_GSS_HANDLE_REQ_FN req_cb,
-                              void *req_cookie)
+TR_GSS_RC tr_gss_handle_connection(int conn,
+                                   const char *acceptor_service,
+                                   const char *acceptor_hostname,
+                                   TR_GSS_AUTH_FN auth_cb,
+                                   void *auth_cookie,
+                                   TR_GSS_HANDLE_REQ_FN req_cb,
+                                   void *req_cookie)
 {
   TALLOC_CTX *tmp_ctx = talloc_new(NULL);
   gss_ctx_id_t gssctx = GSS_C_NO_CONTEXT;
@@ -229,6 +229,7 @@ void tr_gss_handle_connection(int conn,
   TR_MSG *req_msg = NULL;
   TR_MSG *resp_msg = NULL;
   char *resp_str = NULL;
+  TR_GSS_RC rc = TR_GSS_ERROR;
 
   tr_debug("tr_gss_handle_connection: Attempting to accept %s connection on fd %d.",
            acceptor_service, conn);
@@ -294,6 +295,10 @@ void tr_gss_handle_connection(int conn,
     goto cleanup;
   }
 
+  /* we successfully sent a response */
+  rc = TR_GSS_SUCCESS;
+
 cleanup:
   talloc_free(tmp_ctx);
+  return rc;
 }
index 850f51b..e0cf770 100644 (file)
 typedef int (TR_GSS_AUTH_FN)(gss_name_t, TR_NAME *, void *);
 typedef TR_MSG *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, TR_MSG *, void *);
 
-void tr_gss_handle_connection(int conn, const char *acceptor_service, const char *acceptor_hostname, TR_GSS_AUTH_FN auth_cb,
-                              void *auth_cookie, TR_GSS_HANDLE_REQ_FN req_cb, void *req_cookie);
+typedef enum tr_gss_rc {
+  TR_GSS_SUCCESS = 0, /* success */
+  TR_GSS_ERROR,       /* unspecified error */
+} TR_GSS_RC;
+
+TR_GSS_RC tr_gss_handle_connection(int conn,
+                                   const char *acceptor_service,
+                                   const char *acceptor_hostname,
+                                   TR_GSS_AUTH_FN auth_cb,
+                                   void *auth_cookie,
+                                   TR_GSS_HANDLE_REQ_FN req_cb,
+                                   void *req_cookie);
 
 #endif //TRUST_ROUTER_TR_GSS_H
index cb70d3b..b0a0400 100644 (file)
@@ -237,11 +237,23 @@ int mons_accept(MONS_INSTANCE *mons, int listen)
 
   if (pid == 0) {
     close(listen);
-    tr_gss_handle_connection(conn,
-                             "trustmonitor", mons->hostname, /* acceptor name */
-                             mons->auth_handler, mons->cookie, /* auth callback and cookie */
-                             mons_req_cb, mons /* req callback and cookie */
-    );
+    switch(tr_gss_handle_connection(conn,
+                                    "trustmonitor", mons->hostname, /* acceptor name */
+                                    mons->auth_handler, mons->cookie, /* auth callback and cookie */
+                                    mons_req_cb, mons /* req callback and cookie */
+    )) {
+      case TR_GSS_SUCCESS:
+        /* do nothing */
+        break;
+
+      case TR_GSS_ERROR:
+        tr_debug("mons_accept: Error returned by tr_gss_handle_connection()");
+        break;
+
+      default:
+        tr_err("mons_accept: Unexpected value returned by tr_gss_handle_connection()");
+        break;
+    }
     close(conn);
     exit(0); /* exit to kill forked child process */
   }
index 74e790e..0569333 100644 (file)
@@ -397,6 +397,54 @@ nfds_t tids_get_listener(TIDS_INSTANCE *tids,
   return (int)n_fd;
 }
 
+/* Strings used to report results from the handler process. The
+ * TIDS_MAX_MESSAGE_LEN must be longer than the longest message, including
+ * null termination (i.e., strlen() + 1) */
+#define TIDS_MAX_MESSAGE_LEN (10)
+#define TIDS_SUCCESS_MESSAGE "OK"
+#define TIDS_ERROR_MESSAGE   "ERR"
+
+/**
+ * Process to handle an incoming TIDS request
+ *
+ * This should be run in the child process after a fork(). Handles
+ * the request, writes the result to result_fd, and terminates via exit().
+ * Never returns to the caller.
+ *
+ * @param tids TID server instance
+ * @param conn_fd file descriptor for the incoming connection
+ * @param result_fd writable file descriptor for the result, or 0 to disable reporting
+ */
+static void tids_handle_proc(TIDS_INSTANCE *tids, int conn_fd, int result_fd)
+{
+  const char *response_message = NULL;
+
+  switch(tr_gss_handle_connection(conn_fd,
+                                  "trustidentity", tids->hostname, /* acceptor name */
+                                  tids->auth_handler, tids->cookie, /* auth callback and cookie */
+                                  tids_req_cb, tids /* req callback and cookie */
+  )) {
+    case TR_GSS_SUCCESS:
+      response_message = TIDS_SUCCESS_MESSAGE;
+      break;
+
+    case TR_GSS_ERROR:
+    default:
+      response_message = TIDS_ERROR_MESSAGE;
+      break;
+  }
+
+  if (0 != result_fd) {
+    /* write strlen + 1 to include the null termination */
+    if (write(result_fd, response_message, strlen(response_message) + 1) < 0)
+      tr_err("tids_accept: child process unable to write to pipe");
+  }
+
+  close(result_fd);
+  close(conn_fd);
+  exit(0); /* exit to kill forked child process */
+}
+
 /* Accept and process a connection on a port opened with tids_get_listener() */
 int tids_accept(TIDS_INSTANCE *tids, int listen)
 {
@@ -422,26 +470,22 @@ int tids_accept(TIDS_INSTANCE *tids, int listen)
   }
 
   if (pid == 0) {
-    close(pipe_fd[0]); /* child only writes */
-    close(listen);
-    tr_gss_handle_connection(conn,
-                             "trustidentity", tids->hostname, /* acceptor name */
-                             tids->auth_handler, tids->cookie, /* auth callback and cookie */
-                             tids_req_cb, tids /* req callback and cookie */
-    );
-    if (write(pipe_fd[1], "OK\0", 3) < 0)
-      tr_crit("tids_accept: child process unable to write to pipe");
-    close(pipe_fd[1]);
-    close(conn);
-    exit(0); /* exit to kill forked child process */
+    /* Only the child process gets here */
+    close(pipe_fd[0]); /* close the read end of the pipe, the child only writes */
+    close(listen); /* close the child process's handle on the listen port */
+
+    tids_handle_proc(tids, conn, pipe_fd[1]); /* never returns */
   }
 
   /* Only the parent process gets here */
-  close(pipe_fd[1]); /* parent only listens */
-  close(conn); /* connection belongs to the child */
+  close(pipe_fd[1]); /* close the write end of the pipe, the parent only listens */
+  close(conn); /* connection belongs to the child, so close parent's handle */
+
+  /* remember the PID of our child process */
+  tr_info("tids_accept: Spawned TID process %d to handle incoming connection.", pid);
   tp.pid = pid;
   tp.read_fd = pipe_fd[0];
-  g_array_append_val(tids->pids, tp); /* remember the PID of our child process */
+  g_array_append_val(tids->pids, tp);
 
   /* clean up any processes that have completed */
   tids_sweep_procs(tids);
@@ -463,7 +507,7 @@ void tids_sweep_procs(TIDS_INSTANCE *tids)
 {
   guint ii;
   struct tid_process tp = {0};
-  char result[10] = {0};
+  char result[TIDS_MAX_MESSAGE_LEN] = {0};
   ssize_t result_len;
   int status;
   int wait_rc;
@@ -506,15 +550,15 @@ void tids_sweep_procs(TIDS_INSTANCE *tids)
     }
 
     /* read the pipe - if the TID request worked, it will have written status before terminating */
-    result_len = read(tp.read_fd, result, sizeof(result)/sizeof(result[0]));
+    result_len = read(tp.read_fd, result, TIDS_MAX_MESSAGE_LEN);
     close(tp.read_fd);
 
-    if ((result_len > 0) && (strcmp(result, "OK") == 0)) {
+    if ((result_len > 0) && (strcmp(result, TIDS_SUCCESS_MESSAGE) == 0)) {
       tids->req_count++;
-      tr_debug("tids_sweep_procs: TID process %d succeeded", tp.pid);
+      tr_info("tids_sweep_procs: TID process %d exited successfully.", tp.pid);
     } else {
       tids->error_count++;
-      tr_debug("tids_sweep_procs: TID process %d failed", tp.pid);
+      tr_info("tids_sweep_procs: TID process %d exited with an error.", tp.pid);
     }
   }
 }