Fix multi-listener LB client auth/re-encryption

This patch corrects a bug with mutli-listener load balancers that
are using either TLS client authentication and/or backend
re-encryption.

Depends-On: https://review.opendev.org/733933
Change-Id: Ib7b083e1dfbfd7afcca870ed6f60a871b2e19253
Story: 2006822
Task: 37394
This commit is contained in:
Michael Johnson 2019-11-08 11:40:52 -08:00
parent cb650e7e4c
commit 0c1e588d79
5 changed files with 91 additions and 69 deletions

View File

@ -148,9 +148,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",
@ -174,22 +171,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)
@ -211,10 +214,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)

View File

@ -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."""
@ -122,14 +120,13 @@ class JinjaTemplater(object):
return JINJA_ENV.get_template(os.path.basename(self.haproxy_template))
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
"""
@ -138,10 +135,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)
@ -154,8 +149,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
@ -165,9 +159,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,
@ -217,9 +209,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
@ -253,23 +244,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:
@ -278,7 +275,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
@ -383,7 +380,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
@ -397,10 +394,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:

View File

@ -76,12 +76,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),
@ -124,7 +131,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),
@ -812,7 +825,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}})
@ -852,7 +865,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,
@ -909,13 +922,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)
@ -923,7 +936,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):
@ -935,7 +948,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):
@ -1053,6 +1066,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),
@ -1080,6 +1094,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),
@ -1162,6 +1177,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(

View File

@ -756,14 +756,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,
@ -805,7 +807,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',

View File

@ -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.