From 9a0563bf450e2138e4d67682641de617364ae05f Mon Sep 17 00:00:00 2001 From: Nobuto Murata Date: Thu, 17 Aug 2017 19:24:52 +0700 Subject: [PATCH] Make ssl_ca optional if ssl_cert+ssl_key provided ssl_ca is not necessary when ssl_cert is signed by a trusted CA, such as GeoTrust, because a trusted cert chain is in the system already. Users can just provide ssl_cert and ssl_key to enable SSL endpoint in that case. Closes-Bug: #1711354 Change-Id: I4a34df1a2c2bf5705e02b713d968a22f4bbf57cf --- README.md | 15 +++++-- hooks/keystone_context.py | 25 +++++++---- unit_tests/test_keystone_contexts.py | 63 +++++++++++++++++++++------- 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index d93b20a6..2809ff17 100644 --- a/README.md +++ b/README.md @@ -110,10 +110,17 @@ and sync across peers. The cert will be distributed to all service endpoints which will be configured to use https as well as configuring themselves to be used as https. -When configuring the charms to use SSL there are two means of configuring a CA. -The user can provide their own using the options ssl_ca, ssl_cert, ssl_key -which are available on all endpoint charms or, if not provided, the keystone -charm will automatically generate a CA and certs to distribute to endpoints. +When configuring the charms to use SSL there are three charm config options as +ssl_ca, ssl_cert and ssl_key. + +- The user can provide their own CA, SSL cert and key using the options ssl_ca, + ssl_cert, ssl_key. + +- The user can provide SSL cert and key using ssl_cert and ssl_key when the cert + is signed by a trusted CA. + +- If not provided, the keystone charm will automatically generate a CA and certs + to distribute to endpoints. When the charm configures itself as a CA (generally only recommended for test purposes) it will elect an "ssl-cert-master" whose duty is to generate the CA diff --git a/hooks/keystone_context.py b/hooks/keystone_context.py index 71d07bfd..87d724d0 100644 --- a/hooks/keystone_context.py +++ b/hooks/keystone_context.py @@ -51,10 +51,9 @@ CA_CERT_PATH = '/usr/local/share/ca-certificates/keystone_juju_ca_cert.crt' def is_cert_provided_in_config(): - ca = config('ssl_ca') cert = config('ssl_cert') key = config('ssl_key') - return bool(ca and cert and key) + return bool(cert and key) class SSLContext(context.ApacheSSLContext): @@ -114,17 +113,25 @@ class SSLContext(context.ApacheSSLContext): "master is elected", level=INFO) return + cert = config('ssl_cert') + key = config('ssl_key') + ca_cert = config('ssl_ca') - if ca_cert is None: + if ca_cert: + ca_cert = b64decode(ca_cert) + elif not (cert and key): + # NOTE(hopem): if a cert and key are provided as config we don't + # mandate that a CA is also provided since it isn't necessarily + # needed. As a result we only generate a custom CA if we are also + # generating cert and key. ca = get_ca(user=SSH_USER) ca_cert = ca.get_ca_bundle() - else: - ca_cert = b64decode(ca_cert) - # Ensure accessible by keystone ssh user and group (unison) - install_ca_cert(ca_cert) - ensure_permissions(CA_CERT_PATH, user=SSH_USER, group=KEYSTONE_USER, - perms=0o0644) + if ca_cert: + # Ensure accessible by keystone ssh user and group (unison) + install_ca_cert(ca_cert) + ensure_permissions(CA_CERT_PATH, user=SSH_USER, + group=KEYSTONE_USER, perms=0o0644) def canonical_names(self): addresses = self.get_network_addresses() diff --git a/unit_tests/test_keystone_contexts.py b/unit_tests/test_keystone_contexts.py index 04e9c611..fda0bed6 100644 --- a/unit_tests/test_keystone_contexts.py +++ b/unit_tests/test_keystone_contexts.py @@ -29,7 +29,6 @@ TO_PATCH = [ 'config', 'determine_apache_port', 'determine_api_port', - 'is_cert_provided_in_config', ] @@ -38,30 +37,62 @@ class TestKeystoneContexts(CharmTestCase): def setUp(self): super(TestKeystoneContexts, self).setUp(context, TO_PATCH) - @patch.object(context, 'is_cert_provided_in_config') + def test_is_cert_provided_in_config(self): + config = {'ssl_cert': 'somecert', 'ssl_key': 'greatkey'} + + def fake_config(key): + return config.get(key) + + self.config.side_effect = fake_config + self.assertTrue(context.is_cert_provided_in_config()) + + del config['ssl_cert'] + self.assertFalse(context.is_cert_provided_in_config()) + @patch.object(context, 'mkdir') @patch('keystone_utils.get_ca') @patch('keystone_utils.ensure_permissions') - @patch('keystone_utils.determine_ports') - @patch('keystone_utils.is_ssl_cert_master') - @patch.object(context, 'log') - def test_apache_ssl_context_ssl_not_master(self, - mock_log, - mock_is_ssl_cert_master, - mock_determine_ports, - mock_ensure_permissions, - mock_get_ca, - mock_mkdir, - mock_cert_provided_in_config): - mock_cert_provided_in_config.return_value = False - mock_is_ssl_cert_master.return_value = False - + @patch('keystone_utils.determine_ports', lambda: None) + @patch('keystone_utils.is_ssl_cert_master', lambda: False) + @patch.object(context, 'is_cert_provided_in_config', lambda: False) + @patch.object(context, 'log', lambda *args, **kwargs: None) + def test_apache_ssl_context_ssl_not_master(self, mock_ensure_permissions, + mock_get_ca, mock_mkdir): context.ApacheSSLContext().configure_cert('foo') context.ApacheSSLContext().configure_ca() self.assertTrue(mock_mkdir.called) self.assertTrue(mock_ensure_permissions.called) self.assertFalse(mock_get_ca.called) + @patch('keystone_utils.ensure_permissions') + @patch.object(context, 'install_ca_cert') + @patch.object(context, 'b64decode') + @patch.object(context, 'mkdir', lambda *args: None) + @patch('keystone_utils.get_ca', lambda: None) + @patch('keystone_utils.determine_ports', lambda: None) + @patch('keystone_utils.is_ssl_cert_master', lambda: True) + @patch.object(context, 'log', lambda *args, **kwargs: None) + def test_apache_ssl_context_ssl_configure_ca(self, mock_b64decode, + mock_install_ca_cert, + mock_ensure_permissions): + config = {'ssl_cert': 'somecert', 'ssl_key': 'greatkey'} + + def fake_config(key): + return config.get(key) + + self.config.side_effect = fake_config + + context.ApacheSSLContext().configure_ca() + self.assertFalse(mock_b64decode.called) + self.assertFalse(mock_install_ca_cert.called) + self.assertFalse(mock_ensure_permissions.called) + + config['ssl_ca'] = 'foofoofalalala' + context.ApacheSSLContext().configure_ca() + self.assertTrue(mock_b64decode.called) + self.assertTrue(mock_install_ca_cert.called) + self.assertTrue(mock_ensure_permissions.called) + @patch('charmhelpers.contrib.hahelpers.cluster.relation_ids') @patch('charmhelpers.contrib.openstack.ip.unit_get') @patch('charmhelpers.contrib.openstack.ip.service_name')