Patchwork [3,of,6] sslutil: require a server hostname when wrapping sockets (API)

login
register
mail settings
Submitter Gregory Szorc
Date March 28, 2016, 6:21 a.m.
Message ID <731b3acf1d2bb3fa60c2.1459146092@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/14101/
State Superseded
Commit 1fde84d42f9c923b7e17b38894136af5f5a0b179
Headers show

Comments

Gregory Szorc - March 28, 2016, 6:21 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1459145827 25200
#      Sun Mar 27 23:17:07 2016 -0700
# Node ID 731b3acf1d2bb3fa60c2d05778e32b9cb512ff83
# Parent  cf65be71e39936624bf39041c93b94e66a45b881
sslutil: require a server hostname when wrapping sockets (API)

This function is used to wrap client sockets. If we don't pass a
server hostname, we can't validate server certificates. And if we
can't validate server certificates, we have no trust and therefore
no security. So require the hostname be specified.

As part of this, we remove the .ciphers() check from
validator.__call__ because it is redundant with what happens in
wrapsocket() (validator.__call__ is now called unconditionally).

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -123,16 +123,19 @@  def wrapsocket(sock, keyfile, certfile, 
       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.
     """
+    if not serverhostname:
+        raise error.Abort('serverhostname argument 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
@@ -166,20 +169,18 @@  def wrapsocket(sock, keyfile, certfile, 
 
     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)
+    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.
@@ -270,18 +271,16 @@  class validator(object):
         self.ui = ui
         self.host = host
 
     def __call__(self, sock, strict=False):
         host = self.host
         cacerts = self.ui.config('web', 'cacerts')
         hostfingerprints = self.ui.configlist('hostfingerprints', host)
 
-        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: '