From 82f6f63fee6ee9dc38ce5a93f1f09891b7852aef Mon Sep 17 00:00:00 2001 From: Shawn Wang Date: Mon, 5 Feb 2024 11:26:01 -0800 Subject: [PATCH] Add support of pinning NSX leaf cert In order to support cert pinning in WCP, this change adds exact cert match for checking NSX manager authenticity. Setting "nsx_cert_der" enables this mode, where the pritotity is below ca cert and above thumbprints. Currently in nsxlib, the call chain to manage HTTPs connextion is: 1. NSXHTTPAdapter (subclass of urllib3 HTTPAdapter) 2. urllib3 PoolManager 3. urllib3 HTTPSConnectionPool 4. urllib3 HTTPSConnection In order to inject custom TLS cert validation, we have to override the connect() function in HTTPSConnection level. Introducing a child class of HTTPSConnectionPool is also needed to pass the new param. Pool manager only needs overrding two attrs to allow passing the new param and properly binding to the new child class of connection pool. When leaf cert verification is not used, the native urllib3 behavior will be kept to reduce regression risk. Change-Id: Icecf30b6df3b60fbeac20cf79586827f3370ce13 --- vmware_nsxlib/tests/unit/v3/test_cluster.py | 33 ++++++++- vmware_nsxlib/v3/cluster.py | 79 +++++++++++++++++++-- vmware_nsxlib/v3/config.py | 17 ++++- 3 files changed, 117 insertions(+), 12 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/test_cluster.py b/vmware_nsxlib/tests/unit/v3/test_cluster.py index cf95017a..110d7bce 100644 --- a/vmware_nsxlib/tests/unit/v3/test_cluster.py +++ b/vmware_nsxlib/tests/unit/v3/test_cluster.py @@ -154,7 +154,7 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, - thumbprint=None, assert_hostname=None) + thumbprint=None, assert_hostname=None, nsx_cert_der=None) @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") @@ -178,7 +178,7 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, - thumbprint=None, assert_hostname=False) + thumbprint=None, assert_hostname=False, nsx_cert_der=None) @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") @@ -202,7 +202,34 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_adaptor_init.assert_called_once_with( pool_connections=1, pool_maxsize=1, max_retries=100, pool_block=False, - thumbprint="thumbprint", assert_hostname=None) + thumbprint="thumbprint", assert_hostname=None, + nsx_cert_der=None) + + @mock.patch("vmware_nsxlib.v3.debug_retry.RetryDebug.from_int") + @mock.patch("vmware_nsxlib.v3.cluster.NSXHTTPAdapter.__init__") + def test_new_connection_with_cert_der(self, mock_adaptor_init, + mock_retry): + mock_api = mock.Mock() + mock_api.nsxlib_config = mock.Mock() + mock_api.nsxlib_config.retries = 100 + mock_api.nsxlib_config.insecure = False + mock_api.nsxlib_config.ssl_assert_hostname = None + mock_adaptor_init.return_value = None + mock_retry.return_value = 100 + provider = cluster.NSXRequestsHTTPProvider() + with mock.patch.object(cluster.TimeoutSession, 'request', + return_value=get_sess_create_resp()): + session = provider.new_connection( + mock_api, cluster.Provider( + '9.8.7.6', 'https://9.8.7.6', None, + None, None, None, "nsx_cert_der")) + + self.assertIsNone(session.verify) + mock_adaptor_init.assert_called_once_with( + pool_connections=1, pool_maxsize=1, + max_retries=100, pool_block=False, + thumbprint=None, assert_hostname=None, + nsx_cert_der="nsx_cert_der") def test_validate_connection_keep_alive(self): mock_conn = mocks.MockRequestSessionApi() diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index f78d8718..5ee29675 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -15,9 +15,11 @@ # import abc +import collections import contextlib import copy import datetime +import functools import inspect import itertools import logging @@ -26,6 +28,10 @@ import time from urllib import parse as urlparse import urllib3 +from urllib3 import connection +from urllib3 import connectionpool +from urllib3 import exceptions as urllib_exc +from urllib3 import poolmanager import eventlet from eventlet import greenpool @@ -212,15 +218,18 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): # NSX v3 doesn't use redirects session.max_redirects = 0 + thumbprint = None + nsx_cert_der = None if config.insecure: # no verification on server certificate session.verify = False - thumbprint = None elif provider.ca_file: # verify using the said ca bundle path session.verify = provider.ca_file - thumbprint = None + elif provider.nsx_cert_der: + session.verify = None + nsx_cert_der = provider.nsx_cert_der elif provider.thumbprint: # verify using the thumbprint session.verify = None @@ -228,14 +237,14 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): else: # verify using the default system root CAs session.verify = True - thumbprint = None # we are pooling with eventlet in the cluster class adapter = NSXHTTPAdapter( pool_connections=1, pool_maxsize=1, max_retries=RetryDebug.from_int(config.retries), pool_block=False, thumbprint=thumbprint, - assert_hostname=config.ssl_assert_hostname) + assert_hostname=config.ssl_assert_hostname, + nsx_cert_der=nsx_cert_der) session.mount('http://', adapter) session.mount('https://', adapter) @@ -327,6 +336,7 @@ class NSXHTTPAdapter(adapters.HTTPAdapter): def __init__(self, *args, **kwargs): self.thumbprint = kwargs.pop("thumbprint", None) self.assert_hostname = kwargs.pop("assert_hostname", None) + self.nsx_cert_der = kwargs.pop("nsx_cert_der", None) super(NSXHTTPAdapter, self).__init__(*args, **kwargs) def init_poolmanager(self, *args, **kwargs): @@ -334,8 +344,63 @@ class NSXHTTPAdapter(adapters.HTTPAdapter): kwargs["assert_fingerprint"] = self.thumbprint if self.assert_hostname is not None: kwargs["assert_hostname"] = self.assert_hostname + if self.nsx_cert_der: + kwargs["nsx_cert_der"] = self.nsx_cert_der super(NSXHTTPAdapter, self).init_poolmanager(*args, **kwargs) + # Inject custom leaf cert validation only when needed + if not self.nsx_cert_der: + return + # Override HTTPs pool class + self.poolmanager.pool_classes_by_scheme = { + "http": connectionpool.HTTPConnectionPool, + "https": NSXHTTPSConnectionPool} + # Extend pool manager kwarg list to allow passing nsx_cert_der + # to NSXHTTPSConnection level + NSXPoolKey = collections.namedtuple( + "NSXPoolKey", (*poolmanager._key_fields, "key_nsx_cert_der")) + self.poolmanager.key_fn_by_scheme = { + "http": functools.partial( + poolmanager._default_key_normalizer, NSXPoolKey), + "https": functools.partial( + poolmanager._default_key_normalizer, NSXPoolKey), + } + + +class NSXHTTPSConnection(connection.HTTPSConnection): + """Subclass of urllib HTTPSConnection with exact cert verification. + + nsx_cert_der will be compared byte-to-byte with server cert without + checking the content of the certificate. + + Should only be used when NSX is to be authenticated with leaf cert + instead of proper CA bundle. + """ + nsx_cert_der = None + + def __init__(self, *args, **kwargs): + self.nsx_cert_der = kwargs.pop("nsx_cert_der", None) + super(NSXHTTPSConnection, self).__init__(*args, **kwargs) + + def connect(self): + # Call super.connect() first to do proper init including self.sock + super(NSXHTTPSConnection, self).connect() + if self.nsx_cert_der: + peer_cert = self.sock.getpeercert(binary_form=True) + if peer_cert != self.nsx_cert_der: + raise urllib_exc.SSLError( + "NSX cert doesn't match exactly with provided cert pem.") + self.is_verified = True + + +class NSXHTTPSConnectionPool(connectionpool.HTTPSConnectionPool): + """Subclass of urllib HTTPSConnectionPool for exact cert verification. + + Should only be used when NSX is to be authenticated with leaf cert + instead of proper CA bundle. + """ + ConnectionCls = NSXHTTPSConnection + class ClusterHealth(object): """Indicator of overall cluster health. @@ -367,13 +432,14 @@ class Provider(object): """ def __init__(self, provider_id, provider_url, username, password, ca_file, - thumbprint=None): + thumbprint=None, nsx_cert_der=None): self.id = provider_id self.url = provider_url self.username = username self.password = password self.ca_file = ca_file self.thumbprint = thumbprint + self.nsx_cert_der = nsx_cert_der def __str__(self): return str(self.url) @@ -903,5 +969,6 @@ class NSXClusteredAPI(ClusteredAPI): self.nsxlib_config.username(provider_index), self.nsxlib_config.password(provider_index), self.nsxlib_config.ca_file(provider_index), - self.nsxlib_config.thumbprint(provider_index))) + self.nsxlib_config.thumbprint(provider_index), + self.nsxlib_config.nsx_cert_der(provider_index))) return providers diff --git a/vmware_nsxlib/v3/config.py b/vmware_nsxlib/v3/config.py index bc65dad7..e4a65a44 100644 --- a/vmware_nsxlib/v3/config.py +++ b/vmware_nsxlib/v3/config.py @@ -90,12 +90,14 @@ class NsxLibConfig(object): :param ca_file: Specify a CA bundle file to use in verifying the NSX Manager server certificate. This option is ignored if "insecure" is set to True. If "insecure" is set to False - and "ca_file" is unset, the "thumbprint" will be used. + and "ca_file" is unset, the "nsx_cert_der" will be used. + If "nsx_cert_der" is unset , "thumbprint" will be used. If "thumbprint" is unset, the system root CAs will be used to verify the server certificate. :param thumbprint: Specify a thumbprint string to use in verifying the NSX Manager server certificate. This option is ignored - if "insecure" is set to True or "ca_file" is defined. + if "insecure" is set to True, or defining either of + "ca_file", "nsx_cert_der". :param token_provider: None, or instance of implemented AbstractJWTProvider which will return the JSON Web Token used in the requests in NSX for authorization. @@ -166,6 +168,10 @@ class NsxLibConfig(object): does not need to check the endpoint's accessibility. By default, this option is set to True. + :param nsx_cert_der: Specify one or a list of NSX manager leaf TLS + certificates in ASN1 / DER byte encoding to use for + verifying NSX Managers. This option is ignored + if "insecure" is set to True or "ca_file" is defined. -- Additional parameters which are relevant only for the Policy manager: :param allow_passthrough: If True, use nsx manager api for cases which are @@ -208,7 +214,8 @@ class NsxLibConfig(object): exception_config=None, api_log_mode=None, enable_health_check=True, - ssl_assert_hostname=None): + ssl_assert_hostname=None, + nsx_cert_der=None): self.nsx_api_managers = nsx_api_managers self._username = username @@ -242,6 +249,7 @@ class NsxLibConfig(object): self.api_log_mode = api_log_mode self.enable_health_check = enable_health_check self.ssl_assert_hostname = ssl_assert_hostname + self._nsx_cert_der = nsx_cert_der if len(nsx_api_managers) == 1 and not self.cluster_unavailable_retry: LOG.warning("When only one endpoint is provided, keepalive probes" @@ -287,3 +295,6 @@ class NsxLibConfig(object): def thumbprint(self, index): return self._attribute_by_index(self._thumbprint, index) + + def nsx_cert_der(self, index): + return self._attribute_by_index(self._nsx_cert_der, index)