OpenSSL fixes and backwards compat break.

Drop SSL_clear() after SSL_new() which causes protocol negotiation failures for TLS1.0 vs TLS1.1/1.2 in OpenSSL 1.0.1b.

Remove SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS (+dont_insert_empty_fragments) from default of openssl_options.

6 files changed, 58 insertions, 16 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index e719855f8..016f3f075 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -14333,16 +14333,12 @@ harm. This option overrides the &%pipe_as_creator%& option of the &(pipe)&
 transport driver.
 
 
-.option openssl_options main "string list" +dont_insert_empty_fragments
+.option openssl_options main "string list" unset
 .cindex "OpenSSL "compatibility options"
 This option allows an administrator to adjust the SSL options applied
 by OpenSSL to connections.  It is given as a space-separated list of items,
-each one to be +added or -subtracted from the current value.  The default
-value is one option which happens to have been set historically.  You can
-remove all options with:
-.code
-openssl_options = -all
-.endd
+each one to be +added or -subtracted from the current value.
+
 This option is only available if Exim is built against OpenSSL.  The values
 available for this option vary according to the age of your OpenSSL install.
 The &"all"& value controls a subset of flags which are available, typically
@@ -14354,12 +14350,19 @@ names lose the leading &"SSL_OP_"& and are lower-cased.
 Note that adjusting the options can have severe impact upon the security of
 SSL as used by Exim.  It is possible to disable safety checks and shoot
 yourself in the foot in various unpleasant ways.  This option should not be
-adjusted lightly.  An unrecognised item will be detected at by invoking Exim
-with the &%-bV%& flag.
+adjusted lightly.  An unrecognised item will be detected at startup, by
+invoking Exim with the &%-bV%& flag.
+
+.new
+Historical note: prior to release 4.78, Exim defaulted this value to
+"+dont_insert_empty_fragments", which may still be needed for compatibility
+with some clients, but which lowers security by increasing exposure to
+some now infamous attacks.
+.wen
 
 An example:
 .code
-openssl_options = -all +microsoft_big_sslv3_buffer
+openssl_options = -all +microsoft_big_sslv3_buffer +dont_insert_empty_fragments
 .endd
 
 Possible options may include:
diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index b41783d71..a491cf973 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -69,6 +69,10 @@ PP/15 LDAP: Check for errors of TLS initialisation, to give correct
       diagnostics.
       Report and patch from Dmitry Banschikov.
 
+PP/16 Removed "dont_insert_empty_fragments" fron "openssl_options".
+      Removed SSL_clear() after SSL_new() which led to protocol negotiation
+      failures.  We appear to now support TLS1.1+ with Exim.
+
 
 Exim version 4.77
 -----------------
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index b962b61a2..0aee33cec 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -33,6 +33,15 @@ Version 4.78
     into the DBM library.  Can be used with gsasl to access sasldb2 files as
     used by Cyrus SASL.
 
+ 6. OpenSSL now supports TLS1.1 and TLS1.2 with OpenSSL 1.0.1.
+
+    Avoid release 1.0.1a if you can.  Note that the default value of
+    "openssl_options" is no longer "+dont_insert_empty_fragments", as that
+    increased susceptibility to attack.  This may still have interoperability
+    implications for very old clients (see version 4.31 change 37) but
+    administrators can choose to make the trade-off themselves and restore
+    compatibility at the cost of session security.
+
 
 Version 4.77
 ------------
diff --git a/doc/doc-txt/OptionLists.txt b/doc/doc-txt/OptionLists.txt
index 5313fd17c..b10f3f1aa 100644
--- a/doc/doc-txt/OptionLists.txt
+++ b/doc/doc-txt/OptionLists.txt
@@ -373,7 +373,7 @@ once                                 string*         unset         autoreply
 once_file_size                       integer         0             autoreply         3.20
 once_repeat                          time            0s            autoreply         2.95
 one_time                             boolean         false         redirect          4.00
-openssl_options                      string "+dont_insert_empty_fragments" main      4.73
+openssl_options                      string          unset         main              4.73 default to unset in 4.78
 optional                             boolean         false         iplookup          4.00
 oracle_servers                       string          unset         main              4.00
 owners                               string list     unset         redirect          4.00
diff --git a/src/README.UPDATING b/src/README.UPDATING
index 3dff7c094..5b6bea869 100644
--- a/src/README.UPDATING
+++ b/src/README.UPDATING
@@ -47,6 +47,22 @@ Exim version 4.78
 
    "openssl_options" gains "no_tlsv1_1", "no_tlsv1_2" and "no_compression".
 
+   COMPATIBILITY WARNING: The default value of "openssl_options" is no longer
+   "+dont_insert_empty_fragments".  We default to unset.  That old default was
+   grandfathered in from before openssl_options became a configuration option.
+   Empty fragments are inserted by default through TLS1.0, to partially defend
+   against certain attacks; TLS1.1+ change the protocol so that this is not
+   needed.  The DIEF SSL option was required for some old releases of mail
+   clients which did not gracefully handle the empty fragments, and was
+   initially set in Exim release 4.31 (see ChangeLog, item 37).
+
+   If you still have affected mail-clients, and you see SSL protocol failures
+   with this release of Exim, set:
+     openssl_options = +dont_insert_empty_fragments
+   in the main section of your Exim configuration file.  You're trading off
+   security for compatibility.  Exim is now defaulting to higher security and
+   rewarding more modern clients.
+
  * Ldap lookups returning multi-valued attributes now separate the attributes
    with only a comma, not a comma-space sequence.  Also, an actual comma within
    a returned attribute is doubled.  This makes it possible to parse the
diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index e2e150c0a..5e8c804e5 100644
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -695,7 +695,19 @@ else if (verify_check_host(&tls_try_verify_hosts) == OK)
 /* Prepare for new connection */
 
 if ((ssl = SSL_new(ctx)) == NULL) return tls_error(US"SSL_new", NULL, NULL);
-SSL_clear(ssl);
+
+/* Warning: we used to SSL_clear(ssl) here, it was removed.
+ *
+ * With the SSL_clear(), we get strange interoperability bugs with
+ * OpenSSL 1.0.1b and TLS1.1/1.2.  It looks as though this may be a bug in
+ * OpenSSL itself, as a clear should not lead to inability to follow protocols.
+ *
+ * The SSL_clear() call is to let an existing SSL* be reused, typically after
+ * session shutdown.  In this case, we have a brand new object and there's no
+ * obvious reason to immediately clear it.  I'm guessing that this was
+ * originally added because of incomplete initialisation which the clear fixed,
+ * in some historic release.
+ */
 
 /* Set context and tell client to go ahead, except in the case of TLS startup
 on connection, where outputting anything now upsets the clients and tends to
@@ -1332,10 +1344,8 @@ uschar keep_c;
 BOOL adding, item_parsed;
 
 result = 0L;
-/* We grandfather in as default the one option which we used to set always. */
-#ifdef SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS
-result |= SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS;
-#endif
+/* Prior to 4.78 we or'd in SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS; removed
+ * from default because it increases BEAST susceptibility. */
 
 if (option_spec == NULL)
   {