Patchwork [2,of,8] sslutil: move SSLContext.verify_mode value into _hostsettings

login
register
mail settings
Submitter Gregory Szorc
Date May 28, 2016, 8:04 p.m.
Message ID <0d473c6da97a25e1b142.1464465864@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15230/
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 1464460881 25200
#      Sat May 28 11:41:21 2016 -0700
# Node ID 0d473c6da97a25e1b142436101502ea56aea456a
# Parent  281a76456d7e198949e97612f277e49c93e98b8d
sslutil: move SSLContext.verify_mode value into _hostsettings

_determinecertoptions() and _hostsettings() are redundant with each
other. _hostsettings() is used the flexible API we want.

We start the process of removing _determinecertoptions() by moving
some of the logic for the verify_mode value into _hostsettings().

As part of this, _determinecertoptions() now takes a settings dict
as its argument. This is technically API incompatible. But since
_determinecertoptions() came into existence a few days ago as part
of this release, I'm not flagging it as such.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -109,39 +109,45 @@  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': [],
+        # 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))
 
+    # If a host cert fingerprint is defined, it is the only thing that
+    # 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.
+
     return s
 
-def _determinecertoptions(ui, host):
+def _determinecertoptions(ui, settings):
     """Determine certificate options for a connections.
 
     Returns a tuple of (cert_reqs, ca_certs).
     """
-    # If a host key fingerprint is on file, it is the only thing that matters
-    # and CA certs don't come into play.
-    hostfingerprint = ui.config('hostfingerprints', host)
-    if hostfingerprint:
-        return ssl.CERT_NONE, None
-
-    # The code below sets up CA verification arguments. If --insecure is
-    # used, we don't take CAs into consideration, so return early.
-    if ui.insecureconnections:
+    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)
@@ -176,17 +182,18 @@  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')
 
-    cert_reqs, ca_certs = _determinecertoptions(ui, serverhostname)
+    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
@@ -229,17 +236,17 @@  def wrapsocket(sock, keyfile, certfile, 
     # closed
     # - see http://bugs.python.org/issue13721
     if not sslsocket.cipher():
         raise error.Abort(_('ssl connection failed'))
 
     sslsocket._hgstate = {
         'caloaded': caloaded,
         'hostname': serverhostname,
-        'settings': _hostsettings(ui, serverhostname),
+        'settings': settings,
         'ui': ui,
     }
 
     return sslsocket
 
 def _verifycert(cert, hostname):
     '''Verify that cert (in socket.getpeercert() format) matches hostname.
     CRLs is not handled.