diff mbox series

[Openvpn-devel] Add SSL_CTX_get_max_proto_version() not in openssl 1.0

Message ID 1516423647-21932-1-git-send-email-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Add SSL_CTX_get_max_proto_version() not in openssl 1.0 | expand

Commit Message

Selva Nair Jan. 19, 2018, 5:47 p.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

- No change in functionality. This is used in a subsequent
  patch for extending TLS1.2 support with cryptoapicert

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/openssl_compat.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Steffan Karger Jan. 19, 2018, 10:50 p.m. UTC | #1 
Hi,

On 20-01-18 05:47, selva.nair@gmail.com wrote:
> From: Selva Nair <selva.nair@gmail.com>
> 
> - No change in functionality. This is used in a subsequent
>   patch for extending TLS1.2 support with cryptoapicert
> 
> Signed-off-by: Selva Nair <selva.nair@gmail.com>
> ---
>  src/openvpn/openssl_compat.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index 9f1e92a..c94341a 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -670,6 +670,29 @@ SSL_CTX_get_min_proto_version(SSL_CTX *ctx)
>  }
>  #endif /* SSL_CTX_get_min_proto_version */
>  
> +#ifndef SSL_CTX_get_max_proto_version
> +/** Return the max SSL protocol version currently enabled in the context.
> + *  If no valid version >= TLS1.0 is found, return 0. */
> +static inline int
> +SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
> +{
> +    long sslopt = SSL_CTX_get_options(ctx);
> +    if (!(sslopt & SSL_OP_NO_TLSv1_2))
> +    {
> +	return TLS1_2_VERSION;
> +    }
> +    if (!(sslopt & SSL_OP_NO_TLSv1_1))
> +    {
> +	return TLS1_1_VERSION;
> +    }
> +    if (!(sslopt & SSL_OP_NO_TLSv1))
> +    {
> +	return TLS1_VERSION;
> +    }
> +    return 0;
> +}
> +#endif /* SSL_CTX_get_max_proto_version */
> +
>  #ifndef SSL_CTX_set_min_proto_version
>  /** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */
>  static inline int
> 

Looks good and compiles fine.

Acked-by: Steffan Karger <steffan@karger.me>

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Steffan Karger Jan. 19, 2018, 11:14 p.m. UTC | #2 
Hi,

On 20-01-18 10:50, Steffan Karger wrote:
> On 20-01-18 05:47, selva.nair@gmail.com wrote:
>> From: Selva Nair <selva.nair@gmail.com>
>>
>> - No change in functionality. This is used in a subsequent
>>   patch for extending TLS1.2 support with cryptoapicert
>>
>> Signed-off-by: Selva Nair <selva.nair@gmail.com>
>> ---
>>  src/openvpn/openssl_compat.h | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
>> index 9f1e92a..c94341a 100644
>> --- a/src/openvpn/openssl_compat.h
>> +++ b/src/openvpn/openssl_compat.h
>> @@ -670,6 +670,29 @@ SSL_CTX_get_min_proto_version(SSL_CTX *ctx)
>>  }
>>  #endif /* SSL_CTX_get_min_proto_version */
>>  
>> +#ifndef SSL_CTX_get_max_proto_version
>> +/** Return the max SSL protocol version currently enabled in the context.
>> + *  If no valid version >= TLS1.0 is found, return 0. */
>> +static inline int
>> +SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
>> +{
>> +    long sslopt = SSL_CTX_get_options(ctx);
>> +    if (!(sslopt & SSL_OP_NO_TLSv1_2))
>> +    {
>> +	return TLS1_2_VERSION;
>> +    }
>> +    if (!(sslopt & SSL_OP_NO_TLSv1_1))
>> +    {
>> +	return TLS1_1_VERSION;
>> +    }
>> +    if (!(sslopt & SSL_OP_NO_TLSv1))
>> +    {
>> +	return TLS1_VERSION;
>> +    }
>> +    return 0;
>> +}
>> +#endif /* SSL_CTX_get_max_proto_version */
>> +
>>  #ifndef SSL_CTX_set_min_proto_version
>>  /** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */
>>  static inline int
>>
> 
> Looks good and compiles fine.
> 
> Acked-by: Steffan Karger <steffan@karger.me>

Sorry, one more thing:  the current patch is only okay for master, as
2.4 still supports openssl 0.9.8 and 1.0.0, which do not have the
SSL_OP_NO_TLSv1_1 and SSL_OP_NO_TLSv1_2 defines (the TLSx_VERSION ones
*are* available though).  If you want this patch backported to
release/2.4, it needs #ifdefs like get_min_proto_version has.

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair Jan. 20, 2018, 2:22 a.m. UTC | #3 
Hi,

On Sat, Jan 20, 2018 at 5:14 AM, Steffan Karger <steffan@karger.me> wrote:
> Hi,
>
> On 20-01-18 10:50, Steffan Karger wrote:
>> On 20-01-18 05:47, selva.nair@gmail.com wrote:
>>> From: Selva Nair <selva.nair@gmail.com>
>>>
>>> - No change in functionality. This is used in a subsequent
>>>   patch for extending TLS1.2 support with cryptoapicert
>>>
>>> Signed-off-by: Selva Nair <selva.nair@gmail.com>
>>> ---
>>>  src/openvpn/openssl_compat.h | 23 +++++++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>
>>> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
>>> index 9f1e92a..c94341a 100644
>>> --- a/src/openvpn/openssl_compat.h
>>> +++ b/src/openvpn/openssl_compat.h
>>> @@ -670,6 +670,29 @@ SSL_CTX_get_min_proto_version(SSL_CTX *ctx)
>>>  }
>>>  #endif /* SSL_CTX_get_min_proto_version */
>>>
>>> +#ifndef SSL_CTX_get_max_proto_version
>>> +/** Return the max SSL protocol version currently enabled in the context.
>>> + *  If no valid version >= TLS1.0 is found, return 0. */
>>> +static inline int
>>> +SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
>>> +{
>>> +    long sslopt = SSL_CTX_get_options(ctx);
>>> +    if (!(sslopt & SSL_OP_NO_TLSv1_2))
>>> +    {
>>> +    return TLS1_2_VERSION;
>>> +    }
>>> +    if (!(sslopt & SSL_OP_NO_TLSv1_1))
>>> +    {
>>> +    return TLS1_1_VERSION;
>>> +    }
>>> +    if (!(sslopt & SSL_OP_NO_TLSv1))
>>> +    {
>>> +    return TLS1_VERSION;
>>> +    }
>>> +    return 0;
>>> +}
>>> +#endif /* SSL_CTX_get_max_proto_version */
>>> +
>>>  #ifndef SSL_CTX_set_min_proto_version
>>>  /** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */
>>>  static inline int
>>>
>>
>> Looks good and compiles fine.
>>
>> Acked-by: Steffan Karger <steffan@karger.me>
>
> Sorry, one more thing:  the current patch is only okay for master, as
> 2.4 still supports openssl 0.9.8 and 1.0.0, which do not have the
> SSL_OP_NO_TLSv1_1 and SSL_OP_NO_TLSv1_2 defines (the TLSx_VERSION ones
> *are* available though).  If you want this patch backported to
> release/2.4, it needs #ifdefs like get_min_proto_version has.

Yeah, I meant it to go into master only (hence no ifdefs). Is it good
to have it in 2.4 too?
If so I will send a back-ported patch.

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 Jan. 20, 2018, 2:36 a.m. UTC | #4 
Your patch has been applied to the master branch.

commit 9e272106029a41b2110c10334ba8cae0f4afb1b4
Author: Selva Nair
Date:   Fri Jan 19 23:47:27 2018 -0500

     Add SSL_CTX_get_max_proto_version() not in openssl 1.0

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Steffan Karger <steffan.karger@fox-it.com>
     Message-Id: <1516423647-21932-1-git-send-email-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16287.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
Selva Nair Jan. 20, 2018, 3:05 a.m. UTC | #5 
Hi,

If/when the TLS1.2+ support for cryptoapicert is ready for merge, is
it a candidate for 2.4? Technically its a new feature but considering
the "popular belief" that TLS1.1 needs to be shunned, and 2.5 is still
far away, its worth considering..

In that case this patch would need to be backported to 2.4, so good to
know what's the general consensus.

Thanks,

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 Jan. 20, 2018, 5:52 a.m. UTC | #6 
Hi

On Sat, Jan 20, 2018 at 09:05:50AM -0500, Selva Nair wrote:
> If/when the TLS1.2+ support for cryptoapicert is ready for merge, is
> it a candidate for 2.4? Technically its a new feature but considering
> the "popular belief" that TLS1.1 needs to be shunned, and 2.5 is still
> far away, its worth considering..
> 
> In that case this patch would need to be backported to 2.4, so good to
> know what's the general consensus.

I would argue that TLS 1.2 is officially supported by 2.4, and as such
it's a bug if cryptoapicert doesn't :-)

Given that the changes are fairly well localized and will not affect
other platforms, and are likely to not affect users that do not use
cryptoapicert, I would tend to "make sure that all we offer works with
TLS 1.2" -> include in 2.4

But I let Steffan or David overrule me here if they find the patch too
intrusive.

gert
Steffan Karger Jan. 20, 2018, 6:09 a.m. UTC | #7 
Hi,

On 20-01-18 17:52, Gert Doering wrote:
> On Sat, Jan 20, 2018 at 09:05:50AM -0500, Selva Nair wrote:
>> If/when the TLS1.2+ support for cryptoapicert is ready for merge, is
>> it a candidate for 2.4? Technically its a new feature but considering
>> the "popular belief" that TLS1.1 needs to be shunned, and 2.5 is still
>> far away, its worth considering..
>>
>> In that case this patch would need to be backported to 2.4, so good to
>> know what's the general consensus.
> 
> I would argue that TLS 1.2 is officially supported by 2.4, and as such
> it's a bug if cryptoapicert doesn't :-)
> 
> Given that the changes are fairly well localized and will not affect
> other platforms, and are likely to not affect users that do not use
> cryptoapicert, I would tend to "make sure that all we offer works with
> TLS 1.2" -> include in 2.4

Completely agree.  I would like to see this backported to 2.4.

-Steffan
------------------------------------------------------------------------------
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

diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index 9f1e92a..c94341a 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -670,6 +670,29 @@  SSL_CTX_get_min_proto_version(SSL_CTX *ctx)
 }
 #endif /* SSL_CTX_get_min_proto_version */
 
+#ifndef SSL_CTX_get_max_proto_version
+/** Return the max SSL protocol version currently enabled in the context.
+ *  If no valid version >= TLS1.0 is found, return 0. */
+static inline int
+SSL_CTX_get_max_proto_version(SSL_CTX *ctx)
+{
+    long sslopt = SSL_CTX_get_options(ctx);
+    if (!(sslopt & SSL_OP_NO_TLSv1_2))
+    {
+	return TLS1_2_VERSION;
+    }
+    if (!(sslopt & SSL_OP_NO_TLSv1_1))
+    {
+	return TLS1_1_VERSION;
+    }
+    if (!(sslopt & SSL_OP_NO_TLSv1))
+    {
+	return TLS1_VERSION;
+    }
+    return 0;
+}
+#endif /* SSL_CTX_get_max_proto_version */
+
 #ifndef SSL_CTX_set_min_proto_version
 /** Mimics SSL_CTX_set_min_proto_version for OpenSSL < 1.1 */
 static inline int