From 1270fc1a93fdd5a5bfc4220919f8297e8e01af42 Mon Sep 17 00:00:00 2001 From: Anna Khmelnitsky Date: Thu, 16 Feb 2017 11:50:56 -0800 Subject: [PATCH] Replace client cert file with cert provider In nsxlib configuration, replace client certificate file with a broader concept of provider: apart from certificate file name, the provider can implement __enter__ and __exit__ routines to handle file creation and disposal Change-Id: I0c11107324786cf0852b054f32940422dffef5bb --- .../tests/unit/v3/nsxlib_testcase.py | 3 +- vmware_nsxlib/tests/unit/v3/test_cluster.py | 10 ++- vmware_nsxlib/v3/client_cert.py | 23 +++++ vmware_nsxlib/v3/cluster.py | 83 +++++++++++-------- vmware_nsxlib/v3/config.py | 14 ++-- vmware_nsxlib/v3/nsx_constants.py | 3 - 6 files changed, 85 insertions(+), 51 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py index aa9ec2bc..6ad95741 100644 --- a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py +++ b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py @@ -22,6 +22,7 @@ from requests import exceptions as requests_exceptions from vmware_nsxlib import v3 from vmware_nsxlib.v3 import client as nsx_client +from vmware_nsxlib.v3 import client_cert from vmware_nsxlib.v3 import cluster as nsx_cluster from vmware_nsxlib.v3 import config @@ -111,7 +112,7 @@ def get_default_nsxlib_config(): def get_nsxlib_config_with_client_cert(): return config.NsxLibConfig( - client_cert_file=CLIENT_CERT, + client_cert_provider=client_cert.ClientCertProvider(CLIENT_CERT), retries=NSX_HTTP_RETRIES, insecure=NSX_INSECURE, ca_file=NSX_CERT, diff --git a/vmware_nsxlib/tests/unit/v3/test_cluster.py b/vmware_nsxlib/tests/unit/v3/test_cluster.py index 747c57d3..0933163b 100644 --- a/vmware_nsxlib/tests/unit/v3/test_cluster.py +++ b/vmware_nsxlib/tests/unit/v3/test_cluster.py @@ -23,6 +23,7 @@ from requests import exceptions as requests_exceptions from vmware_nsxlib.tests.unit.v3 import mocks from vmware_nsxlib.tests.unit.v3 import nsxlib_testcase from vmware_nsxlib.v3 import client +from vmware_nsxlib.v3 import client_cert from vmware_nsxlib.v3 import cluster from vmware_nsxlib.v3 import exceptions as nsxlib_exc @@ -47,6 +48,7 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_api.nsxlib_config.ca_file = None mock_api.nsxlib_config.http_timeout = 99 mock_api.nsxlib_config.conn_idle_timeout = 39 + mock_api.nsxlib_config.client_cert_provider = None provider = cluster.NSXRequestsHTTPProvider() session = provider.new_connection( mock_api, cluster.Provider('9.8.7.6', 'https://9.8.7.6', @@ -66,15 +68,17 @@ class RequestsHTTPProviderTestCase(unittest.TestCase): mock_api.nsxlib_config.ca_file = None mock_api.nsxlib_config.http_timeout = 99 mock_api.nsxlib_config.conn_idle_timeout = 39 + cert_provider_inst = client_cert.ClientCertProvider( + '/etc/cert.pem') + mock_api.nsxlib_config.client_cert_provider = cert_provider_inst provider = cluster.NSXRequestsHTTPProvider() session = provider.new_connection( mock_api, cluster.Provider('9.8.7.6', 'https://9.8.7.6', - None, None, None, - '/etc/cert.pem')) + None, None, None)) self.assertEqual(session.auth, None) self.assertEqual(session.verify, False) - self.assertEqual(session.cert, '/etc/cert.pem') + self.assertEqual(session.cert_provider, cert_provider_inst) self.assertEqual(session.timeout, 99) def test_validate_connection(self): diff --git a/vmware_nsxlib/v3/client_cert.py b/vmware_nsxlib/v3/client_cert.py index 95ea1063..7289fb0e 100644 --- a/vmware_nsxlib/v3/client_cert.py +++ b/vmware_nsxlib/v3/client_cert.py @@ -344,3 +344,26 @@ class ClientCertificateManager(object): self._nsx_trust_management.delete_identity(details['id']) self._nsx_trust_management.create_identity(self._identity, nsx_cert_id) + + +class ClientCertProvider(object): + """Basic implementation for client certificate provider + + Responsible for preparing, providing and disposing client certificate + file. Basic implementation assumes the file exists in the file system + and does not take responsibility of deleting this sensitive information + after use. + Inheriting objects should make use of __enter__ and __exit__ APIs to + prepare and dispose the certificate file data. + """ + def __init__(self, filename): + self._filename = filename + + def __enter__(self): + return self + + def __exit__(self, type, value, traceback): + pass + + def filename(self): + return self._filename diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index 6921492c..e59871e2 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -20,6 +20,7 @@ import datetime import eventlet import itertools import logging +import OpenSSL import requests import six import six.moves.urllib.parse as urlparse @@ -33,7 +34,6 @@ from requests import exceptions as requests_exceptions from vmware_nsxlib._i18n import _, _LI, _LW from vmware_nsxlib.v3 import client as nsx_client from vmware_nsxlib.v3 import exceptions -from vmware_nsxlib.v3 import nsx_constants LOG = log.getLogger(__name__) @@ -88,14 +88,55 @@ class TimeoutSession(requests.Session): def __init__(self, timeout, read_timeout): self.timeout = timeout self.read_timeout = read_timeout + self.cert_provider = None super(TimeoutSession, self).__init__() + @property + def cert_provider(self): + return self._cert_provider + + @cert_provider.setter + def cert_provider(self, value): + self._cert_provider = value + # wrapper timeouts at the session level # see: https://goo.gl/xNk7aM def request(self, *args, **kwargs): if 'timeout' not in kwargs: kwargs['timeout'] = (self.timeout, self.read_timeout) - return super(TimeoutSession, self).request(*args, **kwargs) + + if not self._cert_provider: + return super(TimeoutSession, self).request(*args, **kwargs) + + if self.cert is not None: + # connection should be open (unless server closed it), + # in which case cert is not needed + try: + return super(TimeoutSession, self).request(*args, **kwargs) + except OpenSSL.SSL.Error as e: + # This is most probably "client cert not found" error (this + # happens when server closed the connection and requests + # reopen it). Try reloading client cert. + LOG.warning(_LW("SSL error: %s, retrying..") % e) + except OSError as e: + # Lack of client cert file can come in form of OSError, + # in this case filename will appear in the error. Try + # reloading client cert. + if self._cert_provider.filename() not in str(e): + raise e + # Don't expose cert file name to the logs + LOG.warning(_LW("Reloading client certificate..")) + + # The following with statement allows for preparing certificate and + # private key file and dispose it once connections are spawned + # (since PK is sensitive information, immediate disposal is + # important). This is done of first request of the session or when + # above exceptions indicate cert is missing. + with self._cert_provider: + self.cert = self._cert_provider.filename() + ret = super(TimeoutSession, self).request(*args, **kwargs) + + return ret class NSXRequestsHTTPProvider(AbstractHTTPProvider): @@ -122,8 +163,8 @@ class NSXRequestsHTTPProvider(AbstractHTTPProvider): config = cluster_api.nsxlib_config session = TimeoutSession(config.http_timeout, config.http_read_timeout) - if provider.client_cert_file: - session.cert = provider.client_cert_file + if config.client_cert_provider: + session.cert_provider = config.client_cert_provider else: session.auth = (provider.username, provider.password) @@ -178,13 +219,11 @@ class Provider(object): Which has a unique id a connection URL, and the credential details. """ - def __init__(self, provider_id, provider_url, - username, password, ca_file, client_cert_file=None): + def __init__(self, provider_id, provider_url, username, password, ca_file): self.id = provider_id self.url = provider_url self.username = username self.password = password - self.client_cert_file = client_cert_file self.ca_file = ca_file def __str__(self): @@ -207,12 +246,6 @@ class Endpoint(object): self._state = EndpointState.INITIALIZED self._last_updated = datetime.datetime.now() - def regenerate_pool(self): - self.pool = pools.Pool(min_size=self.pool.min_size, - max_size=self.pool.max_size, - order_as_stack=True, - create=self.pool.create) - @property def last_updated(self): return self._last_updated @@ -267,7 +300,6 @@ class ClusteredAPI(object): self._http_provider = http_provider self._keepalive_interval = keepalive_interval - self._callbacks = {} def _init_cluster(*args, **kwargs): self._init_endpoints(providers, @@ -288,6 +320,7 @@ class ClusteredAPI(object): def _conn(): # called when a pool needs to create a new connection return self._http_provider.new_connection(self, p) + return _conn self._endpoints = {} @@ -366,30 +399,11 @@ class ClusteredAPI(object): if up == len(self._endpoints) else ClusterHealth.ORANGE) - def subscribe(self, callback, event): - if event in self._callbacks: - self._callbacks[event].append(callback) - else: - self._callbacks[event] = [callback] - - def _notify(self, event): - if event in self._callbacks: - for callback in self._callbacks[event]: - callback() - def _validate(self, endpoint): try: with endpoint.pool.item() as conn: self._http_provider.validate_connection(self, endpoint, conn) endpoint.set_state(EndpointState.UP) - except exceptions.ClientCertificateNotTrusted: - LOG.warning(_LW("Failed to validate API cluster endpoint " - "'%(ep)s' due to untrusted client certificate"), - {'ep': endpoint}) - # allow nsxlib user to reload certificate that possibly changed - self._notify(nsx_constants.ON_CLIENT_CERT_UNTRUSTED) - # regenerate connection pool based on new certificate - endpoint.regenerate_pool() except Exception as e: endpoint.set_state(EndpointState.DOWN) LOG.warning(_LW("Failed to validate API cluster endpoint " @@ -525,6 +539,5 @@ class NSXClusteredAPI(ClusteredAPI): urlparse.urlunparse(conf_url), self.nsxlib_config.username(provider_index), self.nsxlib_config.password(provider_index), - self.nsxlib_config.ca_file(provider_index), - self.nsxlib_config.client_cert_file(provider_index))) + self.nsxlib_config.ca_file(provider_index))) return providers diff --git a/vmware_nsxlib/v3/config.py b/vmware_nsxlib/v3/config.py index b032085f..7c74c1b2 100644 --- a/vmware_nsxlib/v3/config.py +++ b/vmware_nsxlib/v3/config.py @@ -25,10 +25,9 @@ class NsxLibConfig(object): and port 443 for https. :param username: User name for the NSX manager :param password: Password for the NSX manager - :param client_cert_file: Specify a file containing client certificate and - private key. If specified, nsxlib will use client - cert authentication instead of basic - authentication. + :param client_cert_provider: None, or ClientCertProvider object. + If specified, nsxlib will use client cert auth + instead of basic authentication. :param insecure: If true, the NSX Manager server certificate is not verified. If false the CA bundle specified via "ca_file" will be used or if unsest the default system root CAs @@ -71,7 +70,7 @@ class NsxLibConfig(object): nsx_api_managers=None, username=None, password=None, - client_cert_file=None, + client_cert_provider=None, insecure=True, ca_file=None, concurrent_connections=10, @@ -91,7 +90,6 @@ class NsxLibConfig(object): self.nsx_api_managers = nsx_api_managers self._username = username self._password = password - self._client_cert_file = client_cert_file self._ca_file = ca_file self.insecure = insecure self.concurrent_connections = concurrent_connections @@ -100,6 +98,7 @@ class NsxLibConfig(object): self.http_read_timeout = http_read_timeout self.conn_idle_timeout = conn_idle_timeout self.http_provider = http_provider + self.client_cert_provider = client_cert_provider self.max_attempts = max_attempts self.plugin_scope = plugin_scope self.plugin_tag = plugin_tag @@ -125,8 +124,5 @@ class NsxLibConfig(object): def password(self, index): return self._attribute_by_index(self._password, index) - def client_cert_file(self, index): - return self._attribute_by_index(self._client_cert_file, index) - def ca_file(self, index): return self._attribute_by_index(self._ca_file, index) diff --git a/vmware_nsxlib/v3/nsx_constants.py b/vmware_nsxlib/v3/nsx_constants.py index 8dbb249e..55e80639 100644 --- a/vmware_nsxlib/v3/nsx_constants.py +++ b/vmware_nsxlib/v3/nsx_constants.py @@ -111,6 +111,3 @@ ERR_CODE_IPAM_IP_NOT_IN_POOL = 5110 ERR_CODE_IPAM_RANGE_MODIFY = 5602 ERR_CODE_IPAM_RANGE_DELETE = 5015 ERR_CODE_IPAM_RANGE_SHRUNK = 5016 - -# NsxLib events -ON_CLIENT_CERT_UNTRUSTED = 'on_client_cert_untrusted'