Patchwork [3,of,8] sslutil: move CA file processing into _hostsettings()

login
register
mail settings
Submitter Gregory Szorc
Date May 28, 2016, 8:04 p.m.
Message ID <f38165c251143b5ac249.1464465865@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15231/
State Accepted
Headers show

Comments

Gregory Szorc - May 28, 2016, 8:04 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1464465213 25200
#      Sat May 28 12:53:33 2016 -0700
# Node ID f38165c251143b5ac249773a12a1f993da539974
# Parent  0d473c6da97a25e1b142436101502ea56aea456a
sslutil: move CA file processing into _hostsettings()

The CA file processing code has been moved from _determinecertoptions
into _hostsettings(). As part of the move, the logic has been changed
slightly and the "cacerts" variable has been renamed to "cafile" to
match the argument used by SSLContext.load_verify_locations().

Since _determinecertoptions() no longer contains any meaningful
code, it has been removed.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -109,16 +109,19 @@  except AttributeError:
 def _hostsettings(ui, hostname):
     """Obtain security settings for a hostname.
 
     Returns a dict of settings relevant to that hostname.
     """
     s = {
         # List of 2-tuple of (hash algorithm, hash).
         'certfingerprints': [],
+        # Path to file containing concatenated CA certs. Used by
+        # SSLContext.load_verify_locations().
+        'cafile': None,
         # ssl.CERT_* constant used by SSLContext.verify_mode.
         'verifymode': None,
     }
 
     # Fingerprints from [hostfingerprints] are always SHA-1.
     for fingerprint in ui.configlist('hostfingerprints', hostname, []):
         fingerprint = fingerprint.replace(':', '').lower()
         s['certfingerprints'].append(('sha1', fingerprint))
@@ -127,55 +130,49 @@  def _hostsettings(ui, hostname):
     # matters. No need to validate CA certs.
     if s['certfingerprints']:
         s['verifymode'] = ssl.CERT_NONE
 
     # If --insecure is used, don't take CAs into consideration.
     elif ui.insecureconnections:
         s['verifymode'] = ssl.CERT_NONE
 
-    # TODO assert verifymode is not None once we integrate cacert
-    # checking in this function.
+    # Try to hook up CA certificate validation unless something above
+    # makes it not necessary.
+    if s['verifymode'] is None:
+        # Find global certificates file in config.
+        cafile = ui.config('web', 'cacerts')
+
+        if cafile:
+            cafile = util.expandpath(cafile)
+            if not os.path.exists(cafile):
+                raise error.Abort(_('could not find web.cacerts: %s') % cafile)
+        else:
+            # No global CA certs. See if we can load defaults.
+            cafile = _defaultcacerts()
+            if cafile:
+                ui.debug('using %s to enable OS X system CA\n' % cafile)
+
+        s['cafile'] = cafile
+
+        # Require certificate validation if CA certs are being loaded and
+        # verification hasn't been disabled above.
+        if cafile or _canloaddefaultcerts:
+            s['verifymode'] = ssl.CERT_REQUIRED
+        else:
+            # At this point we don't have a fingerprint, aren't being
+            # explicitly insecure, and can't load CA certs. Connecting
+            # at this point is insecure. But we do it for BC reasons.
+            # TODO abort here to make secure by default.
+            s['verifymode'] = ssl.CERT_NONE
+
+    assert s['verifymode'] is not None
 
     return s
 
-def _determinecertoptions(ui, settings):
-    """Determine certificate options for a connections.
-
-    Returns a tuple of (cert_reqs, ca_certs).
-    """
-    if settings['verifymode'] == ssl.CERT_NONE:
-        return ssl.CERT_NONE, None
-
-    cacerts = ui.config('web', 'cacerts')
-
-    # If a value is set in the config, validate against a path and load
-    # and require those certs.
-    if cacerts:
-        cacerts = util.expandpath(cacerts)
-        if not os.path.exists(cacerts):
-            raise error.Abort(_('could not find web.cacerts: %s') % cacerts)
-
-        return ssl.CERT_REQUIRED, cacerts
-
-    # No CAs in config. See if we can load defaults.
-    cacerts = _defaultcacerts()
-
-    # We found an alternate CA bundle to use. Load it.
-    if cacerts:
-        ui.debug('using %s to enable OS X system CA\n' % cacerts)
-        ui.setconfig('web', 'cacerts', cacerts, 'defaultcacerts')
-        return ssl.CERT_REQUIRED, cacerts
-
-    # FUTURE this can disappear once wrapsocket() is secure by default.
-    if _canloaddefaultcerts:
-        return ssl.CERT_REQUIRED, None
-
-    return ssl.CERT_NONE, None
-
 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:
@@ -183,17 +180,16 @@  def wrapsocket(sock, keyfile, certfile, 
     * serverhostname - The expected hostname of the remote server. If the
       server (and client) support SNI, this tells the server which certificate
       to use.
     """
     if not serverhostname:
         raise error.Abort('serverhostname argument is required')
 
     settings = _hostsettings(ui, serverhostname)
-    cert_reqs, ca_certs = _determinecertoptions(ui, settings)
 
     # 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
     # support TLS 1.2.
     #
     # The PROTOCOL_TLSv* constants select a specific TLS version
     # only (as opposed to multiple versions). So the method for
@@ -210,26 +206,26 @@  def wrapsocket(sock, keyfile, certfile, 
 
     # TODO use ssl.create_default_context() on modernssl.
     sslcontext = SSLContext(protocol)
 
     # This is a no-op on old Python.
     sslcontext.options |= OP_NO_SSLv2 | OP_NO_SSLv3
 
     # This still works on our fake SSLContext.
-    sslcontext.verify_mode = cert_reqs
+    sslcontext.verify_mode = settings['verifymode']
 
     if certfile is not None:
         def password():
             f = keyfile or certfile
             return ui.getpass(_('passphrase for %s: ') % f, '')
         sslcontext.load_cert_chain(certfile, keyfile, password)
 
-    if ca_certs is not None:
-        sslcontext.load_verify_locations(cafile=ca_certs)
+    if settings['cafile'] is not None:
+        sslcontext.load_verify_locations(cafile=settings['cafile'])
         caloaded = True
     else:
         # This is a no-op on old Python.
         sslcontext.load_default_certs()
         caloaded = _canloaddefaultcerts
 
     sslsocket = sslcontext.wrap_socket(sock, server_hostname=serverhostname)
     # check if wrap_socket failed silently because socket had been