From 852191feb22f0968c4338c06952ba88191df8087 Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Mon, 11 Jun 2018 13:53:42 -0400 Subject: [PATCH] Allow using spaces for primary common name in SSL certificates The common name is used as a file name inside the HAproxy configuration file. However, a common name can include spaces and it will result in a configuration file that simply doesn't work because of the spaces. The patch changes the functionality so that it instead creates a SHA1 hash of the certificate and uses that as the file name to avoid those issues. Change-Id: I039ed0b40df8b72a1238f8896548fe77086c530c --- .../drivers/haproxy/rest_api_driver.py | 2 +- octavia/common/jinja/haproxy/jinja_cfg.py | 2 +- octavia/common/tls_utils/cert_parser.py | 2 ++ .../agent/api_server/test_listener.py | 6 ++++-- .../drivers/haproxy/test_rest_api_driver.py | 19 ++++++++++++++----- .../common/jinja/haproxy/test_jinja_cfg.py | 6 ++++-- .../common/sample_configs/sample_certs.py | 1 + .../unit/common/tls_utils/test_cert_parser.py | 4 ++++ 8 files changed, 31 insertions(+), 11 deletions(-) diff --git a/octavia/amphorae/drivers/haproxy/rest_api_driver.py b/octavia/amphorae/drivers/haproxy/rest_api_driver.py index 479804b1d7..cf035c9b95 100644 --- a/octavia/amphorae/drivers/haproxy/rest_api_driver.py +++ b/octavia/amphorae/drivers/haproxy/rest_api_driver.py @@ -183,7 +183,7 @@ class HaproxyAmphoraLoadBalancerDriver( for cert in certs: pem = cert_parser.build_pem(cert) md5 = hashlib.md5(pem).hexdigest() # nosec - name = '{cn}.pem'.format(cn=cert.primary_cn) + name = '{id}.pem'.format(id=cert.id) self._apply(self._upload_cert, listener, pem, md5, name) return {'tls_cert': tls_cert, 'sni_certs': sni_certs} diff --git a/octavia/common/jinja/haproxy/jinja_cfg.py b/octavia/common/jinja/haproxy/jinja_cfg.py index 262b4fbfb9..0f0690f63e 100644 --- a/octavia/common/jinja/haproxy/jinja_cfg.py +++ b/octavia/common/jinja/haproxy/jinja_cfg.py @@ -205,7 +205,7 @@ class JinjaTemplater(object): ret_value['default_tls_path'] = '%s.pem' % ( os.path.join(self.base_crt_dir, listener.id, - tls_cert.primary_cn)) + tls_cert.id)) if listener.sni_containers: ret_value['crt_dir'] = os.path.join(self.base_crt_dir, listener.id) if listener.default_pool: diff --git a/octavia/common/tls_utils/cert_parser.py b/octavia/common/tls_utils/cert_parser.py index fd58f10580..b3c6409ceb 100644 --- a/octavia/common/tls_utils/cert_parser.py +++ b/octavia/common/tls_utils/cert_parser.py @@ -14,6 +14,7 @@ # under the License. import base64 +import hashlib from cryptography.hazmat import backends from cryptography.hazmat.primitives import serialization @@ -360,6 +361,7 @@ def load_certificates_data(cert_mngr, listener, context=None): def _map_cert_tls_container(cert): return data_models.TLSContainer( + id=hashlib.sha1(cert.get_certificate()).hexdigest(), primary_cn=get_primary_cn(cert), private_key=prepare_private_key( cert.get_private_key(), diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_listener.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_listener.py index 3310b5baa7..4d482ec7a7 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_listener.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_listener.py @@ -41,6 +41,7 @@ class ListenerTestCase(base.TestCase): def test_parse_haproxy_config(self): # template_tls tls_tupe = sample_configs.sample_tls_container_tuple( + id='tls_container_id', certificate='imaCert1', private_key='imaPrivateKey1', primary_cn='FakeCN') rendered_obj = self.jinja_cfg.render_loadbalancer_obj( @@ -57,7 +58,7 @@ class ListenerTestCase(base.TestCase): self.assertEqual('/var/lib/octavia/sample_listener_id_1.sock', res['stats_socket']) self.assertEqual( - '/var/lib/octavia/certs/sample_listener_id_1/FakeCN.pem', + '/var/lib/octavia/certs/sample_listener_id_1/tls_container_id.pem', res['ssl_crt']) # render_template_tls_no_sni @@ -66,6 +67,7 @@ class ListenerTestCase(base.TestCase): sample_configs.sample_listener_tuple( proto='TERMINATED_HTTPS', tls=True), tls_cert=sample_configs.sample_tls_container_tuple( + id='tls_container_id', certificate='ImAalsdkfjCert', private_key='ImAsdlfksdjPrivateKey', primary_cn="FakeCN")) @@ -77,7 +79,7 @@ class ListenerTestCase(base.TestCase): self.assertEqual(BASE_AMP_PATH + '/sample_listener_id_1.sock', res['stats_socket']) self.assertEqual( - BASE_CRT_PATH + '/sample_listener_id_1/FakeCN.pem', + BASE_CRT_PATH + '/sample_listener_id_1/tls_container_id.pem', res['ssl_crt']) # render_template_http diff --git a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py index f9efec4b3e..1b32f143bf 100644 --- a/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py +++ b/octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver.py @@ -106,8 +106,17 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): # verify result # this is called 3 times - self.driver.client.get_cert_md5sum.assert_called_with( - self.amp, self.sl.id, sample_certs.X509_CERT_CN_3 + '.pem') + gcm_calls = [ + mock.call(self.amp, self.sl.id, + self.sl.default_tls_container.id + '.pem'), + mock.call(self.amp, self.sl.id, + sconts[0].id + '.pem'), + mock.call(self.amp, self.sl.id, + sconts[1].id + '.pem') + ] + self.driver.client.get_cert_md5sum.assert_has_calls(gcm_calls, + any_order=True) + # this is called three times (last MD5 matches) fp1 = b'\n'.join([sample_certs.X509_CERT, sample_certs.X509_CERT_KEY, @@ -120,11 +129,11 @@ class TestHaproxyAmphoraLoadBalancerDriverTest(base.TestCase): sample_certs.X509_IMDS]) + b'\n' ucp_calls = [ mock.call(self.amp, self.sl.id, - sample_certs.X509_CERT_CN + '.pem', fp1), + self.sl.default_tls_container.id + '.pem', fp1), mock.call(self.amp, self.sl.id, - sample_certs.X509_CERT_CN_2 + '.pem', fp2), + sconts[0].id + '.pem', fp2), mock.call(self.amp, self.sl.id, - sample_certs.X509_CERT_CN_3 + '.pem', fp3) + sconts[1].id + '.pem', fp3) ] self.driver.client.upload_cert_pem.assert_has_calls(ucp_calls, any_order=True) diff --git a/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py b/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py index a5778673be..7a27ae471d 100644 --- a/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py +++ b/octavia/tests/unit/common/jinja/haproxy/test_jinja_cfg.py @@ -36,7 +36,7 @@ class TestHaproxyCfg(base.TestCase): " redirect scheme https if !{ ssl_fc }\n" " bind 10.0.0.2:443 " "ssl crt /var/lib/octavia/certs/" - "sample_listener_id_1/FakeCN.pem " + "sample_listener_id_1/tls_container_id.pem " "crt /var/lib/octavia/certs/sample_listener_id_1\n" " mode http\n" " default_backend sample_pool_id_1\n" @@ -59,6 +59,7 @@ class TestHaproxyCfg(base.TestCase): "weight 13 check inter 30s fall 3 rise 2 cookie " "sample_member_id_2\n\n") tls_tupe = sample_configs.sample_tls_container_tuple( + id='tls_container_id', certificate='imaCert1', private_key='imaPrivateKey1', primary_cn='FakeCN') rendered_obj = self.jinja_cfg.render_loadbalancer_obj( @@ -78,7 +79,7 @@ class TestHaproxyCfg(base.TestCase): " redirect scheme https if !{ ssl_fc }\n" " bind 10.0.0.2:443 " "ssl crt /var/lib/octavia/certs/" - "sample_listener_id_1/FakeCN.pem\n" + "sample_listener_id_1/tls_container_id.pem\n" " mode http\n" " default_backend sample_pool_id_1\n" " timeout client 50000\n\n") @@ -104,6 +105,7 @@ class TestHaproxyCfg(base.TestCase): sample_configs.sample_listener_tuple( proto='TERMINATED_HTTPS', tls=True), tls_cert=sample_configs.sample_tls_container_tuple( + id='tls_container_id', certificate='ImAalsdkfjCert', private_key='ImAsdlfksdjPrivateKey', primary_cn="FakeCN")) diff --git a/octavia/tests/unit/common/sample_configs/sample_certs.py b/octavia/tests/unit/common/sample_configs/sample_certs.py index 2bc858a650..a3470de727 100644 --- a/octavia/tests/unit/common/sample_configs/sample_certs.py +++ b/octavia/tests/unit/common/sample_configs/sample_certs.py @@ -20,6 +20,7 @@ import six X509_CERT_CN = 'www.example.com' +X509_CERT_SHA1 = '9965834d856a7e24459522af0b91df69323947b3' X509_CERT = b"""-----BEGIN CERTIFICATE----- MIIE8TCCAtmgAwIBAgICEAEwDQYJKoZIhvcNAQELBQAwIzEhMB8GA1UEAwwYY2Et diff --git a/octavia/tests/unit/common/tls_utils/test_cert_parser.py b/octavia/tests/unit/common/tls_utils/test_cert_parser.py index 0a751aa4ca..aa039fde1f 100644 --- a/octavia/tests/unit/common/tls_utils/test_cert_parser.py +++ b/octavia/tests/unit/common/tls_utils/test_cert_parser.py @@ -165,6 +165,7 @@ class TestTLSParseUtils(base.TestCase): @mock.patch('octavia.certificates.common.cert.Cert') def test_map_cert_tls_container(self, cert_mock): tls = data_models.TLSContainer( + id=sample_certs.X509_CERT_SHA1, primary_cn=sample_certs.X509_CERT_CN, certificate=sample_certs.X509_CERT, private_key=sample_certs.X509_CERT_KEY_ENCRYPTED, @@ -176,6 +177,9 @@ class TestTLSParseUtils(base.TestCase): cert_mock.get_private_key_passphrase.return_value = tls.passphrase with mock.patch.object(cert_parser, 'get_host_names') as cp: cp.return_value = {'cn': sample_certs.X509_CERT_CN} + self.assertEqual( + tls.id, cert_parser._map_cert_tls_container( + cert_mock).id) self.assertEqual( tls.primary_cn, cert_parser._map_cert_tls_container( cert_mock).primary_cn)