Revert "TLS by default for the overcloud"

This is needed to land I2c5b511df50f29c63aa613899c2bebb506360bf4,
see the reasoning on that patch.

This reverts commit 6fa7a0974a.

Partial-Bug: #1771627
Change-Id: Ia86842b0b1f42512f25390d6bdb695e0f8133c6d

releasenotes/notes/Public-TLS-by-default-ea0bc712f475472f.yaml

@@ -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.

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

scripts/tripleo-overcloud-cert

@@ -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

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

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

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

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'

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):