[PATCH 1 of 6] sslutil: perform certificate verification at socket wrap time

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


# HG changeset patch
# User Gregory Szorc <gregory.szorc at gmail.com>
# Date 1459145188 25200
#      Sun Mar 27 23:06:28 2016 -0700
# Node ID 78f292d3f2c09f55d1aa62e5926b3888635a2426
# Parent  36c21f6ed25641681e7c586ba2196a9d50939aff
sslutil: perform certificate verification at socket wrap time

Previously, the pattern has been:

* Wrap socket with SSL/TLS
* Validate certificate on socket using a separate function

This pattern was established before SSLContext existed and before
Python supported certificate verification in the standard library.
The pattern was also prone to bugs because callers may wrap the
socket but forget to verify the certificate!

This patch adds verification to wrapsocket() because it makes
sense in a SSLContext world and because we should maximize the
probability that good security is attained.

Callers of the validator in httpconnection.py and url.py have been
removed to prevent double verification.

We still have one caller to sslutil.validator() in mail.py. It's use
of sslutil is a bit more complex and fixing it will be done in a
separate patch.

The added argument to wrapsocket() is a bit hacky. The "strict"
argument to the validator is only used in mail.py. IMO, the
existence of this argument is a bit unfortunate: email.py
has a one-off mechanism for specifying 3 levels of validation.
It's reasons are valid. But the validation mechanism really needs
to be unified across all consumers. I plan to eventually address
this. However, it will require a bit more refactoring. For now,
live with the ugly named argument.

diff --git a/mercurial/httpconnection.py b/mercurial/httpconnection.py
--- a/mercurial/httpconnection.py
+++ b/mercurial/httpconnection.py
@@ -278,11 +278,10 @@ class http2handler(urllib2.HTTPHandler, 
 
         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),
                              **kwargs)
         return con
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -102,28 +102,36 @@ except AttributeError:
             }
 
             if self._supportsciphers:
                 args['ciphers'] = self._ciphers
 
             return ssl.wrap_socket(socket, **args)
 
 def wrapsocket(sock, keyfile, certfile, ui, cert_reqs=ssl.CERT_NONE,
-               ca_certs=None, serverhostname=None):
+               ca_certs=None, serverhostname=None,
+               requirefingerprintwhennocacerts=False):
     """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:
 
     * serverhostname - The expected hostname of the remote server. If the
       server (and client) support SNI, this tells the server which certificate
-      to use.
+      to use. It is also used for verifying that the hostname of the remote
+      certificate matches the expected hostname.
+
+    * requirefingerprintwhennocacerts - Boolean indicating whether to require
+      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.
     """
     # 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
@@ -157,16 +165,22 @@ def wrapsocket(sock, keyfile, certfile, 
         sslcontext.load_default_certs()
 
     sslsocket = sslcontext.wrap_socket(sock, server_hostname=serverhostname)
     # check if wrap_socket failed silently because socket had been
     # closed
     # - see http://bugs.python.org/issue13721
     if not sslsocket.cipher():
         raise error.Abort(_('ssl connection failed'))
+
+    # We can only verify if a hostname is known.
+    if serverhostname:
+        verifier = validator(ui, serverhostname)
+        verifier(sslsocket, strict=requirefingerprintwhennocacerts)
+
     return sslsocket
 
 def _verifycert(cert, hostname):
     '''Verify that cert (in socket.getpeercert() format) matches hostname.
     CRLs is not handled.
 
     Returns error message if any problems are found and None on success.
     '''
diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -350,17 +350,16 @@ 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)
 
     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