Hi,
"already" - I see you just added it ;-)
But really great Jeroen for implementing these - thanks.
Just a few comments - I see you also added tls_compression - maybe you
should consider also actually implementing it? ;-)
Also I would recommend logging a failure if a wrong tls_eccurve is
specified as I do - you may also want to use openssl's automatic method of
enabled the elliptic curve instead of prime256v1.
I would recommend that you change tls_versions to a negative list instead
of a possible list and thereby follow the nature of openssl and other
projects, eg. sendmail, apache. The reasoning for this is that you want to
disable old protocols but always automatically want to support newer
protocols. At the moment you have no control over possible newer protocols
which openssl support. They would with your patch be added anyway so the
list would only specify a subset of protocols actually known by cyrus
imap. E.g. if openssl starts to support e.g. tls1.3 that would actually be
supported in cyrus without any upgrade but be counterintuitive since it
would not have to be listed in tls_versions.
Turning it around also makes transitioning easier for administrators, so
they do not have to make sure to update their protocol list in Cyrus IMAP
upon updating e.g. OpenSSL. Hence the SSL_OP_NO-options are "no"-options.
/Kristian
On Fri, 17 Oct 2014 12:34:21 +0200, Jeroen van Meeuwen (Kolab Systems)
<vanmeeu...@kolabsys.com> wrote:
On 2014-10-16 19:32, Kristian Kræmmer Nielsen wrote:
Hi,
Patch attached.
Something similar is already in cyrus-imapd-2.4:
http://git.cyrusimap.org/cyrus-imapd/commit/?h=cyrus-imapd-2.4&id=4b26d2d7244eeaa481871c337e57cd393fd76dfe
For master / 2.5, I have a push pending of a similar nature, while it
also addresses some client vs. server certificate chain configuration
options (i.e. Internet-facing public CA, verify client certificates
against private CA, offer client certificates between Cyrus IMAP
servers, and allow requiring certs to be set to "optional" or
"required").
Kind regards,
Jeroen van Meeuwen
