Clean up parsing of port ranges

conf_ports() parses ranges of ports for the -t, -u, -T and -U options.
The code is quite difficult to the follow, to the point that clang-tidy
and cppcheck disagree on whether one of the pointers can be NULL at some
points.

Rework the code with the use of two new helper functions:
  * parse_port_range() operates a bit like strtoul(), but can parse a whole
    port range specification (e.g. '80' or '1000-1015')
  * next_chunk() does the necessary wrapping around strchr() to advance to
    just after the next given delimiter, while cleanly handling if there
    are no more delimiters

The new version is easier to follow, and also removes some cppcheck
warnings.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2022-09-28 14:33:12 +10:00 committed by Stefano Brivio
parent a1a058533c
commit 84fec4e998

254
conf.c
View file

@ -106,6 +106,66 @@ static int get_bound_ports_ns(void *arg)
return 0; return 0;
} }
/**
* next_chunk - Return the next piece of a string delimited by a character
* @s: String to search
* @c: Delimiter character
*
* Returns: If another @c is found in @s, returns a pointer to the
* character *after* the delimiter, if no further @c is in
* @s, return NULL
*/
static char *next_chunk(const char *s, char c)
{
char *sep = strchr(s, c);
return sep ? sep + 1 : NULL;
}
/**
* port_range - Represents a non-empty range of ports
* @first: First port number in the range
* @last: Last port number in the range (inclusive)
*
* Invariant: @last >= @first
*/
struct port_range {
in_port_t first, last;
};
/**
* parse_port_range() - Parse a range of port numbers '<first>[-<last>]'
* @s: String to parse
* @endptr: Update to the character after the parsed range (similar to
* strtol() etc.)
* @range: Update with the parsed values on success
*
* Return: -EINVAL on parsing error, -ERANGE on out of range port
* numbers, 0 on success
*/
static int parse_port_range(const char *s, char **endptr,
struct port_range *range)
{
unsigned long first, last;
last = first = strtoul(s, endptr, 10);
if (*endptr == s) /* Parsed nothing */
return -EINVAL;
if (**endptr == '-') { /* we have a last value too */
const char *lasts = *endptr + 1;
last = strtoul(lasts, endptr, 10);
if (*endptr == lasts) /* Parsed nothing */
return -EINVAL;
}
if ((last < first) || (last >= NUM_PORTS))
return -ERANGE;
range->first = first;
range->last = last;
return 0;
}
/** /**
* conf_ports() - Parse port configuration options, initialise UDP/TCP sockets * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets
* @c: Execution context * @c: Execution context
@ -119,11 +179,11 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
struct port_fwd *fwd) struct port_fwd *fwd)
{ {
char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf; char addr_buf[sizeof(struct in6_addr)] = { 0 }, *addr = addr_buf;
int start_src, end_src, start_dst, end_dst, exclude_only = 1, i;
uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 };
char buf[BUFSIZ], *sep, *spec, *p; char buf[BUFSIZ], *spec, *p;
sa_family_t af = AF_UNSPEC; sa_family_t af = AF_UNSPEC;
unsigned port; bool exclude_only = true;
unsigned i;
if (!strcmp(optarg, "none")) { if (!strcmp(optarg, "none")) {
if (fwd->mode) if (fwd->mode)
@ -183,65 +243,29 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
addr = NULL; addr = NULL;
} }
if (strspn(spec, "0123456789-,:~") != strlen(spec))
goto bad;
/* Mark all exclusions first, they might be given after base ranges */ /* Mark all exclusions first, they might be given after base ranges */
p = spec; p = spec;
start_src = end_src = -1;
do { do {
while (*p != '~' && start_src == -1) { struct port_range xrange;
exclude_only = 0;
if (!(p = strchr(p, ','))) if (*p != '~') {
break; /* Not an exclude range, parse later */
exclude_only = false;
p++; continue;
} }
if (!p || !*p)
break;
if (*p == '~') if (parse_port_range(p, &p, &xrange))
p++; goto bad;
if ((*p != '\0') && (*p != ',')) /* Garbage after the range */
errno = 0;
port = strtoul(p, &sep, 10);
if (sep == p)
break;
if (port >= NUM_PORTS || errno)
goto bad; goto bad;
switch (*sep) { for (i = xrange.first; i <= xrange.last; i++) {
case '-': if (bitmap_isset(exclude, i))
if (start_src == -1) /* ~22-... */ goto overlap;
start_src = port;
break;
case ',':
case 0:
if (start_src == -1) /* ~80 */
start_src = end_src = port;
else if (end_src == -1) /* ~22-25 */
end_src = port;
else
goto bad;
if (start_src > end_src) /* ~80-22 */ bitmap_set(exclude, i);
goto bad;
for (i = start_src; i <= end_src; i++) {
if (bitmap_isset(exclude, i))
goto overlap;
bitmap_set(exclude, i);
}
start_src = end_src = -1;
break;
default:
goto bad;
} }
p = sep + 1; } while ((p = next_chunk(p, ',')));
} while (*sep);
if (exclude_only) { if (exclude_only) {
for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { for (i = 0; i < PORT_EPHEMERAL_MIN; i++) {
@ -260,109 +284,47 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
} }
/* Now process base ranges, skipping exclusions */ /* Now process base ranges, skipping exclusions */
start_src = end_src = start_dst = end_dst = -1;
p = spec; p = spec;
do { do {
while (*p == '~') { struct port_range orig_range, mapped_range;
if (!(p = strchr(p, ',')))
break;
p++;
}
if (!p || !*p)
break;
errno = 0; if (*p == '~')
port = strtoul(p, &sep, 10); /* Exclude range, already parsed */
if (sep == p) continue;
break;
if (port >= NUM_PORTS || errno) if (parse_port_range(p, &p, &orig_range))
goto bad; goto bad;
/* -p 22 if (*p == ':') { /* There's a range to map to as well */
* ^ start_src end_src == start_dst == end_dst == -1 if (parse_port_range(p + 1, &p, &mapped_range))
*
* -p 22-25
* | ^ end_src
* ` start_src start_dst == end_dst == -1
*
* -p 80:8080
* | ^ start_dst
* ` start_src end_src == end_dst == -1
*
* -p 22-80:8022-8080
* | | | ^ end_dst
* | | ` start_dst
* | ` end_dst
* ` start_src
*/
switch (*sep) {
case '-':
if (start_src == -1) { /* 22-... */
start_src = port;
} else {
if (!end_src) /* 22:8022-8080 */
goto bad;
start_dst = port; /* 22-80:8022-... */
}
break;
case ':':
if (start_src == -1) /* 80:... */
start_src = end_src = port;
else if (end_src == -1) /* 22-80:... */
end_src = port;
else /* 22-80:8022:... */
goto bad; goto bad;
break; if ((mapped_range.last - mapped_range.first) !=
case ',': (orig_range.last - orig_range.first))
case 0:
if (start_src == -1) /* 80 */
start_src = end_src = port;
else if (end_src == -1) /* 22-25 */
end_src = port;
else if (start_dst == -1) /* 80:8080 */
start_dst = end_dst = port;
else if (end_dst == -1) /* 22-80:8022-8080 */
end_dst = port;
else
goto bad; goto bad;
} else {
if (start_src > end_src) /* 80-22 */ mapped_range = orig_range;
goto bad;
if (start_dst > end_dst) /* 22-80:8080:8022 */
goto bad;
if (end_dst != -1 &&
end_dst - start_dst != end_src - start_src)
goto bad; /* 22-81:8022:8080 */
for (i = start_src; i <= end_src; i++) {
if (bitmap_isset(fwd->map, i))
goto overlap;
if (bitmap_isset(exclude, i))
continue;
bitmap_set(fwd->map, i);
if (start_dst != -1) {
/* 80:8080 or 22-80:8080:8080 */
fwd->delta[i] = (in_port_t)(start_dst -
start_src);
}
if (optname == 't')
tcp_sock_init(c, 0, af, addr, i);
else if (optname == 'u')
udp_sock_init(c, 0, af, addr, i);
}
start_src = end_src = start_dst = end_dst = -1;
break;
} }
p = sep + 1;
} while (*sep); if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */
goto bad;
for (i = orig_range.first; i <= orig_range.last; i++) {
if (bitmap_isset(fwd->map, i))
goto overlap;
if (bitmap_isset(exclude, i))
continue;
bitmap_set(fwd->map, i);
fwd->delta[i] = mapped_range.first - orig_range.first;
if (optname == 't')
tcp_sock_init(c, 0, af, addr, i);
else if (optname == 'u')
udp_sock_init(c, 0, af, addr, i);
}
} while ((p = next_chunk(p, ',')));
return 0; return 0;
bad: bad: