Patchwork [4,of,5] sslutil: more robustly detect protocol support

login
register
mail settings
Submitter Gregory Szorc
Date July 17, 2016, 6:28 p.m.
Message ID <306645544688957bf872.1468780107@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15918/
State Accepted
Headers show

Comments

Gregory Szorc - July 17, 2016, 6:28 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468779391 25200
#      Sun Jul 17 11:16:31 2016 -0700
# Node ID 306645544688957bf8729e1b03301e5240b0b8ed
# Parent  418f9f9b31c31e53bd233047be49e3993ceccfc1
sslutil: more robustly detect protocol support

The Python ssl module conditionally sets the TLS 1.1 and TLS 1.2
constants depending on whether HAVE_TLSv1_2 is defined. Yes, these
are both tied to the same constant (I would think there would be
separate constants for each version). Perhaps support for TLS 1.1
and 1.2 were added at the same time and the assumption is that
OpenSSL either has neither or both. I don't know.

Strictly speaking, we should introduce hghave tests for TLS 1.2 and
modify tests accordingly. However, I'm inclined to leave things as
they are because I'm curious what environments don't have TLS 1.2
in 2016 and want someone to report a bug so we can find and
erradicate installs/distributions that are shipping an insecure
Python 2.7.
Yuya Nishihara - July 18, 2016, 9:42 a.m.
On Sun, 17 Jul 2016 11:28:27 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1468779391 25200
> #      Sun Jul 17 11:16:31 2016 -0700
> # Node ID 306645544688957bf8729e1b03301e5240b0b8ed
> # Parent  418f9f9b31c31e53bd233047be49e3993ceccfc1
> sslutil: more robustly detect protocol support

Queued 1-3 and 5, thanks.

> +# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
> +# against don't support them.
> +supportedprotocols = set(['tls1.0'])
> +if getattr(ssl, 'PROTOCOL_TLSv1_1', 0):
> +    supportedprotocols.add('tls1.1')
> +if getattr(ssl, 'PROTOCOL_TLSv1_2', 0):
> +    supportedprotocols.add('tls1.2')

I might be too strict, but PROTOCOL_* constants aren't bit flags, so 0 is
valid value. Can we use safehasattr() instead?

> -    if not modernssl:
> +    if supportedprotocols == set(['tls1.0']):
>          if protocol != 'tls1.0':
>              raise error.Abort(_('current Python does not support protocol '
>                                  'setting %s') % protocol,
>                                hint=_('upgrade Python or disable setting since '
>                                       'only TLS 1.0 is supported'))
>  
>          return ssl.PROTOCOL_TLSv1, 0

Confirmed that legacy ssl module doesn't support tls1.1+. If it did, SSL2 and 3
would be enabled since options were noop.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -32,16 +32,24 @@  from . import (
 configprotocols = set([
     'tls1.0',
     'tls1.1',
     'tls1.2',
 ])
 
 hassni = getattr(ssl, 'HAS_SNI', False)
 
+# TLS 1.1 and 1.2 may not be supported if the OpenSSL Python is compiled
+# against don't support them.
+supportedprotocols = set(['tls1.0'])
+if getattr(ssl, 'PROTOCOL_TLSv1_1', 0):
+    supportedprotocols.add('tls1.1')
+if getattr(ssl, 'PROTOCOL_TLSv1_2', 0):
+    supportedprotocols.add('tls1.2')
+
 try:
     # ssl.SSLContext was added in 2.7.9 and presence indicates modern
     # SSL/TLS features are available.
     SSLContext = ssl.SSLContext
     modernssl = True
     _canloaddefaultcerts = util.safehasattr(SSLContext, 'load_default_certs')
 except AttributeError:
     modernssl = False
@@ -143,25 +151,23 @@  def _hostsettings(ui, hostname):
     def validateprotocol(protocol, key):
         if protocol not in configprotocols:
             raise error.Abort(
                 _('unsupported protocol from hostsecurity.%s: %s') %
                 (key, protocol),
                 hint=_('valid protocols: %s') %
                      ' '.join(sorted(configprotocols)))
 
-    # Legacy Python can only do TLS 1.0. We default to TLS 1.1+ where we
-    # can because TLS 1.0 has known vulnerabilities (like BEAST and POODLE).
-    # We allow users to downgrade to TLS 1.0+ via config options in case a
-    # legacy server is encountered.
-    if modernssl:
+    # We default to TLS 1.1+ where we can because TLS 1.0 has known
+    # vulnerabilities (like BEAST and POODLE). We allow users to downgrade to
+    # TLS 1.0+ via config options in case a legacy server is encountered.
+    if 'tls1.1' in supportedprotocols:
         defaultprotocol = 'tls1.1'
     else:
-        # Let people on legacy Python versions know they are borderline
-        # secure.
+        # Let people know they are borderline secure.
         # We don't document this config option because we want people to see
         # the bold warnings on the web site.
         # internal config: hostsecurity.disabletls10warning
         if not ui.configbool('hostsecurity', 'disabletls10warning'):
             ui.warn(_('warning: connecting to %s using legacy security '
                       'technology (TLS 1.0); see '
                       'https://mercurial-scm.org/wiki/SecureConnections for '
                       'more info\n') % hostname)
@@ -283,17 +289,17 @@  def protocolsettings(protocol):
     # support TLS 1.2.
     #
     # The PROTOCOL_TLSv* constants select a specific TLS version
     # only (as opposed to multiple versions). So the method for
     # supporting multiple TLS versions is to use PROTOCOL_SSLv23 and
     # disable protocols via SSLContext.options and OP_NO_* constants.
     # However, SSLContext.options doesn't work unless we have the
     # full/real SSLContext available to us.
-    if not modernssl:
+    if supportedprotocols == set(['tls1.0']):
         if protocol != 'tls1.0':
             raise error.Abort(_('current Python does not support protocol '
                                 'setting %s') % protocol,
                               hint=_('upgrade Python or disable setting since '
                                      'only TLS 1.0 is supported'))
 
         return ssl.PROTOCOL_TLSv1, 0