Patchwork [V3] sslutil: more robustly detect protocol support

login
register
mail settings
Submitter Gregory Szorc
Date July 18, 2016, 6:31 p.m.
Message ID <c1f0a96351be26cd58f7.1468866669@gps-mbp.local>
Download mbox | patch
Permalink /patch/15948/
State Accepted
Headers show

Comments

Gregory Szorc - July 18, 2016, 6:31 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468866447 25200
#      Mon Jul 18 11:27:27 2016 -0700
# Node ID c1f0a96351be26cd58f7722954903826d7ef3548
# Parent  953839de96ab574caa40557c542c262286c6287c
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.

As part of developing this patch, it was discovered that Apple's
/usr/bin/python2.7 does not support TLS 1.1 and 1.2 (only TLS 1.0)!
On OS X 10.11, Apple Python has the modern ssl module including
SSLContext, but it doesn't appear to negotiate TLS 1.1+ nor does
it expose the constants related to TLS 1.1+. Since this code is
doing more robust feature detection (and not assuming modern ssl
implies TLS 1.1+ support), we now get TLS 1.0 warnings when running
on Apple Python. Hence the test changes.

I'm not super thrilled about shipping a Mercurial that always
whines about TLS 1.0 on OS X. We may want a follow-up patch to
suppress this warning.
Matt Mackall - July 19, 2016, 4:17 a.m.
On Mon, 2016-07-18 at 11:31 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1468866447 25200
> #      Mon Jul 18 11:27:27 2016 -0700
> # Node ID c1f0a96351be26cd58f7722954903826d7ef3548
> # Parent  953839de96ab574caa40557c542c262286c6287c
> sslutil: more robustly detect protocol support

Queued for default, thanks.
-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -36,8 +36,16 @@  configprotocols = set([
 ])
 
 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 = set(['tls1.0'])
+if util.safehasattr(ssl, 'PROTOCOL_TLSv1_1'):
+    supportedprotocols.add('tls1.1')
+if util.safehasattr(ssl, 'PROTOCOL_TLSv1_2'):
+    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
@@ -147,17 +155,15 @@  def _hostsettings(ui, hostname):
                 (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'):
@@ -287,9 +293,9 @@  def protocolsettings(protocol):
     # 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 '
@@ -440,10 +446,14 @@  def wrapserversocket(sock, ui, certfile=
     exactprotocol = ui.config('devel', 'serverexactprotocol')
     if exactprotocol == 'tls1.0':
         protocol = ssl.PROTOCOL_TLSv1
     elif exactprotocol == 'tls1.1':
+        if 'tls1.1' not in supportedprotocols:
+            raise error.Abort(_('TLS 1.1 not supported by this Python'))
         protocol = ssl.PROTOCOL_TLSv1_1
     elif exactprotocol == 'tls1.2':
+        if 'tls1.2' not in supportedprotocols:
+            raise error.Abort(_('TLS 1.2 not supported by this Python'))
         protocol = ssl.PROTOCOL_TLSv1_2
     elif exactprotocol:
         raise error.Abort(_('invalid value for serverexactprotocol: %s') %
                           exactprotocol)
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -438,8 +438,13 @@  def has_defaultcacertsloaded():
         ctx.load_default_certs()
 
     return len(ctx.get_ca_certs()) > 0
 
+@check("tls1.2", "TLS 1.2 protocol support")
+def has_tls1_2():
+    from mercurial import sslutil
+    return 'tls1.2' in sslutil.supportprotocols
+
 @check("windows", "Windows")
 def has_windows():
     return os.name == 'nt'
 
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -81,8 +81,9 @@  we are able to load CA certs.
 #endif
 
 #if defaultcacertsloaded
   $ hg clone https://localhost:$HGPORT/ copy-pull
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   (using CA certificates from *; if you see this message, your Mercurial install is not properly configured; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this message) (glob) (?)
   abort: error: *certificate verify failed* (glob)
   [255]
 #endif
@@ -107,8 +108,9 @@  A malformed per-host certificate file wi
 
   $ echo baddata > badca.pem
 #if sslcontext
   $ hg --config hostsecurity.localhost:verifycertsfile=badca.pem clone https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: error loading CA file badca.pem: * (glob)
   (file is empty or malformed?)
   [255]
 #else
@@ -122,8 +124,9 @@  A per-host certificate mismatching the s
 
 (modern ssl is able to discern whether the loaded cert is a CA cert)
 #if sslcontext
   $ hg --config hostsecurity.localhost:verifycertsfile="$CERTSDIR/client-cert.pem" clone https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   (an attempt was made to load CA certificates but none were loaded; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
   abort: error: *certificate verify failed* (glob)
   [255]
 #else
@@ -257,8 +260,9 @@  empty cacert file
 
 #if sslcontext
   $ hg --config web.cacerts=emptycafile -R copy-pull pull
   pulling from https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: error loading CA file emptycafile: * (glob)
   (file is empty or malformed?)
   [255]
 #else
@@ -352,20 +356,23 @@  configs.
 
 #if sslcontext
 Setting ciphers to an invalid value aborts
   $ P="$CERTSDIR" hg --config hostsecurity.ciphers=invalid -R copy-pull id https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: could not set ciphers: No cipher can be selected.
   (change cipher string (invalid) in config)
   [255]
 
   $ P="$CERTSDIR" hg --config hostsecurity.localhost:ciphers=invalid -R copy-pull id https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: could not set ciphers: No cipher can be selected.
   (change cipher string (invalid) in config)
   [255]
 
 Changing the cipher string works
 
   $ P="$CERTSDIR" hg --config hostsecurity.ciphers=HIGH -R copy-pull id https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   5fed3813f7f5
 #endif
 
 Fingerprints
@@ -434,9 +441,9 @@  Ports used by next test. Kill servers.
   $ killdaemons.py hg0.pid
   $ killdaemons.py hg1.pid
   $ killdaemons.py hg2.pid
 
-#if sslcontext
+#if sslcontext tls1.2
 Start servers running supported TLS versions
 
   $ cd test
   $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
@@ -569,8 +576,9 @@  Start hgweb that requires client certifi
 
 without client certificate:
 
   $ P="$CERTSDIR" hg id https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: error: *handshake failure* (glob)
   [255]
 
 with client certificate:
@@ -583,15 +591,18 @@  with client certificate:
   > EOT
 
   $ P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
   > --config auth.l.key="$CERTSDIR/client-key-decrypted.pem"
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   5fed3813f7f5
 
   $ printf '1234\n' | env P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
   > --config ui.interactive=True --config ui.nontty=True
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   passphrase for */client-key.pem: 5fed3813f7f5 (glob)
 
   $ env P="$CERTSDIR" hg id https://localhost:$HGPORT/
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   abort: error: * (glob)
   [255]
 
 #endif
diff --git a/tests/test-patchbomb-tls.t b/tests/test-patchbomb-tls.t
--- a/tests/test-patchbomb-tls.t
+++ b/tests/test-patchbomb-tls.t
@@ -68,8 +68,9 @@  we are able to load CA certs:
   $ try
   this patch series consists of 1 patches.
   
   
+  warning: connecting to localhost using legacy security technology (TLS 1.0); see https://mercurial-scm.org/wiki/SecureConnections for more info (?)
   (using CA certificates from *; if you see this message, your Mercurial install is not properly configured; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this message) (glob) (?)
   (?i)abort: .*?certificate.verify.failed.* (re)
   [255]