MEDIUM: tools: make str2sa_range() validate callers' port specifications

Now str2sa_range() will enforce the caller's port specification passed
using the PA_O_PORT_* flags, and will return an error on failure. For
optional ports, values 0-65535 will be enforced. For mandatory ports,
values 1-65535 are enforced. In case of ranges, it is also verified that
the upper bound is not lower than the lower bound, as this used to result
in empty listeners.

I couldn't find an easy way to test this using VTC since the purpose is
to trigger parse errors, so instead a test file is provided as
tests/ports.cfg with comments about what errors are expected for each
line.

diff --git a/src/check.c b/src/check.c
index c326c00a29b2779c5c77e20accbacdd263d69244..1d3a307b63da3823fc9b2f9e644600e16bda4d5c 100644 (file)
--- a/src/check.c
+++ b/src/check.c
@@ -2638,7 +2638,8 @@ static int srv_parse_addr(char **args, int *cur_arg, struct proxy *curpx, struct
                goto error;
        }
 
-       sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, errmsg, NULL, NULL, PA_O_RESOLVE);
+       sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, errmsg, NULL, NULL,
+                         PA_O_RESOLVE | PA_O_PORT_OK);
        if (!sk) {
                memprintf(errmsg, "'%s' : %s", args[*cur_arg], *errmsg);
                goto error;

diff --git a/src/tools.c b/src/tools.c
index 29526c26278ac99f4d56d0335db55a5e2c330b3f..4f61ae256298e5ad5b5cab49c02a1dd1f23246dc 100644 (file)
--- a/src/tools.c
+++ b/src/tools.c
@@ -999,6 +999,10 @@ struct sockaddr_storage *str2sa_range(const char *str, int *port, int *low, int
                        /* Found a colon before a closing-bracket, must be a port separator.
                         * This guarantee backward compatibility.
                         */
+                       if (!(opts & PA_O_PORT_OK)) {
+                               memprintf(err, "port specification not permitted here in '%s'", str);
+                               goto out;
+                       }
                        *chr++ = '\0';
                        port1 = chr;
                }
@@ -1007,24 +1011,57 @@ struct sockaddr_storage *str2sa_range(const char *str, int *port, int *low, int
                         * or directly ending with a closing-bracket.
                         * However, no port.
                         */
+                       if (opts & PA_O_PORT_MAND) {
+                               memprintf(err, "missing port specification in '%s'", str);
+                               goto out;
+                       }
                        port1 = "";
                }
 
                if (isdigit((unsigned char)*port1)) {   /* single port or range */
                        port2 = strchr(port1, '-');
-                       if (port2)
+                       if (port2) {
+                               if (!(opts & PA_O_PORT_RANGE)) {
+                                       memprintf(err, "port range not permitted here in '%s'", str);
+                                       goto out;
+                               }
                                *port2++ = '\0';
+                       }
                        else
                                port2 = port1;
                        portl = atoi(port1);
                        porth = atoi(port2);
+
+                       if (portl < !!(opts & PA_O_PORT_MAND) || portl > 65535) {
+                               memprintf(err, "invalid port '%s'", port1);
+                               goto out;
+                       }
+
+                       if (porth < !!(opts & PA_O_PORT_MAND) || porth > 65535) {
+                               memprintf(err, "invalid port '%s'", port2);
+                               goto out;
+                       }
+
+                       if (portl > porth) {
+                               memprintf(err, "invalid port range '%d-%d'", portl, porth);
+                               goto out;
+                       }
+
                        porta = portl;
                }
                else if (*port1 == '-') { /* negative offset */
+                       if (!(opts & PA_O_PORT_OFS)) {
+                               memprintf(err, "port offset not permitted here in '%s'", str);
+                               goto out;
+                       }
                        portl = atoi(port1 + 1);
                        porta = -portl;
                }
                else if (*port1 == '+') { /* positive offset */
+                       if (!(opts & PA_O_PORT_OFS)) {
+                               memprintf(err, "port offset not permitted here in '%s'", str);
+                               goto out;
+                       }
                        porth = atoi(port1 + 1);
                        porta = porth;
                }
@@ -1032,6 +1069,10 @@ struct sockaddr_storage *str2sa_range(const char *str, int *port, int *low, int
                        memprintf(err, "invalid character '%c' in port number '%s' in '%s'\n", *port1, port1, str);
                        goto out;
                }
+               else if (opts & PA_O_PORT_MAND) {
+                       memprintf(err, "missing port specification in '%s'", str);
+                       goto out;
+               }
 
                /* first try to parse the IP without resolving. If it fails, it
                 * tells us we need to keep a copy of the FQDN to resolve later

diff --git a/tests/ports.cfg b/tests/ports.cfg
new file mode 100644 (file)
index 0000000..7210c3c
--- /dev/null
+++ b/tests/ports.cfg
@@ -0,0 +1,73 @@
+# This is used to validate the address/port parser using "haproxy -c -f $file".
+# Some errors will be returned, they are expected to match the documented ones.
+
+frontend f1
+        log 127.0.0.1 local0
+        log 127.0.0.1:10000 local0
+        log 127.0.0.1:10001-10010 local0    # port range not permitted here in '127.0.0.1:10001-10010'
+        log 127.0.0.1:+10011 local0         # port offset not permitted here in ':::+10011'
+        log 127.0.0.1:-10012 local0         # port offset not permitted here in ':::-10012'
+
+        bind :                              # missing port specification in ':'
+        bind :11001
+        bind :::11002
+        bind :::11003-11010
+        bind :::+11011                      # port offset not permitted here in ':::+11011'
+        bind :::-11012                      # port offset not permitted here in ':::-11012'
+
+frontend f2
+        bind :::0                           # invalid port '0'
+        bind :::0-11                        # invalid port '0'
+        bind :::65016-                      # invalid port ''
+        bind :::65016-1024                  # invalid port range '65016-1024'
+        bind :::65016--1024                 # invalid port '-1024'
+        bind :::66016-1024                  # invalid port '66016'
+
+backend b2
+        source :12001
+        source :::12002
+        source :::12003-12010               # port range not permitted here in '127.0.0.1:12003-12010'
+        source :::+12011                    # port offset not permitted here in ':::+12011'
+        source :::-12012                    # port offset not permitted here in ':::-12012'
+
+backend b3
+        server s1 :
+        server s2 localhost:13001
+        server s3 :13002
+        server s4 :+13003
+        server s5 :-13004
+        server s6 :13005-13010              # port range not permitted here in ':13005-13010'
+
+backend b4
+        server s1 : addr 0.0.0.1:14001      # addr: port specification not permitted here
+
+backend b5
+        server s1 : source localhost:15000
+        server s1 : source 0.0.0.1:15001
+        server s2 : source 0.0.0.1:+15002   # port offset not permitted here in '0.0.0.1:+15002'
+        server s3 : source 0.0.0.1:-15003   # port offset not permitted here in '0.0.0.1:-15003'
+        server s4 : source 0.0.0.1:15004-15010
+
+backend b6
+        server s1 : source 0.0.0.0 usesrc localhost:16000
+        server s1 : source 0.0.0.0 usesrc 0.0.0.1:16001
+        server s2 : source 0.0.0.0 usesrc 0.0.0.1:+16002      # port offset not permitted here in '0.0.0.1:+16002'
+        server s3 : source 0.0.0.0 usesrc 0.0.0.1:-16003      # port offset not permitted here in '0.0.0.1:-16003'
+        server s4 : source 0.0.0.0 usesrc 0.0.0.1:16004-16010 # port range not permitted here in '0.0.0.1:16004-16010'
+
+backend b7
+        server s1 : socks4 0.0.0.1              # missing port specification in '0.0.0.1'
+        server s2 : socks4 localhost:18000
+        server s2 : socks4 0.0.0.1:18001
+        server s3 : socks4 0.0.0.1:+18002       # port offset not permitted here in '0.0.0.1:+18002'
+        server s4 : socks4 0.0.0.1:-18003       # port offset not permitted here in '0.0.0.1:-18003'
+        server s5 : socks4 0.0.0.1:18004-18010  # port range not permitted here in '0.0.0.1:18004-18010'
+
+backend b8
+        tcp-check connect addr 0.0.0.1
+        tcp-check connect addr 0.0.0.1:
+        tcp-check connect addr localhost:19000
+        tcp-check connect addr 0.0.0.1:19001
+        tcp-check connect addr 0.0.0.1:+19002       # port offset not permitted here in '0.0.0.1:+19002'
+        tcp-check connect addr 0.0.0.1:-19003       # port offset not permitted here in '0.0.0.1:-19003'
+        tcp-check connect addr 0.0.0.1:19004-19005  # port range not permitted here in '0.0.0.1:19004-19010'