From f61f33a1b274c2a42afd96aab19ee8e1d8b121cc Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 27 Mar 2020 18:55:49 +0100 Subject: [PATCH] BUG/MINOR: checks: Respect the no-check-ssl option This options is used to force a non-SSL connection to check a SSL server or to invert a check-ssl option inherited from the default section. The use_ssl field in the check structure is used to know if a SSL connection must be used (use_ssl=1) or not (use_ssl=0). The server configuration is used by default. The problem is that we cannot distinguish the default case (no specific SSL check option) and the case of an explicit non-SSL check. In both, use_ssl is set to 0. So the server configuration is always used. For a SSL server, when no-check-ssl option is set, the check is still performed using a SSL configuration. To fix the bug, instead of a boolean value (0=TCP, 1=SSL), we use a ternary value : * 0 = use server config * 1 = force SSL * -1 = force non-SSL The same is done for the server parameter. It is not really necessary for now. But it is a good way to know is the server no-ssl option is set. In addition, the PR_O_TCPCHK_SSL proxy option is no longer used to set use_ssl to 1 for a check. Instead the flag is directly tested to prepare or destroy the server SSL context. This patch should be backported as far as 1.8. --- include/types/checks.h | 2 +- include/types/server.h | 2 +- src/cfgparse.c | 4 ++-- src/checks.c | 7 ++++--- src/haproxy.c | 2 +- src/ssl_sock.c | 8 ++++---- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/include/types/checks.h b/include/types/checks.h index cbab28f8b..e94824b8a 100644 --- a/include/types/checks.h +++ b/include/types/checks.h @@ -167,7 +167,7 @@ struct check { short status, code; /* check result, check code */ unsigned short port; /* the port to use for the health checks */ char desc[HCHK_DESC_LEN]; /* health check description */ - int use_ssl; /* use SSL for health checks */ + char use_ssl; /* use SSL for health checks (1: on, 0: server mode, -1: off) */ int send_proxy; /* send a PROXY protocol header with checks */ struct list *tcpcheck_rules; /* tcp-check send / expect rules */ struct tcpcheck_rule *current_step; /* current step when using tcpcheck */ diff --git a/include/types/server.h b/include/types/server.h index 48582e613..a21411962 100644 --- a/include/types/server.h +++ b/include/types/server.h @@ -201,7 +201,7 @@ struct server { enum obj_type obj_type; /* object type == OBJ_TYPE_SERVER */ enum srv_state next_state, cur_state; /* server state among SRV_ST_* */ enum srv_admin next_admin, cur_admin; /* server maintenance status : SRV_ADMF_* */ - unsigned char use_ssl; /* ssl enabled */ + char use_ssl; /* ssl enabled (1: on, 0: disabled, -1 forced off) */ unsigned int pp_opts; /* proxy protocol options (SRV_PP_*) */ struct server *next; int cklen; /* the len of the cookie, to speed up checks */ diff --git a/src/cfgparse.c b/src/cfgparse.c index 59bdb3bab..926cd1d66 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -3252,7 +3252,7 @@ out_uri_auth_compat: } /* this will also properly set the transport layer for prod and checks */ - if (newsrv->use_ssl || newsrv->check.use_ssl) { + if (newsrv->use_ssl == 1 || newsrv->check.use_ssl == 1 || (newsrv->proxy->options & PR_O_TCPCHK_SSL)) { if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->prepare_srv) cfgerr += xprt_get(XPRT_SSL)->prepare_srv(newsrv); } @@ -4005,7 +4005,7 @@ out_uri_auth_compat: p = curpeers->remote; while (p) { if (p->srv) { - if (p->srv->use_ssl && xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->prepare_srv) + if (p->srv->use_ssl == 1 && xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->prepare_srv) cfgerr += xprt_get(XPRT_SSL)->prepare_srv(p->srv); } p = p->next; diff --git a/src/checks.c b/src/checks.c index 22a2c501b..f52647b72 100644 --- a/src/checks.c +++ b/src/checks.c @@ -1371,7 +1371,7 @@ static void __event_srv_chk_r(struct conn_stream *cs) default: /* good connection is enough for pure TCP check */ if (!(conn->flags & CO_FL_WAIT_XPRT) && !check->type) { - if (check->use_ssl) + if (check->use_ssl == 1) set_server_check_status(check, HCHK_STATUS_L6OK, NULL); else set_server_check_status(check, HCHK_STATUS_L4OK, NULL); @@ -2366,7 +2366,7 @@ static struct task *process_chk_conn(struct task *t, void *context, unsigned sho if (check->result == CHK_RES_UNKNOWN) { /* good connection is enough for pure TCP check */ if (!(conn->flags & CO_FL_WAIT_XPRT) && !check->type) { - if (check->use_ssl) + if (check->use_ssl == 1) set_server_check_status(check, HCHK_STATUS_L6OK, NULL); else set_server_check_status(check, HCHK_STATUS_L4OK, NULL); @@ -3670,7 +3670,8 @@ int srv_check_healthcheck_port(struct check *chk) * default, unless one is specified. */ if (!chk->port && !is_addr(&chk->addr)) { - chk->use_ssl |= (srv->use_ssl || (srv->proxy->options & PR_O_TCPCHK_SSL)); + if (!chk->use_ssl) + chk->use_ssl = srv->use_ssl; chk->send_proxy |= (srv->pp_opts); } diff --git a/src/haproxy.c b/src/haproxy.c index 4af56fbc5..a702e7869 100644 --- a/src/haproxy.c +++ b/src/haproxy.c @@ -2686,7 +2686,7 @@ void deinit(void) free(s->available_conns); free(s->curr_idle_thr); - if (s->use_ssl || s->check.use_ssl) { + if (s->use_ssl == 1 || s->check.use_ssl == 1 || (s->proxy->options & PR_O_TCPCHK_SSL)) { if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->destroy_srv) xprt_get(XPRT_SSL)->destroy_srv(s); } diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 12b40123d..224de2866 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -5925,9 +5925,9 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) return cfgerr; } } - if (srv->use_ssl) + if (srv->use_ssl == 1) srv->xprt = &ssl_sock; - if (srv->check.use_ssl) + if (srv->check.use_ssl == 1) srv->check.xprt = &ssl_sock; ctx = SSL_CTX_new(SSLv23_client_method()); @@ -9956,7 +9956,7 @@ static int srv_parse_crt(char **args, int *cur_arg, struct proxy *px, struct ser /* parse the "no-check-ssl" server keyword */ static int srv_parse_no_check_ssl(char **args, int *cur_arg, struct proxy *px, struct server *newsrv, char **err) { - newsrv->check.use_ssl = 0; + newsrv->check.use_ssl = -1; free(newsrv->ssl_ctx.ciphers); newsrv->ssl_ctx.ciphers = NULL; newsrv->ssl_ctx.options &= ~global_ssl.connect_default_ssloptions; @@ -9983,7 +9983,7 @@ static int srv_parse_no_send_proxy_cn(char **args, int *cur_arg, struct proxy *p /* parse the "no-ssl" server keyword */ static int srv_parse_no_ssl(char **args, int *cur_arg, struct proxy *px, struct server *newsrv, char **err) { - newsrv->use_ssl = 0; + newsrv->use_ssl = -1; free(newsrv->ssl_ctx.ciphers); newsrv->ssl_ctx.ciphers = NULL; return 0;