From f8f94ffc9c26bb8c2ff261972ce2f6c4fd731f6d Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Wed, 3 Sep 2025 15:29:56 +0200 Subject: [PATCH] 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. --- include/haproxy/server.h | 2 +- src/server.c | 53 ++++++++++++++++++++++++++++------------ src/server_state.c | 8 ++++-- src/tcp_act.c | 4 +-- 4 files changed, 46 insertions(+), 21 deletions(-) diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 5cd75a774..7f38cd53b 100644 --- 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 7b56dc7f8..a41ffed16 100644 --- a/src/server.c +++ b/src/server.c @@ -54,7 +54,7 @@ #include #include - +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 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 ssl' expects 'on' or 'off'.\n"); diff --git a/src/server_state.c b/src/server_state.c index 55321db25..098af82be 100644 --- 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 d5e69793b..677dc0770 100644 --- 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; }