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
---
V3:
Fix clearing SSL_OP_CIPHER_SERVER_PREFERENCE when force-tlsXY methods
are used

V2:
patch doesn't change defaults and implements prefer-client-ciphers
instead of "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 06a1a2a..fea06cb 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -10592,6 +10592,11 @@ npn <protocols>
   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 | <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..8aae395 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 48ad1b2..acb7d28 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 },
 }};
 
-- 
2.7.4


Reply via email to