Patchwork [4,of,5,V2] sslutil: convert socket validation from a class to a function (API)

login
register
mail settings
Submitter Gregory Szorc
Date May 15, 2016, 11:18 p.m.
Message ID <e69e18b04ba5d395d506.1463354321@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15127/
State Accepted
Headers show

Comments

Gregory Szorc - May 15, 2016, 11:18 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1463337518 25200
#      Sun May 15 11:38:38 2016 -0700
# Node ID e69e18b04ba5d395d50671e69d6762c5107554bc
# Parent  7b8fd0e68768a980b114176cf809c6f1b283133d
sslutil: convert socket validation from a class to a function (API)

Now that the socket validator doesn't have any instance state,
we can make it a generic function.

The "validator" class has been converted into the "validatesocket"
function and all consumers have been updated.

Patch

diff --git a/mercurial/httpconnection.py b/mercurial/httpconnection.py
--- a/mercurial/httpconnection.py
+++ b/mercurial/httpconnection.py
@@ -279,11 +279,11 @@  class http2handler(urlreq.httphandler, u
 
         kwargs['keyfile'] = keyfile
         kwargs['certfile'] = certfile
 
         kwargs.update(sslutil.sslkwargs(self.ui, host))
 
         con = HTTPConnection(host, port, use_ssl=True,
                              ssl_wrap_socket=sslutil.wrapsocket,
-                             ssl_validator=sslutil.validator(self.ui, host),
+                             ssl_validator=sslutil.validatesocket,
                              **kwargs)
         return con
diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -134,17 +134,17 @@  def _smtp(ui):
     s.connect(host=mailhost, port=mailport)
     if starttls:
         ui.note(_('(using starttls)\n'))
         s.ehlo()
         s.starttls()
         s.ehlo()
     if (starttls or smtps) and verifycert:
         ui.note(_('(verifying remote certificate)\n'))
-        sslutil.validator(ui, mailhost)(s.sock, verifycert == 'strict')
+        sslutil.validatesocket(s.sock, verifycert == 'strict')
     username = ui.config('smtp', 'username')
     password = ui.config('smtp', 'password')
     if username and not password:
         password = ui.getpass()
     if username and password:
         ui.note(_('(authenticating to mail server as %s)\n') %
                   (username))
         try:
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -286,82 +286,82 @@  def sslkwargs(ui, host):
 
     # FUTURE this can disappear once wrapsocket() is secure by default.
     if _canloaddefaultcerts:
         kws['cert_reqs'] = ssl.CERT_REQUIRED
         return kws
 
     return kws
 
-class validator(object):
-    def __init__(self, ui=None, host=None):
-        pass
-
-    def __call__(self, sock, strict=False):
-        host = sock._hgstate['hostname']
-        ui = sock._hgstate['ui']
-
-        if not sock.cipher(): # work around http://bugs.python.org/issue13721
-            raise error.Abort(_('%s ssl connection error') % host)
-        try:
-            peercert = sock.getpeercert(True)
-            peercert2 = sock.getpeercert()
-        except AttributeError:
-            raise error.Abort(_('%s ssl connection error') % host)
-
-        if not peercert:
-            raise error.Abort(_('%s certificate error: '
-                               'no certificate received') % host)
+def validatesocket(sock, strict=False):
+    """Validate a socket meets security requiremnets.
 
-        # If a certificate fingerprint is pinned, use it and only it to
-        # validate the remote cert.
-        hostfingerprints = ui.configlist('hostfingerprints', host)
-        peerfingerprint = util.sha1(peercert).hexdigest()
-        nicefingerprint = ":".join([peerfingerprint[x:x + 2]
-            for x in xrange(0, len(peerfingerprint), 2)])
-        if hostfingerprints:
-            fingerprintmatch = False
-            for hostfingerprint in hostfingerprints:
-                if peerfingerprint.lower() == \
-                        hostfingerprint.replace(':', '').lower():
-                    fingerprintmatch = True
-                    break
-            if not fingerprintmatch:
-                raise error.Abort(_('certificate for %s has unexpected '
-                                   'fingerprint %s') % (host, nicefingerprint),
-                                 hint=_('check hostfingerprint configuration'))
-            ui.debug('%s certificate matched fingerprint %s\n' %
-                     (host, nicefingerprint))
-            return
+    The passed socket must have been created with ``wrapsocket()``.
+    """
+    host = sock._hgstate['hostname']
+    ui = sock._hgstate['ui']
 
-        # If insecure connections were explicitly requested via --insecure,
-        # print a warning and do no verification.
-        #
-        # It may seem odd that this is checked *after* host fingerprint pinning.
-        # This is for backwards compatibility (for now). The message is also
-        # the same as below for BC.
-        if ui.insecureconnections:
-            ui.warn(_('warning: %s certificate with fingerprint %s not '
-                      'verified (check hostfingerprints or web.cacerts '
-                      'config setting)\n') %
-                    (host, nicefingerprint))
-            return
+    if not sock.cipher(): # work around http://bugs.python.org/issue13721
+        raise error.Abort(_('%s ssl connection error') % host)
+    try:
+        peercert = sock.getpeercert(True)
+        peercert2 = sock.getpeercert()
+    except AttributeError:
+        raise error.Abort(_('%s ssl connection error') % host)
 
-        if not sock._hgstate['caloaded']:
-            if strict:
-                raise error.Abort(_('%s certificate with fingerprint %s not '
-                                    'verified') % (host, nicefingerprint),
-                                  hint=_('check hostfingerprints or '
-                                         'web.cacerts config setting'))
-            else:
-                ui.warn(_('warning: %s certificate with fingerprint %s '
-                          'not verified (check hostfingerprints or '
-                          'web.cacerts config setting)\n') %
-                        (host, nicefingerprint))
+    if not peercert:
+        raise error.Abort(_('%s certificate error: '
+                           'no certificate received') % host)
 
-            return
+    # If a certificate fingerprint is pinned, use it and only it to
+    # validate the remote cert.
+    hostfingerprints = ui.configlist('hostfingerprints', host)
+    peerfingerprint = util.sha1(peercert).hexdigest()
+    nicefingerprint = ":".join([peerfingerprint[x:x + 2]
+        for x in xrange(0, len(peerfingerprint), 2)])
+    if hostfingerprints:
+        fingerprintmatch = False
+        for hostfingerprint in hostfingerprints:
+            if peerfingerprint.lower() == \
+                    hostfingerprint.replace(':', '').lower():
+                fingerprintmatch = True
+                break
+        if not fingerprintmatch:
+            raise error.Abort(_('certificate for %s has unexpected '
+                               'fingerprint %s') % (host, nicefingerprint),
+                             hint=_('check hostfingerprint configuration'))
+        ui.debug('%s certificate matched fingerprint %s\n' %
+                 (host, nicefingerprint))
+        return
 
-        msg = _verifycert(peercert2, host)
-        if msg:
-            raise error.Abort(_('%s certificate error: %s') % (host, msg),
-                             hint=_('configure hostfingerprint %s or use '
-                                    '--insecure to connect insecurely') %
-                                  nicefingerprint)
+    # If insecure connections were explicitly requested via --insecure,
+    # print a warning and do no verification.
+    #
+    # It may seem odd that this is checked *after* host fingerprint pinning.
+    # This is for backwards compatibility (for now). The message is also
+    # the same as below for BC.
+    if ui.insecureconnections:
+        ui.warn(_('warning: %s certificate with fingerprint %s not '
+                  'verified (check hostfingerprints or web.cacerts '
+                  'config setting)\n') %
+                (host, nicefingerprint))
+        return
+
+    if not sock._hgstate['caloaded']:
+        if strict:
+            raise error.Abort(_('%s certificate with fingerprint %s not '
+                                'verified') % (host, nicefingerprint),
+                              hint=_('check hostfingerprints or '
+                                     'web.cacerts config setting'))
+        else:
+            ui.warn(_('warning: %s certificate with fingerprint %s '
+                      'not verified (check hostfingerprints or '
+                      'web.cacerts config setting)\n') %
+                    (host, nicefingerprint))
+
+        return
+
+    msg = _verifycert(peercert2, host)
+    if msg:
+        raise error.Abort(_('%s certificate error: %s') % (host, msg),
+                         hint=_('configure hostfingerprint %s or use '
+                                '--insecure to connect insecurely') %
+                              nicefingerprint)
diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -351,17 +351,17 @@  if has_https:
 
             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))
-            sslutil.validator(self.ui, host)(self.sock)
+            sslutil.validatesocket(self.sock)
 
     class httpshandler(keepalive.KeepAliveHandler, urlreq.httpshandler):
         def __init__(self, ui):
             keepalive.KeepAliveHandler.__init__(self)
             urlreq.httpshandler.__init__(self)
             self.ui = ui
             self.pwmgr = passwordmgr(self.ui)