From a3818d823167fd02fe29c0e53998aee4197b7969 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 30 May 2018 00:58:13 -0400 Subject: [PATCH] Refactor host validation and parsing, move methods out of tr_util.[ch] * Limit hostname validation to avoiding ambiguity about whether a port is part of the string * Refactor hostname/port parsing - new function is tr_parse_host() in tr_inet_util.c - handles both hostname and port - works with strings, not TR_NAME * Move hostname related methods out of tr_util.c Changes to make the rest of the codebase work with these updates will be in the next commit. --- common/tr_inet_util.c | 205 ++++++++++++++++++++++++------------------------- common/tr_util.c | 116 ---------------------------- include/tr_inet_util.h | 8 +- include/tr_util.h | 6 +- 4 files changed, 107 insertions(+), 228 deletions(-) diff --git a/common/tr_inet_util.c b/common/tr_inet_util.c index 65b746f..6d84918 100644 --- a/common/tr_inet_util.c +++ b/common/tr_inet_util.c @@ -32,9 +32,11 @@ * */ +#include #include #include #include +#include #include @@ -55,16 +57,6 @@ static int is_valid_address(int af, const char *s) } /** - * Determine whether a string is a valid IPv4 address - * @param s string to validate - * @return 1 if a valid reference, 0 otherwise - */ -int is_valid_ipv4_address(const char *s) -{ - return is_valid_address(AF_INET, s); -} - -/** * Determine whether a string is a valid IPv6 address reference * * I.e., an IPv6 address in brackets @@ -72,7 +64,7 @@ int is_valid_ipv4_address(const char *s) * @param s string to validate * @return 1 if a valid reference, 0 otherwise */ -int is_valid_ipv6_reference(const char *s) +static int tr_valid_ipv6_reference(const char *s) { char *cpy; size_t len; @@ -88,65 +80,53 @@ int is_valid_ipv6_reference(const char *s) return 0; /* make a null-terminated copy of the string omitting the brackets */ - cpy = strndup(s+1, len-2); + cpy = talloc_strndup(NULL, s+1, len-2); if (cpy == NULL) - return -1; + return 0; /* an error occurred - fail safe */ valid_ipv6 = is_valid_address(AF_INET6, cpy); - free(cpy); + talloc_free(cpy); return valid_ipv6; } -static int is_valid_dns_char(char c) +/** + * Validate a host string + * + * The intention is to reject strings that may appear to contain a ':port' spec. + * Takes a permissive view of valid: a hostname is valid if either it is a + * bracketed IPv6 address reference ([address]) or has no brackets or colons. + * This accepts all valid DNS names and IPv4 addresses, as well as many invalid + * hostnames. This is ok for accepting a hostname that will later be resolved + * because invalid names will fail to resolve. It should *not* be used to ensure + * a hostname is compliant with RFC! + * + * Ignores a trailing colon followed by decimal digits. + * + * @param s string to validate + * @return 1 if a valid host specification, 0 otherwise + */ +static int tr_valid_host(const char *s) { - /* digits ok */ - if ( ('0' <= c) && ('9' >= c)) - return 1; - - /* letters ok */ - if ( (('a' <= c) && ('z' >= c)) - || (('A' <= c) && ('Z' >= c)) ) - return 1; + if (strchr(s, '[') || strchr(s, ']') || strchr(s, ':')) + return tr_valid_ipv6_reference(s); - /* '-' ok */ - if ('-' == c) - return 1; - - /* everything else illegal */ - return 0; + return 1; } /** - * Helper to validate a DNS label - * - * Checks whether the string starting at s[start] and ending at s[end-1] - * is a valid DNS label. - * - * Does not check the length of the label. + * Check that all characters are decimal digits * * @param s - * @param start - * @param end - * @return + * @return 1 if all digits, 0 otherwise */ -static int is_valid_dns_label(const char *s, size_t start, size_t end) +static int tr_str_all_digits(const char *s) { - size_t ii; - - /* Check that there is at least one character. - * Be careful - size_t is unsigned */ - if (start >= end) - return 0; - - /* must neither start nor end with '-' */ - if ((s[start] == '-') - || s[end-1] == '-') + if (s == NULL) return 0; - /* make sure all characters are valid */ - for (ii=start; ii '9')) return 0; } @@ -154,69 +134,88 @@ static int is_valid_dns_label(const char *s, size_t start, size_t end) } /** - * Determine whether a string is a valid DNS name + * Validate and parse a hostname or hostname/port * - * Does not check the length of the name or of the indivdiual - * labels in it. + * If port_out is not null, accepts a port as well. This is + * stored in *port_out. If no port is given, a 0 is stored. + * If an invalid port is given, -1 is stored. * - * @param s string to validate - * @return 1 if a valid DNS name, 0 otherwise + * If the hostname is invalid, null is returned and no value + * is written to *port_out. + * + * If port_out is null, null will be returned if the string + * contains a port. + * + * The return value must be freed with talloc_free unless + * it is null. + * + * @param mem_ctx talloc context for hostname result + * @param s string to parse + * @param port_out pointer to an allocated integer, or NULL + * @return pointer to the hostname or null on error */ -int is_valid_dns_name(const char *s) +char *tr_parse_host(TALLOC_CTX *mem_ctx, const char *s, int *port_out) { - size_t label_start; - size_t label_end; + const char *colon; + char *hostname; + long int port; - /* reject some trivial cases */ - if ((s == NULL) - || (*s == '\0')) - return 0; + if (s == NULL) + return NULL; + + /* If we are accepting a port, find the last colon. */ + if (port_out == NULL) + colon = NULL; + else + colon = strrchr(s, ':'); + + /* Get a copy of the hostname portion, which may be the entire string. */ + if (colon == NULL) + hostname = talloc_strdup(NULL, s); + else + hostname = talloc_strndup(NULL, s, colon-s); + + if (hostname == NULL) + return NULL; /* failed to dup the hostname */ + + /* Check that the hostname is valid; if not, return null and ignore the port. */ + if (! tr_valid_host(hostname)) { + talloc_free(hostname); + return NULL; + } - /* Walk along with the end counter until we encounter a '.'. When that - * happens, we have a complete DNS label. Validate that, then set the start - * counter to one character past the end pointer, which will either be the - * next character in the DNS name or the null terminator. Since we stop as - * soon as the end counter reaches a null character, this will never refer - * to an invalid address. */ - for (label_start = 0, label_end = 0; - s[label_end] != '\0'; - label_end++) { - if (s[label_end] == '.') { - if (! is_valid_dns_label(s, label_start, label_end)) - return 0; - - label_start = label_end+1; + /* If we are accepting a port, parse and validate it. */ + if (port_out != NULL) { + if (colon == NULL) { + *port_out = 0; + } else { + port = strtol(colon+1, NULL, 10); + if (tr_str_all_digits(colon+1) && (port > 0) && (port <= 65535)) + *port_out = (int) port; + else + *port_out = -1; } } - if (s[label_start] == '\0') - return 1; /* we must have ended on a '.' */ - - /* There was one more label to validate */ - return is_valid_dns_label(s, label_start, label_end); + return hostname; } -/** - * Validate a host string - * - * Valid formats: - * IPv4 address (dotted quad) - * IPv6 address in brackets (e.g., [::1]) - * DNS hostname (labels made of alphanumerics or "-", separated by dots) - * - * @param s string to validate - * @return 1 if a valid host specification, 0 otherwise - */ -int is_valid_host(const char *s) +TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port) { - if (is_valid_ipv4_address(s)) - return 1; + TR_NAME *retval = NULL; + char *s = NULL; + char *hn_s = tr_name_strdup(hn); - if (is_valid_ipv6_reference(s)) - return 1; + if (!hn_s) + return NULL; - if (is_valid_dns_name(s)) - return 1; + s = talloc_asprintf(NULL, "%s:%d", hn_s, port); + free(hn_s); - return 0; -} + if (s) { + retval = tr_new_name(s); + talloc_free(s); + } + + return retval; +} \ No newline at end of file diff --git a/common/tr_util.c b/common/tr_util.c index ad10671..c3d03fb 100644 --- a/common/tr_util.c +++ b/common/tr_util.c @@ -36,10 +36,8 @@ #include #include #include -#include #include #include -#include void tr_bin_to_hex(const unsigned char * bin, size_t bin_len, char * hex_out, size_t hex_len) @@ -212,117 +210,3 @@ struct timespec *tr_clock_convert(clockid_t from, const struct timespec *when, } return dst; } - -TR_NAME *tr_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; -} - -/** - * Parse the port from a hostname:port string - * - * @param s string to parse - * @return the specified port, 0 if none specified, -1 if invalid - */ -int tr_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_port, &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 hostname and port - * - * @param s - * @param hn_dest - * @param p_dest - * @return 0 on success, -1 on error - */ -int tr_parse_hostname_and_port(const char *s, TR_NAME **hn_dest, int *p_dest) -{ - if ((hn_dest == NULL) || (p_dest == NULL)) - return -1; - - *hn_dest = tr_parse_hostname(s); - if (*hn_dest == NULL) - return -1; - - *p_dest = tr_parse_port(s); - if ((*p_dest < 0) || (*p_dest > 65535)) { - tr_free_name(*hn_dest); - *hn_dest = NULL; - return -1; - } - - return 0; -} - -TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port) -{ - TR_NAME *retval = NULL; - char *s = NULL; - char *hn_s = tr_name_strdup(hn); - - if (!hn_s) - return NULL; - - s = talloc_asprintf(NULL, "%s:%d", hn_s, port); - free(hn_s); - - if (s) { - retval = tr_new_name(s); - talloc_free(s); - } - - return retval; -} diff --git a/include/tr_inet_util.h b/include/tr_inet_util.h index 348332a..44d8ba5 100644 --- a/include/tr_inet_util.h +++ b/include/tr_inet_util.h @@ -36,10 +36,10 @@ #define TRUST_ROUTER_TR_INET_UTIL_H #include +#include -int is_valid_ipv4_address(const char *s); -int is_valid_ipv6_reference(const char *s); -int is_valid_dns_name(const char *s); -int is_valid_host(const char *s); +char *tr_parse_host(TALLOC_CTX *mem_ctx, const char *s, int *port_out); + +TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port); #endif //TRUST_ROUTER_TR_INET_UTIL_H diff --git a/include/tr_util.h b/include/tr_util.h index d42f875..9b2da06 100644 --- a/include/tr_util.h +++ b/include/tr_util.h @@ -35,7 +35,7 @@ #ifndef TR_UTIL_H #define TR_UTIL_H -#include +#include #include /* NB, tr_bin_to_hex() is also prototyped in trust_router/tr_dh.h */ @@ -47,9 +47,5 @@ int tr_sub_timespec(const struct timespec *ts1_copy, const struct timespec *ts2_ char *timespec_to_str(const struct timespec *ts); struct timespec *tr_clock_convert(clockid_t from, const struct timespec *when, clockid_t to, struct timespec *dst); -TR_NAME *tr_parse_hostname(const char *s); -int tr_parse_port(const char *s); -int tr_parse_hostname_and_port(const char *s, TR_NAME **hn_dest, int *p_dest); -TR_NAME *tr_hostname_and_port_to_name(TR_NAME *hn, int port); #endif /* TR_UTIL_H */ -- 2.1.4