openstack/octavia
Code Issues Proposed changes

Fix multi-listener LB client auth/re-encryption

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

Change-Id: Ib7b083e1dfbfd7afcca870ed6f60a871b2e19253
Story: 2006822
Task: 37394
This commit is contained in:
Michael Johnson 2019-11-08 11:40:52 -08:00
parent 7d23a711dd
commit cccd47e05a
4 changed files with 84 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
octavia/amphorae/drivers/haproxy/rest_api_driver.py
View File

@ -151,9 +151,6 @@ class HaproxyAmphoraLoadBalancerDriver(
has_tcp = False has_tcp = False
certs = {} certs = {}
client_ca_filename = None
crl_filename = None
pool_tls_certs = dict()
listeners_to_update = [] listeners_to_update = []
for listener in loadbalancer.listeners: for listener in loadbalancer.listeners:
LOG.debug("%s updating listener %s on amphora %s", LOG.debug("%s updating listener %s on amphora %s",
@ -177,22 +174,28 @@ class HaproxyAmphoraLoadBalancerDriver(
listener.tls_certificate_id: listener.tls_certificate_id:
self._process_tls_certificates( self._process_tls_certificates(
listener, amphora, obj_id)['tls_cert']}) listener, amphora, obj_id)['tls_cert']})
client_ca_filename = self._process_secret( certs.update({listener.client_ca_tls_certificate_id:
listener, listener.client_ca_tls_certificate_id, self._process_secret(
amphora, obj_id) listener,
crl_filename = self._process_secret( listener.client_ca_tls_certificate_id,
listener, listener.client_crl_container_id, amphora, obj_id)})
amphora, obj_id) certs.update({listener.client_crl_container_id:
pool_tls_certs = self._process_listener_pool_certs( self._process_secret(
listener, amphora, obj_id) listener,
listener.client_crl_container_id,
amphora, obj_id)})
certs.update(self._process_listener_pool_certs(
listener, amphora, obj_id))
if split_config: if split_config:
config = self.jinja_split.build_config( config = self.jinja_split.build_config(
host_amphora=amphora, listener=listener, host_amphora=amphora, listener=listener,
haproxy_versions=haproxy_versions, haproxy_versions=haproxy_versions,
client_ca_filename=client_ca_filename, client_ca_filename=certs[
client_crl=crl_filename, listener.client_ca_tls_certificate_id],
pool_tls_certs=pool_tls_certs) client_crl=certs[listener.client_crl_container_id],
pool_tls_certs=certs)
self.clients[amphora.api_version].upload_config( self.clients[amphora.api_version].upload_config(
amphora, listener.id, config, amphora, listener.id, config,
timeout_dict=timeout_dict) timeout_dict=timeout_dict)
@ -214,10 +217,8 @@ class HaproxyAmphoraLoadBalancerDriver(
# Generate HaProxy configuration from listener object # Generate HaProxy configuration from listener object
config = self.jinja_combo.build_config( config = self.jinja_combo.build_config(
host_amphora=amphora, listeners=listeners_to_update, host_amphora=amphora, listeners=listeners_to_update,
haproxy_versions=haproxy_versions, tls_certs=certs,
client_ca_filename=client_ca_filename, haproxy_versions=haproxy_versions)
client_crl=crl_filename,
pool_tls_certs=pool_tls_certs)
self.clients[amphora.api_version].upload_config( self.clients[amphora.api_version].upload_config(
amphora, loadbalancer.id, config, amphora, loadbalancer.id, config,
timeout_dict=timeout_dict) timeout_dict=timeout_dict)

75
octavia/common/jinja/haproxy/combined_listeners/jinja_cfg.py
View File

@ -81,9 +81,8 @@ class JinjaTemplater(object):
self.log_server = log_server self.log_server = log_server
self.connection_logging = connection_logging self.connection_logging = connection_logging
def build_config(self, host_amphora, listeners, haproxy_versions, def build_config(self, host_amphora, listeners, tls_certs,
socket_path=None, client_ca_filename=None, haproxy_versions, socket_path=None):
client_crl=None, pool_tls_certs=None):
"""Convert a logical configuration to the HAProxy version """Convert a logical configuration to the HAProxy version
:param host_amphora: The Amphora this configuration is hosted on :param host_amphora: The Amphora this configuration is hosted on
@ -102,10 +101,9 @@ class JinjaTemplater(object):
feature_compatibility[constants.HTTP_REUSE] = True feature_compatibility[constants.HTTP_REUSE] = True
return self.render_loadbalancer_obj( return self.render_loadbalancer_obj(
host_amphora, listeners, socket_path=socket_path, host_amphora, listeners, tls_certs=tls_certs,
feature_compatibility=feature_compatibility, socket_path=socket_path,
client_ca_filename=client_ca_filename, client_crl=client_crl, feature_compatibility=feature_compatibility)
pool_tls_certs=pool_tls_certs)
def _get_template(self): def _get_template(self):
"""Returns the specified Jinja configuration template.""" """Returns the specified Jinja configuration template."""
@ -141,14 +139,13 @@ class JinjaTemplater(object):
return log_format return log_format
def render_loadbalancer_obj(self, host_amphora, listeners, def render_loadbalancer_obj(self, host_amphora, listeners,
socket_path=None, feature_compatibility=None, tls_certs=None, socket_path=None,
client_ca_filename=None, client_crl=None, feature_compatibility=None):
pool_tls_certs=None):
"""Renders a templated configuration from a load balancer object """Renders a templated configuration from a load balancer object
:param host_amphora: The Amphora this configuration is hosted on :param host_amphora: The Amphora this configuration is hosted on
:param listener: The listener configuration :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 :param socket_path: The socket path for Haproxy process
:return: Rendered configuration :return: Rendered configuration
""" """
@ -157,10 +154,8 @@ class JinjaTemplater(object):
host_amphora, host_amphora,
listeners[0].load_balancer, listeners[0].load_balancer,
listeners, listeners,
feature_compatibility, tls_certs,
client_ca_filename=client_ca_filename, feature_compatibility,)
client_crl=client_crl,
pool_tls_certs=pool_tls_certs)
if not socket_path: if not socket_path:
socket_path = '%s/%s.sock' % (self.base_amp_path, socket_path = '%s/%s.sock' % (self.base_amp_path,
listeners[0].load_balancer.id) listeners[0].load_balancer.id)
@ -176,8 +171,7 @@ class JinjaTemplater(object):
constants=constants) constants=constants)
def _transform_loadbalancer(self, host_amphora, loadbalancer, listeners, def _transform_loadbalancer(self, host_amphora, loadbalancer, listeners,
feature_compatibility, client_ca_filename=None, tls_certs, feature_compatibility):
client_crl=None, pool_tls_certs=None):
"""Transforms a load balancer into an object that will """Transforms a load balancer into an object that will
be processed by the templating system be processed by the templating system
@ -187,9 +181,7 @@ class JinjaTemplater(object):
if listener.protocol == constants.PROTOCOL_UDP: if listener.protocol == constants.PROTOCOL_UDP:
continue continue
listener_transforms.append(self._transform_listener( listener_transforms.append(self._transform_listener(
listener, feature_compatibility, loadbalancer, listener, tls_certs, feature_compatibility, loadbalancer))
client_ca_filename=client_ca_filename, client_crl=client_crl,
pool_tls_certs=pool_tls_certs))
ret_value = { ret_value = {
'id': loadbalancer.id, 'id': loadbalancer.id,
@ -239,9 +231,8 @@ class JinjaTemplater(object):
'vrrp_priority': amphora.vrrp_priority 'vrrp_priority': amphora.vrrp_priority
} }
def _transform_listener(self, listener, feature_compatibility, def _transform_listener(self, listener, tls_certs, feature_compatibility,
loadbalancer, client_ca_filename=None, loadbalancer):
client_crl=None, pool_tls_certs=None):
"""Transforms a listener into an object that will """Transforms a listener into an object that will
be processed by the templating system be processed by the templating system
@ -278,25 +269,29 @@ class JinjaTemplater(object):
CONF.haproxy_amphora.base_cert_dir, CONF.haproxy_amphora.base_cert_dir,
loadbalancer.id, '{}.pem'.format(listener.id)) loadbalancer.id, '{}.pem'.format(listener.id))
if listener.client_ca_tls_certificate_id: if tls_certs is not None:
ret_value['client_ca_tls_path'] = '%s' % ( if listener.client_ca_tls_certificate_id:
os.path.join(self.base_crt_dir, loadbalancer.id, ret_value['client_ca_tls_path'] = '%s' % (
client_ca_filename)) os.path.join(
ret_value['client_auth'] = CLIENT_AUTH_MAP.get( self.base_crt_dir, loadbalancer.id,
listener.client_authentication) tls_certs[listener.client_ca_tls_certificate_id]))
if listener.client_crl_container_id: ret_value['client_auth'] = CLIENT_AUTH_MAP.get(
ret_value['client_crl_path'] = '%s' % ( listener.client_authentication)
os.path.join(self.base_crt_dir, loadbalancer.id, client_crl))
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 = [] pools = []
pool_gen = (pool for pool in listener.pools if pool_gen = (pool for pool in listener.pools if
pool.provisioning_status != constants.PENDING_DELETE) pool.provisioning_status != constants.PENDING_DELETE)
for x in pool_gen: for pool in pool_gen:
kwargs = {} kwargs = {}
if pool_tls_certs and pool_tls_certs.get(x.id): if tls_certs is not None and tls_certs.get(pool.id):
kwargs = {'pool_tls_certs': pool_tls_certs.get(x.id)} kwargs = {'pool_tls_certs': tls_certs.get(pool.id)}
pools.append(self._transform_pool( pools.append(self._transform_pool(
x, feature_compatibility, **kwargs)) pool, feature_compatibility, **kwargs))
ret_value['pools'] = pools ret_value['pools'] = pools
policy_gen = (policy for policy in listener.l7policies if policy_gen = (policy for policy in listener.l7policies if
policy.provisioning_status != constants.PENDING_DELETE) policy.provisioning_status != constants.PENDING_DELETE)
@ -307,7 +302,7 @@ class JinjaTemplater(object):
break break
l7policies = [self._transform_l7policy( l7policies = [self._transform_l7policy(
x, feature_compatibility, pool_tls_certs) x, feature_compatibility, tls_certs)
for x in policy_gen] for x in policy_gen]
ret_value['l7policies'] = l7policies ret_value['l7policies'] = l7policies
return ret_value return ret_value
@ -416,7 +411,7 @@ class JinjaTemplater(object):
} }
def _transform_l7policy(self, l7policy, feature_compatibility, def _transform_l7policy(self, l7policy, feature_compatibility,
pool_tls_certs=None): tls_certs=None):
"""Transforms an L7 policy into an object that will """Transforms an L7 policy into an object that will
be processed by the templating system be processed by the templating system
@ -432,10 +427,10 @@ class JinjaTemplater(object):
l7policy.redirect_pool.provisioning_status != l7policy.redirect_pool.provisioning_status !=
constants.PENDING_DELETE): constants.PENDING_DELETE):
kwargs = {} kwargs = {}
if pool_tls_certs and pool_tls_certs.get( if tls_certs is not None and tls_certs.get(
l7policy.redirect_pool.id): l7policy.redirect_pool.id):
kwargs = {'pool_tls_certs': 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( ret_value['redirect_pool'] = self._transform_pool(
l7policy.redirect_pool, feature_compatibility, **kwargs) l7policy.redirect_pool, feature_compatibility, **kwargs)
else: else:

32
octavia/tests/unit/common/jinja/haproxy/combined_listeners/test_jinja_cfg.py
View File

@ -75,12 +75,19 @@ class TestHaproxyCfg(base.TestCase):
"weight 13 check inter 30s fall 3 rise 2 cookie " "weight 13 check inter 30s fall 3 rise 2 cookie "
"sample_member_id_2\n\n").format( "sample_member_id_2\n\n").format(
maxconn=constants.HAPROXY_MAX_MAXCONN) 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( rendered_obj = self.jinja_cfg.render_loadbalancer_obj(
sample_configs_combined.sample_amphora_tuple(), sample_configs_combined.sample_amphora_tuple(),
[sample_configs_combined.sample_listener_tuple( [sample_configs_combined.sample_listener_tuple(
proto='TERMINATED_HTTPS', tls=True, sni=True, proto='TERMINATED_HTTPS', tls=True, sni=True,
client_ca_cert=True, client_crl_cert=True)], client_ca_cert=True, client_crl_cert=True)],
client_ca_filename='client_ca.pem', client_crl='SHA_ID.pem') tls_tupe)
self.assertEqual( self.assertEqual(
sample_configs_combined.sample_base_expected_config( sample_configs_combined.sample_base_expected_config(
frontend=fe, backend=be), frontend=fe, backend=be),
@ -122,7 +129,13 @@ class TestHaproxyCfg(base.TestCase):
rendered_obj = self.jinja_cfg.render_loadbalancer_obj( rendered_obj = self.jinja_cfg.render_loadbalancer_obj(
sample_configs_combined.sample_amphora_tuple(), sample_configs_combined.sample_amphora_tuple(),
[sample_configs_combined.sample_listener_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( self.assertEqual(
sample_configs_combined.sample_base_expected_config( sample_configs_combined.sample_base_expected_config(
frontend=fe, backend=be), frontend=fe, backend=be),
@ -826,7 +839,7 @@ class TestHaproxyCfg(base.TestCase):
sample_configs_combined.sample_amphora_tuple(), sample_configs_combined.sample_amphora_tuple(),
[sample_configs_combined.sample_listener_tuple( [sample_configs_combined.sample_listener_tuple(
pool_cert=True, tls_enabled=True)], pool_cert=True, tls_enabled=True)],
pool_tls_certs={ tls_certs={
'sample_pool_id_1': 'sample_pool_id_1':
{'client_cert': cert_file_path, {'client_cert': cert_file_path,
'ca_cert': None, 'crl': None}}) 'ca_cert': None, 'crl': None}})
@ -866,7 +879,7 @@ class TestHaproxyCfg(base.TestCase):
[sample_configs_combined.sample_listener_tuple( [sample_configs_combined.sample_listener_tuple(
pool_cert=True, pool_ca_cert=True, pool_crl=True, pool_cert=True, pool_ca_cert=True, pool_crl=True,
tls_enabled=True)], tls_enabled=True)],
pool_tls_certs={ tls_certs={
'sample_pool_id_1': 'sample_pool_id_1':
{'client_cert': pool_client_cert, {'client_cert': pool_client_cert,
'ca_cert': pool_ca_cert, 'ca_cert': pool_ca_cert,
@ -923,13 +936,13 @@ class TestHaproxyCfg(base.TestCase):
def test_transform_listener(self): def test_transform_listener(self):
in_listener = sample_configs_combined.sample_listener_tuple() 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) in_listener.load_balancer)
self.assertEqual(sample_configs_combined.RET_LISTENER, ret) self.assertEqual(sample_configs_combined.RET_LISTENER, ret)
def test_transform_listener_with_l7(self): def test_transform_listener_with_l7(self):
in_listener = sample_configs_combined.sample_listener_tuple(l7=True) 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) in_listener.load_balancer)
self.assertEqual(sample_configs_combined.RET_LISTENER_L7, ret) 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_amphora = sample_configs_combined.sample_amphora_tuple()
in_listener = sample_configs_combined.sample_listener_tuple() in_listener = sample_configs_combined.sample_listener_tuple()
ret = self.jinja_cfg._transform_loadbalancer( 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) self.assertEqual(sample_configs_combined.RET_LB, ret)
def test_transform_amphora(self): def test_transform_amphora(self):
@ -949,7 +962,7 @@ class TestHaproxyCfg(base.TestCase):
in_amphora = sample_configs_combined.sample_amphora_tuple() in_amphora = sample_configs_combined.sample_amphora_tuple()
in_listener = sample_configs_combined.sample_listener_tuple(l7=True) in_listener = sample_configs_combined.sample_listener_tuple(l7=True)
ret = self.jinja_cfg._transform_loadbalancer( 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) self.assertEqual(sample_configs_combined.RET_LB_L7, ret)
def test_transform_l7policy(self): def test_transform_l7policy(self):
@ -1067,6 +1080,7 @@ class TestHaproxyCfg(base.TestCase):
rendered_obj = j_cfg.build_config( rendered_obj = j_cfg.build_config(
sample_amphora, sample_amphora,
[sample_proxy_listener], [sample_proxy_listener],
tls_certs=None,
haproxy_versions=("1", "8", "1")) haproxy_versions=("1", "8", "1"))
self.assertEqual( self.assertEqual(
sample_configs_combined.sample_base_expected_config(backend=be), sample_configs_combined.sample_base_expected_config(backend=be),
@ -1094,6 +1108,7 @@ class TestHaproxyCfg(base.TestCase):
rendered_obj = j_cfg.build_config( rendered_obj = j_cfg.build_config(
sample_amphora, sample_amphora,
[sample_proxy_listener], [sample_proxy_listener],
tls_certs=None,
haproxy_versions=("1", "5", "18")) haproxy_versions=("1", "5", "18"))
self.assertEqual( self.assertEqual(
sample_configs_combined.sample_base_expected_config(backend=be), sample_configs_combined.sample_base_expected_config(backend=be),
@ -1175,6 +1190,7 @@ class TestHaproxyCfg(base.TestCase):
rendered_obj = j_cfg.build_config( rendered_obj = j_cfg.build_config(
sample_configs_combined.sample_amphora_tuple(), sample_configs_combined.sample_amphora_tuple(),
[sample_listener], [sample_listener],
tls_certs=None,
haproxy_versions=("1", "5", "18")) haproxy_versions=("1", "5", "18"))
self.assertEqual( self.assertEqual(
sample_configs_combined.sample_base_expected_config( sample_configs_combined.sample_base_expected_config(

6
releasenotes/notes/fix-client-auth-single-process-749af7791454ff03.yaml Normal file
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.