BUG/MEDIUM: server: avoid changing healthcheck ctx with set server ssl

While giving a fresh try to `set server ssl` (which I wrote), I realised
the behavior is a bit inconsistent. Indeed when using this command over
a server with ssl enabled for the data path but also for the health
check path we have:

- data and health check done using tls
- emit `set server be_foo/srv0 ssl off`
- data path and health check path becomes plain text
- emit `set server be_foo/srv0 ssl on`
- data path becomes tls and health check path remains plain text

while I thought the end result would be:
- data path and health check path comes back in tls

In the current code we indeed erase all connections while deactivating,
but restore only the data path while activating.  I made this mistake in
the past because I was testing with a case where the health check plain
text by default.

There are several ways to solve this issue. The cleanest one would
probably be to avoid changing the health check connection when we use
`set server ssl` command, and create a new command `set server
ssl-check` to change this. For now I assumed this would be ok to simply
avoid changing the health check path and be more consistent.

This patch tries to address that and also update the documentation. It
should not break the existing usage with health check on plain text, as
in this case they should have `no-check-ssl` in defaults.  Without this
patch, it makes the command unusable in an env where you have a list of
server to add along the way with initial `server-template`, and all
using tls for data and healthcheck path.

For 2.6 we should probably reconsider and add `set server ssl-check`
command for better granularity of cases.

If this solution is accepted, this patch should be backported up to >=
2.4.

The alternative solution was to restore the previous state, but I
believe this will create even more confusion in the future.

Signed-off-by: William Dauchy <wdauchy@gmail.com>

diff --git a/doc/management.txt b/doc/management.txt
index 147e059f6db198f61fd8a0d493abb13ad97337e0..b96cbc8789022cf34628e1f9b12c70149b0d21f6 100644 (file)
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -2187,6 +2187,8 @@ set server <backend>/<server> fqdn <FQDN>
 
 set server <backend>/<server> ssl [ on | off ]
   This option configures SSL ciphering on outgoing connections to the server.
+  When switch off, all traffic becomes plain text; health check path is not
+  changed.
 
 set severity-output [ none | number | string ]
   Change the severity output format of the stats socket connected to for the

diff --git a/src/server.c b/src/server.c
index 2061153bc04c78c1d9d330576979205c135d12b2..d165f50b936fbeeafdf3f411003acca011e93b64 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2116,7 +2116,7 @@ void srv_set_ssl(struct server *s, int use_ssl)
        if (s->use_ssl)
                s->xprt = xprt_get(XPRT_SSL);
        else
-               s->xprt = s->check.xprt = s->agent.xprt = xprt_get(XPRT_RAW);
+               s->xprt = xprt_get(XPRT_RAW);
 }
 
 #endif /* USE_OPENSSL */