sslutil: config option to specify TLS protocol version
authorGregory Szorc <gregory.szorc@gmail.com>
Thu, 14 Jul 2016 20:47:22 -0700
changeset 29559 7dec5e441bf7
parent 29558 a935cd7d51a6
child 29560 303e9300772a
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.
mercurial/help/config.txt
mercurial/sslutil.py
tests/test-https.t
--- a/mercurial/help/config.txt	Thu Jul 14 20:07:10 2016 -0700
+++ b/mercurial/help/config.txt	Thu Jul 14 20:47:22 2016 -0700
@@ -1000,10 +1000,22 @@
 ``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.
 
@@ -1026,6 +1038,10 @@
 
     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``
@@ -1058,6 +1074,13 @@
     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``
 --------------
 
--- a/mercurial/sslutil.py	Thu Jul 14 20:07:10 2016 -0700
+++ b/mercurial/sslutil.py	Thu Jul 14 20:47:22 2016 -0700
@@ -29,14 +29,13 @@
 # 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
@@ -136,7 +135,7 @@
 
     # 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
@@ -145,19 +144,26 @@
     # 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
+
+    # 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)))
 
-    # 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
+    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)
 
-    # Prevent CRIME.
-    # There is no guarantee this attribute is defined on the module.
-    s['ctxoptions'] |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
+    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.
@@ -250,6 +256,46 @@
 
     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'))
+
+    # Prevent CRIME.
+    # There is no guarantee this attribute is defined on the module.
+    options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
+
+    return ssl.PROTOCOL_SSLv23, options
+
 def wrapsocket(sock, keyfile, certfile, ui, serverhostname=None):
     """Add SSL/TLS to a socket.
 
@@ -306,7 +352,7 @@
 
     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.
@@ -318,6 +364,13 @@
                       '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
@@ -349,14 +402,28 @@
 
     Typically ``cafile`` is only defined if ``requireclientcert`` is true.
     """
+    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)
+
     if modernssl:
         # We /could/ use create_default_context() here since it doesn't load
-        # CAs when configured for client auth.
-        sslcontext = SSLContext(ssl.PROTOCOL_SSLv23)
-        # SSLv2 and SSLv3 are broken. Ban them outright.
-        sslcontext.options |= OP_NO_SSLv2 | OP_NO_SSLv3
-        # Prevent CRIME
-        sslcontext.options |= getattr(ssl, 'OP_NO_COMPRESSION', 0)
+        # CAs when configured for client auth. However, it is hard-coded to
+        # use ssl.PROTOCOL_SSLv23 which may not be appropriate here.
+        sslcontext = SSLContext(protocol)
+        sslcontext.options |= options
+
         # Improve forward secrecy.
         sslcontext.options |= getattr(ssl, 'OP_SINGLE_DH_USE', 0)
         sslcontext.options |= getattr(ssl, 'OP_SINGLE_ECDH_USE', 0)
--- a/tests/test-https.t	Thu Jul 14 20:07:10 2016 -0700
+++ b/tests/test-https.t	Thu Jul 14 20:47:22 2016 -0700
@@ -345,11 +345,79 @@
   $ hg -R copy-pull id https://127.0.0.1:$HGPORT/ --config hostfingerprints.127.0.0.1=ecd87cd6b386d04fc1b8b41c9d8f5e168eef1c03
   5fed3813f7f5
 
-HGPORT1 is reused below for tinyproxy tests. Kill that server.
+Ports used by next test. Kill servers.
+
+  $ killdaemons.py hg0.pid
   $ killdaemons.py hg1.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
 
 Prepare for connecting through proxy
 
+  $ hg serve -R test -p $HGPORT -d --pid-file=hg0.pid --certificate=$PRIV
+  $ cat hg0.pid >> $DAEMON_PIDS
+  $ hg serve -R test -p $HGPORT2 -d --pid-file=hg2.pid --certificate=server-expired.pem
+  $ cat hg2.pid >> $DAEMON_PIDS
+tinyproxy.py doesn't fully detach, so killing it may result in extra output
+from the shell. So don't kill it.
   $ tinyproxy.py $HGPORT1 localhost >proxy.log </dev/null 2>&1 &
   $ while [ ! -f proxy.pid ]; do sleep 0; done
   $ cat proxy.pid >> $DAEMON_PIDS