Merge "Fix multi-listener LB client auth/re-encryption"

octavia/amphorae/drivers/haproxy/rest_api_driver.py

@@ -151,9 +151,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",
@@ -177,22 +174,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)
@@ -214,10 +217,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)

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,25 +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 = []
         pool_gen = (pool for pool in listener.pools if
                     pool.provisioning_status != constants.PENDING_DELETE)
-        for x in pool_gen:
+        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
         policy_gen = (policy for policy in listener.l7policies if
                       policy.provisioning_status != constants.PENDING_DELETE)
@@ -307,7 +302,7 @@ class JinjaTemplater(object):
                     break
 
         l7policies = [self._transform_l7policy(
-                      x, feature_compatibility, pool_tls_certs)
+                      x, feature_compatibility, tls_certs)
                       for x in policy_gen]
         ret_value['l7policies'] = l7policies
         return ret_value
@@ -416,7 +411,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
@@ -432,10 +427,10 @@ class JinjaTemplater(object):
             l7policy.redirect_pool.provisioning_status !=
                 constants.PENDING_DELETE):
             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:

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(

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.