Use TR_MSG instead of encoded strings in GSS request handler interface
authorJennifer Richards <jennifer@painless-security.com>
Tue, 17 Apr 2018 16:27:15 +0000 (12:27 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Tue, 17 Apr 2018 16:27:15 +0000 (12:27 -0400)
Also some further cleanup of header files and data types.

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

index 39a13a9..e2483b9 100644 (file)
@@ -221,6 +221,9 @@ void tr_gss_handle_connection(int conn,
   TALLOC_CTX *tmp_ctx = talloc_new(NULL);
   gss_ctx_id_t gssctx = GSS_C_NO_CONTEXT;
   char *req_str = NULL;
+  size_t req_len = 0;
+  TR_MSG *req_msg = NULL;
+  TR_MSG *resp_msg = NULL;
   char *resp_str = NULL;
 
   if (tr_gss_auth_connection(conn,
@@ -236,34 +239,52 @@ void tr_gss_handle_connection(int conn,
   tr_debug("tr_gss_handle_connection: Connection authorized");
 
   // TODO: should there be a timeout on this?
-  while (1) {  /* continue until an error breaks us out */
+  do {
+    /* continue until an error breaks us out */
     // try to read a request
     req_str = tr_gss_read_req(tmp_ctx, conn, gssctx);
 
-    if ( req_str == NULL) {
+    if (req_str == NULL) {
       // an error occurred, give up
       tr_notice("tr_gss_handle_connection: Error reading request");
       goto cleanup;
-    } else if (strlen(req_str) > 0) {
-      // we got a request message, exit the loop and process it
-      break;
     }
 
-    // no error, but no message, keep waiting for one
-    talloc_free(req_str); // this would be cleaned up anyway, but may as well free it
+    req_len = strlen(req_str);
+
+    /* If we got no characters, we will loop again. Free the empty response for the next loop. */
+    if (req_len == 0)
+      talloc_free(req_str);
+
+  } while (req_len == 0);
+
+  /* Decode the request */
+  req_msg = tr_msg_decode(tmp_ctx, req_str, req_len);
+  if (req_msg == NULL) {
+    tr_notice("tr_gss_handle_connection: Error decoding response");
+    goto cleanup;
   }
 
   /* Hand off the request for processing and get the response */
-  resp_str = req_cb(tmp_ctx, req_str, req_cookie);
+  resp_msg = req_cb(tmp_ctx, req_msg, req_cookie);
 
-  if (resp_str == NULL) {
+  if (resp_msg == NULL) {
     // no response, clean up
     goto cleanup;
   }
 
+  /* Encode the response */
+  resp_str = tr_msg_encode(tmp_ctx, resp_msg);
+  if (resp_str == NULL) {
+    /* We apparently can't encode a response, so just return */
+    tr_err("tr_gss_handle_connection: Error encoding response");
+    goto cleanup;
+  }
+
   // send the response
   if (tr_gss_write_resp(conn, gssctx, resp_str)) {
-    tr_notice("tr_gss_handle_connection: Error writing response");
+    tr_err("tr_gss_handle_connection: Error writing response");
+    goto cleanup;
   }
 
 cleanup:
index 0b19830..27fc00e 100644 (file)
@@ -41,8 +41,8 @@
 #include <jansson.h>
 #include <gmodule.h>
 #include <gssapi.h>
-
-//#include <trp_internal.h>
+#include <trust_router/tid.h>
+#include <trp_internal.h>
 #include <tr_gss_names.h>
 #include <tr_gss_client.h>
 #include <tr_name_internal.h>
@@ -126,8 +126,8 @@ struct mons_instance {
   const char *hostname;
   unsigned int port;
   TR_GSS_NAMES *authorized_gss_names;
-  void *tids; // TODO sort out header file cycles and use typed pointers
-  void *trps; // TODO sort out header file cycles and use typed pointers
+  TIDS_INSTANCE *tids;
+  TRPS_INSTANCE *trps;
   MONS_REQ_FUNC *req_handler;
   MONS_AUTH_FUNC *auth_handler;
   void *cookie;
index 30abc6d..d035a4e 100644 (file)
@@ -38,7 +38,7 @@
 #include <tr_msg.h>
 
 typedef int (TR_GSS_AUTH_FN)(gss_name_t, TR_NAME *, void *);
-typedef char *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, const char *, void *);
+typedef TR_MSG *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, TR_MSG *, void *);
 
 void tr_gss_handle_connection(int conn, const char *acceptor_name, const char *acceptor_realm, TR_GSS_AUTH_FN auth_cb,
                               void *auth_cookie, TR_GSS_HANDLE_REQ_FN req_cb, void *req_cookie);
index d79cbcb..d2a5c8a 100644 (file)
@@ -44,6 +44,8 @@
 #include <sys/wait.h>
 #include <tr_gss.h>
 
+#include "mons_handlers.h"
+
 /**
  * Allocate a new MONS_INSTANCE
  *
@@ -78,9 +80,30 @@ MONS_INSTANCE *mons_new(TALLOC_CTX *mem_ctx)
  * @param data pointer to a MONS_INSTANCE
  * @return pointer to the response string or null to send no response
  */
-static char *mons_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data)
+static TR_MSG *mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, void *data)
 {
-  return "This is a response.";
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
+  //MONS_INSTANCE *mons = talloc_get_type_abort(data, MONS_INSTANCE);
+  MON_REQ *req = NULL;
+  TR_MSG *resp_msg = NULL; /* This is the response value */
+
+  /* Validate inputs */
+  if (req_msg == NULL)
+    goto cleanup;
+
+  req = tr_msg_get_mon_req(req_msg);
+  if (req == NULL) {
+    /* this is an internal error */
+    tr_err("mons_req_cb: Received incorrect message type (was %d, expected %d)",
+           tr_msg_get_msg_type(req_msg),
+           MON_REQUEST);
+    /* TODO send an error response */
+    goto cleanup;
+  }
+
+cleanup:
+  talloc_free(tmp_ctx);
+  return resp_msg;
 }
 
 /**
index a94c902..2e5dbea 100644 (file)
@@ -255,41 +255,45 @@ int tids_send_response (TIDS_INSTANCE *tids, TID_REQ *req, TID_RESP *resp)
  * @param data pointer to a TIDS_INSTANCE
  * @return pointer to the response string or null to send no response
  */
-static char *tids_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data)
+static TR_MSG *tids_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *mreq, void *data)
 {
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
   TIDS_INSTANCE *tids = talloc_get_type_abort(data, TIDS_INSTANCE);
-  TR_MSG *mreq = NULL;
   TID_REQ *req = NULL;
   TID_RESP *resp = NULL;
-  char *resp_str = NULL;
+  TR_MSG *resp_msg = NULL; /* this is the return value */
   int rc = 0;
 
-  mreq = tr_msg_decode(NULL, req_str, strlen(req_str)); // allocates memory on success!
-  if (mreq == NULL) {
-    tr_debug("tids_req_cb: Error decoding request.");
-    return NULL;
-  }
-
   /* If this isn't a TID Request, just drop it. */
   if (mreq->msg_type != TID_REQUEST) {
-    tr_msg_free_decoded(mreq);
     tr_debug("tids_req_cb: Not a TID request, dropped.");
-    return NULL;
+    goto cleanup;
   }
 
   /* Get a handle on the request itself. Don't free req - it belongs to mreq */
   req = tr_msg_get_req(mreq);
 
-  /* Allocate a response structure and populate common fields. The resp is in req's talloc context,
-   * which will be cleaned up when mreq is freed. */
-  resp = tids_create_response(req, req);
+  /* Allocate a response message */
+  resp_msg = talloc(tmp_ctx, TR_MSG);
+  if (resp_msg == NULL) {
+    /* We cannot create a response message, so all we can really do is emit
+     * an error message and return. */
+    tr_crit("tids_req_cb: Error allocating response message.");
+    goto cleanup;
+  }
+
+  /* Allocate a response structure and populate common fields. Put it in the
+   * response message's talloc context. */
+  resp = tids_create_response(resp_msg, req);
   if (resp == NULL) {
     /* If we were unable to create a response, we cannot reply. Log an
      * error if we can, then drop the request. */
-    tr_msg_free_decoded(mreq);
     tr_crit("tids_req_cb: Error creating response structure.");
-    return NULL;
+    resp_msg = NULL; /* the contents are in tmp_ctx, so they will still be cleaned up */
+    goto cleanup;
   }
+  /* Now officially assign the response to the message. */
+  tr_msg_set_resp(resp_msg, resp);
 
   /* Handle the request and fill in resp */
   rc = tids_handle_request(tids, req, resp);
@@ -298,12 +302,12 @@ static char *tids_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data)
     /* Fall through, to send the response, either way */
   }
 
-  /* Convert the completed response into an encoded response */
-  resp_str = tids_encode_response(mem_ctx, resp);
+  /* put the response message in the caller's context */
+  talloc_steal(mem_ctx, resp_msg);
 
-  /* Finished; free the request and return */
-  tr_msg_free_decoded(mreq); // this frees req and resp, too
-  return resp_str;
+cleanup:
+  talloc_free(tmp_ctx);
+  return resp_msg;
 }
 
 TIDS_INSTANCE *tids_new(TALLOC_CTX *mem_ctx)