[PATCH 4 of 6] sslutil: inline sslkwargs() into wrapsocket()

Gregory Szorc gregory.szorc at gmail.com
Mon Mar 28 02:21:33 EDT 2016


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1459116402 25200
#      Sun Mar 27 15:06:42 2016 -0700
# Node ID cbe771e8d36d3e9685ede77ea37c42d4b4868cb8
# Parent  731b3acf1d2bb3fa60c2d05778e32b9cb512ff83
sslutil: inline sslkwargs() into wrapsocket()

sslkwargs() is a glorified wrapper to determine sane defaults for
the ca_certs and cert_reqs arguments to wrapsocket(). I don't see
a compelling reason for this functionality not to be in
wrapsocket() itself.

As part of moving the code, we change a variable name so "cacerts"
and "ca_certs" don't exist in the same function. This violates the
common convention that security code be hard to read. However,
Mercurial isn't written by monkeys, so this convention doesn't
apply.

diff --git a/mercurial/httpconnection.py b/mercurial/httpconnection.py
--- a/mercurial/httpconnection.py
+++ b/mercurial/httpconnection.py
@@ -273,15 +273,14 @@ class http2handler(urllib2.HTTPHandler, 
         if ':' in host and '[' not in host or ']:' in host:
             host, port = host.rsplit(':', 1)
             port = int(port)
             if '[' in host:
                 host = host[1:-1]
 
         kwargs['keyfile'] = keyfile
         kwargs['certfile'] = certfile
-
-        kwargs.update(sslutil.sslkwargs(self.ui, host))
+        kwargs['ui'] = self.ui
 
         con = HTTPConnection(host, port, use_ssl=True,
                              ssl_wrap_socket=sslutil.wrapsocket,
                              **kwargs)
         return con
diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -106,27 +106,19 @@ def _smtp(ui):
     # False. The first two perform hostname and fingerprint verification.
     # "strict" requires that a CA cert be trusted or a fingerprint be defined.
     verifycert = ui.config('smtp', 'verifycert', 'strict')
     if verifycert not in ['strict', 'loose']:
         if util.parsebool(verifycert) is not False:
             raise error.Abort(_('invalid smtp.verifycert configuration: %s')
                              % (verifycert))
         verifycert = False
-    if (starttls or smtps) and verifycert:
-        sslkwargs = sslutil.sslkwargs(ui, mailhost)
-
-        sslkwargs['serverhostname'] = mailhost
-
-        # Passed to the validator.
-        if verifycert == 'strict':
-            sslkwargs['requirefingerprintwhennocacerts'] = True
-    else:
-        # 'ui' is required by sslutil.wrapsocket() and set by sslkwargs()
-        sslkwargs = {'ui': ui, 'serverhostname': mailhost}
+    sslkwargs = {'ui': ui, 'serverhostname': mailhost}
+    if (starttls or smtps) and verifycert == 'strict':
+        sslkwargs['requirefingerprintwhennocacerts'] = True
     if smtps:
         ui.note(_('(using smtps)\n'))
         s = SMTPS(sslkwargs, local_hostname=local_hostname)
     elif starttls:
         s = STARTTLS(sslkwargs, local_hostname=local_hostname)
     else:
         s = smtplib.SMTP(local_hostname=local_hostname)
     if smtps:
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -126,16 +126,37 @@ def wrapsocket(sock, keyfile, certfile, 
       certificate fingerprints to be specified when CA certs are explicitly
       disabled (e.g. when executing in insecure mode). This is currently only
       used by the SMTP code. This argument should be removed in favor of a
       unified API for certificate verification.
     """
     if not serverhostname:
         raise error.Abort('serverhostname argument required')
 
+    # If we don't have a fingerprint pinned and we don't pass in a CA
+    # file path to use, try to find a CA path from the config or the system
+    # defaults.
+    if not ui.config('hostfingerprints', serverhostname) and not ca_certs:
+        capath = ui.config('web', 'cacerts')
+        if capath == '!':
+            pass
+        elif capath:
+            capath = util.expandpath(capath)
+            if not os.path.exists(capath):
+                raise error.Abort(_('could not find web.cacerts: %s') % capath)
+        else:
+            capath = _defaultcacerts()
+            if capath and capath != '!':
+                ui.debug('using %s to enable OS X system CA\n' % capath)
+            ui.setconfig('web', 'cacerts', capath, 'defaultcacerts')
+
+        if capath != '!':
+            ca_certs = capath
+            cert_reqs = ssl.CERT_REQUIRED
+
     # 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
     # supporting multiple TLS versions is to use PROTOCOL_SSLv23 and
@@ -238,39 +259,16 @@ def _defaultcacerts():
     if _plainapplepython():
         dummycert = os.path.join(os.path.dirname(__file__), 'dummycert.pem')
         if os.path.exists(dummycert):
             return dummycert
     if _canloaddefaultcerts:
         return None
     return '!'
 
-def sslkwargs(ui, host):
-    kws = {'ui': ui}
-    hostfingerprint = ui.config('hostfingerprints', host)
-    if hostfingerprint:
-        return kws
-    cacerts = ui.config('web', 'cacerts')
-    if cacerts == '!':
-        pass
-    elif cacerts:
-        cacerts = util.expandpath(cacerts)
-        if not os.path.exists(cacerts):
-            raise error.Abort(_('could not find web.cacerts: %s') % cacerts)
-    else:
-        cacerts = _defaultcacerts()
-        if cacerts and cacerts != '!':
-            ui.debug('using %s to enable OS X system CA\n' % cacerts)
-        ui.setconfig('web', 'cacerts', cacerts, 'defaultcacerts')
-    if cacerts != '!':
-        kws.update({'ca_certs': cacerts,
-                    'cert_reqs': ssl.CERT_REQUIRED,
-                    })
-    return kws
-
 class validator(object):
     def __init__(self, ui, host):
         self.ui = ui
         self.host = host
 
     def __call__(self, sock, strict=False):
         host = self.host
         cacerts = self.ui.config('web', 'cacerts')
diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -349,17 +349,17 @@ if has_https:
             self.sock = _create_connection((self.host, self.port))
 
             host = self.host
             if self.realhostport: # use CONNECT proxy
                 _generic_proxytunnel(self)
                 host = self.realhostport.rsplit(':', 1)[0]
             self.sock = sslutil.wrapsocket(
                 self.sock, self.key_file, self.cert_file, serverhostname=host,
-                **sslutil.sslkwargs(self.ui, host))
+                ui=self.ui)
 
     class httpshandler(keepalive.KeepAliveHandler, urllib2.HTTPSHandler):
         def __init__(self, ui):
             keepalive.KeepAliveHandler.__init__(self)
             urllib2.HTTPSHandler.__init__(self)
             self.ui = ui
             self.pwmgr = passwordmgr(self.ui)
 


More information about the Mercurial-devel mailing list