OpenSSL 1.1.0 support

[oerdnj]

Hi,

Debian Stretch (and possibly other distributions) are going to switch to OpenSSL 1.1.0.

Building with OpenSSL 1.1.0 breaks, so adding support to 2.5.x branch would be much appreciated. The original bug report and build log can be found here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=828276

[elliefm]

I've got openssl-1.1.0-pre5 installed and can reproduce the break. On with fixing it...

[elliefm]

Work in progress here: elliefm/cyrus-imapd@cyrus-imapd-2.5...v25/openssl-1.1

Builds and seems to work okay provided:
a) imap/tls_th-lock.[ch] are removed from imap_libcyrus_imap_la_SOURCES in Makefile.am
b) murder is not enabled at configure time

I'm still trying to figure out what to do with imap/tls_th-lock.[ch]. These files are providing the wrapper such that we can meet the constraint,

OpenSSL 1.0.2:
OpenSSL can safely be used in multi-threaded applications provided that at least two callback functions are set, locking_function and threadid_func.

The murder component is the only part of Cyrus that uses threads.

The docs for 1.1.0 seem to be saying that this is unnecessary for 1.1.0 -- that, provided OpenSSL was compiled with threading support, it will be thread safe without any work on our part:

OpenSSL 1.1.0:
OpenSSL can be safely used in multi-threaded applications provided that support for the underlying OS threading API is built-in.

So I'm starting to suspect it'll just be a matter of some more conditional compilation to make these functions into no-ops when linking with OpenSSL 1.1.0. And maybe detecting whether or not OpenSSL has "support ... built-in", and rejecting it if not.

[elliefm]

Okay that was pretty easy to integrate after all, it's now also on my github (link in previous comment)

It's now building and passing tests for me with both 1.0.2h and 1.1.0-pre5.

@oerdnj, how does it look to you?

I still mean to squash those commits down a bit, then once we're looking good I'll send them upstream, and also forward port to master, and backport as far as seems reasonable

[oerdnj]

Looks mostly good to me. One nitpick though:

+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+static SSL_SESSION *get_session_cb(SSL *ssl __attribute__((unused)),
+                  const unsigned char *id, int idlen, int *copy)
+#else
 static SSL_SESSION *get_session_cb(SSL *ssl __attribute__((unused)),
                   unsigned char *id, int idlen, int *copy)
+#endif

Is this necessary? Would declaring get_session_cb with const unsigned char *id for OpenSSL 1.0.x break anything?

BTW There's similar problem with cyrus-sasl2: cyrusimap/cyrus-sasl#4 (Perhaps I can convince you to look at it as well? :))

[elliefm]

It is necessary, as it turns out. I tried doing it the simple way at first, but it fell apart when I did a test build with 1.0.2h, so, mess it is. :(

I don't really know cyrus-sasl at all (have never even successfully built it from source...). I can have a look maybe next week if no-one else has already, but don't expect as quick a turnaround I suppose

[ksmurchison]

Your diffs look correct to me. I was actually working on this myself and came up with similar stuff. I'll let you continue your work and shelve mine for now.

[elliefm]

History cleaned up, and now on the real cyrus-imapd-2.5 branch.

Next: forward/back port...

[elliefm]

master was easy, same set of commits (modulo tabs/spaces)

[elliefm]

Now on cyrus-imapd-2.4 and cyrus-imapd-2.3 branches too