From 021e0a6a4660135f06fc7c4d0a4b0c76b8772a7f Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 4 Sep 2020 13:52:27 +0200 Subject: [PATCH] Generate a TLS certificate and send it to ironic Adds a new flag (on by default) that enables generating a TLS certificate and sending it to ironic via heartbeat. Whether ironic supports auto-generated certificates is determined by checking its API version. Change-Id: I01f83dd04cfec2adc9e2a6b9c531391773ed36e5 Depends-On: https://review.opendev.org/747136 Depends-On: https://review.opendev.org/749975 Story: #2007214 Task: #40604 --- ironic_python_agent/agent.py | 26 +++- ironic_python_agent/api/app.py | 13 +- ironic_python_agent/config.py | 6 + ironic_python_agent/hardware.py | 8 ++ ironic_python_agent/ironic_api_client.py | 14 ++- ironic_python_agent/tests/unit/test_agent.py | 47 +++++++- .../tests/unit/test_ironic_api_client.py | 32 +++++ .../tests/unit/test_tls_utils.py | 68 +++++++++++ ironic_python_agent/tls_utils.py | 111 ++++++++++++++++++ lower-constraints.txt | 3 + .../notes/auto-tls-b52b873663f35618.yaml | 8 ++ requirements.txt | 1 + 12 files changed, 331 insertions(+), 6 deletions(-) create mode 100644 ironic_python_agent/tests/unit/test_tls_utils.py create mode 100644 ironic_python_agent/tls_utils.py create mode 100644 releasenotes/notes/auto-tls-b52b873663f35618.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 1cfe2e202..af2d0cc9a 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -135,6 +135,7 @@ class IronicPythonAgentHeartbeater(threading.Thread): uuid=self.agent.get_node_uuid(), advertise_address=self.agent.advertise_address, advertise_protocol=self.agent.advertise_protocol, + generated_cert=self.agent.generated_cert, ) self.error_delay = self.initial_delay LOG.info('heartbeat successful') @@ -216,6 +217,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin): # got upgraded somewhere along the way. self.agent_token_required = cfg.CONF.agent_token_required self.iscsi_started = False + self.generated_cert = None def get_status(self): """Retrieve a serializable status. @@ -370,9 +372,31 @@ class IronicPythonAgent(base.ExecuteCommandMixin): LOG.warning("No valid network interfaces found. " "Node lookup will probably fail.") + def _start_auto_tls(self): + # NOTE(dtantsur): if listen_tls is True, assume static TLS + # configuration and don't auto-generate anything. + if cfg.CONF.listen_tls or not cfg.CONF.enable_auto_tls: + LOG.debug('Automated TLS is disabled') + return None, None + + if not self.api_url or not self.api_client.supports_auto_tls(): + LOG.warning('Ironic does not support automated TLS') + return None, None + + self.set_agent_advertise_addr() + + LOG.info('Generating TLS parameters automatically for IP %s', + self.advertise_address.hostname) + tls_info = hardware.dispatch_to_managers( + 'generate_tls_certificate', self.advertise_address.hostname) + self.generated_cert = tls_info.text + self.advertise_protocol = 'https' + return tls_info.path, tls_info.private_key_path + def serve_ipa_api(self): """Serve the API until an extension terminates it.""" - self.api.start() + cert_file, key_file = self._start_auto_tls() + self.api.start(cert_file, key_file) if not self.standalone and self.api_url: # Don't start heartbeating until the server is listening self.heartbeater.start() diff --git a/ironic_python_agent/api/app.py b/ironic_python_agent/api/app.py index a379a7e04..467f8d9c8 100644 --- a/ironic_python_agent/api/app.py +++ b/ironic_python_agent/api/app.py @@ -16,6 +16,7 @@ import json from ironic_lib import metrics_utils from oslo_log import log +from oslo_service import sslutils from oslo_service import wsgi import werkzeug from werkzeug import exceptions as http_exc @@ -126,12 +127,20 @@ class Application(object): response = self.handle_exception(environ, exc) return response(environ, start_response) - def start(self): + def start(self, tls_cert_file=None, tls_key_file=None): """Start the API service in the background.""" + if tls_cert_file and tls_key_file: + sslutils.register_opts(self._conf) + self._conf.set_override('cert_file', tls_cert_file, group='ssl') + self._conf.set_override('key_file', tls_key_file, group='ssl') + use_tls = True + else: + use_tls = self._conf.listen_tls + self.service = wsgi.Server(self._conf, 'ironic-python-agent', app=self, host=self.agent.listen_address.hostname, port=self.agent.listen_address.port, - use_ssl=self._conf.listen_tls) + use_ssl=use_tls) self.service.start() LOG.info('Started API service on port %s', self.agent.listen_address.port) diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 16372c97f..c7afee015 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -66,6 +66,12 @@ cli_opts = [ 'key_file, and, if desired, ca_file to validate client ' 'certificates.'), + cfg.BoolOpt('enable_auto_tls', + default=True, + help='Enables auto-generating TLS parameters when listen_tls ' + 'is False and ironic API version indicates support for ' + 'automatic agent TLS.'), + cfg.StrOpt('advertise_host', default=APARAMS.get('ipa-advertise-host', None), help='The host to tell Ironic to reply and send ' diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 399d70609..2b6f795db 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -40,6 +40,7 @@ from ironic_python_agent import errors from ironic_python_agent.extensions import base as ext_base from ironic_python_agent import netutils from ironic_python_agent import raid_utils +from ironic_python_agent import tls_utils from ironic_python_agent import utils _global_managers = None @@ -648,6 +649,9 @@ class HardwareManager(object, metaclass=abc.ABCMeta): def get_interface_info(self, interface_name): raise errors.IncompatibleHardwareMethodError() + def generate_tls_certificate(self, ip_address): + raise errors.IncompatibleHardwareMethodError() + def erase_block_device(self, node, block_device): """Attempt to erase a block device. @@ -2091,6 +2095,10 @@ class GenericHardwareManager(HardwareManager): # The result is asynchronous, wait here. cmd.join() + def generate_tls_certificate(self, ip_address): + """Generate a TLS certificate for the IP address.""" + return tls_utils.generate_tls_certificate(ip_address) + def _compare_extensions(ext1, ext2): mgr1 = ext1.obj diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index a8923060e..ac923d513 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -32,9 +32,10 @@ LOG = log.getLogger(__name__) MIN_IRONIC_VERSION = (1, 31) AGENT_VERSION_IRONIC_VERSION = (1, 36) AGENT_TOKEN_IRONIC_VERSION = (1, 62) +AGENT_VERIFY_CA_IRONIC_VERSION = (1, 68) # NOTE(dtantsur): change this constant every time you add support for more # versions to ensure that we send the highest version we know about. -MAX_KNOWN_VERSION = AGENT_TOKEN_IRONIC_VERSION +MAX_KNOWN_VERSION = AGENT_VERIFY_CA_IRONIC_VERSION class APIClient(object): @@ -101,7 +102,11 @@ class APIClient(object): return MIN_IRONIC_VERSION return self._ironic_api_version - def heartbeat(self, uuid, advertise_address, advertise_protocol='http'): + def supports_auto_tls(self): + return self._get_ironic_api_version() >= AGENT_VERIFY_CA_IRONIC_VERSION + + def heartbeat(self, uuid, advertise_address, advertise_protocol='http', + generated_cert=None): path = self.heartbeat_api.format(uuid=uuid) data = {'callback_url': self._get_agent_url(advertise_address, @@ -115,9 +120,14 @@ class APIClient(object): if api_ver >= AGENT_VERSION_IRONIC_VERSION: data['agent_version'] = version.version_info.release_string() + if api_ver >= AGENT_VERIFY_CA_IRONIC_VERSION and generated_cert: + data['agent_verify_ca'] = generated_cert + api_ver = min(MAX_KNOWN_VERSION, api_ver) headers = self._get_ironic_api_version_header(api_ver) + LOG.debug('Heartbeat: announcing callback URL %s, API version is ' + '%d.%d', data['callback_url'], *api_ver) try: response = self._request('POST', path, data=data, headers=headers) except requests.exceptions.ConnectionError as e: diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 013eb83d9..451fb87f1 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -31,6 +31,7 @@ from ironic_python_agent import hardware from ironic_python_agent import inspector from ironic_python_agent import netutils from ironic_python_agent.tests.unit import base as ironic_agent_base +from ironic_python_agent import tls_utils from ironic_python_agent import utils EXPECTED_ERROR = RuntimeError('command execution failed') @@ -858,12 +859,55 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest): @mock.patch( 'ironic_python_agent.hardware_managers.cna._detect_cna_card', mock.Mock()) + @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', autospec=True) @mock.patch.object(hardware, 'get_managers', autospec=True) def test_run(self, mock_get_managers, mock_list_hardware, - mock_wsgi): + mock_wsgi, mock_dispatch): + wsgi_server_request = mock_wsgi.return_value + + def set_serve_api(): + self.agent.serve_api = False + + wsgi_server_request.start.side_effect = set_serve_api + + mock_dispatch.return_value = tls_utils.TlsCertificate( + 'I am a cert', '/path/to/cert', '/path/to/key') + + self.agent.heartbeater = mock.Mock() + self.agent.api_client = mock.Mock() + self.agent.api_client.lookup_node = mock.Mock() + + self.agent.run() + + self.assertTrue(mock_get_managers.called) + mock_wsgi.assert_called_once_with(CONF, 'ironic-python-agent', + app=self.agent.api, + host=mock.ANY, port=9999, + use_ssl=True) + wsgi_server_request.start.assert_called_once_with() + mock_dispatch.assert_called_once_with('generate_tls_certificate', + mock.ANY) + + self.assertEqual('/path/to/cert', CONF.ssl.cert_file) + self.assertEqual('/path/to/key', CONF.ssl.key_file) + self.assertEqual('https', self.agent.advertise_protocol) + + self.assertFalse(self.agent.heartbeater.called) + self.assertFalse(self.agent.api_client.lookup_node.called) + + @mock.patch( + 'ironic_python_agent.hardware_managers.cna._detect_cna_card', + mock.Mock()) + @mock.patch('oslo_service.wsgi.Server', autospec=True) + @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', + autospec=True) + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_run_no_tls(self, mock_get_managers, mock_list_hardware, + mock_wsgi): + CONF.set_override('enable_auto_tls', False) wsgi_server_request = mock_wsgi.return_value def set_serve_api(): @@ -883,6 +927,7 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest): host=mock.ANY, port=9999, use_ssl=False) wsgi_server_request.start.assert_called_once_with() + self.assertEqual('http', self.agent.advertise_protocol) self.assertFalse(self.agent.heartbeater.called) self.assertFalse(self.agent.api_client.lookup_node.called) diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index bd710e77f..78e4a93e7 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -205,6 +205,38 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): 'callback_url': 'http://[fc00:1111::4]:9999'} self.assertEqual(jsonutils.dumps(expected_data), data) + def test_successful_heartbeat_with_verify_ca(self): + response = FakeResponse(status_code=202) + + self.api_client.session.request = mock.Mock() + self.api_client.session.request.return_value = response + self.api_client._ironic_api_version = ( + ironic_api_client.AGENT_VERIFY_CA_IRONIC_VERSION) + self.api_client.agent_token = 'magical' + + self.api_client.heartbeat( + uuid='deadbeef-dabb-ad00-b105-f00d00bab10c', + advertise_address=('192.0.2.1', '9999'), + advertise_protocol='https', + generated_cert='I am a cert', + ) + + heartbeat_path = 'v1/heartbeat/deadbeef-dabb-ad00-b105-f00d00bab10c' + request_args = self.api_client.session.request.call_args[0] + data = self.api_client.session.request.call_args[1]['data'] + self.assertEqual('POST', request_args[0]) + self.assertEqual(API_URL + heartbeat_path, request_args[1]) + expected_data = { + 'callback_url': 'https://192.0.2.1:9999', + 'agent_token': 'magical', + 'agent_version': version.version_info.release_string(), + 'agent_verify_ca': 'I am a cert'} + self.assertEqual(jsonutils.dumps(expected_data), data) + headers = self.api_client.session.request.call_args[1]['headers'] + self.assertEqual( + '%d.%d' % ironic_api_client.AGENT_VERIFY_CA_IRONIC_VERSION, + headers['X-OpenStack-Ironic-API-Version']) + def test_heartbeat_requests_exception(self): self.api_client.session.request = mock.Mock() self.api_client.session.request.side_effect = Exception('api is down!') diff --git a/ironic_python_agent/tests/unit/test_tls_utils.py b/ironic_python_agent/tests/unit/test_tls_utils.py new file mode 100644 index 000000000..1c847cb6b --- /dev/null +++ b/ironic_python_agent/tests/unit/test_tls_utils.py @@ -0,0 +1,68 @@ +# Copyright 2020 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import ipaddress +import os +import tempfile +from unittest import mock + +from cryptography.hazmat import backends +from cryptography import x509 + +from ironic_python_agent.tests.unit import base as ironic_agent_base +from ironic_python_agent import tls_utils + + +class GenerateTestCase(ironic_agent_base.IronicAgentTest): + + def setUp(self): + super().setUp() + tempdir = tempfile.mkdtemp() + self.crt_file = os.path.join(tempdir, 'localhost.crt') + self.key_file = os.path.join(tempdir, 'localhost.key') + + def test__generate(self): + result = tls_utils._generate_tls_certificate(self.crt_file, + self.key_file, + 'localhost', '127.0.0.1') + self.assertTrue(result.startswith("-----BEGIN CERTIFICATE-----\n"), + result) + self.assertTrue(result.endswith("\n-----END CERTIFICATE-----\n"), + result) + self.assertTrue(os.path.exists(self.key_file)) + with open(self.crt_file, 'rt') as fp: + self.assertEqual(result, fp.read()) + + cert = x509.load_pem_x509_certificate(result.encode(), + backends.default_backend()) + self.assertEqual([(x509.NameOID.COMMON_NAME, 'localhost')], + [(item.oid, item.value) for item in cert.subject]) + subject_alt_name = cert.extensions.get_extension_for_oid( + x509.ExtensionOID.SUBJECT_ALTERNATIVE_NAME) + self.assertTrue(subject_alt_name.critical) + self.assertEqual( + [ipaddress.IPv4Address('127.0.0.1')], + subject_alt_name.value.get_values_for_type(x509.IPAddress)) + self.assertEqual( + [], subject_alt_name.value.get_values_for_type(x509.DNSName)) + + @mock.patch('ironic_python_agent.netutils.get_hostname', autospec=True) + @mock.patch('os.makedirs', autospec=True) + @mock.patch.object(tls_utils, '_generate_tls_certificate', autospec=True) + def test_generate(self, mock_generate, mock_makedirs, mock_hostname): + result = tls_utils.generate_tls_certificate('127.0.0.1') + mock_generate.assert_called_once_with(result.path, + result.private_key_path, + mock_hostname.return_value, + '127.0.0.1') diff --git a/ironic_python_agent/tls_utils.py b/ironic_python_agent/tls_utils.py new file mode 100644 index 000000000..f38fb7e23 --- /dev/null +++ b/ironic_python_agent/tls_utils.py @@ -0,0 +1,111 @@ +# Copyright 2020 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import collections +import datetime +import ipaddress +import os + +from cryptography.hazmat import backends +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import serialization +from cryptography import x509 + +from ironic_python_agent import netutils + + +def _create_private_key(output): + """Create a new private key and write it to a file. + + Using elliptic curve keys since they are 2x smaller than RSA ones of + the same security (the NIST P-256 curve we use roughly corresponds + to RSA with 3072 bits). + + :param output: Output file name. + :return: a private key object. + """ + private_key = ec.generate_private_key(ec.SECP256R1(), + backends.default_backend()) + pkey_bytes = private_key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.PKCS8, + encryption_algorithm=serialization.NoEncryption() + ) + with open(output, 'wb') as fp: + fp.write(pkey_bytes) + + return private_key + + +def _generate_tls_certificate(output, private_key_output, + common_name, ip_address, + valid_for_days=30): + """Generate a self-signed TLS certificate. + + :param output: Output file name for the certificate. + :param private_key_output: Output file name for the private key. + :param common_name: Content for the common name field (e.g. host name). + :param ip_address: IP address the certificate will be valid for. + :param valid_for_days: Number of days the certificate will be valid for. + :return: the generated certificate as a string. + """ + if isinstance(ip_address, str): + ip_address = ipaddress.ip_address(ip_address) + + private_key = _create_private_key(private_key_output) + + subject = x509.Name([ + x509.NameAttribute(x509.NameOID.COMMON_NAME, common_name), + ]) + alt_name = x509.SubjectAlternativeName([x509.IPAddress(ip_address)]) + cert = (x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(subject) + .public_key(private_key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(datetime.datetime.utcnow()) + .not_valid_after(datetime.datetime.utcnow() + + datetime.timedelta(days=valid_for_days)) + .add_extension(alt_name, critical=True) + .sign(private_key, hashes.SHA256(), backends.default_backend())) + pub_bytes = cert.public_bytes(serialization.Encoding.PEM) + with open(output, "wb") as f: + f.write(pub_bytes) + return pub_bytes.decode('utf-8') + + +TlsCertificate = collections.namedtuple('TlsCertificate', + ['text', 'path', 'private_key_path']) + + +def generate_tls_certificate(ip_address, common_name=None, + valid_for_days=90): + """Generate a self-signed TLS certificate. + + :param ip_address: IP address the certificate will be valid for. + :param common_name: Content for the common name field (e.g. host name). + Defaults to the current host name. + :param valid_for_days: Number of days the certificate will be valid for. + :return: a TlsCertificate object. + """ + root = '/run/ironic-python-agent' + cert_path = os.path.join(root, 'agent.crt') + key_path = os.path.join(root, 'agent.key') + + os.makedirs(root, exist_ok=True) + common_name = netutils.get_hostname() + content = _generate_tls_certificate(cert_path, key_path, + common_name, ip_address) + return TlsCertificate(content, cert_path, key_path) diff --git a/lower-constraints.txt b/lower-constraints.txt index a97dc7b74..d5960cf40 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -4,8 +4,10 @@ Babel==2.5.3 bandit==1.1.0 bashate==0.5.1 certifi==2018.1.18 +cffi==1.14.0 chardet==3.0.4 coverage==4.0 +cryptography==2.3 debtcollector==1.19.0 doc8==0.6.0 docutils==0.14 @@ -53,6 +55,7 @@ pep8==1.5.7 Pint==0.5 psutil==3.2.2 pycodestyle==2.3.1 +pycparser==2.18 pyflakes==0.8.1 Pygments==2.2.0 pyinotify==0.9.6 diff --git a/releasenotes/notes/auto-tls-b52b873663f35618.yaml b/releasenotes/notes/auto-tls-b52b873663f35618.yaml new file mode 100644 index 000000000..8ef344dbb --- /dev/null +++ b/releasenotes/notes/auto-tls-b52b873663f35618.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + When a recent enough version of ironic is detected and ``listen_tls`` is + ``False``, agent will now generate a self-signed TLS certificate and send + it to ironic on heartbeat. This ensures encrypted communication from + ironic to the agent. Set ``enable_auto_tls`` to ``False`` to disable this + behavior. diff --git a/requirements.txt b/requirements.txt index a21908da9..1f66f3424 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,3 +18,4 @@ rtslib-fb>=2.1.65 # Apache-2.0 stevedore>=1.20.0 # Apache-2.0 ironic-lib>=4.1.0 # Apache-2.0 Werkzeug>=1.0.1 # BSD License +cryptography>=2.3 # BSD/Apache-2.0