BUG/MEDIUM: server/log: "mode log" after server keyword causes crash

In 9a74a6c ("MAJOR: log: introduce log backends"), a mistake was made:
it was assumed that the proxy mode was already known during server
keyword parsing in parse_server() function, but this is wrong.

Indeed, "mode log" can be declared late in the proxy section. Due to this,
a simple config like this will cause the process to crash:

   |backend test
   |
   |  server name 127.0.0.1:8080
   |  mode log

In order to fix this, we relax some checks in _srv_parse_init() and store
the address protocol from str2sa_range() in server struct, then we set-up
a postparsing function that is to be called after config parsing to
finish the server checks/initialization that depend on the proxy mode
to be known. We achieve this by checking the PR_CAP_LB capability from
the parent proxy to know if we're in such case where the effective proxy
mode is not yet known (it is assumed that other proxies which are implicit
ones don't provide this possibility and thus don't suffer from this
constraint).

Only then, if the capability is not found, we immediately perform the
server checks that depend on the proxy mode, else the check is postponed
and it will automatically be performed during postparsing thanks to the
REGISTER_POST_SERVER_CHECK() hook.

Note that we remove the SRV_PARSE_IN_LOG_BE flag because it was introduced
in the above commit and it is no longer relevant.

No backport needed unless 9a74a6c gets backported.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 9b03600034e722297dbf5c7829e5f1fdf49c14f1..6d0fa0fba9ecd48f8ca08202cbe77bc451d5f05a 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -275,6 +275,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 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 */
@@ -626,7 +627,6 @@ struct srv_kw_list {
 #define SRV_PARSE_PARSE_ADDR      0x08    /* required to parse the server address in the second argument */
 #define SRV_PARSE_DYNAMIC         0x10    /* dynamic server created at runtime with cli */
 #define SRV_PARSE_INITIAL_RESOLVE 0x20    /* resolve immediately the fqdn to an ip address */
-#define SRV_PARSE_IN_LOG_BE       0x40    /* keyword in log backend */
 
 #endif /* _HAPROXY_SERVER_T_H */
 

diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 7ed67aa32854698499567b6c2d440a6aa91233f6..0b6f7fa07572fe5bde4105174d222d61e8169216 100644 (file)
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -438,7 +438,6 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
        if ((strcmp(args[0], "server") == 0)) {
                err_code |= parse_server(file, linenum, args,
                                         curproxy, curr_defproxy,
-                                        (curproxy->mode == PR_MODE_SYSLOG ? SRV_PARSE_IN_LOG_BE : 0) |
                                         SRV_PARSE_PARSE_ADDR);
 
                if (err_code & ERR_FATAL)
@@ -447,7 +446,6 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
        else if (strcmp(args[0], "default-server") == 0) {
                err_code |= parse_server(file, linenum, args,
                                         curproxy, curr_defproxy,
-                                        (curproxy->mode == PR_MODE_SYSLOG ? SRV_PARSE_IN_LOG_BE : 0) |
                                         SRV_PARSE_DEFAULT_SERVER);
 
                if (err_code & ERR_FATAL)
@@ -456,7 +454,6 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
        else if (strcmp(args[0], "server-template") == 0) {
                err_code |= parse_server(file, linenum, args,
                                         curproxy, curr_defproxy,
-                                        (curproxy->mode == PR_MODE_SYSLOG ? SRV_PARSE_IN_LOG_BE : 0) |
                                         SRV_PARSE_TEMPLATE|SRV_PARSE_PARSE_ADDR);
 
                if (err_code & ERR_FATAL)

diff --git a/src/server.c b/src/server.c
index 4873c1438a135e605a894c9dbfc301844e79470a..ea6a325be6ded1522229774e46c313704c201974 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2336,6 +2336,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->svc_port = src->svc_port;
        }
 
@@ -2706,6 +2707,93 @@ static int _srv_parse_tmpl_init(struct server *srv, struct proxy *px)
        return i - srv->tmpl_info.nb_low;
 }
 
+/* Ensure server config will work with effective proxy mode
+ *
+ * This function is expected to be called after _srv_parse_init() initialization
+ * but only when the effective server's proxy mode is known, which is not always
+ * the case during parsing time, in which case the function will be called during
+ * postparsing thanks to the _srv_postparse() below.
+ *
+ * Returns ERR_NONE on success else a combination or ERR_CODE.
+ */
+static int _srv_check_proxy_mode(struct server *srv, char postparse)
+{
+       int err_code = ERR_NONE;
+
+       if (postparse && !(srv->proxy->cap & PR_CAP_LB))
+               return ERR_NONE; /* nothing to do, the check was already performed during parsing */
+
+       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
+                */
+               goto out;
+       }
+       if (srv->proxy->mode == PR_MODE_SYSLOG) {
+               /* mode log enabled:
+                * pre-resolve related log target from known infos
+                */
+
+               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");
+                       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;
+               }
+       }
+       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));
+                       err_code |= ERR_ALERT | ERR_FATAL;
+               }
+       }
+ out:
+       if (srv->conf.file)
+               reset_usermsgs_ctx();
+
+       return err_code;
+}
+
+/* Perform some server postparsing checks / tasks:
+ * We must be careful that checks / postinits performed within this function
+ * don't depend or conflict with other postcheck functions that are registered
+ * using REGISTER_POST_SERVER_CHECK() hook.
+ *
+ * Returns ERR_NONE on success else a combination or ERR_CODE.
+ */
+static int _srv_postparse(struct server *srv)
+{
+       int err_code = ERR_NONE;
+
+       err_code |= _srv_check_proxy_mode(srv, 1);
+
+       return err_code;
+}
+REGISTER_POST_SERVER_CHECK(_srv_postparse);
+
 /* Allocate a new server pointed by <srv> and try to parse the first arguments
  * in <args> as an address for a server or an address-range for a template or
  * nothing for a default-server. <cur_arg> is incremented to the next argument.
@@ -2725,7 +2813,6 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
        const char *err = NULL;
        int err_code = 0;
        char *fqdn = NULL;
-       struct protocol *proto;
        int tmpl_range_low = 0, tmpl_range_high = 0;
        char *errmsg = NULL;
 
@@ -2815,12 +2902,11 @@ 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, &proto,
+               sk = str2sa_range(args[*cur_arg], &port, &port1, &port2, NULL, &newsrv->addr_proto,
                                  &errmsg, NULL, &fqdn,
-                                 (parse_flags & SRV_PARSE_IN_LOG_BE ? PA_O_DGRAM : PA_O_CONNECT) |
                                  (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) |
-                                 PA_O_STREAM | PA_O_XPRT);
+                                 PA_O_STREAM | PA_O_DGRAM | PA_O_XPRT);
                if (!sk) {
                        ha_alert("%s\n", errmsg);
                        err_code |= ERR_ALERT | ERR_FATAL;
@@ -2858,35 +2944,6 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
                        }
                }
 
-               if ((parse_flags & SRV_PARSE_IN_LOG_BE) && proto) {
-                       /* mode log enabled, and found proto:
-                        * pre-resolve related log target from known infos
-                        */
-                       newsrv->log_target = malloc(sizeof(*newsrv->log_target));
-                       if (!newsrv->log_target) {
-                               ha_alert("memory error when allocating log server\n");
-                               err_code |= ERR_ALERT | ERR_FATAL;
-                               goto out;
-                       }
-                       newsrv->log_target->addr = &newsrv->addr;
-                       switch (proto->xprt_type) {
-                               case PROTO_TYPE_DGRAM:
-                                       newsrv->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.
-                                        */
-                                       newsrv->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;
-                       }
-               }
-
                newsrv->addr = *sk;
                newsrv->svc_port = port;
                /*
@@ -2939,6 +2996,14 @@ static int _srv_parse_init(struct server **srv, char **args, int *cur_arg,
        }
 
        free(fqdn);
+       if (!(curproxy->cap & PR_CAP_LB)) {
+               /* No need to wait for effective proxy mode, it is already known:
+                * Only general purpose user-declared proxies ("listen", "frontend", "backend")
+                * offer the possibility to configure the mode of the proxy. Hopefully for us,
+                * they have the PR_CAP_LB set.
+                */
+               return _srv_check_proxy_mode(newsrv, 0);
+       }
        return 0;
 
 out: