Repository: python-zk/kazoo (python)

Issue:
Add support for SNI hostnames
## What would you like to be added

Send the SNI header with TLS connections.

## Why is this needed

Currently when connecting to TLS hosts, only the IP is used. Some sites are behind passthrough proxies that proxy based upon hostname in the SNI header. Without support for SNI, these zookeeper ensembles cannot be reached via the library.

Pull request description:
Added support for SNI
Fixes #754

## Why is this needed?

Supports SNI

## Proposed Changes

Add server_hostname to the wrap call

## Does this PR introduce any breaking change?

No


Patch:
diff --git a/kazoo/client.py b/kazoo/client.py
index 27b7c38..7ee4e83 100644
--- a/kazoo/client.py
+++ b/kazoo/client.py
@@ -120,6 +120,7 @@ class KazooClient(object):
         ca=None,
         use_ssl=False,
         verify_certs=True,
+        check_hostname=False,
         **kwargs,
     ):
         """Create a :class:`KazooClient` instance. All time arguments
@@ -182,6 +183,8 @@ class KazooClient(object):
         :param use_ssl: argument to control whether SSL is used or not
         :param verify_certs: when using SSL, argument to bypass
             certs verification
+        :param check_hostname: when using SSL, check the hostname
+            against the hostname in the cert
 
         Basic Example:
 
@@ -237,6 +240,7 @@ class KazooClient(object):
 
         self.use_ssl = use_ssl
         self.verify_certs = verify_certs
+        self.check_hostname = check_hostname
         self.certfile = certfile
         self.keyfile = keyfile
         self.keyfile_password = keyfile_password
@@ -758,8 +762,10 @@ class KazooClient(object):
             raise ConnectionLoss("No connection to server")
 
         peer = self._connection._socket.getpeername()[:2]
+        peer_host = self._connection._socket.getpeername()[1]
         sock = self.handler.create_connection(
             peer,
+            hostname=peer_host,
             timeout=self._session_timeout / 1000.0,
             use_ssl=self.use_ssl,
             ca=self.ca,
@@ -767,6 +773,7 @@ class KazooClient(object):
             keyfile=self.keyfile,
             keyfile_password=self.keyfile_password,
             verify_certs=self.verify_certs,
+            check_hostname=self.check_hostname,
         )
         sock.sendall(cmd)
         result = sock.recv(8192)
diff --git a/kazoo/handlers/utils.py b/kazoo/handlers/utils.py
index f227baa..206806f 100644
--- a/kazoo/handlers/utils.py
+++ b/kazoo/handlers/utils.py
@@ -196,6 +196,7 @@ def create_tcp_socket(module):
 def create_tcp_connection(
     module,
     address,
+    hostname=None,
     timeout=None,
     use_ssl=False,
     ca=None,
@@ -203,6 +204,7 @@ def create_tcp_connection(
     keyfile=None,
     keyfile_password=None,
     verify_certs=True,
+    check_hostname=False,
     options=None,
     ciphers=None,
 ):
@@ -237,11 +239,14 @@ def create_tcp_connection(
 
             # Load default CA certs
             context.load_default_certs(ssl.Purpose.SERVER_AUTH)
+            if check_hostname and not verify_certs:
+                raise ValueError(
+                    "verify_certs must be True when"
+                    + " check_hostname is True"
+                )
             # We must set check_hostname to False prior to setting
             # verify_mode to CERT_NONE.
-            # TODO: Make hostname verification configurable as some users may
-            # elect to use it.
-            context.check_hostname = False
+            context.check_hostname = check_hostname
             context.verify_mode = (
                 ssl.CERT_REQUIRED if verify_certs else ssl.CERT_NONE
             )
@@ -258,7 +263,9 @@ def create_tcp_connection(
                 addrs = socket.getaddrinfo(
                     address[0], address[1], 0, socket.SOCK_STREAM
                 )
-                conn = context.wrap_socket(module.socket(addrs[0][0]))
+                conn = context.wrap_socket(
+                    module.socket(addrs[0][0]), server_hostname=hostname
+                )
                 conn.settimeout(timeout_at)
                 conn.connect(address)
                 sock = conn
diff --git a/kazoo/protocol/connection.py b/kazoo/protocol/connection.py
index ad4f3b1..ba30b84 100644
--- a/kazoo/protocol/connection.py
+++ b/kazoo/protocol/connection.py
@@ -703,6 +703,7 @@ class ConnectionHandler(object):
         with self._socket_error_handling():
             self._socket = self.handler.create_connection(
                 address=(hostip, port),
+                hostname=host,
                 timeout=client._session_timeout / 1000.0,
                 use_ssl=self.client.use_ssl,
                 keyfile=self.client.keyfile,
@@ -710,6 +711,7 @@ class ConnectionHandler(object):
                 ca=self.client.ca,
                 keyfile_password=self.client.keyfile_password,
                 verify_certs=self.client.verify_certs,
+                check_hostname=self.client.check_hostname,
             )
 
         self._socket.setblocking(0)


Test patch:
diff --git a/kazoo/tests/test_utils.py b/kazoo/tests/test_utils.py
index b56744c..96d484a 100644
--- a/kazoo/tests/test_utils.py
+++ b/kazoo/tests/test_utils.py
@@ -30,6 +30,59 @@ class TestCreateTCPConnection(unittest.TestCase):
                     timeout = call_args[0][1]
                     assert timeout >= 0, "socket timeout must be nonnegative"
 
+    def test_ssl_server_hostname(self):
+        from kazoo.handlers import utils
+        from kazoo.handlers.utils import create_tcp_connection, socket, ssl
+
+        with patch.object(utils, "_set_default_tcpsock_options"):
+            with patch.object(ssl.SSLContext, "wrap_socket") as wrap_socket:
+                create_tcp_connection(
+                    socket,
+                    ("127.0.0.1", 2181),
+                    timeout=1.5,
+                    hostname="fakehostname",
+                    use_ssl=True,
+                )
+
+                for call_args in wrap_socket.call_args_list:
+                    server_hostname = call_args[1]["server_hostname"]
+                    assert server_hostname == "fakehostname"
+
+    def test_ssl_server_check_hostname(self):
+        from kazoo.handlers import utils
+        from kazoo.handlers.utils import create_tcp_connection, socket, ssl
+
+        with patch.object(utils, "_set_default_tcpsock_options"):
+            with patch.object(
+                ssl.SSLContext, "wrap_socket", autospec=True
+            ) as wrap_socket:
+                create_tcp_connection(
+                    socket,
+                    ("127.0.0.1", 2181),
+                    timeout=1.5,
+                    hostname="fakehostname",
+                    use_ssl=True,
+                    check_hostname=True,
+                )
+
+                for call_args in wrap_socket.call_args_list:
+                    ssl_context = call_args[0][0]
+                    assert ssl_context.check_hostname
+
+    def test_ssl_server_check_hostname_config_validation(self):
+        from kazoo.handlers.utils import create_tcp_connection, socket
+
+        with pytest.raises(ValueError):
+            create_tcp_connection(
+                socket,
+                ("127.0.0.1", 2181),
+                timeout=1.5,
+                hostname="fakehostname",
+                use_ssl=True,
+                verify_certs=False,
+                check_hostname=True,
+            )
+
     def test_timeout_arg_eventlet(self):
         if not EVENTLET_HANDLER_AVAILABLE:
             pytest.skip("eventlet handler not available.")