From fdd11b54a5e3d7a9ee89628baba2990e4e00abdd Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Thu, 17 Nov 2016 13:26:28 +0200 Subject: [PATCH] Configure and use SSL-related requests options This patch adds standard SSL options to IPA config and makes use of them when making HTTP requests. For now, a single set of certificates is used when needed. In the future configuration can be expanded to allow per-service certificates. Besides, the 'insecure' option (defaults to False) can be overridden through kernel command line parameter 'ipa-insecure'. This will allow running IPA in CI-like environments with self-signed SSL certificates. Change-Id: I259d9b3caa9ba1dc3d7382f375b8e086a5348d80 Closes-Bug: #1642515 --- doc/source/index.rst | 68 +++++++++++++++++++ .../ironic_python_agent.conf.sample | 28 +++++++- ironic_python_agent/agent.py | 3 + ironic_python_agent/config.py | 18 +++++ ironic_python_agent/extensions/standby.py | 6 +- ironic_python_agent/inspector.py | 4 +- ironic_python_agent/ironic_api_client.py | 6 ++ .../tests/unit/extensions/test_standby.py | 5 ++ .../tests/unit/test_inspector.py | 3 + ironic_python_agent/tests/unit/test_utils.py | 30 ++++++++ ironic_python_agent/utils.py | 17 +++++ .../notes/handle-ssl-063a91fb7bdcf9b9.yaml | 15 ++++ 12 files changed, 198 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/handle-ssl-063a91fb7bdcf9b9.yaml diff --git a/doc/source/index.rst b/doc/source/index.rst index d322deda1..edce7ad15 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -226,6 +226,74 @@ ironic-python-agent.service unit in cloud-config.yaml [5]_. * ``--debug``: Enables debug logging. +IPA and SSL +=========== + +During its operation IPA makes HTTP requests to a number of other services, +currently including + +- ironic for lookup/heartbeats +- ironic-inspector to publish results of introspection +- HTTP image storage to fetch the user image to be written to the node's disk + (Object storage service or other service storing user images + when ironic is running in a standalone mode) + +When these services are configured to require SSL-encrypted connections, +IPA can be configured to either properly use such secure connections or +ignore verifying such SSL connections. + +Configuration mostly happens in the IPA config file +(default is ``/etc/ironic_python_agent/ironic_python_agent.conf``) +or command line arguments passed to ``ironic-python-agent``, +and it is possible to provide some options via kernel command line arguments +instead. + +Available options in the ``[DEFAULT]`` config file section are: + +insecure + Whether to verify server SSL certificates. + When not specified explicitly, defaults to the value of ``ipa-insecure`` + kernel command line argument (converted to boolean). + The default for this kernel command line argument is taken to be ``False``. + Overriding it to ``True`` by adding ``ipa-insecure=1`` to the value of + ``[pxe]pxe_append_params`` in ironic configuration file will allow running + the same IPA-based deploy ramdisk in a CI-like environment when services + are using secure HTTPS endpoints with self-signed certificates without + adding a custom CA file to the deploy ramdisk (see below). + +cafile + Path to the PEM encoded Certificate Authority file. + When not specified, available system-wide list of CAs will be used to + verify server certificates. + Thus in order to use IPA with HTTPS endpoints of other services in + a secure fashion (with ``insecure`` option being ``False``, see above), + operators should either ensure that certificates of those services + are verifiable by root CAs present in the deploy ramdisk, + or add a custom CA file to the ramdisk and set this IPA option to point + to this file at ramdisk build time. + +certfile + Path to PEM encoded client certificate cert file. + This option must be used when services are configured to require client + certificates on SSL-secured connections. + This cert file must be added to the deploy ramdisk and path + to it specified for IPA via this option at ramdisk build time. + This option has an effect only when the ``keyfile`` option is also set. + +keyfile + Path to PEM encoded client certificate key file. + This option must be used when services are configured to require client + certificates on SSL-secured connections. + This key file must be added to the deploy ramdisk and path + to it specified for IPA via this option at ramdisk build time. + This option has an effect only when the ``certfile`` option is also set. + +Currently a single set of cafile/certfile/keyfile options is used for all +HTTP requests to the other services. + +Securing IPA's HTTP server itself with SSL is not yet supported in default +ramdisk builds. + Hardware Managers ================= diff --git a/etc/ironic_python_agent/ironic_python_agent.conf.sample b/etc/ironic_python_agent/ironic_python_agent.conf.sample index 037965ca4..0da7c78e8 100644 --- a/etc/ironic_python_agent/ironic_python_agent.conf.sample +++ b/etc/ironic_python_agent/ironic_python_agent.conf.sample @@ -5,9 +5,10 @@ # # URL of the Ironic API. Can be supplied as "ipa-api-url" -# kernel parameter. (string value) +# kernel parameter.The value must start with either http:// or +# https://. (string value) # Deprecated group/name - [DEFAULT]/api_url -#api_url = http://127.0.0.1:6385 +#api_url = # The IP address to listen on. Can be supplied as "ipa-listen- # host" kernel parameter. (string value) @@ -43,7 +44,7 @@ # Deprecated group/name - [DEFAULT]/ip_lookup_sleep #ip_lookup_sleep = 10 -# The interface to use when looking for an IPaddress. Can be +# The interface to use when looking for an IP address. Can be # supplied as "ipa-network-interface" kernel parameter. # (string value) # Deprecated group/name - [DEFAULT]/network_interface @@ -122,6 +123,27 @@ # (integer value) #disk_wait_delay = 3 +# Verify HTTPS connections. Can be supplied as "ipa-insecure" +# kernel parameter. (boolean value) +#insecure = false + +# Path to PEM encoded Certificate Authority file to use when +# verifying HTTPS connections. Default is to use available +# system-wide configured CAs. (string value) +#cafile = + +# Path to PEM encoded client certificate cert file. Must be +# provided together with "keyfile" option. Default is to not +# present any client certificates to the server. (string +# value) +#certfile = + +# Path to PEM encoded client certificate key file. Must be +# provided together with "certfile" option. Default is to not +# present any client certificates to the server. (string +# value) +#keyfile = + # # From oslo.log # diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 281c48032..8aa1f77b2 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -162,6 +162,9 @@ class IronicPythonAgent(base.ExecuteCommandMixin): lookup_timeout, lookup_interval, standalone, hardware_initialization_delay=0): super(IronicPythonAgent, self).__init__() + if bool(cfg.CONF.keyfile) != bool(cfg.CONF.certfile): + LOG.warning("Only one of 'keyfile' and 'certfile' options is " + "defined in config file. Its value will be ignored.") self.ext_mgr = extension.ExtensionManager( namespace='ironic_python_agent.extensions', invoke_on_load=True, diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 2fccb3a29..ad01d5823 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -180,6 +180,24 @@ cli_opts = [ 'in inventory. ' 'Can be supplied as "ipa-disk-wait-delay" ' 'kernel parameter.'), + cfg.BoolOpt('insecure', + default=APARAMS.get('ipa-insecure', False), + help='Verify HTTPS connections. Can be supplied as ' + '"ipa-insecure" kernel parameter.'), + cfg.StrOpt('cafile', + help='Path to PEM encoded Certificate Authority file ' + 'to use when verifying HTTPS connections. ' + 'Default is to use available system-wide configured CAs.'), + cfg.StrOpt('certfile', + help='Path to PEM encoded client certificate cert file. ' + 'Must be provided together with "keyfile" option. ' + 'Default is to not present any client certificates to ' + 'the server.'), + cfg.StrOpt('keyfile', + help='Path to PEM encoded client certificate key file. ' + 'Must be provided together with "certfile" option. ' + 'Default is to not present any client certificates to ' + 'the server.'), ] CONF.register_cli_opts(cli_opts) diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index 1ba396ea1..be9a19521 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -19,6 +19,7 @@ import six import time from oslo_concurrency import processutils +from oslo_config import cfg from oslo_log import log from ironic_lib import disk_utils @@ -27,6 +28,7 @@ from ironic_python_agent.extensions import base from ironic_python_agent import hardware from ironic_python_agent import utils +CONF = cfg.CONF LOG = log.getLogger(__name__) IMAGE_CHUNK_SIZE = 1024 * 1024 # 1MB @@ -227,7 +229,9 @@ class ImageDownload(object): if no_proxy: os.environ['no_proxy'] = no_proxy proxies = image_info.get('proxies', {}) - resp = requests.get(url, stream=True, proxies=proxies) + verify, cert = utils.get_ssl_client_options(CONF) + resp = requests.get(url, stream=True, proxies=proxies, + verify=verify, cert=cert) if resp.status_code != 200: msg = ('Received status code {} from {}, expected 200. Response ' 'body: {}').format(resp.status_code, url, resp.text) diff --git a/ironic_python_agent/inspector.py b/ironic_python_agent/inspector.py index 97d827744..823a23a49 100644 --- a/ironic_python_agent/inspector.py +++ b/ironic_python_agent/inspector.py @@ -118,7 +118,9 @@ def call_inspector(data, failures): encoder = encoding.RESTJSONEncoder() data = encoder.encode(data) - resp = requests.post(CONF.inspection_callback_url, data=data) + verify, cert = utils.get_ssl_client_options(CONF) + resp = requests.post(CONF.inspection_callback_url, data=data, + verify=verify, cert=cert) if resp.status_code >= 400: LOG.error('inspector error %d: %s, proceeding with lookup', resp.status_code, resp.content.decode('utf-8')) diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index 60c635cad..35d2deebf 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -13,6 +13,7 @@ # limitations under the License. +from oslo_config import cfg from oslo_log import log from oslo_serialization import jsonutils from oslo_service import loopingcall @@ -21,8 +22,10 @@ import requests from ironic_python_agent import encoding from ironic_python_agent import errors from ironic_python_agent import netutils +from ironic_python_agent import utils +CONF = cfg.CONF LOG = log.getLogger(__name__) @@ -57,10 +60,13 @@ class APIClient(object): 'Accept': 'application/json', }) + verify, cert = utils.get_ssl_client_options(CONF) return self.session.request(method, request_url, headers=headers, data=data, + verify=verify, + cert=cert, **kwargs) def heartbeat(self, uuid, advertise_address): diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index b76dd5961..a57a55329 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -299,6 +299,7 @@ class TestStandbyExtension(test_base.BaseTestCase): standby._download_image(image_info) requests_mock.assert_called_once_with(image_info['urls'][0], + cert=None, verify=True, stream=True, proxies={}) write = file_mock.write write.assert_any_call('some') @@ -329,6 +330,7 @@ class TestStandbyExtension(test_base.BaseTestCase): standby._download_image(image_info) self.assertEqual(no_proxy, os.environ['no_proxy']) requests_mock.assert_called_once_with(image_info['urls'][0], + cert=None, verify=True, stream=True, proxies=proxies) write = file_mock.write write.assert_any_call('some') @@ -767,6 +769,7 @@ class TestStandbyExtension(test_base.BaseTestCase): self.agent_extension._stream_raw_image_onto_device(image_info, '/dev/foo') requests_mock.assert_called_once_with(image_info['urls'][0], + cert=None, verify=True, stream=True, proxies={}) expected_calls = [mock.call('some'), mock.call('content')] file_mock.write.assert_has_calls(expected_calls) @@ -790,6 +793,7 @@ class TestStandbyExtension(test_base.BaseTestCase): self.agent_extension._stream_raw_image_onto_device, image_info, '/dev/foo') requests_mock.assert_called_once_with(image_info['urls'][0], + cert=None, verify=True, stream=True, proxies={}) # Assert write was only called once and failed! file_mock.write.assert_called_once_with('some') @@ -863,5 +867,6 @@ class TestImageDownload(test_base.BaseTestCase): self.assertEqual(content, list(image_download)) requests_mock.assert_called_once_with(image_info['urls'][0], + cert=None, verify=True, stream=True, proxies={}) self.assertEqual(image_info['checksum'], image_download.md5sum()) diff --git a/ironic_python_agent/tests/unit/test_inspector.py b/ironic_python_agent/tests/unit/test_inspector.py index 95b5b0599..2e47e8719 100644 --- a/ironic_python_agent/tests/unit/test_inspector.py +++ b/ironic_python_agent/tests/unit/test_inspector.py @@ -145,6 +145,7 @@ class TestCallInspector(test_base.BaseTestCase): res = inspector.call_inspector(data, failures) mock_post.assert_called_once_with('url', + cert=None, verify=True, data='{"data": 42, "error": null}') self.assertEqual(mock_post.return_value.json.return_value, res) @@ -157,6 +158,7 @@ class TestCallInspector(test_base.BaseTestCase): res = inspector.call_inspector(data, failures) mock_post.assert_called_once_with('url', + cert=None, verify=True, data='{"data": 42, "error": "boom"}') self.assertEqual(mock_post.return_value.json.return_value, res) @@ -168,6 +170,7 @@ class TestCallInspector(test_base.BaseTestCase): res = inspector.call_inspector(data, failures) mock_post.assert_called_once_with('url', + cert=None, verify=True, data='{"data": 42, "error": null}') self.assertIsNone(res) diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 487e6aeee..0f8e3b852 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -455,3 +455,33 @@ class TestUtils(testtools.TestCase): file_list=['/var/log'], io_dict={'iptables': mock.ANY, 'ip_addr': mock.ANY, 'ps': mock.ANY, 'dmesg': mock.ANY, 'df': mock.ANY}) + + def test_get_ssl_client_options(self): + # defaults + conf = mock.Mock(insecure=False, cafile=None, + keyfile=None, certfile=None) + self.assertEqual((True, None), utils.get_ssl_client_options(conf)) + + # insecure=True overrides cafile + conf = mock.Mock(insecure=True, cafile='spam', + keyfile=None, certfile=None) + self.assertEqual((False, None), utils.get_ssl_client_options(conf)) + + # cafile returned as verify when not insecure + conf = mock.Mock(insecure=False, cafile='spam', + keyfile=None, certfile=None) + self.assertEqual(('spam', None), utils.get_ssl_client_options(conf)) + + # only both certfile and keyfile produce non-None result + conf = mock.Mock(insecure=False, cafile=None, + keyfile=None, certfile='ham') + self.assertEqual((True, None), utils.get_ssl_client_options(conf)) + + conf = mock.Mock(insecure=False, cafile=None, + keyfile='ham', certfile=None) + self.assertEqual((True, None), utils.get_ssl_client_options(conf)) + + conf = mock.Mock(insecure=False, cafile=None, + keyfile='spam', certfile='ham') + self.assertEqual((True, ('ham', 'spam')), + utils.get_ssl_client_options(conf)) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 6045f6b49..ddfe25caf 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -416,3 +416,20 @@ def collect_system_logs(journald_max_lines=None): try_get_command_output(io_dict, name, cmd) return gzip_and_b64encode(io_dict=io_dict, file_list=file_list) + + +def get_ssl_client_options(conf): + """Format SSL-related requests options. + + :param conf: oslo_config CONF object + :returns: tuple of 'verify' and 'cert' values to pass to requests + """ + if conf.insecure: + verify = False + else: + verify = conf.cafile or True + if conf.certfile and conf.keyfile: + cert = (conf.certfile, conf.keyfile) + else: + cert = None + return verify, cert diff --git a/releasenotes/notes/handle-ssl-063a91fb7bdcf9b9.yaml b/releasenotes/notes/handle-ssl-063a91fb7bdcf9b9.yaml new file mode 100644 index 000000000..9115db150 --- /dev/null +++ b/releasenotes/notes/handle-ssl-063a91fb7bdcf9b9.yaml @@ -0,0 +1,15 @@ +--- +features: + - Ironic Python Agent now can access other services + (Ironic, Inspector, image backend) when those are configured with + HTTPS endpoints and use custom server certificates or require + client certificates. + It is possible to add a custom Certificate Authority (CA) file and client + certificate files to the deploy ramdisk during build, + and provide paths to those as corresponding new options + in ``ironic_python_agent.conf`` configuration file. + Validation of server certificate can be turned off with ``insecure`` + config option or via ``ipa-insecure`` kernel boot parameter. + This should make it possible to run IPA in CI-like environments that use + HTTPS endpoints with self-signed sertificates. +