Parse hostname/port for AAA server addresses
authorJennifer Richards <jennifer@painless-security.com>
Thu, 24 May 2018 17:34:20 +0000 (13:34 -0400)
committerJennifer Richards <jennifer@painless-security.com>
Thu, 24 May 2018 17:34:20 +0000 (13:34 -0400)
  * Add methods to create a TR_AAA_SERVER from a hostname:port string
    - also a version starting from a TR_NAME, which is a bit of a
      misuse of the TR_NAME
  * Update code to use the new methods instead
  * tr_aaa_server_new() no longer sets the hostname
  * tr_aaa_server_set_port() only uses default port when port == 0,
    otherwise allows any value
  * refactor tr_cfg_parse_one_aaa_server() to better use talloc
  * Raise error in tr_tids_req_handler() if AAA server allocation fails

common/tests/commtest.c
common/tr_aaa_server.c
common/tr_config_realms.c
include/tr_aaa_server.h
tr/tr_tid.c

index 72a2844..d2becc0 100644 (file)
@@ -239,11 +239,9 @@ struct aaa_entry {
 static TR_AAA_SERVER *aaa_entry_to_aaa_server(TALLOC_CTX *mem_ctx, struct aaa_entry *ae)
 {
   TALLOC_CTX *tmp_ctx=talloc_new(NULL);
-  TR_AAA_SERVER *aaa=tr_aaa_server_new(tmp_ctx, tr_new_name(ae->hostname));
+  TR_AAA_SERVER *aaa=tr_aaa_server_from_string(tmp_ctx, ae->hostname);
 
-  if ((aaa==NULL) || (aaa->hostname==NULL))
-    aaa=NULL;
-  else
+  if (aaa)
     talloc_steal(mem_ctx, aaa);
 
   talloc_free(tmp_ctx); 
index e8df13d..b008e5d 100644 (file)
@@ -47,12 +47,13 @@ static int tr_aaa_server_destructor(void *obj)
   return 0;
 }
 
-TR_AAA_SERVER *tr_aaa_server_new(TALLOC_CTX *mem_ctx, TR_NAME *hostname)
+TR_AAA_SERVER *tr_aaa_server_new(TALLOC_CTX *mem_ctx)
 {
   TR_AAA_SERVER *aaa=talloc(mem_ctx, TR_AAA_SERVER);
   if (aaa!=NULL) {
     aaa->next=NULL;
-    aaa->hostname=hostname;
+    aaa->hostname = NULL;
+    tr_aaa_server_set_port(aaa, 0); /* go through setter to guarantee consistent default */
     talloc_set_destructor((void *)aaa, tr_aaa_server_destructor);
   }
   return aaa;
@@ -63,6 +64,136 @@ void tr_aaa_server_free(TR_AAA_SERVER *aaa)
   talloc_free(aaa);
 }
 
+/**
+ * Parse the port from a hostname:port string
+ *
+ * @param s string to parse
+ * @return the specified port, 0 if none specified, -1 if invalid
+ */
+static int tr_aaa_server_parse_port(const char *s)
+{
+  const char *s_port;
+  char *end_of_conversion;
+  long int port; /* long instead of int because we use strtol */
+
+  /* Find the first colon */
+  s_port = strchr(s, ':'); /* port starts at s_port + 1 */
+  if (s_port == NULL)
+    return 0; /* no port */
+
+  /* Check that the last colon is the same as the first */
+  if (strrchr(s, ':') != s_port)
+    return -1; /* multiple colons are invalid*/
+
+  s_port += 1; /* port now starts at s_port */
+
+  /* Parse the port number */
+  port = strtol(s, &end_of_conversion, /* base */ 10);
+
+  /* validate */
+  if ((end_of_conversion == s_port) /* there was no port, just a colon */
+      || (*end_of_conversion != '\0') /* did not reach the end of the string */
+      || (port <= 0) || (port > 65535)) {
+    return -1;
+  }
+
+  return (int) port;
+}
+
+/**
+ * Parse a hostname out of a hostname:port string
+ *
+ * The ":port" section is optional. Ignores the string after the first colon.
+ * Does not validate the port section of the name.
+ *
+ * An empty hostname is allowed (but s must not be null)
+ *
+ * @param s
+ * @return TR_NAME or null on error (i.e., out-of-memory)
+ */
+static TR_NAME *tr_aaa_server_parse_hostname(const char *s)
+{
+  const char *colon;
+  char *hostname;
+  size_t hostname_len;
+  TR_NAME *retval;
+
+  if (s == NULL)
+    return NULL;
+
+  /* find the colon */
+  colon = strchr(s, ':');
+  if (colon == NULL)
+    return tr_new_name(s); /* there was no colon, take the whole string */
+
+  /* make a copy of the hostname portion of the string */
+  hostname_len = colon - s;
+  hostname = malloc(hostname_len + 1); /* +1 for the null termination */
+  if (hostname == NULL)
+    return NULL;
+
+  /* copy up to the colon, add a null termination, and make a TR_NAME */
+  strncpy(hostname, s, hostname_len);
+  hostname[hostname_len] = '\0';
+  retval = tr_new_name(hostname);
+
+  /* clean up and return */
+  free(hostname);
+  return retval;
+}
+
+/**
+ * Allocate a AAA server record and fill it in by parsing a hostname:port string
+ *
+ * Does not validate hostname or port values. The port will be -1 if the port
+ * could not be parsed properly.
+ *
+ * @return newly allocated TR_AAA_SERVER in the mem_ctx context, or NULL on error
+ */
+TR_AAA_SERVER *tr_aaa_server_from_string(TALLOC_CTX *mem_ctx, const char *s)
+{
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
+  TR_AAA_SERVER *aaa = tr_aaa_server_new(tmp_ctx);
+
+  if (aaa == NULL)
+    goto failed;
+
+  tr_aaa_server_set_hostname(aaa, tr_aaa_server_parse_hostname(s));
+  if (tr_aaa_server_get_hostname(aaa) == NULL)
+    goto failed;
+
+  tr_aaa_server_set_port(aaa, tr_aaa_server_parse_port(s));
+  talloc_steal(mem_ctx, aaa); /*put this in the caller's context */
+  goto succeeded;
+
+failed:
+  aaa = NULL; /* talloc will free the memory if it was allocated */
+
+succeeded:
+  talloc_free(tmp_ctx);
+  return aaa;
+}
+
+
+/**
+ * Allocate a AAA server record and fill it in by parsing a hostname:port TR_NAME
+ *
+ * Does not validate hostname or port values. The port will be -1 if the port
+ * could not be parsed properly.
+ *
+ * @return newly allocated TR_AAA_SERVER in the mem_ctx context, or NULL on error
+ */
+TR_AAA_SERVER *tr_aaa_server_from_name(TALLOC_CTX *mem_ctx, TR_NAME *n)
+{
+  TR_AAA_SERVER *aaa = NULL;
+  char *s = tr_name_strdup(n);
+  if (s != NULL) {
+    aaa = tr_aaa_server_from_string(mem_ctx, s);
+    free(s);
+  }
+  return aaa;
+}
+
 TR_AAA_SERVER_ITER *tr_aaa_server_iter_new(TALLOC_CTX *mem_ctx)
 {
   return talloc(mem_ctx, TR_AAA_SERVER_ITER);
@@ -120,7 +251,9 @@ int tr_aaa_server_get_port(TR_AAA_SERVER *aaa)
 /**
  * Set the port for a AAA server
  *
- * If port is outside the range 1-65535, uses the standard TID port (12309).
+ * If port is 0, uses the standard TID port (12309). Other invalid values are stored
+ * as-is.
+ *
  * Does nothing if aaa is null.
  *
  * @param aaa
@@ -131,7 +264,7 @@ void tr_aaa_server_set_port(TR_AAA_SERVER *aaa, int port)
   if (aaa == NULL)
     return;
 
-  if ((port <= 0) || (port > 65535))
+  if (port == 0)
     port = TID_PORT;
 
   aaa->port = port;
index 2f6fa09..881a881 100644 (file)
 
 TR_AAA_SERVER *tr_cfg_parse_one_aaa_server(TALLOC_CTX *mem_ctx, json_t *jaddr, TR_CFG_RC *rc)
 {
+  TALLOC_CTX *tmp_ctx = talloc_new(NULL);
   TR_AAA_SERVER *aaa = NULL;
-  TR_NAME *name=NULL;
 
   if ((!jaddr) || (!json_is_string(jaddr))) {
     tr_debug("tr_cfg_parse_one_aaa_server: Bad parameters.");
     *rc = TR_CFG_BAD_PARAMS;
-    return NULL;
+    goto cleanup;
   }
 
-  name=tr_new_name(json_string_value(jaddr));
-  if (name==NULL) {
-    tr_debug("tr_cfg_parse_one_aaa_server: Out of memory allocating hostname.");
+  aaa = tr_aaa_server_from_string(mem_ctx, json_string_value(jaddr));
+  if (aaa == NULL) {
+    tr_debug("tr_cfg_parse_one_aaa_server: Out of memory allocating AAA server.");
     *rc = TR_CFG_NOMEM;
-    return NULL;
+    goto cleanup
   }
 
-  aaa=tr_aaa_server_new(mem_ctx, name);
-  if (aaa==NULL) {
-    tr_free_name(name);
-    tr_debug("tr_cfg_parse_one_aaa_server: Out of memory allocating AAA server.");
-    *rc = TR_CFG_NOMEM;
-    return NULL;
+  if (tr_aaa_server_get_hostname(aaa)->len == 0) {
+    tr_debug("tr_cfg_parse_one_aaa_server: Empty hostname for AAA server not allowed");
+    *rc = TR_CFG_NOPARSE;
+    goto cleanup;
+  }
+
+  if ((tr_aaa_server_get_port(aaa) <= 0)
+      || (tr_aaa_server_get_port(aaa) > 65535)) {
+    tr_debug("tr_cfg_parse_one_aaa_server: Invalid AAA server port");
+    *rc = TR_CFG_NOPARSE;
+    goto cleanup;
   }
 
+  /* success ! */
+  *rc = TR_CFG_SUCCESS;
+  talloc_steal(mem_ctx, aaa);
+
+cleanup:
+  if (*rc != TR_CFG_SUCCESS)
+    aaa = NULL;
+  talloc_free(tmp_ctx);
   return aaa;
 }
 
index dfe356d..2c4cd20 100644 (file)
@@ -49,13 +49,15 @@ typedef struct tr_aaa_server_iter {
   TR_AAA_SERVER *this;
 } TR_AAA_SERVER_ITER;
 
-TR_AAA_SERVER *tr_aaa_server_new(TALLOC_CTX *mem_ctx, TR_NAME *hostname);
+TR_AAA_SERVER *tr_aaa_server_new(TALLOC_CTX *mem_ctx);
 void tr_aaa_server_free(TR_AAA_SERVER *aaa);
 
 TR_NAME *tr_aaa_server_get_hostname(TR_AAA_SERVER *aaa);
 void tr_aaa_server_set_hostname(TR_AAA_SERVER *aaa, TR_NAME *hostname);
 int tr_aaa_server_get_port(TR_AAA_SERVER *aaa);
 void tr_aaa_server_set_port(TR_AAA_SERVER *aaa, int port);
+TR_AAA_SERVER *tr_aaa_server_from_string(TALLOC_CTX *mem_ctx, const char *s);
+TR_AAA_SERVER *tr_aaa_server_from_name(TALLOC_CTX *mem_ctx, TR_NAME *n);
 
 TR_AAA_SERVER_ITER *tr_aaa_server_iter_new(TALLOC_CTX *mem_ctx);
 void tr_aaa_server_iter_free(TR_AAA_SERVER_ITER *iter);
index 0e324eb..b0f6602 100644 (file)
@@ -511,7 +511,12 @@ static int tr_tids_req_handler(TIDS_INSTANCE *tids,
                                              &idp_shared);
     } else {
       tr_debug("tr_tids_req_handler: route not local.");
-      aaa_servers = tr_aaa_server_new(tmp_ctx, trp_route_get_next_hop(route));
+      aaa_servers = tr_aaa_server_from_name(tmp_ctx, trp_route_get_next_hop(route)); /* cleaned up via talloc */
+      if (aaa_servers == NULL) {
+        tr_err("tr_tids_req_handler: error allocating next hop");
+        retval=-1;
+        goto cleanup;
+      }
       idp_shared = 0;
     }