On Thu, Nov 25, 2021 at 11:02:52AM +0100, Amaury Denoyelle wrote:
> On Thu, Nov 25, 2021 at 11:42:01AM +0300, Dmitry Sivachenko wrote:
> > On 24 Nov 2021, at 12:57, Christopher Faulet <cfau...@haproxy.com> wrote:
> > > > > Hi,
> > > > HAProxy 2.4.9 was released on 2021/11/23. It added 36 new commits
> > > after version 2.4.8.
> > >
> > Hello,
> > version 2.4.9 fails to build with OpenSSL turned off:
> > src/server.c:207:51: error: no member named 'ssl_ctx' in 'struct server'
> > if (srv->mux_proto || srv->use_ssl != 1 || !srv->ssl_ctx.alpn_str) {
> > ~~~ ^
> > src/server.c:241:37: error: no member named 'ssl_ctx' in 'struct server'
> > const struct ist alpn = ist2(srv->ssl_ctx.alpn_str,
> > ~~~ ^
> > src/server.c:242:37: error: no member named 'ssl_ctx' in 'struct server'
> > srv->ssl_ctx.alpn_len);
> > ~~~ ^
> > Version 2.4.8 builds fine.
> >
> >
>
> Thanks for your report. One of my commit to handle properly websocket on
> the server side introduces this issue. I'm working on a fix.
Please try the two attached patches. They re-backport something that
we earlier failed to backport that simplifies the ugly ifdefs everywhere
that virtually break every single backport related to SSL.
For me they work with/without SSL and with older versions (tested as far
as 0.9.8).
Thanks,
Willy
>From ce5ca630697a069ffbd81169663e5dbeb554179a Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Wed, 6 Oct 2021 11:23:32 +0200
Subject: CLEANUP: servers: do not include openssl-compat
This is exactly the same as for listeners, servers only include
openssl-compat to provide the SSL_CTX type to use as two pointers to
contexts, and to detect if NPN, ALPN, and cipher suites are supported,
and save up to 5 pointers in the ssl_ctx struct if not supported. This
is pointless, as these ones have all been supported for about a decade,
and including this file comes with a long dependency chain that impacts
lots of other files. The ctx was made a void*.
Now the build time was significantly reduced, from 9.2 to 8.1 seconds,
thanks to opensslconf.h being included "only" 456 times instead of 2424
previously!
The total number of lines of code compiled was reduced by 15%.
(cherry picked from commit 340ef2502eae2a37781e460d3590982c0e437fbd)
[wt: this is backported to get rid of the painful #ifdef around SSL
fields that regularly break backports]
Signed-off-by: Willy Tarreau <w...@1wt.eu>
---
include/haproxy/server-t.h | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 429195388..32b649bf3 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -35,9 +35,7 @@
#include <haproxy/freq_ctr-t.h>
#include <haproxy/listener-t.h>
#include <haproxy/obj_type-t.h>
-#include <haproxy/openssl-compat.h>
#include <haproxy/resolvers-t.h>
-#include <haproxy/ssl_sock-t.h>
#include <haproxy/stats-t.h>
#include <haproxy/task-t.h>
#include <haproxy/thread-t.h>
@@ -341,7 +339,7 @@ struct server {
#ifdef USE_OPENSSL
char *sni_expr; /* Temporary variable to store a sample
expression for SNI */
struct {
- SSL_CTX *ctx;
+ void *ctx;
struct {
unsigned char *ptr;
int size;
@@ -353,9 +351,7 @@ struct server {
__decl_thread(HA_RWLOCK_T lock); /* lock the cache and SSL_CTX
during commit operations */
char *ciphers; /* cipher suite to use if
non-null */
-#ifdef HAVE_SSL_CTX_SET_CIPHERSUITES
char *ciphersuites; /* TLS 1.3 cipher suite
to use if non-null */
-#endif
int options; /* ssl options */
int verify; /* verify method (set of
SSL_VERIFY_* flags) */
struct tls_version_filter methods; /* ssl methods */
@@ -363,14 +359,10 @@ struct server {
char *ca_file; /* CAfile to use on verify */
char *crl_file; /* CRLfile to use on verify */
struct sample_expr *sni; /* sample expression for SNI */
-#ifdef OPENSSL_NPN_NEGOTIATED
char *npn_str; /* NPN protocol string */
int npn_len; /* NPN protocol string length */
-#endif
-#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
char *alpn_str; /* ALPN protocol string */
int alpn_len; /* ALPN protocol string length
*/
-#endif
} ssl_ctx;
#ifdef USE_QUIC
struct quic_transport_params quic_params; /* QUIC transport parameters
*/
--
2.28.0
>From 6d395b766fd816cf2e7feea3286a689e635e35f9 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Wed, 6 Oct 2021 14:48:37 +0200
Subject: CLEANUP: server: always include the storage for SSL settings
The SSL stuff in struct server takes less than 3% of it and requires
lots of annoying ifdefs in the code just to take care of the cases
where the field is absent. Let's get rid of this and stop including
openssl-compat from server.c to detect NPN and ALPN capabilities.
This reduces the total LoC by another 0.4%.
(cherry picked from commit 80527bcb9d51d8506c8e7ef95de9c30d30722719)
Signed-off-by: Christopher Faulet <cfau...@haproxy.com>
(cherry picked from commit 5279e61cee28b7012619906048edd2c8a9c89059)
[wt: backported again to fix backport issues around SSL fields. It
previously broke due to the absence of 'CLEANUP: servers: do not
include openssl-compat' that was backported now]
Signed-off-by: Willy Tarreau <w...@1wt.eu>
---
include/haproxy/server-t.h | 2 --
src/server.c | 21 +++------------------
2 files changed, 3 insertions(+), 20 deletions(-)
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 32b649bf3..90485f0c4 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -336,7 +336,6 @@ struct server {
unsigned int init_addr_methods; /* initial address setting,
3-bit per method, ends at 0, enough to store 10 entries */
enum srv_log_proto log_proto; /* used proto to emit messages
on server lines from ring section */
-#ifdef USE_OPENSSL
char *sni_expr; /* Temporary variable to store a sample
expression for SNI */
struct {
void *ctx;
@@ -367,7 +366,6 @@ struct server {
#ifdef USE_QUIC
struct quic_transport_params quic_params; /* QUIC transport parameters
*/
struct eb_root cids; /* QUIC connections IDs. */
-#endif
#endif
struct resolv_srvrq *srvrq; /* Pointer representing the DNS
SRV requeest, if any */
struct list srv_rec_item; /* to attach server to a srv
record item */
diff --git a/src/server.c b/src/server.c
index 54637dc9c..ea3271957 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1943,7 +1943,6 @@ const char *server_parse_maxconn_change_request(struct
server *sv,
return NULL;
}
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
static struct sample_expr *srv_sni_sample_parse_expr(struct server *srv,
struct proxy *px,
const char *file, int
linenum, char **err)
{
@@ -1983,7 +1982,6 @@ static int server_parse_sni_expr(struct server *newsrv,
struct proxy *px, char *
return 0;
}
-#endif
static void display_parser_err(const char *file, int linenum, char **args, int
cur_arg, int err_code, char **err)
{
@@ -2080,14 +2078,11 @@ static void srv_ssl_settings_cpy(struct server *srv,
struct server *src)
if (src->ssl_ctx.methods.max)
srv->ssl_ctx.methods.max = src->ssl_ctx.methods.max;
-#ifdef HAVE_SSL_CTX_SET_CIPHERSUITES
if (src->ssl_ctx.ciphersuites != NULL)
srv->ssl_ctx.ciphersuites = strdup(src->ssl_ctx.ciphersuites);
-#endif
if (src->sni_expr != NULL)
srv->sni_expr = strdup(src->sni_expr);
-#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
if (src->ssl_ctx.alpn_str) {
srv->ssl_ctx.alpn_str = malloc(src->ssl_ctx.alpn_len);
if (srv->ssl_ctx.alpn_str) {
@@ -2096,8 +2091,7 @@ static void srv_ssl_settings_cpy(struct server *srv,
struct server *src)
srv->ssl_ctx.alpn_len = src->ssl_ctx.alpn_len;
}
}
-#endif
-#ifdef OPENSSL_NPN_NEGOTIATED
+
if (src->ssl_ctx.npn_str) {
srv->ssl_ctx.npn_str = malloc(src->ssl_ctx.npn_len);
if (srv->ssl_ctx.npn_str) {
@@ -2106,7 +2100,6 @@ static void srv_ssl_settings_cpy(struct server *srv,
struct server *src)
srv->ssl_ctx.npn_len = src->ssl_ctx.npn_len;
}
}
-#endif
}
#endif
@@ -2463,13 +2456,13 @@ static int _srv_parse_tmpl_init(struct server *srv,
struct proxy *px)
srv_settings_cpy(newsrv, srv, 1);
srv_prepare_for_resolution(newsrv, srv->hostname);
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
+
if (newsrv->sni_expr) {
newsrv->ssl_ctx.sni = srv_sni_sample_parse_expr(newsrv,
px, NULL, 0, NULL);
if (!newsrv->ssl_ctx.sni)
goto err;
}
-#endif
+
/* append to list of servers available to receive an hostname */
if (newsrv->srvrq)
LIST_APPEND(&newsrv->srvrq->attached_servers,
&newsrv->srv_rec_item);
@@ -2488,9 +2481,7 @@ static int _srv_parse_tmpl_init(struct server *srv,
struct proxy *px)
err:
_srv_parse_set_id_from_prefix(srv, srv->tmpl_info.prefix,
srv->tmpl_info.nb_low);
if (newsrv) {
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
release_sample_expr(newsrv->ssl_ctx.sni);
-#endif
free_check(&newsrv->agent);
free_check(&newsrv->check);
LIST_DELETE(&newsrv->global_list);
@@ -2748,7 +2739,6 @@ static int _srv_parse_kw(struct server *srv, char **args,
int *cur_arg,
return err_code;
}
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
/* This function is first intended to be used through parse_server to
* initialize a new server on startup.
*/
@@ -2767,7 +2757,6 @@ static int _srv_parse_sni_expr_init(char **args, int
cur_arg,
return ret;
}
-#endif
/* Server initializations finalization.
* Initialize health check, agent check and SNI expression if enabled.
@@ -2780,9 +2769,7 @@ static int _srv_parse_finalize(char **args, int cur_arg,
struct server *srv, struct proxy *px,
int parse_flags, char **errmsg)
{
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
int ret;
-#endif
if (srv->do_check && srv->trackit) {
memprintf(errmsg, "unable to enable checks and tracking at the
same time!");
@@ -2795,10 +2782,8 @@ static int _srv_parse_finalize(char **args, int cur_arg,
return ERR_ALERT | ERR_FATAL;
}
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
if ((ret = _srv_parse_sni_expr_init(args, cur_arg, srv, px, errmsg)) !=
0)
return ret;
-#endif
/* A dynamic server is disabled on startup. It must not be counted as
* an active backend entry.
--
2.28.0
