BUG/MEDIUM: server: Use sni as pool connection name for SSL server only

By default, for a given server, when no pool-conn-name is specified, the
configured sni is used. However, this must only be done when SSL is in-use
for the server. Of course, it is uncommon to have a sni expression for
now-ssl server. But this may happen.

In addition, the SSL may be disabled via the CLI. In that case, the
pool-conn-name must be discarded if it was copied from the sni. And, we must
of course take care to set it if the ssl is enabled.

Finally, when the attac-srv action is checked, we now checked the
pool-conn-name expression.

This patch should be backported as far as 3.0. It relies on "MINOR: server:
Parse sni and pool-conn-name expressions in a dedicated function" which
should be backported too.

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 5cd75a774417773211eb69460e7a31f0d9f412c2..7f38cd53bd853c4eb4244dd9f8c35532e363868a 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -75,7 +75,7 @@ void srv_take(struct server *srv);
 struct server *srv_drop(struct server *srv);
 void srv_free_params(struct server *srv);
 int srv_preinit(struct server *srv);
-void srv_set_ssl(struct server *s, int use_ssl);
+int srv_set_ssl(struct server *s, int use_ssl);
 const char *srv_adm_st_chg_cause(enum srv_adm_st_chg_cause cause);
 const char *srv_op_st_chg_cause(enum srv_op_st_chg_cause cause);
 void srv_event_hdl_publish_check(struct server *srv, struct check *check);

diff --git a/src/server.c b/src/server.c
index 7b56dc7f8fc89e9f92da44237c5084db17d29b5b..a41ffed16f7d58f00f2174b8648044de7ef929ba 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -54,7 +54,7 @@
 #include <haproxy/xxhash.h>
 #include <haproxy/event_hdl.h>
 
-
+static inline int _srv_parse_exprs(struct server *srv, struct proxy *px, char **errmsg);
 static void srv_update_status(struct server *s, int type, int cause);
 static int srv_apply_lastaddr(struct server *srv, int *err_code);
 static void srv_cleanup_connections(struct server *srv);
@@ -2761,16 +2761,26 @@ static void srv_ssl_settings_cpy(struct server *srv, const struct server *src)
  *
  * Must be called with the server lock held.
  */
-void srv_set_ssl(struct server *s, int use_ssl)
+int srv_set_ssl(struct server *s, int use_ssl)
 {
        if (s->use_ssl == use_ssl)
-               return;
+               return 0;
 
        s->use_ssl = use_ssl;
-       if (s->use_ssl)
+       if (s->use_ssl) {
+               if (_srv_parse_exprs(s, s->proxy, NULL))
+                       return -1;
                s->xprt = xprt_get(XPRT_SSL);
-       else
+       }
+       else {
+               if (s->sni_expr && s->pool_conn_name && strcmp(s->sni_expr, s->pool_conn_name) == 0) {
+                       release_sample_expr(s->pool_conn_name_expr);
+                       s->pool_conn_name_expr = NULL;
+               }
                s->xprt = xprt_get(XPRT_RAW);
+       }
+
+       return 0;
 }
 
 #endif /* USE_OPENSSL */
@@ -3294,23 +3304,26 @@ static inline int _srv_parse_exprs(struct server *srv, struct proxy *px, char **
 {
        int ret = 0;
 
-       /* Use sni as fallback if pool_conn_name isn't set */
-       if (!srv->pool_conn_name && srv->sni_expr) {
-               srv->pool_conn_name = strdup(srv->sni_expr);
-               if (!srv->pool_conn_name) {
-                       memprintf(errmsg, "cannot duplicate sni expression (out of memory)");
-                       ret = ERR_ALERT | ERR_FATAL;
-                       goto out;
+       if (srv->use_ssl == 1) {
+               /* Use sni as fallback if pool_conn_name isn't set, but only if
+                * the server is configured to use SSL */
+               if (!srv->pool_conn_name && srv->sni_expr) {
+                       srv->pool_conn_name = strdup(srv->sni_expr);
+                       if (!srv->pool_conn_name) {
+                               memprintf(errmsg, "cannot duplicate sni expression (out of memory)");
+                               ret = ERR_ALERT | ERR_FATAL;
+                               goto out;
+                       }
                }
        }
 
-       if (srv->sni_expr) {
+       if (srv->sni_expr && !srv->ssl_ctx.sni) {
                ret = parse_srv_expr(srv->sni_expr, &srv->ssl_ctx.sni, px, errmsg);
                if (ret)
                        goto out;
        }
 
-       if (srv->pool_conn_name) {
+       if (srv->pool_conn_name && !srv->pool_conn_name_expr) {
                ret = parse_srv_expr(srv->pool_conn_name, &srv->pool_conn_name_expr, px, errmsg);
                if (ret)
                        goto out;
@@ -5560,6 +5573,8 @@ static int cli_parse_set_server(char **args, char *payload, struct appctx *appct
        }
        else if (strcmp(args[3], "ssl") == 0) {
 #ifdef USE_OPENSSL
+               char *err = NULL;
+
                if (sv->flags & SRV_F_DYNAMIC) {
                        cli_err(appctx, "'set server <srv> ssl' not supported on dynamic servers\n");
                        goto out;
@@ -5573,9 +5588,15 @@ static int cli_parse_set_server(char **args, char *payload, struct appctx *appct
 
                HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
                if (strcmp(args[4], "on") == 0) {
-                       srv_set_ssl(sv, 1);
+                       if (srv_set_ssl(sv, 1)) {
+                               cli_dynerr(appctx, memprintf(&err, "failed to enable ssl for server %s.\n", args[2]));
+                               goto out;
+                       }
                } else if (strcmp(args[4], "off") == 0) {
-                       srv_set_ssl(sv, 0);
+                       if (srv_set_ssl(sv, 0)) {
+                               cli_dynerr(appctx, memprintf(&err, "failed to disable ssl for server %s.\n", args[2]));
+                               goto out;
+                       }
                } else {
                        HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
                        cli_err(appctx, "'set server <srv> ssl' expects 'on' or 'off'.\n");

diff --git a/src/server_state.c b/src/server_state.c
index 55321db2567bfc4c5908da58f24fa5443ca26981..098af82be902f44f4502a5239e07fc6f1c39cecf 100644 (file)
--- a/src/server_state.c
+++ b/src/server_state.c
@@ -440,8 +440,12 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
                use_ssl = strtol(params[16], &p, 10);
 
                /* configure ssl if connection has been initiated at startup */
-               if (srv->ssl_ctx.ctx != NULL)
-                       srv_set_ssl(srv, use_ssl);
+               if (srv->ssl_ctx.ctx != NULL) {
+                       if (srv_set_ssl(srv, use_ssl)) {
+                               chunk_appendf(msg, ", failed to %s ssl for server '%s'", (use_ssl ? "enable" : "disable"), srv->id);
+                               goto out;
+                       }
+               }
 #endif
        }
 

diff --git a/src/tcp_act.c b/src/tcp_act.c
index d5e69793bcd11c80ac07cff217e1788138e0b991..677dc0770e0704a5fc1bb3c2b4db8174b926ce41 100644 (file)
--- a/src/tcp_act.c
+++ b/src/tcp_act.c
@@ -514,12 +514,12 @@ static int tcp_check_attach_srv(struct act_rule *rule, struct proxy *px, char **
        }
 
        if (rule->arg.attach_srv.name) {
-               if (!srv->pool_conn_name) {
+               if (!srv->pool_conn_name_expr) {
                        memprintf(err, "attach-srv rule has a name argument while server '%s/%s' does not use pool-conn-name; either reconfigure the server or remove the name argument from this attach-srv rule", ist0(be_name), ist0(sv_name));
                        return 0;
                }
        } else {
-               if (srv->pool_conn_name) {
+               if (srv->pool_conn_name_expr) {
                        memprintf(err, "attach-srv rule has no name argument while server '%s/%s' uses pool-conn-name; either add a name argument to the attach-srv rule or reconfigure the server", ist0(be_name), ist0(sv_name));
                        return 0;
                }