Thread: [Accel-ppp-users] [PATCH 0/3] utils: implement IPv4 parsing primitive for iprange
Status: Beta
Brought to you by:
xebd
From: Guillaume N. <g....@al...> - 2018-12-07 16:37:54
|
This series extends utils.c with IPv4 parsing functions. The objective is to simplify the code having to deal with IPv4 text representations (for parsing and for printing). Patch 1 redefines u_parse_ip4addr() to make it work like its IPv6 counterpart. Patch 2 defines u_ip4str(), to easily print an IPv4 from a printf()-like function. It also adds u_parse_ip4cidr() and u_parse_ip4range() to simplify the parsing of IPv4 addresse ranges. Patch 3 uses these new functions to simplify the parsing of the [client-ip-range] section (accel-ppp.conf). Next, I'm planning to rework the RADIUS Framed-Route attribute parsing if this approach is considered useful. Guillaume Nault (3): utils: rework u_parse_ip4addr() utils: add IPv4 string parsing helpers iprange: rework range parsing using u_parse_*() functions accel-pppd/iprange.c | 159 +++++++++++++++++-------------------------- accel-pppd/utils.c | 130 ++++++++++++++++++++++++++++------- accel-pppd/utils.h | 7 +- 3 files changed, 172 insertions(+), 124 deletions(-) -- 2.20.0.rc1 |
From: Guillaume N. <g....@al...> - 2018-12-07 16:37:55
|
Define the IPv4 counterparts of u_ip6str() and u_parse_ip6cidr(). Also add the special u_parse_ip4range() which will be useful for parsing the [client-ip-range] section of accel-ppp.conf. Signed-off-by: Guillaume Nault <g....@al...> --- accel-pppd/utils.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++ accel-pppd/utils.h | 3 ++ 2 files changed, 82 insertions(+) diff --git a/accel-pppd/utils.c b/accel-pppd/utils.c index b1590544..dd928c1a 100644 --- a/accel-pppd/utils.c +++ b/accel-pppd/utils.c @@ -30,6 +30,21 @@ char __export *u_ip6str(const struct in6_addr *addr, char *buf) return buf; } +/* Convenient wrapper around inet_ntop() to print IPv4 addresses. + * It stores a string representation of addr into buf, which must be at + * least INET_ADDRSTRLEN bytes long. + * + * Returns buf, which is guaranteed to contain a valid string even if an error + * occured. + */ +char __export *u_ip4str(const struct in_addr *addr, char *buf) +{ + if (!inet_ntop(AF_INET, addr, buf, INET_ADDRSTRLEN)) + snprintf(buf, INET_ADDRSTRLEN, "< ERROR! >"); + + return buf; +} + void __export u_inet_ntoa(in_addr_t addr, char *str) { addr = ntohl(addr); @@ -242,6 +257,70 @@ size_t __export u_parse_ip6cidr(const char *str, struct in6_addr *netp, uint8_t return ptr - str; } +/* Parse an IPv4 network prefix in CIDR notation (for example "192.0.2.0/24"). + * The IP address must be in dotted-decimal format. + * Returns the number of bytes parsed, or 0 if str doesn't start with an IPv4 + * network prefix. + */ +size_t __export u_parse_ip4cidr(const char *str, struct in_addr *netp, uint8_t *plen) +{ + const char *ptr = str; + size_t len; + + len = u_parse_ip4addr(ptr, netp); + if (!len) + return 0; + + ptr += len; + if (*ptr != '/') + return 0; + + len = u_parse_u8(++ptr, plen); + if (!len) + return 0; + + if (*plen > 32) + return 0; + + ptr += len; + + return ptr - str; +} + +/* Parse an IPv4 address range (for example "192.0.2.0-255"). + * The IP address must be in dotted-decimal format. The number following '-' + * is the upper bound of the address' least significant byte (the lower bound + * is given by the address itself). The upper bound must be bigger or equal + * than the lower bound. + * + * Returns the number of bytes parsed, or 0 if str doesn't start with an IPv4 + * address range. + */ +size_t __export u_parse_ip4range(const char *str, struct in_addr *base_ip, uint8_t *max) +{ + const char *ptr = str; + size_t len; + + len = u_parse_ip4addr(ptr, base_ip); + if (!len) + return 0; + + ptr += len; + if (*ptr != '-') + return 0; + + len = u_parse_u8(++ptr, max); + if (!len) + return 0; + + if (*max < (ntohl(base_ip->s_addr) & 0xff)) + return 0; + + ptr += len; + + return ptr - str; +} + int __export u_randbuf(void *buf, size_t buf_len, int *err) { uint8_t *u8buf = buf; diff --git a/accel-pppd/utils.h b/accel-pppd/utils.h index a7c38973..06859a6b 100644 --- a/accel-pppd/utils.h +++ b/accel-pppd/utils.h @@ -5,6 +5,7 @@ #include <stdint.h> char *u_ip6str(const struct in6_addr *addr, char *buf); +char *u_ip4str(const struct in_addr *addr, char *buf); void u_inet_ntoa(in_addr_t, char *str); int u_readlong(long int *dst, const char *src, long int min, long int max); @@ -20,6 +21,8 @@ size_t u_parse_ip6addr(const char *str, struct in6_addr *addr); size_t u_parse_ip4addr(const char *str, struct in_addr *addr); size_t u_parse_ip6cidr(const char *str, struct in6_addr *netp, uint8_t *plen); +size_t u_parse_ip4cidr(const char *str, struct in_addr *netp, uint8_t *plen); +size_t u_parse_ip4range(const char *str, struct in_addr *base_ip, uint8_t *max); int u_randbuf(void *buf, size_t buf_len, int *err); -- 2.20.0.rc1 |
From: Guillaume N. <g....@al...> - 2018-12-07 16:37:57
|
Redefine u_parse_ip4addr() to match the behaviour of other u_parse_*() functions: * Drop the err_msg parameter. * Return the number of bytes parsed instead of an error number. * Remove support for fancy IPv4 address notations. There is currently only one user of u_parse_ip4addr() (in iprange.c). Dropping the fancy IPv4 address representations is probably not going to harm anyone (quite the opposite as many users don't realise that leading 0 means octal and that plain integers can be considered IPv4 addresses). Signed-off-by: Guillaume Nault <g....@al...> --- accel-pppd/iprange.c | 3 +-- accel-pppd/utils.c | 51 ++++++++++++++++++++++---------------------- accel-pppd/utils.h | 4 ++-- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/accel-pppd/iprange.c b/accel-pppd/iprange.c index f7b77a27..40c938e2 100644 --- a/accel-pppd/iprange.c +++ b/accel-pppd/iprange.c @@ -51,7 +51,6 @@ static int parse_iprange(const char *str, struct iprange_t **range) char ipstr[CIDR_MAXLEN + 1] = { 0 }; struct iprange_t *_range; struct in_addr addr; - const char *errmsg; char *suffix_str; uint32_t ipmin; uint32_t ipmax; @@ -88,7 +87,7 @@ static int parse_iprange(const char *str, struct iprange_t **range) *suffix_str = '\0'; ++suffix_str; - if (u_parse_ip4addr(ipstr, &addr, &errmsg)) { + if (!u_parse_ip4addr(ipstr, &addr)) { log_error("iprange: impossible to parse range \"%s\":" " invalid IPv4 address \"%s\"\n", str, ipstr); diff --git a/accel-pppd/utils.c b/accel-pppd/utils.c index 544c59e7..b1590544 100644 --- a/accel-pppd/utils.c +++ b/accel-pppd/utils.c @@ -1,7 +1,6 @@ #include <arpa/inet.h> #include <ctype.h> #include <errno.h> -#include <netdb.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h> @@ -167,31 +166,6 @@ size_t __export u_parse_u32(const char *str, uint32_t *val) return endptr - str; } -int __export u_parse_ip4addr(const char *src, struct in_addr *addr, - const char **err_msg) -{ - struct addrinfo hint = { - .ai_flags = AI_NUMERICHOST, - .ai_family = AF_INET, - .ai_socktype = 0, - .ai_protocol = 0, - }; - struct addrinfo *ainfo; - int err; - - err = getaddrinfo(src, NULL, &hint, &ainfo); - if (err) { - *err_msg = gai_strerror(err); - return err; - } - - *addr = ((struct sockaddr_in *)ainfo->ai_addr)->sin_addr; - - freeaddrinfo(ainfo); - - return 0; -} - /* Parse an IPv6 address (for example "2001:db8::1"). * Returns the number of bytes parsed, or 0 if str doesn't start with an IPv6 * address. @@ -214,6 +188,31 @@ size_t __export u_parse_ip6addr(const char *str, struct in6_addr *addr) return len; } +/* Parse an IPv4 address in dotted-decimal format (for example "198.51.100.1"). + * Other formats (hex "0xc6.0x33.0x64.0x1", octal "0306.063.0144.01", mixed + * "0xc6.51.0144.1", non dotted-quad "198.51.25601"...) are rejected. + * + * Returns the number of bytes parsed, or 0 if str doesn't start with an IPv4 + * address. + */ +size_t __export u_parse_ip4addr(const char *str, struct in_addr *addr) +{ + char buf[INET_ADDRSTRLEN]; + size_t len; + + len = strspn(str, ".0123456789"); + if (!len || len >= sizeof(buf)) + return 0; + + memcpy(buf, str, len); + buf[len] = '\0'; + + if (inet_pton(AF_INET, buf, addr) != 1) + return 0; + + return len; +} + /* Parse an IPv6 network prefix in CIDR notation (for example "2001:db8::/32"). * Returns the number of bytes parsed, or 0 if str doesn't start with an IPv6 * network prefix. diff --git a/accel-pppd/utils.h b/accel-pppd/utils.h index d3e06083..a7c38973 100644 --- a/accel-pppd/utils.h +++ b/accel-pppd/utils.h @@ -16,9 +16,9 @@ size_t u_parse_u8(const char *str, uint8_t *val); size_t u_parse_u16(const char *str, uint16_t *val); size_t u_parse_u32(const char *str, uint32_t *val); -int u_parse_ip4addr(const char *src, struct in_addr *addr, - const char **err_msg); size_t u_parse_ip6addr(const char *str, struct in6_addr *addr); +size_t u_parse_ip4addr(const char *str, struct in_addr *addr); + size_t u_parse_ip6cidr(const char *str, struct in6_addr *netp, uint8_t *plen); int u_randbuf(void *buf, size_t buf_len, int *err); -- 2.20.0.rc1 |
From: Guillaume N. <g....@al...> - 2018-12-07 16:37:58
|
Now that we have primitives for parsing IPv4 ranges, let's use them to simplify parse_iprange(). Try u_parse_ip4cidr() first. In case of failure, try u_parse_ip4range(). If any of them succeeds, verify that there aren't spurious data following the range definition. If everything is valid, either load the range or disable the module (if the range is 0.0.0.0/0). The diff is a bit ugly, but the implementation should be much clearer. Signed-off-by: Guillaume Nault <g....@al...> --- accel-pppd/iprange.c | 158 +++++++++++++++++-------------------------- 1 file changed, 63 insertions(+), 95 deletions(-) diff --git a/accel-pppd/iprange.c b/accel-pppd/iprange.c index 40c938e2..58321bd3 100644 --- a/accel-pppd/iprange.c +++ b/accel-pppd/iprange.c @@ -29,12 +29,6 @@ static pthread_mutex_t iprange_lock = PTHREAD_MUTEX_INITIALIZER; static bool conf_disable = false; static LIST_HEAD(client_ranges); -/* Maximum IPv4 address length with CIDR notation but no extra 0, - * e.g. "0xff.0xff.0xff.0xff/32". - */ -#define CIDR_MAXLEN 22 - - static void free_ranges(struct list_head *head) { struct iprange_t *range; @@ -46,115 +40,89 @@ static void free_ranges(struct list_head *head) } } +/* Parse a [client-ip-iprange] configuration entry. + * Ranges can be defined in CIDR notation ("192.0.2.0/24") or by specifying an + * upper bound for the last IPv4 byte, after a '-' character ("192.0.2.0-255"). + * For simplicity, only mention the CIDR notation in error messages. + */ static int parse_iprange(const char *str, struct iprange_t **range) { - char ipstr[CIDR_MAXLEN + 1] = { 0 }; - struct iprange_t *_range; - struct in_addr addr; - char *suffix_str; - uint32_t ipmin; - uint32_t ipmax; - bool is_cidr; - - /* Extra spaces and comments must have already been removed */ - if (strpbrk(str, " \t#")) { - log_error("iprange: impossible to parse range \"%s\":" - " invalid space or comment character found\n", - str); - return -1; - } + struct iprange_t *new_range; + struct in_addr base_addr; + const char *ptr; + uint32_t ip_min; + uint32_t ip_max; + uint8_t suffix; + size_t len; if (!strcmp(str, "disable")) goto disable; - strncpy(ipstr, str, CIDR_MAXLEN + 1); - if (ipstr[CIDR_MAXLEN] != '\0') { - log_error("iprange: impossible to parse range \"%s\":" - " line too long\n", - str); - return -1; - } - - suffix_str = strpbrk(ipstr, "-/"); - if (!suffix_str) { - log_error("iprange: impossible to parse range \"%s\":" - " unrecognised range format\n", - str); - return -1; - } - - is_cidr = *suffix_str == '/'; - *suffix_str = '\0'; - ++suffix_str; - - if (!u_parse_ip4addr(ipstr, &addr)) { - log_error("iprange: impossible to parse range \"%s\":" - " invalid IPv4 address \"%s\"\n", - str, ipstr); - return -1; - } - ipmin = ntohl(addr.s_addr); + ptr = str; - - /* If is_cidr is set, range is given with CIDR notation, - * e.g. "192.0.2.0/24". - * If unset, range is an IP address where the last octet is replaced by - * an octet range, e.g. "192.0.2.0-255". - */ - if (is_cidr) { - long int prefix_len; + /* Try IPv4 CIDR notation first */ + len = u_parse_ip4cidr(ptr, &base_addr, &suffix); + if (len) { + uint32_t addr_hbo; uint32_t mask; - if (u_readlong(&prefix_len, suffix_str, 0, 32)) { - log_error("iprange: impossible to parse range \"%s\":" - " invalid CIDR prefix length \"/%s\"\n", - str, suffix_str); - return -1; - } + /* Cast to uint64_t to avoid undefined 32 bits shift on 32 bits + * integer if 'suffix' is 0. + */ + mask = (uint64_t)0xffffffff << (32 - suffix); + addr_hbo = ntohl(base_addr.s_addr); + ip_min = addr_hbo & mask; + ip_max = addr_hbo | ~mask; + + if (ip_min != addr_hbo) { + struct in_addr min_addr = { .s_addr = htonl(ip_min) }; + char ipbuf[INET_ADDRSTRLEN]; - /* Interpret /0 as disable request */ - if (prefix_len == 0) { - if (ipmin != INADDR_ANY) - log_warn("iprange: %s is equivalent to 0.0.0.0/0 and disables the iprange module\n", - str); - goto disable; + log_warn("iprange: network %s is equivalent to %s/%hhu\n", + str, u_ip4str(&min_addr, ipbuf), suffix); } + goto addrange; + } - mask = INADDR_BROADCAST << (32 - prefix_len); - if (ipmin != (ipmin & mask)) { - char buf[INET_ADDRSTRLEN] = { 0 }; + /* Not an IPv4 CIDR, try the IPv4 range notation */ + len = u_parse_ip4range(ptr, &base_addr, &suffix); + if (len) { + ip_min = ntohl(base_addr.s_addr); + ip_max = (ip_min & 0xffffff00) | suffix; + goto addrange; + } - ipmin &= mask; - addr.s_addr = htonl(ipmin); - log_warn("iprange: first IP of range %s will be %s\n", - str, inet_ntop(AF_INET, &addr, buf, - sizeof(buf))); - } + log_error("iprange: parsing range \"%s\" failed:" + " expecting an IPv4 network prefix in CIDR notation\n", + str); - ipmax = ipmin | ~mask; - } else { - long int max; + return -1; - if (u_readlong(&max, suffix_str, ipmin & 0xff, 255)) { - log_error("iprange: impossible to parse range \"%s\":" - " invalid upper bound \"-%s\"\n", - str, suffix_str); - return -1; - } +addrange: + ptr += len; - ipmax = (ipmin & 0xffffff00) | max; + if (!u_parse_endstr(ptr)) { + log_error("iprange: parsing range \"%s\" failed:" + " unexpected data at \"%s\"\n", + str, ptr); + return -1; } - _range = _malloc(sizeof(*_range)); - if (!_range) { - log_error("iprange: impossible to allocate range \"%s\":" - " memory allocation failed\n", str); + if (ip_min == INADDR_ANY && ip_max == INADDR_BROADCAST) + goto disable; + + new_range = _malloc(sizeof(*new_range)); + if (!new_range) { + log_error("iprange: impossible to load range \"%s\":" + " memory allocation failed\n", + str); return -1; } - _range->begin = ipmin; - _range->end = ipmax; - *range = _range; + new_range->begin = ip_min; + new_range->end = ip_max; + + *range = new_range; return 0; @@ -175,7 +143,7 @@ static bool load_ranges(struct list_head *list, const char *conf_sect) list_for_each_entry(opt, &s->items, entry) { /* Ignore parsing errors, parse_iprange() already logs suitable - * error message. + * error messages. */ if (parse_iprange(opt->name, &r) < 0) continue; -- 2.20.0.rc1 |