From 87d0426d2e8c8da5b28ae326e414e0b4535b3097 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 24 May 2018 13:34:20 -0400 Subject: [PATCH] Parse hostname/port for AAA server addresses * 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 | 6 +- common/tr_aaa_server.c | 141 ++++++++++++++++++++++++++++++++++++++++++++-- common/tr_config_realms.c | 37 ++++++++---- include/tr_aaa_server.h | 4 +- tr/tr_tid.c | 7 ++- 5 files changed, 173 insertions(+), 22 deletions(-) diff --git a/common/tests/commtest.c b/common/tests/commtest.c index 72a2844..d2becc0 100644 --- a/common/tests/commtest.c +++ b/common/tests/commtest.c @@ -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); diff --git a/common/tr_aaa_server.c b/common/tr_aaa_server.c index e8df13d..b008e5d 100644 --- a/common/tr_aaa_server.c +++ b/common/tr_aaa_server.c @@ -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; diff --git a/common/tr_config_realms.c b/common/tr_config_realms.c index 2f6fa09..881a881 100644 --- a/common/tr_config_realms.c +++ b/common/tr_config_realms.c @@ -55,30 +55,43 @@ 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; } diff --git a/include/tr_aaa_server.h b/include/tr_aaa_server.h index dfe356d..2c4cd20 100644 --- a/include/tr_aaa_server.h +++ b/include/tr_aaa_server.h @@ -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); diff --git a/tr/tr_tid.c b/tr/tr_tid.c index 0e324eb..b0f6602 100644 --- a/tr/tr_tid.c +++ b/tr/tr_tid.c @@ -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; } -- 2.1.4