Patchwork [2,of,5,V3] sslutil: config option to specify TLS protocol version

login
register
mail settings
Submitter Gregory Szorc
Date July 14, 2016, 4:50 a.m.
Message ID <6a6d56e1391ff7e1468e.1468471813@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15836/
State Superseded
Headers show

Comments

Gregory Szorc - July 14, 2016, 4:50 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468468118 25200
#      Wed Jul 13 20:48:38 2016 -0700
# Node ID 6a6d56e1391ff7e1468ef1b44b7e4c5cbe406f7b
# Parent  944f37469b94ea956566a037f9b7ae136a99b119
sslutil: config option to specify TLS protocol version

Currently, Mercurial will use TLS 1.0 or newer when connecting to
remote servers, selecting the highest TLS version supported by both
peers. On older Pythons, only TLS 1.0 is available. On newer Pythons,
TLS 1.1 and 1.2 should be available.

Security-minded people may want to not take any risks running
TLS 1.0 (or even TLS 1.1). This patch gives those people a config
option to explicitly control which TLS versions Mercurial should use.
By providing this option, one can require newer TLS versions
before they are formally deprecated by Mercurial/Python/OpenSSL/etc
and lower their security exposure. This option also provides an
easy mechanism to change protocol policies in Mercurial. If there
is a 0-day and TLS 1.0 is completely broken, we can act quickly
without changing much code.

Because setting the minimum TLS protocol is something you'll likely
want to do globally, this patch introduces a global config option under
[hostsecurity] for that purpose.

wrapserversocket() has been taught a hidden config option to define
the explicit protocol to use. This is queried in this function and
not passed as an argument because I don't want to expose this dangerous
option as part of the Python API. There is a risk someone could footgun
themselves. But the config option is a devel option, has a warning
comment, and I doubt most people are using `hg serve` to run a
production HTTPS server (I would have something not Mercurial/Python
handle TLS). If this is problematic, we can go back to using a
custom extension in tests to coerce the server into bad behavior.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -995,20 +995,32 @@  For example::
 
     [hostfingerprints]
     hg.intevation.de = fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
     hg.intevation.org = fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
 
 ``hostsecurity``
 ----------------
 
-Used to specify per-host security settings.
-
-Options in this section have the form ``hostname``:``setting``. This allows
-multiple settings to be defined on a per-host basis.
+Used to specify global and per-host security settings for connecting to
+other machines.
+
+The following options control default behavior for all hosts.
+
+``minimumprotocol``
+    Defines the minimum channel encryption protocol to use.
+
+    By default, the highest version of TLS - 1.0 or greater - supported by
+    both client and server is used.
+
+    Allowed values are: ``tls1.0`` (the default), ``tls1.1``, ``tls1.2``.
+
+Options in the ``[hostsecurity]`` section can have the form
+``hostname``:``setting``. This allows multiple settings to be defined on a
+per-host basis.
 
 The following per-host settings can be defined.
 
 ``fingerprints``
     A list of hashes of the DER encoded peer/remote certificate. Values have
     the form ``algorithm``:``fingerprint``. e.g.
     ``sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2``.
 
@@ -1021,16 +1033,20 @@  The following per-host settings can be d
     host and Mercurial will require the remote certificate to match one
     of the fingerprints specified. This means if the server updates its
     certificate, Mercurial will abort until a new fingerprint is defined.
     This can provide stronger security than traditional CA-based validation
     at the expense of convenience.
 
     This option takes precedence over ``verifycertsfile``.
 
+``minimumprotocol``
+    This behaves like ``minimumprotocol`` as described above except it
+    only applies to the host on which it is defined.
+
 ``verifycertsfile``
     Path to file a containing a list of PEM encoded certificates used to
     verify the server certificate. Environment variables and ``~user``
     constructs are expanded in the filename.
 
     The server certificate or the certificate's certificate authority (CA)
     must match a certificate from this file or certificate verification
     will fail and connections to the server will be refused.
@@ -1053,16 +1069,23 @@  The following per-host settings can be d
 
 For example::
 
     [hostsecurity]
     hg.example.com:fingerprints = sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2
     hg2.example.com:fingerprints = sha1:914f1aff87249c09b6859b88b1906d30756491ca, sha1:fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
     foo.example.com:verifycertsfile = /etc/ssl/trusted-ca-certs.pem
 
+To change the default minimum protocol version to TLS 1.2 but to allow TLS 1.1
+when connecting to ``hg.example.com``::
+
+    [hostsecurity]
+    minimumprotocol = tls1.2
+    hg.example.com:minimumprotocol = tls1.1
+
 ``http_proxy``
 --------------
 
 Used to access web-based Mercurial repositories through a HTTP
 proxy.
 
 ``host``
     Host name and (optional) port of the proxy server, for example
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -24,24 +24,23 @@  from . import (
 # Python 2.7.9+ overhauled the built-in SSL/TLS features of Python. It added
 # support for TLS 1.1, TLS 1.2, SNI, system CA stores, etc. These features are
 # all exposed via the "ssl" module.
 #
 # Depending on the version of Python being used, SSL/TLS support is either
 # modern/secure or legacy/insecure. Many operations in this module have
 # separate code paths depending on support in Python.
 
-hassni = getattr(ssl, 'HAS_SNI', False)
+configprotocols = set([
+    'tls1.0',
+    'tls1.1',
+    'tls1.2',
+])
 
-try:
-    OP_NO_SSLv2 = ssl.OP_NO_SSLv2
-    OP_NO_SSLv3 = ssl.OP_NO_SSLv3
-except AttributeError:
-    OP_NO_SSLv2 = 0x1000000
-    OP_NO_SSLv3 = 0x2000000
+hassni = getattr(ssl, 'HAS_SNI', False)
 
 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:
@@ -131,34 +130,45 @@  def _hostsettings(ui, hostname):
         # ssl.CERT_* constant used by SSLContext.verify_mode.
         'verifymode': None,
         # Defines extra ssl.OP* bitwise options to set.
         'ctxoptions': None,
     }
 
     # Despite its name, PROTOCOL_SSLv23 selects the highest protocol
     # that both ends support, including TLS protocols. On legacy stacks,
-    # the highest it likely goes in TLS 1.0. On modern stacks, it can
+    # the highest it likely goes is TLS 1.0. On modern stacks, it can
     # 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 modernssl:
-        s['protocol'] = ssl.PROTOCOL_SSLv23
-    else:
-        s['protocol'] = ssl.PROTOCOL_TLSv1
 
-    # SSLv2 and SSLv3 are broken. We ban them outright.
-    # WARNING: ctxoptions doesn't have an effect unless the modern ssl module
-    # is available. Be careful when adding flags!
-    s['ctxoptions'] = OP_NO_SSLv2 | OP_NO_SSLv3
+    # Allow minimum TLS protocol to be specified in the config.
+    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)))
+
+    key = 'minimumprotocol'
+    # Default to TLS 1.0+ as that is what browsers are currently doing.
+    protocol = ui.config('hostsecurity', key, 'tls1.0')
+    validateprotocol(protocol, key)
+
+    key = '%s:minimumprotocol' % hostname
+    protocol = ui.config('hostsecurity', key, protocol)
+    validateprotocol(protocol, key)
+
+    s['protocol'], s['ctxoptions'] = protocolsettings(protocol)
 
     # Look for fingerprints in [hostsecurity] section. Value is a list
     # of <alg>:<fingerprint> strings.
     fingerprints = ui.configlist('hostsecurity', '%s:fingerprints' % hostname,
                                  [])
     for fingerprint in fingerprints:
         if not (fingerprint.startswith(('sha1:', 'sha256:', 'sha512:'))):
             raise error.Abort(_('invalid fingerprint for %s: %s') % (
@@ -241,16 +251,52 @@  def _hostsettings(ui, hostname):
             s['verifymode'] = ssl.CERT_NONE
 
     assert s['protocol'] is not None
     assert s['ctxoptions'] is not None
     assert s['verifymode'] is not None
 
     return s
 
+def protocolsettings(protocol):
+    """Resolve the protocol and context options for a config value."""
+    if protocol not in configprotocols:
+        raise ValueError('protocol value not supported: %s' % protocol)
+
+    # Legacy ssl module only supports up to TLS 1.0. Ideally we'd use
+    # PROTOCOL_SSLv23 and options to disable SSLv2 and SSLv3. However,
+    # SSLContext.options doesn't work in our implementation since we use
+    # a fake SSLContext on these Python versions.
+    if not modernssl:
+        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
+
+    # WARNING: returned options don't work unless the modern ssl module
+    # is available. Be careful when adding options here.
+
+    # SSLv2 and SSLv3 are broken. We ban them outright.
+    options = ssl.OP_NO_SSLv2 | ssl.OP_NO_SSLv3
+
+    if protocol == 'tls1.0':
+        # Defaults above are to use TLS 1.0+
+        pass
+    elif protocol == 'tls1.1':
+        options |= ssl.OP_NO_TLSv1
+    elif protocol == 'tls1.2':
+        options |= ssl.OP_NO_TLSv1 | ssl.OP_NO_TLSv1_1
+    else:
+        raise error.Abort(_('this should not happen'))
+
+    return ssl.PROTOCOL_SSLv23, options
+
 def wrapsocket(sock, keyfile, certfile, ui, serverhostname=None):
     """Add SSL/TLS to a socket.
 
     This is a glorified wrapper for ``ssl.wrap_socket()``. It makes sane
     choices based on what security options are available.
 
     In addition to the arguments supported by ``ssl.wrap_socket``, we allow
     the following additional arguments:
@@ -297,28 +343,35 @@  def wrapsocket(sock, keyfile, certfile, 
         # This is a no-op on old Python.
         sslcontext.load_default_certs()
         caloaded = True
     else:
         caloaded = False
 
     try:
         sslsocket = sslcontext.wrap_socket(sock, server_hostname=serverhostname)
-    except ssl.SSLError:
+    except ssl.SSLError as e:
         # If we're doing certificate verification and no CA certs are loaded,
         # that is almost certainly the reason why verification failed. Provide
         # a hint to the user.
         # Only modern ssl module exposes SSLContext.get_ca_certs() so we can
         # only show this warning if modern ssl is available.
         if (caloaded and settings['verifymode'] == ssl.CERT_REQUIRED and
             modernssl and not sslcontext.get_ca_certs()):
             ui.warn(_('(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)\n'))
+        # Try to print more helpful error messages for known failures.
+        if util.safehasattr(e, 'reason'):
+            if e.reason == 'UNSUPPORTED_PROTOCOL':
+                ui.warn(_('(could not negotiate a common protocol; see '
+                          'https://mercurial-scm.org/wiki/SecureConnections '
+                          'for how to configure Mercurial to avoid this '
+                          'error)\n'))
         raise
 
     # check if wrap_socket failed silently because socket had been
     # closed
     # - see http://bugs.python.org/issue13721
     if not sslsocket.cipher():
         raise error.Abort(_('ssl connection failed'))
 
@@ -341,17 +394,47 @@  def wrapserversocket(sock, ui, certfile=
 
     ``cafile`` defines the path to certificate authorities.
 
     ``requireclientcert`` specifies whether to require client certificates.
 
     Typically ``cafile`` is only defined if ``requireclientcert`` is true.
     """
     if modernssl:
-        sslcontext = ssl.create_default_context(purpose=ssl.Purpose.CLIENT_AUTH)
+        protocol, options = protocolsettings('tls1.0')
+
+        # This config option is intended for use in tests only. It is a giant
+        # footgun to kill security. Don't define it.
+        exactprotocol = ui.config('devel', 'serverexactprotocol')
+        if exactprotocol == 'tls1.0':
+            protocol = ssl.PROTOCOL_TLSv1
+        elif exactprotocol == 'tls1.1':
+            protocol = ssl.PROTOCOL_TLSv1_1
+        elif exactprotocol == 'tls1.2':
+            protocol = ssl.PROTOCOL_TLSv1_2
+        elif exactprotocol:
+            raise error.Abort(_('invalid value for serverexactprotocol: %s') %
+                              exactprotocol)
+
+        # The protocol is pinned as SSLContext construction time and can't be
+        # changed after the fact (even though the class allows you to assign
+        # the variable it has no effect). Unfortunately,
+        # ``create_default_context`` doesn't allow us to pass a custom
+        # protocol.
+        if exactprotocol:
+            sslcontext = ssl.SSLContext(protocol)
+            sslcontext.options |= options
+            # Emulate create_default_context() behavior.
+            sslcontext.options |= ssl.OP_CIPHER_SERVER_PREFERENCE
+            sslcontext.options |= ssl.OP_SINGLE_DH_USE
+            sslcontext.options |= ssl.OP_SINGLE_ECDH_USE
+        else:
+            sslcontext = ssl.create_default_context(
+                purpose=ssl.Purpose.CLIENT_AUTH)
+
     else:
         sslcontext = SSLContext(ssl.PROTOCOL_TLSv1)
 
     # We don't need to apply options to the context here because either
     # a) create_default_context() has appropriate defaults and we don't have
     # any custom settings to apply b) legacy ssl module is already using the
     # appropriate protocol.
 
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -387,18 +387,78 @@  Test https with cert problems through pr
   abort: error: *certificate verify failed* (glob)
   [255]
   $ http_proxy=http://localhost:$HGPORT1/ hg -R copy-pull pull \
   > --config web.cacerts="$CERTSDIR/pub-expired.pem" https://localhost:$HGPORT2/
   pulling from https://localhost:$HGPORT2/
   abort: error: *certificate verify failed* (glob)
   [255]
 
+  $ killdaemons.py hg0.pid
+  $ killdaemons.py proxy.pid
+  $ killdaemons.py hg2.pid
+
+#if sslcontext
+Start servers running supported TLS versions
+
+  $ cd test
+  $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
+  > --config devel.serverexactprotocol=tls1.0
+  $ cat ../hg0.pid >> $DAEMON_PIDS
+  $ hg serve -p $HGPORT1 -d --pid-file=../hg1.pid --certificate=$PRIV \
+  > --config devel.serverexactprotocol=tls1.1
+  $ cat ../hg1.pid >> $DAEMON_PIDS
+  $ hg serve -p $HGPORT2 -d --pid-file=../hg2.pid --certificate=$PRIV \
+  > --config devel.serverexactprotocol=tls1.2
+  $ cat ../hg2.pid >> $DAEMON_PIDS
+  $ cd ..
+
+Clients talking same TLS versions work
+
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.0 id https://localhost:$HGPORT/
+  5fed3813f7f5
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.1 id https://localhost:$HGPORT1/
+  5fed3813f7f5
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT2/
+  5fed3813f7f5
+
+Clients requiring newer TLS version than what server supports fail
+
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.1 id https://localhost:$HGPORT/
+  (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
+  abort: error: *unsupported protocol* (glob)
+  [255]
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT/
+  (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
+  abort: error: *unsupported protocol* (glob)
+  [255]
+  $ P="$CERTSDIR" hg --config hostsecurity.minimumprotocol=tls1.2 id https://localhost:$HGPORT1/
+  (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
+  abort: error: *unsupported protocol* (glob)
+  [255]
+
+The per-host config option overrides the default
+
+  $ P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
+  > --config hostsecurity.minimumprotocol=tls1.2 \
+  > --config hostsecurity.localhost:minimumprotocol=tls1.0
+  5fed3813f7f5
+
+The per-host config option by itself works
+
+  $ P="$CERTSDIR" hg id https://localhost:$HGPORT/ \
+  > --config hostsecurity.localhost:minimumprotocol=tls1.2
+  (could not negotiate a common protocol; see https://mercurial-scm.org/wiki/SecureConnections for how to configure Mercurial to avoid this error)
+  abort: error: *unsupported protocol* (glob)
+  [255]
 
   $ killdaemons.py hg0.pid
+  $ killdaemons.py hg1.pid
+  $ killdaemons.py hg2.pid
+#endif
 
 #if sslcontext
 
 Start hgweb that requires client certificates:
 
   $ cd test
   $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
   > --config devel.servercafile=$PRIV --config devel.serverrequirecert=true