From 53ae85c38e1d70634d06ea0958c17bb5b49d05b7 Mon Sep 17 00:00:00 2001 From: Lukas Tribus Date: Thu, 4 May 2017 15:45:40 +0000 Subject: [PATCH] MINOR: ssl: add prefer-client-ciphers 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. The initial proposal to change the current default and add a "prefer-server-ciphers" option (as implemented in e566ecb) has been declined due to the possible security impact. This patch implements prefer-client-ciphers without changing the defaults. [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 --- doc/configuration.txt | 5 +++++ include/types/listener.h | 1 + src/ssl_sock.c | 12 ++++++++++++ 3 files changed, 18 insertions(+) diff --git a/doc/configuration.txt b/doc/configuration.txt index 06a1a2af3..fea06cb5c 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -10592,6 +10592,11 @@ npn enabled (check with haproxy -vv). Note that the NPN extension has been replaced with the ALPN extension (see the "alpn" keyword). +prefer-client-ciphers + Use the client's preference when selecting the cipher suite, by default + the server's preference is enforced. This option is also available on + global statement "ssl-default-bind-options". + process [ all | odd | even | [-] ] 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 2b8f5feb6..8aae395e0 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_CLIE_CIPH 0x0200 /* prefer client ciphers */ #endif /* ssl "bind" settings */ diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 48ad1b26e..acb7d283f 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -3238,6 +3238,8 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) } if (conf_ssl_options & BC_SSL_O_NO_TLS_TICKETS) ssloptions |= SSL_OP_NO_TICKET; + if (conf_ssl_options & BC_SSL_O_PREF_CLIE_CIPH) + ssloptions &= ~SSL_OP_CIPHER_SERVER_PREFERENCE; SSL_CTX_set_options(ctx, ssloptions); SSL_CTX_set_mode(ctx, sslmode); if (global_ssl.life_time) @@ -6327,6 +6329,13 @@ static int bind_parse_ssl(char **args, int cur_arg, struct proxy *px, struct bin return 0; } +/* parse the "prefer-client-ciphers" bind keyword */ +static int bind_parse_pcc(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err) +{ + conf->ssl_options |= BC_SSL_O_PREF_CLIE_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) { @@ -6870,6 +6879,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-client-ciphers")) + global_ssl.listen_default_ssloptions |= BC_SSL_O_PREF_CLIE_CIPH; else { memprintf(err, "unknown option '%s' on global statement '%s'.", args[i], args[0]); return -1; @@ -7505,6 +7516,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-client-ciphers", bind_parse_pcc, 0 }, /* prefer client ciphers */ { NULL, NULL, 0 }, }};