diff mbox series

[Openvpn-devel] tls fix for upcoming 2.4.5

Message ID 52e860ea74ac958309368374049f14bd.squirrel@webmail.bi.invoca.ch
State Accepted
Headers
Series [Openvpn-devel] tls fix for upcoming 2.4.5 |

Commit Message

Simon Matter March 1, 2018, 12:14 a.m. UTC 
  Hi,

I've just done some test builds with 2.4.5 tagged version.

Attached patch makes it build with older systems. Do you see any issue
with the change?

Regards,
Simon
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Comments

Gert Doering March 1, 2018, 12:23 a.m. UTC | #1 
Hi,

On Thu, Mar 01, 2018 at 12:14:06PM +0100, Simon Matter wrote:
> I've just done some test builds with 2.4.5 tagged version.
> 
> Attached patch makes it build with older systems. Do you see any issue
> with the change?

Which SSL library version needs this?

(I thought we have test systems for everything we officially support, plus
a few more to catch avoidable LibreSSL breakage etc)

gert
Gert Doering March 1, 2018, 12:26 a.m. UTC | #2 
Hi,

On Thu, Mar 01, 2018 at 12:14:06PM +0100, Simon Matter wrote:
> I've just done some test builds with 2.4.5 tagged version.
> 
> Attached patch makes it build with older systems. Do you see any issue
> with the change?

As a side note: this won't make 2.4.5 release, which is already "mostly
done" (things are built, release announcement coming) - but if it's
needed I see no issue with having it in 2.4.6, and release that "quicker
than only in 5 month time" :-)

gert
Simon Matter March 1, 2018, 3:32 a.m. UTC | #3 
> Hi,
>
> On Thu, Mar 01, 2018 at 12:14:06PM +0100, Simon Matter wrote:
>> I've just done some test builds with 2.4.5 tagged version.
>>
>> Attached patch makes it build with older systems. Do you see any issue
>> with the change?
>
> Which SSL library version needs this?
>
> (I thought we have test systems for everything we officially support, plus
> a few more to catch avoidable LibreSSL breakage etc)

I saw that the #ifdefs were forgotten there, because they exist in other
places of the same file but didn't make it into the separate more recent
commit.

Anyway, it's for RHEL5, which is still supported for RedHat customers or
CERN Linux 5 users (SLC5).

I know OpenVPN officially doesn't care for it anymore but I do :-)

Regards,
Simon


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair March 1, 2018, 4:17 a.m. UTC | #4 
On Thu, Mar 1, 2018 at 6:14 AM, Simon Matter <simon.matter@invoca.ch> wrote:
> Hi,
>
> I've just done some test builds with 2.4.5 tagged version.
>
> Attached patch makes it build with older systems. Do you see any issue
> with the change?

.. from the attachment

> --- openvpn-2.4.5/src/openvpn/openssl_compat.h.orig 2018-02-28 21:56:54.000000000 +0100
> +++ openvpn-2.4.5/src/openvpn/openssl_compat.h 2018-03-01 11:44:57.000000000 +0100
> @@ -672,14 +672,18 @@
>      {
>          return TLS1_VERSION;
>      }
> +#ifdef SSL_OP_NO_TLSv1_1
>      if (!(sslopt & SSL_OP_NO_TLSv1_1))
>      {
>          return TLS1_1_VERSION;
>      }
> +#endif
> +#ifdef SSL_OP_NO_TLSv1_2
>      if (!(sslopt & SSL_OP_NO_TLSv1_2))
>      {
>          return TLS1_2_VERSION;
>      }
> +#endif
>      return 0;
>  }
>
> #endif /* SSL_CTX_get_min_proto_version */

These ifdefs are needed for older openssl (e.g., 0.9.8), but how did we miss it?

Turns out commit 2d705accea3e538a555631ef7c39eb4bc4fd4acf cherry-picked
from f8a92a4393a was not fully ripe..

As we do not support Windows build using pre 1.0 openssl, this is the
only change needed. So ACK, assuming a commit message and Author: may
be slapped on during merge.

Acked-by: Selva Nair <selva.nair@gmail.com>

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair March 1, 2018, 9:40 a.m. UTC | #5 
Hi,


>
>> --- openvpn-2.4.5/src/openvpn/openssl_compat.h.orig 2018-02-28 21:56:54.000000000 +0100
>> +++ openvpn-2.4.5/src/openvpn/openssl_compat.h 2018-03-01 11:44:57.000000000 +0100
>> @@ -672,14 +672,18 @@
>>      {
>>          return TLS1_VERSION;
>>      }
>> +#ifdef SSL_OP_NO_TLSv1_1
>>      if (!(sslopt & SSL_OP_NO_TLSv1_1))
>>      {
>>          return TLS1_1_VERSION;
>>      }
>> +#endif
>> +#ifdef SSL_OP_NO_TLSv1_2
>>      if (!(sslopt & SSL_OP_NO_TLSv1_2))
>>      {
>>          return TLS1_2_VERSION;
>>      }
>> +#endif
>>      return 0;
>>  }
>>
>> #endif /* SSL_CTX_get_min_proto_version */
>
> These ifdefs are needed for older openssl (e.g., 0.9.8), but how did we miss it?
>
> Turns out commit 2d705accea3e538a555631ef7c39eb4bc4fd4acf cherry-picked
> from f8a92a4393a was not fully ripe..
>
> As we do not support Windows build using pre 1.0 openssl, this is the
> only change needed. So ACK, assuming a commit message and Author: may
> be slapped on during merge.
>
> Acked-by: Selva Nair <selva.nair@gmail.com>

Forgot to add: 2.4 only.  Master is good as is.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering March 3, 2018, 10:04 p.m. UTC | #6 
Your patch has been applied to the release/2.4 branch.

(I've added a commit message explaining what Selva wrote about the
cherrypick etc., setting you as author)

commit 88abb911ea22a306e87fba58410da45c2baad57f
Author: Simon Matter
Date:   Fri Mar 2 08:49:31 2018 +0100

     Add missing #ifdef SSL_OP_NO_TLSv1_1/2

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <52e860ea74ac958309368374049f14bd.squirrel@webmail.bi.invoca.ch>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16588.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff mbox series

Patch

--- openvpn-2.4.5/src/openvpn/openssl_compat.h.orig	2018-02-28 21:56:54.000000000 +0100
+++ openvpn-2.4.5/src/openvpn/openssl_compat.h	2018-03-01 11:44:57.000000000 +0100
@@ -672,14 +672,18 @@ 
     {
         return TLS1_VERSION;
     }
+#ifdef SSL_OP_NO_TLSv1_1
     if (!(sslopt & SSL_OP_NO_TLSv1_1))
     {
         return TLS1_1_VERSION;
     }
+#endif
+#ifdef SSL_OP_NO_TLSv1_2
     if (!(sslopt & SSL_OP_NO_TLSv1_2))
     {
         return TLS1_2_VERSION;
     }
+#endif
     return 0;
 }
 #endif /* SSL_CTX_get_min_proto_version */