Hi Lukas, Willy, On 05/18/2018 05:55 PM, Lukas Tribus wrote: > Sets OpenSSL 1.1.1's SSL_OP_PRIORITIZE_CHACHA unconditionally, as per [1]: > > When SSL_OP_CIPHER_SERVER_PREFERENCE is set, temporarily reprioritize > ChaCha20-Poly1305 ciphers to the top of the server cipher list if a > ChaCha20-Poly1305 cipher is at the top of the client cipher list. This > helps those clients (e.g. mobile) use ChaCha20-Poly1305 if that cipher > is anywhere in the server cipher list; but still allows other clients to > use AES and other ciphers. Requires SSL_OP_CIPHER_SERVER_PREFERENCE. > > [1] https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_clear_options.html > --- > > RFC because we need to confirm we want to set this option unconditionally. > > We could also add another configuration option, however I don't see the > real benefit honestly: > > Symmetric crypto performance of ChaCha20-Poly1305 should not be a real > world concern at all as it performs very good. If it is a concern, it > should not be in the allowed cipher list in the first place. > > I'm not aware of any clients not supporting a recent AES-GCM (AEAD) > cipher while they do support ChaCha20-Poly1305, it certainly doesn't > make much sense. > > That is why I'm proposing to set this option unconditionally. > > --- > doc/configuration.txt | 3 +++ > src/ssl_sock.c | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/doc/configuration.txt b/doc/configuration.txt > index cbea330..d59a446 100644 > --- a/doc/configuration.txt > +++ b/doc/configuration.txt > @@ -10961,6 +10961,9 @@ 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". > + Note that with OpenSSL >= 1.1.1 ChaCha20-Poly1305 is reprioritized anyway > + (without setting this option), if a ChaCha20-Poly1305 cipher is at the top > of > + the client cipher list. > > process <process-set>[/<thread-set>] > This restricts the list of processes and/or threads on which this listener > is > diff --git a/src/ssl_sock.c b/src/ssl_sock.c > index 7a602ad..5a003dc 100644 > --- a/src/ssl_sock.c > +++ b/src/ssl_sock.c > @@ -1957,6 +1957,9 @@ ssl_sock_generate_certificate_from_conn(struct > bind_conf *bind_conf, SSL *ssl) > #ifndef SSL_MODE_SMALL_BUFFERS /* needs > small_records.patch */ > #define SSL_MODE_SMALL_BUFFERS 0 > #endif > +#ifndef SSL_OP_PRIORITIZE_CHACHA /* needs OpenSSL >= > 1.1.1 */ > +#define SSL_OP_PRIORITIZE_CHACHA 0 > +#endif > > #if (OPENSSL_VERSION_NUMBER < 0x1010000fL) > typedef enum { SET_CLIENT, SET_SERVER } set_context_func; > @@ -3711,6 +3714,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) > SSL_OP_SINGLE_DH_USE | > SSL_OP_SINGLE_ECDH_USE | > SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | > + SSL_OP_PRIORITIZE_CHACHA | > SSL_OP_CIPHER_SERVER_PREFERENCE; > long mode = > SSL_MODE_ENABLE_PARTIAL_WRITE | >
I agree, we could merge it as it is. R, Emeric
