Patch for adding tls_honor_cipher_order

[Jeroen van Meeuwen (Kolab Systems)]

On 2014-10-23 16:04, Wolfgang Breyha wrote:
> Kristian Kræmmer Nielsen wrote on 17/10/14 15:13:
>> The more important part of my previous mail are that there are issues 
>> with
>> the patches that now have been merged into git. E.g. compression is 
>> not
>> merged correctly and it is recommended to do negative list and not
>> positive lists of protocols.
> 
> Yes, you're right. The patches in master tree have broken logic...
> 
> Option documentation says:
>  tls_versions: ssl2 ssl3 tls1_0 tls1_1 tls1_2
>            Disable SSL/TLS protocols not in this list.
> 
> Code says:
> + if (strstr(tls_versions, "tls1_2") == NULL) {
> +#if (OPENSSL_VERSION_NUMBER >= 0x1000105fL)
> + off |= SSL_OP_NO_TLSv1_2;
> +#else
> + syslog(LOG_ERR, "ERROR: TLSv1.2 configured, OpenSSL < 1.0.1e 
> insufficient");
> +#endif
> + }
> 
> Setting the NO_TLSv1_2 option does the opposite of the expected/wanted
> behavior.

You're aware though, that for the code to set NO_TLSv1_2 you would need 
to explicitly set a list of TLS versions that does not include tls1_2, 
such as:

   tls_versions: sslv2 sslv3 tls1_0 tls1_1

Let's not forget the code starts off with SSL_OP_ALL -- probably also 
not the best of ideas.

Should newer versions arrive (say, tls1_3), it would not be suppressed 
(the corresponding NO_TLSv1.3 flag would not be set) until after *both*; 
imap/tls.c is updated to handle a new value for the setting, and your 
configuration is not updated (to include the new value tls1_3 for it 
would otherwise be suppressed).

> I also would prefer a negative list as most other daemons like
> apache, exim, ... use. Maybe a more generic
> tls_openssl_options: no_ssl2 no_ssl3 no_compression 
> prefer_server_cipher_order
> would be better?
> 

A better way of specifying TLS versions would certainly be appreciated, 
especially if the list of options translates to openssl flags directly, 
so we don't have to patch/rebuild every time the flags change in order 
to allow newer/better versions.

I recall needing to upgrade Apache httpd from version 2.2 to 2.4 in 
order to be able to add -TLSv1.1:

   SSLOptions all -SSLv2 -SSLv3 -TLSv1 -TLSv1.1

or, for that matter:

   SSLOptions TLSv1.2

as neither flags were supported by httpd 2.2 -- admittedly, I could have 
backported the fix/enhancement rather than upgrade.

Anyway, it's one of the things I wanted to prevent having to do in Cyrus 
IMAP.

> And yes, you're also right with mentioning that functionality is 
> missing.
>  tls_compression: 0
>    Enable TLS compression. Disabled by default.
> 

This has been an oversight on my part.

>  tls_eccurve: prime256v1
>    Select the elliptic curve used for ECDHE.
> description is there, but there is no code doing it actually. Support 
> for ECDH
> auto mode in Openssl 1.2+ as provided in
> https://bugzilla.cyrusimap.org/attachment.cgi?id=1535
> is missing in the documentation as well.
> 

This patch and various other patches from different people in different 
tickets did not really mix well. Along with the tls_compression having 
been omitted, I did not consider documenting "auto" as a valid 
configuration value.

I'm also not sure what you mean by OpenSSL 1.2+ -- do you mean OpenSSL 
1.0.2+?

Kind regards,

Jeroen van Meeuwen

-- 
Systems Architect, Kolab Systems AG

e: vanmeeuwen at kolabsys.com
m: +41 79 951 9003
w: http://www.kolabsys.com

pgp: 9342 BF08