BUG/MEDIUM: ssl: initialize correctly ssl w/ default-server

This bug was introduced by d817dc73 ("MEDIUM: ssl: Load client
certificates in a ckch for backend servers") in which the creation of
the SSL_CTX for a server was moved to the configuration parser when
using a "crt" keyword instead of being done in ssl_sock_prepare_srv_ctx().

The patch 0498fa40 ("BUG/MINOR: ssl: Default-server configuration ignored by
server") made it worse by setting the same SSL_CTX for every servers
using a default-server. Resulting in any SSL option on a server applied
to every server in its backend.

This patch fixes the issue by reintroducing a string which store the
path of certificate inside the server structure, and loading the
certificate in ssl_sock_prepare_srv_ctx() again.

This is a quick fix to backport, a cleaner way can be achieve by always
creating the SSL_CTX in ssl_sock_prepare_srv_ctx() and splitting
properly the ssl_sock_load_srv_cert() function.

This patch fixes issue #1488.

Must be backported as far as 2.4.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 71b8e4a873e5ecfd6be6c9f3d36aa5513a5bb3f6..09f39333d67ed4387b66c7a7780bee0bb5342ab8 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -357,6 +357,7 @@ struct server {
                char *verify_host;              /* hostname of certificate must match this host */
                char *ca_file;                  /* CAfile to use on verify */
                char *crl_file;                 /* CRLfile to use on verify */
+               char *client_crt;               /* client certificate to send */
                struct sample_expr *sni;        /* sample expression for SNI */
                char *npn_str;                  /* NPN protocol string */
                int npn_len;                    /* NPN protocol string length */

diff --git a/reg-tests/ssl/ssl_default_server.vtc b/reg-tests/ssl/ssl_default_server.vtc
index 485a9ba17162da6ff371de265929a79b4a2a34ec..d5ff73ed69f928b19b379897b206753835e67239 100644 (file)
--- a/reg-tests/ssl/ssl_default_server.vtc
+++ b/reg-tests/ssl/ssl_default_server.vtc
@@ -15,7 +15,7 @@ feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.5-dev0)'"
 feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
 feature ignore_unknown_macro
 
-server s1 -repeat 7 {
+server s1 -repeat 10 {
   rxreq
   txresp
 } -start
@@ -56,7 +56,10 @@ haproxy h1 -conf {
 
     backend third_be
         default-server ssl crt client1.pem ca-file ca-auth.crt verify none
-        server s1 "${tmpdir}/ssl.sock" crt client2_expired.pem
+        server s1 "${tmpdir}/ssl.sock"
+        server s2 "${tmpdir}/ssl.sock" crt client2_expired.pem
+        server s3 "${tmpdir}/ssl.sock"
+        server s4 "${tmpdir}/ssl.sock"
 
     backend fourth_be
         default-server ssl crt client1.pem verify none
@@ -101,6 +104,14 @@ client c1 -connect ${h1_clearlst_sock} {
     expect resp.http.x-ssl == "Ok"
 } -run
 
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/third"
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Ok"
+} -run
+
 client c1 -connect ${h1_clearlst_sock} {
     txreq -url "/third"
     txreq
@@ -109,6 +120,14 @@ client c1 -connect ${h1_clearlst_sock} {
     expect resp.http.x-ssl == "Expired"
 } -run
 
+client c1 -connect ${h1_clearlst_sock} -repeat 2 {
+    txreq -url "/third"
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Ok"
+} -run
+
 client c1 -connect ${h1_clearlst_sock} {
     txreq -url "/fourth"
     txreq

diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index cebab3739bd286bd7a4e448268041d1c4676d2f7..026e16d888f593e339153e9db6c663793614077a 100644 (file)
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -1500,9 +1500,6 @@ static int srv_parse_crl_file(char **args, int *cur_arg, struct proxy *px, struc
 /* parse the "crt" server keyword */
 static int srv_parse_crt(char **args, int *cur_arg, struct proxy *px, struct server *newsrv, char **err)
 {
-       const int create_if_none = newsrv->flags & SRV_F_DYNAMIC ? 0 : 1;
-       int retval = -1;
-       char *path = NULL;
 
        if (!*args[*cur_arg + 1]) {
                memprintf(err, "'%s' : missing certificate file path", args[*cur_arg]);
@@ -1510,16 +1507,11 @@ static int srv_parse_crt(char **args, int *cur_arg, struct proxy *px, struct ser
        }
 
        if ((*args[*cur_arg + 1] != '/') && global_ssl.crt_base)
-               memprintf(&path, "%s/%s", global_ssl.crt_base, args[*cur_arg + 1]);
+               memprintf(&newsrv->ssl_ctx.client_crt, "%s/%s", global_ssl.crt_base, args[*cur_arg + 1]);
        else
-               memprintf(&path, "%s", args[*cur_arg + 1]);
-
-       if (path) {
-               retval = ssl_sock_load_srv_cert(path, newsrv, create_if_none, err);
-               free(path);
-       }
+               memprintf(&newsrv->ssl_ctx.client_crt, "%s", args[*cur_arg + 1]);
 
-       return retval;
+       return 0;
 }
 
 /* parse the "no-check-ssl" server keyword */

diff --git a/src/server.c b/src/server.c
index 4f75dff5a129e2a5954d1d3a20531caf31fb19ea..2061153bc04c78c1d9d330576979205c135d12b2 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -2050,6 +2050,8 @@ static void srv_conn_src_cpy(struct server *srv, struct server *src)
 static void srv_ssl_settings_cpy(struct server *srv, struct server *src)
 {
        /* <src> is the current proxy's default server and SSL is enabled */
+       BUG_ON(src->ssl_ctx.ctx != NULL); /* the SSL_CTX must never be initialized in a default-server */
+
        if (src == &srv->proxy->defsrv && src->use_ssl == 1)
                srv->flags |= SRV_F_DEFSRV_USE_SSL;
 
@@ -2057,10 +2059,11 @@ static void srv_ssl_settings_cpy(struct server *srv, struct server *src)
                srv->ssl_ctx.ca_file = strdup(src->ssl_ctx.ca_file);
        if (src->ssl_ctx.crl_file != NULL)
                srv->ssl_ctx.crl_file = strdup(src->ssl_ctx.crl_file);
+       if (src->ssl_ctx.client_crt != NULL)
+               srv->ssl_ctx.client_crt = strdup(src->ssl_ctx.client_crt);
 
        srv->ssl_ctx.verify = src->ssl_ctx.verify;
 
-       srv->ssl_ctx.ctx = src->ssl_ctx.ctx;
 
        if (src->ssl_ctx.verify_host != NULL)
                srv->ssl_ctx.verify_host = strdup(src->ssl_ctx.verify_host);

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 1fa7374db0716a932f023bf20f85c53c6a8465a2..a14dea3e3e17f537d048db1895541b958a26c4a5 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4837,8 +4837,7 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
 int ssl_sock_prepare_srv_ctx(struct server *srv)
 {
        int cfgerr = 0;
-       SSL_CTX *ctx = srv->ssl_ctx.ctx;
-
+       SSL_CTX *ctx;
        /* Automatic memory computations need to know we use SSL there */
        global.ssl_used_backend = 1;
 
@@ -4853,6 +4852,27 @@ int ssl_sock_prepare_srv_ctx(struct server *srv)
        if (srv->use_ssl == 1)
                srv->xprt = &ssl_sock;
 
+       if (srv->ssl_ctx.client_crt) {
+               const int create_if_none = srv->flags & SRV_F_DYNAMIC ? 0 : 1;
+               char *err = NULL;
+               int err_code = 0;
+
+               /* If there is a crt keyword there, the SSL_CTX will be created here. */
+               err_code = ssl_sock_load_srv_cert(srv->ssl_ctx.client_crt, srv, create_if_none, &err);
+               if (err_code != ERR_NONE) {
+                       if ((err_code & ERR_WARN) && !(err_code & ERR_ALERT))
+                               ha_warning("%s", err);
+                       else
+                               ha_alert("%s", err);
+
+                       if (err_code & (ERR_FATAL|ERR_ABORT))
+                               cfgerr++;
+               }
+               ha_free(&err);
+       }
+
+       ctx = srv->ssl_ctx.ctx;
+
        /* The context will be uninitialized if there wasn't any "cert" option
         * in the server line. */
        if (!ctx) {