Patchwork [03,of,10,v2] sslutil: convert check for TLS 1.1 and TLS 1.2 into assert

login
register
mail settings
Submitter Manuel Jacob
Date May 30, 2020, 10:37 p.m.
Message ID <594ab49f13ab0caba9f4.1590878236@tmp>
Download mbox | patch
Permalink /patch/46422/
State New
Headers show

Comments

Manuel Jacob - May 30, 2020, 10:37 p.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1590783568 -7200
#      Fri May 29 22:19:28 2020 +0200
# Node ID 594ab49f13ab0caba9f4934803932cd9b37e96d1
# Parent  b33cc1f796e9c3aa6df1e51d36f0b3c061dfee9e
# EXP-Topic require_modern_ssl
sslutil: convert check for TLS 1.1 and TLS 1.2 into assert
Yuya Nishihara - May 31, 2020, 6:05 a.m.
On Sun, 31 May 2020 00:37:16 +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <me@manueljacob.de>
> # Date 1590783568 -7200
> #      Fri May 29 22:19:28 2020 +0200
> # Node ID 594ab49f13ab0caba9f4934803932cd9b37e96d1
> # Parent  b33cc1f796e9c3aa6df1e51d36f0b3c061dfee9e
> # EXP-Topic require_modern_ssl
> sslutil: convert check for TLS 1.1 and TLS 1.2 into assert
> 
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> --- a/mercurial/sslutil.py
> +++ b/mercurial/sslutil.py
> @@ -44,13 +44,11 @@ configprotocols = {
>  
>  hassni = getattr(ssl, 'HAS_SNI', False)
>  
> -# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
> -# against doesn't support them.
> -supportedprotocols = {b'tls1.0'}
> -if util.safehasattr(ssl, b'PROTOCOL_TLSv1_1'):
> -    supportedprotocols.add(b'tls1.1')
> -if util.safehasattr(ssl, b'PROTOCOL_TLSv1_2'):
> -    supportedprotocols.add(b'tls1.2')
> +supportedprotocols = {
> +    b'tls1.0',
> +    b'tls1.1',
> +    b'tls1.2',
> +}
>  
>  
>  def _hostsettings(ui, hostname):
> @@ -252,6 +250,11 @@ def protocolsettings(protocol):
>      if protocol not in configprotocols:
>          raise ValueError(b'protocol value not supported: %s' % protocol)
>  
> +    # We already check in setup.py that these attributes are present. To be
> +    # sure, we double-check here.
> +    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_1')
> +    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_2')

I expect there will be v3/v4 of this series. setup.py in v3 only requires
the existence of PROTOCOL_TLSv1_1 or PROTOCOL_TLSv1_2.
Manuel Jacob - May 31, 2020, 6:27 a.m.
On 2020-05-31 08:05, Yuya Nishihara wrote:
> On Sun, 31 May 2020 00:37:16 +0200, Manuel Jacob wrote:
>> # HG changeset patch
>> # User Manuel Jacob <me@manueljacob.de>
>> # Date 1590783568 -7200
>> #      Fri May 29 22:19:28 2020 +0200
>> # Node ID 594ab49f13ab0caba9f4934803932cd9b37e96d1
>> # Parent  b33cc1f796e9c3aa6df1e51d36f0b3c061dfee9e
>> # EXP-Topic require_modern_ssl
>> sslutil: convert check for TLS 1.1 and TLS 1.2 into assert
>> 
>> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
>> --- a/mercurial/sslutil.py
>> +++ b/mercurial/sslutil.py
>> @@ -44,13 +44,11 @@ configprotocols = {
>> 
>>  hassni = getattr(ssl, 'HAS_SNI', False)
>> 
>> -# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is 
>> compiled
>> -# against doesn't support them.
>> -supportedprotocols = {b'tls1.0'}
>> -if util.safehasattr(ssl, b'PROTOCOL_TLSv1_1'):
>> -    supportedprotocols.add(b'tls1.1')
>> -if util.safehasattr(ssl, b'PROTOCOL_TLSv1_2'):
>> -    supportedprotocols.add(b'tls1.2')
>> +supportedprotocols = {
>> +    b'tls1.0',
>> +    b'tls1.1',
>> +    b'tls1.2',
>> +}
>> 
>> 
>>  def _hostsettings(ui, hostname):
>> @@ -252,6 +250,11 @@ def protocolsettings(protocol):
>>      if protocol not in configprotocols:
>>          raise ValueError(b'protocol value not supported: %s' % 
>> protocol)
>> 
>> +    # We already check in setup.py that these attributes are present. 
>> To be
>> +    # sure, we double-check here.
>> +    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_1')
>> +    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_2')
> 
> I expect there will be v3/v4 of this series. setup.py in v3 only 
> requires
> the existence of PROTOCOL_TLSv1_1 or PROTOCOL_TLSv1_2.

I'll rework the branch and it is possible that the following cleanups 
don't need the assertion at all.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -44,13 +44,11 @@  configprotocols = {
 
 hassni = getattr(ssl, 'HAS_SNI', False)
 
-# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
-# against doesn't support them.
-supportedprotocols = {b'tls1.0'}
-if util.safehasattr(ssl, b'PROTOCOL_TLSv1_1'):
-    supportedprotocols.add(b'tls1.1')
-if util.safehasattr(ssl, b'PROTOCOL_TLSv1_2'):
-    supportedprotocols.add(b'tls1.2')
+supportedprotocols = {
+    b'tls1.0',
+    b'tls1.1',
+    b'tls1.2',
+}
 
 
 def _hostsettings(ui, hostname):
@@ -252,6 +250,11 @@  def protocolsettings(protocol):
     if protocol not in configprotocols:
         raise ValueError(b'protocol value not supported: %s' % protocol)
 
+    # We already check in setup.py that these attributes are present. To be
+    # sure, we double-check here.
+    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_1')
+    assert util.safehasattr(ssl, b'PROTOCOL_TLSv1_2')
+
     # Despite its name, PROTOCOL_SSLv23 selects the highest protocol
     # that both ends support, including TLS protocols. On legacy stacks,
     # the highest it likely goes is TLS 1.0. On modern stacks, it can