Currently we unconditionally set SSL_OP_CIPHER_SERVER_PREFERENCE [1], which may not always be a good thing.
The benefit of server side cipher prioritization may not apply to all cases out there, and it appears that the various SSL libs are going away from this recommendation ([2], [3]), as insecure ciphers suites are properly blacklisted/removed and honoring the client's preference is more likely to improve user experience (for example using SW-friendly ciphers on devices without HW AES support). This is especially true for TLSv1.3, which will restrict the cipher suites to just AES-GCM and Chacha20/Poly1305. Apache [4], nginx [5] and others give admins full flexibility, we should as well. This feature was actually implement in 1.5-dev (e566ecb), but later the option was removed (3c4bc6e), as the functionality was unconditionally enabled. This patch adds this feature again in current code. By leveraging ssl-default-bind-options bind line spam can be avoided. I opted for a change in default-behavior, to bring us inline with other products, recommendations and the actual API, as opposed to implement a "no-prefer-server-ciphers" kind of feature. As default behavior is changed, this should not be backported. [1] https://www.openssl.org/docs/man1.0.2/ssl/SSL_CTX_set_options.html [2] https://github.com/openssl/openssl/issues/541 [3] https://github.com/libressl-portable/portable/issues/66 [4] https://httpd.apache.org/docs/2.0/en/mod/mod_ssl.html#sslhonorcipherorder [5] https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_prefer_server_ciphers --- I would like some ACKs before we commit this, thanks! --- doc/configuration.txt | 6 ++++++ include/types/listener.h | 1 + src/ssl_sock.c | 15 +++++++++++++-- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index bfce6b1..a0cac69 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -10592,6 +10592,12 @@ npn <protocols> enabled (check with haproxy -vv). Note that the NPN extension has been replaced with the ALPN extension (see the "alpn" keyword). +prefer-server-ciphers + Use server's preference when selecting the cipher, instead of accepting the + client's preference (which is default since Haproxy 1.8). Before 1.8, server + preference was unconditionally enforced. This option is also available on + global statement "ssl-default-bind-options". + process [ all | odd | even | <number 1-64>[-<number 1-64>] ] This restricts the list of processes on which this listener is allowed to run. It does not enforce any process but eliminates those which do not match. diff --git a/include/types/listener.h b/include/types/listener.h index 2b8f5fe..cd80033 100644 --- a/include/types/listener.h +++ b/include/types/listener.h @@ -114,6 +114,7 @@ enum li_state { #define BC_SSL_O_USE_TLSV12 0x0080 /* force TLSv12 */ /* 0x00F0 reserved for 'force' protocol version options */ #define BC_SSL_O_NO_TLS_TICKETS 0x0100 /* disable session resumption tickets */ +#define BC_SSL_O_PREF_SERV_CIPH 0x0200 /* prefer server ciphers */ #endif /* ssl "bind" settings */ diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 4c1be5a..766b326 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -3179,8 +3179,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) SSL_OP_NO_COMPRESSION | SSL_OP_SINGLE_DH_USE | SSL_OP_SINGLE_ECDH_USE | - SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | - SSL_OP_CIPHER_SERVER_PREFERENCE; + SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION; long sslmode = SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER | @@ -3212,6 +3211,8 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) ssloptions |= SSL_OP_NO_TLSv1_1; if (conf_ssl_options & BC_SSL_O_NO_TLSV12) ssloptions |= SSL_OP_NO_TLSv1_2; + if (conf_ssl_options & BC_SSL_O_PREF_SERV_CIPH) + ssloptions |= SSL_OP_CIPHER_SERVER_PREFERENCE; } if (conf_ssl_options & BC_SSL_O_NO_TLS_TICKETS) ssloptions |= SSL_OP_NO_TICKET; @@ -6281,6 +6282,13 @@ static int bind_parse_ssl(char **args, int cur_arg, struct proxy *px, struct bin return 0; } +/* parse the "prefer-server-ciphers" bind keyword */ +static int bind_parse_pref_srv_ciphers(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err) +{ + conf->ssl_options |= BC_SSL_O_PREF_SERV_CIPH; + return 0; +} + /* parse the "generate-certificates" bind keyword */ static int bind_parse_generate_certs(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err) { @@ -6824,6 +6832,8 @@ static int ssl_parse_default_bind_options(char **args, int section_type, struct } else if (!strcmp(args[i], "no-tls-tickets")) global_ssl.listen_default_ssloptions |= BC_SSL_O_NO_TLS_TICKETS; + else if (!strcmp(args[i], "prefer-server-ciphers")) + global_ssl.listen_default_ssloptions |= BC_SSL_O_PREF_SERV_CIPH; else { memprintf(err, "unknown option '%s' on global statement '%s'.", args[i], args[0]); return -1; @@ -7459,6 +7469,7 @@ static struct bind_kw_list bind_kws = { "SSL", { }, { { "tls-ticket-keys", bind_parse_tls_ticket_keys, 1 }, /* set file to load TLS ticket keys from */ { "verify", bind_parse_verify, 1 }, /* set SSL verify method */ { "npn", bind_parse_npn, 1 }, /* set NPN supported protocols */ + { "prefer-server-ciphers", bind_parse_pref_srv_ciphers,0 }, /* prefer server ciphers */ { NULL, NULL, 0 }, }}; -- 2.7.4
