From 26b4459b06fc3f1c2e9143d4a3ed5a7e90a58ee6 Mon Sep 17 00:00:00 2001 From: James Slagle Date: Wed, 16 May 2018 17:43:59 -0400 Subject: [PATCH] Revert "TLS by default for the overcloud" This is needed to land I2c5b511df50f29c63aa613899c2bebb506360bf4, see the reasoning on that patch. This reverts commit 6fa7a0974a6729513e5d3d087119aa5a54b74c09. Partial-Bug: #1771627 Change-Id: Ia86842b0b1f42512f25390d6bdb695e0f8133c6d --- ...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, 36 insertions(+), 240 deletions(-) delete mode 100644 releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml delete 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 deleted file mode 100644 index 7a2f3a0f9..000000000 --- a/releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml +++ /dev/null @@ -1,8 +0,0 @@ ---- -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 a7d4657d8..c129373ac 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,4 +28,3 @@ 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 deleted file mode 100755 index 3a65b58cf..000000000 --- a/scripts/tripleo-overcloud-cert +++ /dev/null @@ -1,48 +0,0 @@ -#!/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 82acf6214..690b4f523 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,7 +31,6 @@ 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 bae5d458b..e8b8e658f 100644 --- a/sudoers +++ b/sudoers @@ -9,5 +9,4 @@ 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 4c45ec163..9a4ee1d27 100644 --- a/tripleo_common/actions/deployment.py +++ b/tripleo_common/actions/deployment.py @@ -12,17 +12,13 @@ # 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 @@ -207,21 +203,11 @@ 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, - overcloud_cert_path=constants.OVERCLOUD_CERT_PATH, - overcloud_key_path=constants.OVERCLOUD_KEY_PATH): + local_ca_path=constants.LOCAL_CACERT_PATH): cacert_string = self._get_local_cacert(local_ca_path) - 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) + if cacert_string: + parameters['CAMap'] = self._get_updated_camap_entry( + 'undercloud-ca', cacert_string, self._get_camap(env)) def _get_local_cacert(self, local_ca_path): # Since the undercloud has TLS by default, we'll add the undercloud's @@ -237,20 +223,9 @@ 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: { @@ -260,68 +235,6 @@ 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 04cb6b031..6cb3000df 100644 --- a/tripleo_common/constants.py +++ b/tripleo_common/constants.py @@ -57,10 +57,6 @@ 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 01adbca7f..88e451069 100644 --- a/tripleo_common/tests/actions/test_deployment.py +++ b/tripleo_common/tests/actions/test_deployment.py @@ -12,14 +12,6 @@ # 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 @@ -370,44 +362,7 @@ class DeployStackActionTest(base.TestCase): "overcloud-swift-rings", "swift-rings.tar.gz", "overcloud-swift-rings/swift-rings.tar.gz-%d" % 1473366264) - -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): + def test_set_tls_parameters_no_ca_found(self): action = deployment.DeployStackAction(1, 'overcloud', skip_deploy_identifier=True) my_params = {} @@ -416,58 +371,49 @@ class DeployStackActionSetTLSParametersTest(base.TestCase): local_ca_path='/tmp/my-unexistent-file.txt') self.assertEqual(my_params, {}) - @mock.patch('oslo_concurrency.processutils.execute') - def test_ca_found_no_camap_provided(self, mock_execute): + def test_set_tls_parameters_ca_found_no_camap_provided(self): action = deployment.DeployStackAction(1, 'overcloud', skip_deploy_identifier=True) - # Write test data my_params = {} my_env = {'parameter_defaults': {}} - 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') + with tempfile.NamedTemporaryFile() as ca_file: + # Write test data + ca_file.write(b'FAKE CA CERT') + ca_file.flush() - # 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']) + # 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']) - @mock.patch('oslo_concurrency.processutils.execute') - def test_ca_found_camap_provided(self, mock_execute): + def test_set_tls_parameters_ca_found_camap_provided(self): 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': 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') + '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() - # 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']) + # 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']) class OvercloudRcActionTestCase(base.TestCase):