BUG/MEDIUM: server: invalid address (post)parsing checks

This bug was introduced with 29b76ca ("BUG/MEDIUM: server/log: "mode log"
after server keyword causes crash ")

Indeed, we cannot safely rely on addr_proto being set when str2sa_range()
returns in parse_server() (even if SRV_PARSE_PARSE_ADDR is set), because
proto lookup might be bypassed when FQDN addresses are involved.

Unfortunately, the above patch wrongly assumed that proto would always
be set when SRV_PARSE_PARSE_ADDR was passed to parse_server() (so when
str2sa_range() was called), resulting in invalid postparsing checks being
performed, which could as well lead to crashes with log backends
("mode log" set) because some postparsing init was skipped as a result of
proto not being set and this wasn't expected later in the init code.

To fix this, we now make use of the previous patch to perform server's
address compatibility checks on hints that are always set when
str2sa_range() succesfully returns.

For log backend, we're also adding a complementary test to check if the
address family is of expected type, else we report an error, plus we're
moving the postinit logic in log api since _srv_check_proxy_mode() is
only meant to check proxy mode compatibility and we were abusing it.

This patch depends on:
 - "MINOR: tools: make str2sa_range() directly return type hints"

No backport required unless 29b76ca gets backported.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 2f41ba8796a47765182c2d21ecc86f4a8f0e7e52..80d803a95e9eee3654763b02edce1022c136dc55 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -41,6 +41,7 @@
 #include <haproxy/task-t.h>
 #include <haproxy/thread-t.h>
 #include <haproxy/event_hdl-t.h>
+#include <haproxy/tools-t.h>
 
 
 /* server states. Only SRV_ST_STOPPED indicates a down server. */
@@ -285,7 +286,7 @@ struct server {
 
        struct proxy *proxy;                    /* the proxy this server belongs to */
        const struct mux_proto_list *mux_proto; /* the mux to use for all outgoing connections (specified by the "proto" keyword) */
-       struct protocol *addr_proto;            /* server side protocol used for haproxy<->server communication */
+       struct net_addr_type addr_type;         /* server address type (socket and transport hints) */
        struct log_target *log_target;          /* when 'mode log' is enabled, target facility used to transport log messages */
        unsigned maxconn, minconn;              /* max # of active sessions (0 = unlimited), min# for dynamic limit. */
        struct srv_per_thread *per_thr;         /* array of per-thread stuff such as connections lists */

diff --git a/src/log.c b/src/log.c
index 5efc08b3aeaddfa9f128f496ba97615373daad50..c6f817c6657f963a0c551327e3b308d6f7bb758c 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -971,8 +971,30 @@ static int postcheck_log_backend(struct proxy *be)
        /* finish the initialization of proxy's servers */
        srv = be->srv;
        while (srv) {
+               BUG_ON(srv->log_target);
+               BUG_ON(srv->addr_type.proto_type != PROTO_TYPE_DGRAM &&
+                      srv->addr_type.proto_type != PROTO_TYPE_STREAM);
+
+               srv->log_target = malloc(sizeof(*srv->log_target));
+               if (!srv->log_target) {
+                       memprintf(&msg, "memory error when allocating log server '%s'\n", srv->id);
+                       err_code |= ERR_ALERT | ERR_FATAL;
+                       goto end;
+               }
+               srv->log_target->addr = &srv->addr;
+               if (srv->addr_type.proto_type == PROTO_TYPE_DGRAM)
+                       srv->log_target->type = LOG_TARGET_DGRAM;
+               else {
+                       /* for now BUFFER type only supports TCP server to it's almost
+                        * explicit. This will require ring buffer creation during log
+                        * postresolving step.
+                        */
+                       srv->log_target->type = LOG_TARGET_BUFFER;
+               }
+
                if (target_type == -1)
                        target_type = srv->log_target->type;
+
                if (target_type != srv->log_target->type) {
                        memprintf(&msg, "cannot mix server types within a log backend, '%s' srv's network type differs from previous server", srv->id);
                        err_code |= ERR_ALERT | ERR_FATAL;

diff --git a/src/server.c b/src/server.c
index 80e26ab94f829e5c1357f9d7bacc8ca51db4a569..ef41a109a2808c328f5f812df22918a1e8bdf4b3 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2401,7 +2401,7 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
 
        if (srv_tmpl) {
                srv->addr = src->addr;
-               srv->addr_proto = src->addr_proto;
+               srv->addr_type = src->addr_type;
                srv->svc_port = src->svc_port;
        }
 
@@ -2830,47 +2830,39 @@ static int _srv_check_proxy_mode(struct server *srv, char postparse)
        if (srv->conf.file)
                set_usermsgs_ctx(srv->conf.file, srv->conf.line, NULL);
 
-       if (!srv->addr_proto) {
-               /* proto may not be set (ie: if srv_parse_init() was skipped or
-                * SRV_PARSE_PARSE_ADDR was not set, in this case, we have nothing
-                * to check
-                */
+       if (!srv->proxy) {
+               /* proxy mode not known, cannot perform checks (ie: defaults section) */
                goto out;
        }
+
        if (srv->proxy->mode == PR_MODE_SYSLOG) {
-               /* mode log enabled:
-                * pre-resolve related log target from known infos
+               /* log backend server (belongs to proxy with mode log enabled):
+                * perform some compatibility checks
                 */
 
-               BUG_ON(srv->log_target);
-               srv->log_target = malloc(sizeof(*srv->log_target));
-               if (!srv->log_target) {
-                       ha_alert("memory error when allocating log server\n");
+               /* supported address family types are:
+                *   - ipv4
+                *   - ipv6
+                * (UNSPEC is supported because it means it will be resolved later)
+                */
+               if (srv->addr.ss_family != AF_UNSPEC &&
+                   srv->addr.ss_family != AF_INET && srv->addr.ss_family != AF_INET6) {
+                       ha_alert("log server address family not supported for log backend server.\n");
                        err_code |= ERR_ALERT | ERR_FATAL;
                        goto out;
                }
-               srv->log_target->addr = &srv->addr;
-               switch (srv->addr_proto->xprt_type) {
-                       case PROTO_TYPE_DGRAM:
-                               srv->log_target->type = LOG_TARGET_DGRAM;
-                               break;
-                       case PROTO_TYPE_STREAM:
-                               /* for now BUFFER type only supports TCP server to it's almost
-                                * explicit. This will require ring buffer creation during log
-                                * postresolving step.
-                                */
-                               srv->log_target->type = LOG_TARGET_BUFFER;
-                               break;
-                       default:
-                               ha_alert("log server type not supported for log backend server.\n");
-                               err_code |= ERR_ALERT | ERR_FATAL;
-                               break;
+
+               /* only @tcp or @udp address forms (or equivalent) are supported */
+               if (!(srv->addr_type.xprt_type == PROTO_TYPE_DGRAM && srv->addr_type.proto_type == PROTO_TYPE_DGRAM) &&
+                   !(srv->addr_type.xprt_type == PROTO_TYPE_STREAM && srv->addr_type.proto_type == PROTO_TYPE_STREAM)) {
+                       ha_alert("log server address type not supported for log backend server.\n");
+                       err_code |= ERR_ALERT | ERR_FATAL;
                }
        }
        else {
-               /* for now, only TCP expected as srv's proto for other uses */
-               if (srv->addr_proto->xprt_type != PROTO_TYPE_STREAM || !srv->addr_proto->connect) {
-                       ha_alert("unsupported protocol for server address in '%s' backend.\n", proxy_mode_str(srv->proxy->mode));
+               /* for all other proxy modes: only TCP expected as srv's transport type for now */
+               if (srv->addr_type.xprt_type != PROTO_TYPE_STREAM) {
+                       ha_alert("unsupported transport for server address in '%s' backend.\n", proxy_mode_str(srv->proxy->mode));
                        err_code |= ERR_ALERT | ERR_FATAL;
                }
        }
@@ -3006,7 +2998,7 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
                if (!(parse_flags & SRV_PARSE_PARSE_ADDR))
                        goto skip_addr;
 
-               sk = str2sa_range(args[*cur_arg], &port, &port1, &port2, NULL, &newsrv->addr_proto, NULL,
+               sk = str2sa_range(args[*cur_arg], &port, &port1, &port2, NULL, NULL, &newsrv->addr_type,
                                  &errmsg, NULL, &fqdn,
                                  (parse_flags & SRV_PARSE_INITIAL_RESOLVE ? PA_O_RESOLVE : 0) | PA_O_PORT_OK |
                                  (parse_flags & SRV_PARSE_IN_PEER_SECTION ? PA_O_PORT_MAND : PA_O_PORT_OFS) |