From 6fa7a0974a6729513e5d3d087119aa5a54b74c09 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Robles Date: Wed, 21 Mar 2018 16:00:39 +0200 Subject: [PATCH] TLS by default for the overcloud This gets a TLS certificate for the overcloud when necessary: * If no incoming cert/key is provided and we don't expect the overcloud's certmonger instances to request the certificates, we request one to the undercloud's certmonger local CA. * If a certificate was provided, we verify if it's user-provided or if it was autogenerated. - If it was user-provided we pass through that certificate - If it was autogenerated, we request or resubmit the request if it's needed. * We also accept the EnableTLS flag, which the deployer can explicitly turn off if they decide not to use TLS. Depends-On: Ic70dd323b33596eaa3fc18bdc69a7c011ccd7fa1 Change-Id: I3d3cad0eb1396e7bee146794b29badad302efdf3 --- ...ublic-TLS-by-default-ea0bc712f475472f.yaml | 8 ++ requirements.txt | 1 + scripts/tripleo-overcloud-cert | 48 +++++++ setup.cfg | 1 + sudoers | 1 + tripleo_common/actions/deployment.py | 95 +++++++++++++- tripleo_common/constants.py | 4 + .../tests/actions/test_deployment.py | 118 +++++++++++++----- 8 files changed, 240 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml create mode 100755 scripts/tripleo-overcloud-cert diff --git a/releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml b/releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml new file mode 100644 index 000000000..7a2f3a0f9 --- /dev/null +++ b/releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + The default plan deployment workflow now automatically adds the necessary + certificate and key to enable TLS by default in the overcloud. Note that + this doesn't overwrite any certificate or keys given by the deployer; + those still take precedence. This will enable TLS if it isn't already + enabled though. diff --git a/requirements.txt b/requirements.txt index c129373ac..a7d4657d8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,3 +28,4 @@ python-keystoneclient>=3.8.0 # Apache-2.0 keystoneauth1>=3.4.0 # Apache-2.0 tenacity>=4.4.0 # Apache-2.0 futures>=3.0.0;python_version=='2.7' or python_version=='2.6' # BSD +cryptography>=2.1 # BSD/Apache-2.0 diff --git a/scripts/tripleo-overcloud-cert b/scripts/tripleo-overcloud-cert new file mode 100755 index 000000000..3a65b58cf --- /dev/null +++ b/scripts/tripleo-overcloud-cert @@ -0,0 +1,48 @@ +#!/bin/bash +set -x +# Currently action is unused, but it will be. +action=$1 +overcloud_container_name=$2 + +if [[ "$action" == 'request' || "$action" == 'resubmit' ]]; then + overcloud_fqdn=$3 + + OVERCLOUD_CERT_PATH="/etc/pki/tls/certs/overcloud-${overcloud_container_name}-cert.pem" + OVERCLOUD_KEY_PATH="/etc/pki/tls/private/overcloud-${overcloud_container_name}-key.pem" + + # This validates that overcloud_fqdn is actually an FQDN + if [[ ! $(echo "$overcloud_fqdn" | grep -P '(?=^.{1,254}$)(^(?>(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z]{2,})$)') ]] + then + exit 1 + fi + + # Skip request if the request already exists + /usr/bin/getcert list -c local -i "overcloud-${overcloud_container_name}-cert" > /dev/null + request_exists=$? + if [[ $request_exists != 0 || "$action" == 'resubmit' ]]; + then + if [[ "$action" == "request" ]]; then + /usr/bin/getcert request -c local \ + -I "overcloud-${overcloud_container_name}-cert" \ + -f $OVERCLOUD_CERT_PATH \ + -k $OVERCLOUD_KEY_PATH \ + -N "CN=${overcloud_fqdn}" \ + -D "$overcloud_fqdn" \ + -C "/usr/bin/chown mistral:mistral $OVERCLOUD_CERT_PATH $OVERCLOUD_KEY_PATH" \ + -w -v + else + /usr/bin/getcert resubmit -c local \ + -i "overcloud-${overcloud_container_name}-cert" \ + -f $OVERCLOUD_CERT_PATH \ + -N "CN=${overcloud_fqdn}" \ + -D "$overcloud_fqdn" \ + -C "/usr/bin/chown mistral:mistral $OVERCLOUD_CERT_PATH $OVERCLOUD_KEY_PATH" \ + -w -v + fi + fi +elif [[ "$action" == 'query' ]]; then + /usr/bin/getcert list -c local -i "overcloud-${overcloud_container_name}-cert" +else + echo "Unkown action $action" + exit 1 +fi diff --git a/setup.cfg b/setup.cfg index 690b4f523..82acf6214 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,6 +31,7 @@ scripts = scripts/run-validation scripts/tripleo-build-images scripts/tripleo-config-download + scripts/tripleo-overcloud-cert scripts/upgrade-non-controller.sh scripts/upload-puppet-modules scripts/upload-swift-artifacts diff --git a/sudoers b/sudoers index e8b8e658f..bae5d458b 100644 --- a/sudoers +++ b/sudoers @@ -9,4 +9,5 @@ mistral ALL = NOPASSWD: /usr/bin/rm -f /tmp/validations_identity_[A-Za-z0-9_][A- mistral ALL = NOPASSWD: /bin/nova-manage cell_v2 discover_hosts * mistral ALL = NOPASSWD: /usr/bin/tar --ignore-failed-read -C / -cf /var/tmp/undercloud-backup-*.tar * mistral ALL = NOPASSWD: /usr/bin/chown mistral. /var/tmp/undercloud-backup-*/filesystem-*.tar +mistral ALL = NOPASSWD: /usr/bin/tripleo-overcloud-cert * validations ALL = NOPASSWD: ALL diff --git a/tripleo_common/actions/deployment.py b/tripleo_common/actions/deployment.py index 9a4ee1d27..4c45ec163 100644 --- a/tripleo_common/actions/deployment.py +++ b/tripleo_common/actions/deployment.py @@ -12,13 +12,17 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from cryptography.hazmat.backends import default_backend +from cryptography import x509 import json import logging +import os import time from heatclient.common import deployment_utils from heatclient import exc as heat_exc from mistral_lib import actions +from oslo_concurrency import processutils from swiftclient import exceptions as swiftexceptions from tripleo_common.actions import base @@ -203,11 +207,21 @@ class DeployStackAction(templates.ProcessTemplatesAction): return heat.stacks.update(stack.id, **stack_args) def set_tls_parameters(self, parameters, env, - local_ca_path=constants.LOCAL_CACERT_PATH): + local_ca_path=constants.LOCAL_CACERT_PATH, + overcloud_cert_path=constants.OVERCLOUD_CERT_PATH, + overcloud_key_path=constants.OVERCLOUD_KEY_PATH): cacert_string = self._get_local_cacert(local_ca_path) - if cacert_string: - parameters['CAMap'] = self._get_updated_camap_entry( - 'undercloud-ca', cacert_string, self._get_camap(env)) + if not cacert_string: + return + + parameters['CAMap'] = self._get_updated_camap_entry( + 'undercloud-ca', cacert_string, self._get_camap(env)) + cert_path = overcloud_cert_path.format(container=self.container) + key_path = overcloud_key_path.format(container=self.container) + if self._deployment_needs_local_cert(env, cert_path, key_path): + self._request_cert_if_necessary(env, cert_path, key_path) + parameters['SSLCertificate'] = self._get_local_file(cert_path) + parameters['SSLKey'] = self._get_local_file(key_path) def _get_local_cacert(self, local_ca_path): # Since the undercloud has TLS by default, we'll add the undercloud's @@ -223,9 +237,20 @@ class DeployStackAction(templates.ProcessTemplatesAction): except Exception: raise + def _get_local_file(self, path): + with open(path, 'rb') as openfile: + return openfile.read() + def _get_camap(self, env): return env['parameter_defaults'].get('CAMap', {}) + def _get_overcloud_tls_certificate(self, env): + cert_raw = env['parameter_defaults'].get('SSLCertificate', b'') + if hasattr(cert_raw, 'encode'): + return cert_raw.encode('ascii') + else: + return cert_raw + def _get_updated_camap_entry(self, entry_name, cacert, orig_camap): ca_map_entry = { entry_name: { @@ -235,6 +260,68 @@ class DeployStackAction(templates.ProcessTemplatesAction): orig_camap.update(ca_map_entry) return orig_camap + def _cert_is_from_local_issuer(self, cert): + for attribute in cert.issuer: + # '2.5.4.3' is the OID of the certificate issuer's CommonName or CN + # 'Local Signing Authority' is the default CA name that certmonger + # uses for the local CA + if (attribute.oid.dotted_string == '2.5.4.3' and + attribute.value == 'Local Signing Authority'): + return True + return False + + def _deployment_needs_local_cert(self, env, cert_path, key_path): + overcloud_cert_bytes = self._get_overcloud_tls_certificate(env) + overcloud_certmonger = env['parameter_defaults'].get( + 'PublicSSLCertificateAutogenerated', False) + enable_tls_flag = env['parameter_defaults'].get( + 'EnablePublicTLS', True) + if overcloud_cert_bytes: + overcloud_cert = x509.load_pem_x509_certificate( + overcloud_cert_bytes, default_backend()) + if not self._cert_is_from_local_issuer(overcloud_cert): + return False + elif overcloud_certmonger or not enable_tls_flag: + return False + return True + + def _request_cert_if_necessary(self, env, cert_path, key_path): + overcloud_fqdn = env['parameter_defaults'].get( + 'CloudName', 'overcloud.localdomain') + if self._local_cert_request_present(): + # Even if certmonger is tracking the request, the user + # might have deleted the cert file, so we resubmit the request. + if not os.path.isfile(cert_path): + self._request_local_cert( + overcloud_fqdn, + resubmit=True) + if not os.path.isfile(key_path): + raise RuntimeError( + "The key \"{0}\" is not present, you'll need to " + "recreate the certificate manually.".format(key_path)) + else: + self._request_local_cert(overcloud_fqdn) + + def _request_local_cert(self, overcloud_fqdn, resubmit=False): + action = 'request' if not resubmit else 'resubmit' + return processutils.execute( + '/usr/bin/sudo', + '/usr/bin/tripleo-overcloud-cert', + action, + self.container, + overcloud_fqdn) + + def _local_cert_request_present(self): + try: + processutils.execute( + '/usr/bin/sudo', + '/usr/bin/tripleo-overcloud-cert', + 'query', + self.container) + return True + except processutils.ProcessExecutionError: + return False + class OvercloudRcAction(base.TripleOAction): """Generate the overcloudrc and overcloudrc.v3 for a plan diff --git a/tripleo_common/constants.py b/tripleo_common/constants.py index 6cb3000df..04cb6b031 100644 --- a/tripleo_common/constants.py +++ b/tripleo_common/constants.py @@ -57,6 +57,10 @@ DEFAULT_VALIDATIONS_PATH = \ # The path to the local CA certificate installed on the undercloud LOCAL_CACERT_PATH = '/etc/pki/ca-trust/source/anchors/cm-local-ca.pem' +# The path to the locally generated overcloud certificate and key +OVERCLOUD_CERT_PATH = '/etc/pki/tls/certs/overcloud-{container}-cert.pem' +OVERCLOUD_KEY_PATH = '/etc/pki/tls/private/overcloud-{container}-key.pem' + # TRIPLEO_META_USAGE_KEY is inserted into metadata for containers created in # Swift via SwiftPlanStorageBackend to identify them from other containers TRIPLEO_META_USAGE_KEY = 'x-container-meta-usage-tripleo' diff --git a/tripleo_common/tests/actions/test_deployment.py b/tripleo_common/tests/actions/test_deployment.py index 88e451069..01adbca7f 100644 --- a/tripleo_common/tests/actions/test_deployment.py +++ b/tripleo_common/tests/actions/test_deployment.py @@ -12,6 +12,14 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import serialization +from cryptography import x509 +from cryptography.x509.oid import NameOID +from datetime import datetime +from datetime import timedelta import mock import tempfile import yaml @@ -362,7 +370,44 @@ class DeployStackActionTest(base.TestCase): "overcloud-swift-rings", "swift-rings.tar.gz", "overcloud-swift-rings/swift-rings.tar.gz-%d" % 1473366264) - def test_set_tls_parameters_no_ca_found(self): + +class DeployStackActionSetTLSParametersTest(base.TestCase): + + def get_self_signed_certificate_and_private_key(self): + private_key = rsa.generate_private_key(public_exponent=3, + key_size=1024, + backend=default_backend()) + issuer = x509.Name([ + x509.NameAttribute(NameOID.COUNTRY_NAME, u"FI"), + x509.NameAttribute(NameOID.LOCALITY_NAME, u"Helsinki"), + x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"Some Company"), + x509.NameAttribute(NameOID.COMMON_NAME, u"Test Certificate"), + ]) + cert_builder = x509.CertificateBuilder( + issuer_name=issuer, subject_name=issuer, + public_key=private_key.public_key(), + serial_number=x509.random_serial_number(), + not_valid_before=datetime.utcnow(), + not_valid_after=datetime.utcnow() + timedelta(days=10) + ) + cert = cert_builder.sign(private_key, + hashes.SHA256(), + default_backend()) + cert_pem = cert.public_bytes(encoding=serialization.Encoding.PEM) + key_pem = private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=serialization.NoEncryption()) + return cert_pem, key_pem + + def create_temp_file(self, content): + temp_file = tempfile.NamedTemporaryFile() + temp_file.write(content) + temp_file.flush() + self.addCleanup(temp_file.close) + return temp_file.name + + def test_no_ca_found(self): action = deployment.DeployStackAction(1, 'overcloud', skip_deploy_identifier=True) my_params = {} @@ -371,49 +416,58 @@ class DeployStackActionTest(base.TestCase): local_ca_path='/tmp/my-unexistent-file.txt') self.assertEqual(my_params, {}) - def test_set_tls_parameters_ca_found_no_camap_provided(self): + @mock.patch('oslo_concurrency.processutils.execute') + def test_ca_found_no_camap_provided(self, mock_execute): action = deployment.DeployStackAction(1, 'overcloud', skip_deploy_identifier=True) + # Write test data my_params = {} my_env = {'parameter_defaults': {}} - with tempfile.NamedTemporaryFile() as ca_file: - # Write test data - ca_file.write(b'FAKE CA CERT') - ca_file.flush() + cert_pem, _ = self.get_self_signed_certificate_and_private_key() + ca_file_path = self.create_temp_file(cert_pem) + overcloud_cert_path = self.create_temp_file(b'FAKE OVERCLOUD CERT') + overcloud_key_path = self.create_temp_file(b'FAKE OVERCLOUD KEY') - # Test - action.set_tls_parameters(parameters=my_params, env=my_env, - local_ca_path=ca_file.name) - self.assertIn('CAMap', my_params) - self.assertIn('undercloud-ca', my_params['CAMap']) - self.assertIn('content', my_params['CAMap']['undercloud-ca']) - self.assertEqual(b'FAKE CA CERT', - my_params['CAMap']['undercloud-ca']['content']) + # Test + action.set_tls_parameters(parameters=my_params, env=my_env, + local_ca_path=ca_file_path, + overcloud_cert_path=overcloud_cert_path, + overcloud_key_path=overcloud_key_path) + self.assertIn('CAMap', my_params) + self.assertIn('undercloud-ca', my_params['CAMap']) + self.assertIn('content', my_params['CAMap']['undercloud-ca']) + self.assertEqual(cert_pem, + my_params['CAMap']['undercloud-ca']['content']) - def test_set_tls_parameters_ca_found_camap_provided(self): + @mock.patch('oslo_concurrency.processutils.execute') + def test_ca_found_camap_provided(self, mock_execute): action = deployment.DeployStackAction(1, 'overcloud', skip_deploy_identifier=True) + # Write test data + undercloud_pem, _ = self.get_self_signed_certificate_and_private_key() + overcloud_pem, _ = self.get_self_signed_certificate_and_private_key() my_params = {} my_env = { 'parameter_defaults': { - 'CAMap': {'overcloud-ca': {'content': b'ANOTER FAKE CERT'}}}} - with tempfile.NamedTemporaryFile() as ca_file: - # Write test data - ca_file.write(b'FAKE CA CERT') - ca_file.flush() + 'CAMap': {'overcloud-ca': {'content': overcloud_pem}}}} + ca_file_path = self.create_temp_file(undercloud_pem) + overcloud_cert_path = self.create_temp_file(b'FAKE OVERCLOUD CERT') + overcloud_key_path = self.create_temp_file(b'FAKE OVERCLOUD KEY') - # Test - action.set_tls_parameters(parameters=my_params, env=my_env, - local_ca_path=ca_file.name) - self.assertIn('CAMap', my_params) - self.assertIn('undercloud-ca', my_params['CAMap']) - self.assertIn('content', my_params['CAMap']['undercloud-ca']) - self.assertEqual(b'FAKE CA CERT', - my_params['CAMap']['undercloud-ca']['content']) - self.assertIn('overcloud-ca', my_params['CAMap']) - self.assertIn('content', my_params['CAMap']['overcloud-ca']) - self.assertEqual(b'ANOTER FAKE CERT', - my_params['CAMap']['overcloud-ca']['content']) + # Test + action.set_tls_parameters(parameters=my_params, env=my_env, + local_ca_path=ca_file_path, + overcloud_cert_path=overcloud_cert_path, + overcloud_key_path=overcloud_key_path) + self.assertIn('CAMap', my_params) + self.assertIn('undercloud-ca', my_params['CAMap']) + self.assertIn('content', my_params['CAMap']['undercloud-ca']) + self.assertEqual(undercloud_pem, + my_params['CAMap']['undercloud-ca']['content']) + self.assertIn('overcloud-ca', my_params['CAMap']) + self.assertIn('content', my_params['CAMap']['overcloud-ca']) + self.assertEqual(overcloud_pem, + my_params['CAMap']['overcloud-ca']['content']) class OvercloudRcActionTestCase(base.TestCase):