diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 56554f9650..47c720a331 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -149,9 +149,6 @@ class HaproxyAmphoraLoadBalancerDriver( has_tcp = False certs = {} - client_ca_filename = None - crl_filename = None - pool_tls_certs = dict() listeners_to_update = [] for listener in loadbalancer.listeners: LOG.debug("%s updating listener %s on amphora %s", @@ -175,22 +172,28 @@ class HaproxyAmphoraLoadBalancerDriver( listener.tls_certificate_id: self._process_tls_certificates( listener, amphora, obj_id)['tls_cert']}) - client_ca_filename = self._process_secret( - listener, listener.client_ca_tls_certificate_id, - amphora, obj_id) - crl_filename = self._process_secret( - listener, listener.client_crl_container_id, - amphora, obj_id) - pool_tls_certs = self._process_listener_pool_certs( - listener, amphora, obj_id) + certs.update({listener.client_ca_tls_certificate_id: + self._process_secret( + listener, + listener.client_ca_tls_certificate_id, + amphora, obj_id)}) + certs.update({listener.client_crl_container_id: + self._process_secret( + listener, + listener.client_crl_container_id, + amphora, obj_id)}) + + certs.update(self._process_listener_pool_certs( + listener, amphora, obj_id)) if split_config: config = self.jinja_split.build_config( host_amphora=amphora, listener=listener, haproxy_versions=haproxy_versions, - client_ca_filename=client_ca_filename, - client_crl=crl_filename, - pool_tls_certs=pool_tls_certs) + client_ca_filename=certs[ + listener.client_ca_tls_certificate_id], + client_crl=certs[listener.client_crl_container_id], + pool_tls_certs=certs) self.clients[amphora.api_version].upload_config( amphora, listener.id, config, timeout_dict=timeout_dict) @@ -212,10 +215,8 @@ class HaproxyAmphoraLoadBalancerDriver( # Generate HaProxy configuration from listener object config = self.jinja_combo.build_config( host_amphora=amphora, listeners=listeners_to_update, - haproxy_versions=haproxy_versions, - client_ca_filename=client_ca_filename, - client_crl=crl_filename, - pool_tls_certs=pool_tls_certs) + tls_certs=certs, + haproxy_versions=haproxy_versions) self.clients[amphora.api_version].upload_config( amphora, loadbalancer.id, config, timeout_dict=timeout_dict) diff --git a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py b/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py index c9c628af6b..4a8a1f9787 100644 --- a/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py +++ b/octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py @@ -81,9 +81,8 @@ class JinjaTemplater(object): self.log_server = log_server self.connection_logging = connection_logging - def build_config(self, host_amphora, listeners, haproxy_versions, - socket_path=None, client_ca_filename=None, - client_crl=None, pool_tls_certs=None): + def build_config(self, host_amphora, listeners, tls_certs, + haproxy_versions, socket_path=None): """Convert a logical configuration to the HAProxy version :param host_amphora: The Amphora this configuration is hosted on @@ -102,10 +101,9 @@ class JinjaTemplater(object): feature_compatibility[constants.HTTP_REUSE] = True return self.render_loadbalancer_obj( - host_amphora, listeners, socket_path=socket_path, - feature_compatibility=feature_compatibility, - client_ca_filename=client_ca_filename, client_crl=client_crl, - pool_tls_certs=pool_tls_certs) + host_amphora, listeners, tls_certs=tls_certs, + socket_path=socket_path, + feature_compatibility=feature_compatibility) def _get_template(self): """Returns the specified Jinja configuration template.""" @@ -141,14 +139,13 @@ class JinjaTemplater(object): return log_format def render_loadbalancer_obj(self, host_amphora, listeners, - socket_path=None, feature_compatibility=None, - client_ca_filename=None, client_crl=None, - pool_tls_certs=None): + tls_certs=None, socket_path=None, + feature_compatibility=None): """Renders a templated configuration from a load balancer object :param host_amphora: The Amphora this configuration is hosted on :param listener: The listener configuration - :param client_ca_filename: The CA certificate for client authorization + :param tls_certs: Dict of the TLS certificates for the listener :param socket_path: The socket path for Haproxy process :return: Rendered configuration """ @@ -157,10 +154,8 @@ class JinjaTemplater(object): host_amphora, listeners[0].load_balancer, listeners, - feature_compatibility, - client_ca_filename=client_ca_filename, - client_crl=client_crl, - pool_tls_certs=pool_tls_certs) + tls_certs, + feature_compatibility,) if not socket_path: socket_path = '%s/%s.sock' % (self.base_amp_path, listeners[0].load_balancer.id) @@ -176,8 +171,7 @@ class JinjaTemplater(object): constants=constants) def _transform_loadbalancer(self, host_amphora, loadbalancer, listeners, - feature_compatibility, client_ca_filename=None, - client_crl=None, pool_tls_certs=None): + tls_certs, feature_compatibility): """Transforms a load balancer into an object that will be processed by the templating system @@ -187,9 +181,7 @@ class JinjaTemplater(object): if listener.protocol == constants.PROTOCOL_UDP: continue listener_transforms.append(self._transform_listener( - listener, feature_compatibility, loadbalancer, - client_ca_filename=client_ca_filename, client_crl=client_crl, - pool_tls_certs=pool_tls_certs)) + listener, tls_certs, feature_compatibility, loadbalancer)) ret_value = { 'id': loadbalancer.id, @@ -239,9 +231,8 @@ class JinjaTemplater(object): 'vrrp_priority': amphora.vrrp_priority } - def _transform_listener(self, listener, feature_compatibility, - loadbalancer, client_ca_filename=None, - client_crl=None, pool_tls_certs=None): + def _transform_listener(self, listener, tls_certs, feature_compatibility, + loadbalancer): """Transforms a listener into an object that will be processed by the templating system @@ -278,23 +269,29 @@ class JinjaTemplater(object): CONF.haproxy_amphora.base_cert_dir, loadbalancer.id, '{}.pem'.format(listener.id)) - if listener.client_ca_tls_certificate_id: - ret_value['client_ca_tls_path'] = '%s' % ( - os.path.join(self.base_crt_dir, loadbalancer.id, - client_ca_filename)) - ret_value['client_auth'] = CLIENT_AUTH_MAP.get( - listener.client_authentication) - if listener.client_crl_container_id: - ret_value['client_crl_path'] = '%s' % ( - os.path.join(self.base_crt_dir, loadbalancer.id, client_crl)) + if tls_certs is not None: + if listener.client_ca_tls_certificate_id: + ret_value['client_ca_tls_path'] = '%s' % ( + os.path.join( + self.base_crt_dir, loadbalancer.id, + tls_certs[listener.client_ca_tls_certificate_id])) + ret_value['client_auth'] = CLIENT_AUTH_MAP.get( + listener.client_authentication) + + if listener.client_crl_container_id: + ret_value['client_crl_path'] = '%s' % ( + os.path.join(self.base_crt_dir, loadbalancer.id, + tls_certs[listener.client_crl_container_id])) pools = [] - for x in listener.pools: + pool_gen = (pool for pool in listener.pools if + pool.provisioning_status != constants.PENDING_DELETE) + for pool in pool_gen: kwargs = {} - if pool_tls_certs and pool_tls_certs.get(x.id): - kwargs = {'pool_tls_certs': pool_tls_certs.get(x.id)} + if tls_certs is not None and tls_certs.get(pool.id): + kwargs = {'pool_tls_certs': tls_certs.get(pool.id)} pools.append(self._transform_pool( - x, feature_compatibility, **kwargs)) + pool, feature_compatibility, **kwargs)) ret_value['pools'] = pools if listener.default_pool: for pool in pools: @@ -303,7 +300,7 @@ class JinjaTemplater(object): break l7policies = [self._transform_l7policy( - x, feature_compatibility, pool_tls_certs) + x, feature_compatibility, tls_certs) for x in listener.l7policies] ret_value['l7policies'] = l7policies return ret_value @@ -408,7 +405,7 @@ class JinjaTemplater(object): } def _transform_l7policy(self, l7policy, feature_compatibility, - pool_tls_certs=None): + tls_certs=None): """Transforms an L7 policy into an object that will be processed by the templating system @@ -422,10 +419,10 @@ class JinjaTemplater(object): } if l7policy.redirect_pool: kwargs = {} - if pool_tls_certs and pool_tls_certs.get( + if tls_certs is not None and tls_certs.get( l7policy.redirect_pool.id): kwargs = {'pool_tls_certs': - pool_tls_certs.get(l7policy.redirect_pool.id)} + tls_certs.get(l7policy.redirect_pool.id)} ret_value['redirect_pool'] = self._transform_pool( l7policy.redirect_pool, feature_compatibility, **kwargs) else: diff --git a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py b/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py index 8906fec50b..9d6b313dd9 100644 --- a/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py +++ b/octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py @@ -75,12 +75,19 @@ class TestHaproxyCfg(base.TestCase): "weight 13 check inter 30s fall 3 rise 2 cookie " "sample_member_id_2\n\n").format( maxconn=constants.HAPROXY_MAX_MAXCONN) + tls_tupe = {'cont_id_1': + sample_configs_combined.sample_tls_container_tuple( + id='tls_container_id', + certificate='imaCert1', private_key='imaPrivateKey1', + primary_cn='FakeCN'), + 'cont_id_ca': 'client_ca.pem', + 'cont_id_crl': 'SHA_ID.pem'} rendered_obj = self.jinja_cfg.render_loadbalancer_obj( sample_configs_combined.sample_amphora_tuple(), [sample_configs_combined.sample_listener_tuple( proto='TERMINATED_HTTPS', tls=True, sni=True, client_ca_cert=True, client_crl_cert=True)], - client_ca_filename='client_ca.pem', client_crl='SHA_ID.pem') + tls_tupe) self.assertEqual( sample_configs_combined.sample_base_expected_config( frontend=fe, backend=be), @@ -122,7 +129,13 @@ class TestHaproxyCfg(base.TestCase): rendered_obj = self.jinja_cfg.render_loadbalancer_obj( sample_configs_combined.sample_amphora_tuple(), [sample_configs_combined.sample_listener_tuple( - proto='TERMINATED_HTTPS', tls=True)]) + proto='TERMINATED_HTTPS', tls=True)], + tls_certs={'cont_id_1': + sample_configs_combined.sample_tls_container_tuple( + id='tls_container_id', + certificate='ImAalsdkfjCert', + private_key='ImAsdlfksdjPrivateKey', + primary_cn="FakeCN")}) self.assertEqual( sample_configs_combined.sample_base_expected_config( frontend=fe, backend=be), @@ -826,7 +839,7 @@ class TestHaproxyCfg(base.TestCase): sample_configs_combined.sample_amphora_tuple(), [sample_configs_combined.sample_listener_tuple( pool_cert=True, tls_enabled=True)], - pool_tls_certs={ + tls_certs={ 'sample_pool_id_1': {'client_cert': cert_file_path, 'ca_cert': None, 'crl': None}}) @@ -866,7 +879,7 @@ class TestHaproxyCfg(base.TestCase): [sample_configs_combined.sample_listener_tuple( pool_cert=True, pool_ca_cert=True, pool_crl=True, tls_enabled=True)], - pool_tls_certs={ + tls_certs={ 'sample_pool_id_1': {'client_cert': pool_client_cert, 'ca_cert': pool_ca_cert, @@ -923,13 +936,13 @@ class TestHaproxyCfg(base.TestCase): def test_transform_listener(self): in_listener = sample_configs_combined.sample_listener_tuple() - ret = self.jinja_cfg._transform_listener(in_listener, {}, + ret = self.jinja_cfg._transform_listener(in_listener, None, {}, in_listener.load_balancer) self.assertEqual(sample_configs_combined.RET_LISTENER, ret) def test_transform_listener_with_l7(self): in_listener = sample_configs_combined.sample_listener_tuple(l7=True) - ret = self.jinja_cfg._transform_listener(in_listener, {}, + ret = self.jinja_cfg._transform_listener(in_listener, None, {}, in_listener.load_balancer) self.assertEqual(sample_configs_combined.RET_LISTENER_L7, ret) @@ -937,7 +950,7 @@ class TestHaproxyCfg(base.TestCase): in_amphora = sample_configs_combined.sample_amphora_tuple() in_listener = sample_configs_combined.sample_listener_tuple() ret = self.jinja_cfg._transform_loadbalancer( - in_amphora, in_listener.load_balancer, [in_listener], {}) + in_amphora, in_listener.load_balancer, [in_listener], None, {}) self.assertEqual(sample_configs_combined.RET_LB, ret) def test_transform_amphora(self): @@ -949,7 +962,7 @@ class TestHaproxyCfg(base.TestCase): in_amphora = sample_configs_combined.sample_amphora_tuple() in_listener = sample_configs_combined.sample_listener_tuple(l7=True) ret = self.jinja_cfg._transform_loadbalancer( - in_amphora, in_listener.load_balancer, [in_listener], {}) + in_amphora, in_listener.load_balancer, [in_listener], None, {}) self.assertEqual(sample_configs_combined.RET_LB_L7, ret) def test_transform_l7policy(self): @@ -1067,6 +1080,7 @@ class TestHaproxyCfg(base.TestCase): rendered_obj = j_cfg.build_config( sample_amphora, [sample_proxy_listener], + tls_certs=None, haproxy_versions=("1", "8", "1")) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), @@ -1094,6 +1108,7 @@ class TestHaproxyCfg(base.TestCase): rendered_obj = j_cfg.build_config( sample_amphora, [sample_proxy_listener], + tls_certs=None, haproxy_versions=("1", "5", "18")) self.assertEqual( sample_configs_combined.sample_base_expected_config(backend=be), @@ -1175,6 +1190,7 @@ class TestHaproxyCfg(base.TestCase): rendered_obj = j_cfg.build_config( sample_configs_combined.sample_amphora_tuple(), [sample_listener], + tls_certs=None, haproxy_versions=("1", "5", "18")) self.assertEqual( sample_configs_combined.sample_base_expected_config( diff --git a/octavia/tests/unit/common/sample_configs/sample_configs_combined.py b/octavia/tests/unit/common/sample_configs/sample_configs_combined.py index d86d371de5..75cb725379 100644 --- a/octavia/tests/unit/common/sample_configs/sample_configs_combined.py +++ b/octavia/tests/unit/common/sample_configs/sample_configs_combined.py @@ -762,14 +762,16 @@ def sample_pool_tuple(listener_id=None, proto=None, monitor=True, backup_member=False, disabled_member=False, has_http_reuse=True, pool_cert=False, pool_ca_cert=False, pool_crl=False, tls_enabled=False, - hm_host_http_check=False): + hm_host_http_check=False, + provisioning_status=constants.ACTIVE): proto = 'HTTP' if proto is None else proto monitor_proto = proto if monitor_proto is None else monitor_proto in_pool = collections.namedtuple( 'pool', 'id, protocol, lb_algorithm, members, health_monitor, ' 'session_persistence, enabled, operating_status, ' 'tls_certificate_id, ca_tls_certificate_id, ' - 'crl_container_id, tls_enabled, ' + constants.HTTP_REUSE) + 'crl_container_id, tls_enabled, ' + 'provisioning_status, ' + constants.HTTP_REUSE) if (proto == constants.PROTOCOL_UDP and persistence_type == constants.SESSION_PERSISTENCE_SOURCE_IP): kwargs = {'persistence_type': persistence_type, @@ -811,7 +813,7 @@ def sample_pool_tuple(listener_id=None, proto=None, monitor=True, tls_certificate_id='pool_cont_1' if pool_cert else None, ca_tls_certificate_id='pool_ca_1' if pool_ca_cert else None, crl_container_id='pool_crl' if pool_crl else None, - tls_enabled=tls_enabled) + tls_enabled=tls_enabled, provisioning_status=constants.ACTIVE) def sample_member_tuple(id, ip, enabled=True, operating_status='ACTIVE', diff --git a/releasenotes/notes/fix-client-auth-single-process-749af7791454ff03.yaml b/releasenotes/notes/fix-client-auth-single-process-749af7791454ff03.yaml new file mode 100644 index 0000000000..60c15fe725 --- /dev/null +++ b/releasenotes/notes/fix-client-auth-single-process-749af7791454ff03.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes an issue where load balancers with more than one TLS enabled + listener, using client authentication and/or backend re-encryption, + may load incorrect certificates for the listener.