From 0cdc53948f98c39ef635d7bf79b8efcd92f2c73e Mon Sep 17 00:00:00 2001 From: Pavlo Shchelokovskyy Date: Tue, 19 Sep 2017 09:33:15 +0000 Subject: [PATCH] [ansible] configure SSL validation This patch adds a number of configration options and playbook variables that govern SSL validation and authentication when accessing the image store to download the image and configdrive from the deploy ramdisk. Most of them are not yet used in the default set of playbooks provided with the driver (for example some of them require yet unreleased Ansible 2.4), however they can be used by custom playbooks, for example to upload and enable those custom CA bundle and cert files to the ramdisk at deploy time. This patch also sets DevStack to disable validation of image store SSL certificate by ansible deploy interface (similar to how IPA is currently confugured). This patch also caps Anisble < 2.4, since our custom callback plugin has troubles with the newest Ansible 2.4 version (to be fixed later). Change-Id: Id40f1067361cc32e98973c70fc5cd3d4242308d0 Closes-Bug: #1717858 --- devstack/plugin.sh | 22 ++++++++++ doc/source/drivers/ansible.rst | 40 +++++++++++++++++++ ironic_staging_drivers/ansible/deploy.py | 38 ++++++++++++++++++ .../ansible/playbooks/library/stream_url.py | 26 +++++++++--- .../roles/deploy/tasks/configdrive.yaml | 1 + .../roles/deploy/tasks/download.yaml | 1 + .../playbooks/roles/deploy/tasks/write.yaml | 1 + .../ansible/python-requirements.txt | 2 +- .../tests/unit/ansible/test_deploy.py | 6 +++ 9 files changed, 130 insertions(+), 7 deletions(-) diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 83870b4..78282e4 100755 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -108,6 +108,25 @@ function install_drivers_dependencies { done } +function configure_ironic_testing_driver { + if [[ "$IRONIC_STAGING_DRIVER" =~ "ansible" && \ + "$IRONIC_STAGING_DRIVER" =~ "ipmi" ]]; then + echo_summary "Configuring ansible deploy driver interface" + configure_ansible_deploy_driver + else + die $LINENO "Failed to configure ${IRONIC_STAGING_DRIVER} driver/hw type: not supported by devstack plugin or other pre-conditions not met" + fi +} + +function configure_ansible_deploy_driver { + # NOTE(pas-ha) DevStack now defaults to tls-proxy being enabled. + # Using custom CA bundle is not that easy with TinyCore, + # requiring extra rebuild steps and resulting in bigger image, + # so just disable validating SSL certs for now in DevStack + # similar to what ironic does for IPA by default in DevStack + iniset $IRONIC_CONF_FILE ansible image_store_insecure True +} + function set_ironic_testing_driver { if [[ "$IRONIC_STAGING_DRIVER" == "pxe_ipmitool_ansible" && \ "$IRONIC_DEPLOY_DRIVER" == "agent_ipmitool" && \ @@ -168,6 +187,9 @@ if is_service_enabled ir-api ir-cond; then elif [[ "$1" == "stack" && "$2" == "post-config" ]]; then echo_summary "Configuring Ironic-staging-drivers" update_ironic_enabled_drivers + if [[ -n ${IRONIC_STAGING_DRIVER} ]]; then + configure_ironic_testing_driver + fi elif [[ "$1" == "stack" && "$2" == "extra" ]]; then if [ -n $IRONIC_STAGING_DRIVER ]; then set_ironic_testing_driver diff --git a/doc/source/drivers/ansible.rst b/doc/source/drivers/ansible.rst index f7700c5..4c09509 100644 --- a/doc/source/drivers/ansible.rst +++ b/doc/source/drivers/ansible.rst @@ -250,6 +250,25 @@ post_deploy_get_power_state_retry_interval after triggering soft poweroff. Default is 5. +image_store_insecure + Boolean to disable validation of server SSL certificate of + the image store when downloading image and configdrive. + Default is False. + +image_store_cafile + Path to custom PEM CA bundle to use for validation of server SSL + certificate of the image store when downloading image of configdrive. + Is not currently used by default playbooks included with the driver. + +image_store_certfile + Path to client certificate file to use for client SSL authentication + to the image store when downloading image of configdrive. + Is not currently used by default playbooks included with the driver. + +image_store_keyfile + Path to private key file to use for client SSL authentication + to the image store when downloading image of configdrive. + Is not currently used by default playbooks included with the driver. Driver properties for the Node ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -441,6 +460,27 @@ Some more explanations: option ``[ansible]extra_memory``. - if ``checksum`` initially does not start with ``hash-algo:``, hashing algorithm is assumed to be ``md5`` (default in Glance). + - ``validate_certs`` - boolean (``yes/no``) flag that turns validating + image store SSL certificate on or off (default is 'yes'). + Governed by ``[ansible]image_store_insecure`` option + in ironic configuration file. + - ``cafile`` - custom CA bundle to use for validating image store + SSL certificate. + Takes value of ``[ansible]image_store_cafile`` if that is defined. + Currently is not used by default playbooks, as Ansible has no way to + specify the custom CA bundle to use for single HTTPS actions, + however you can use this value in your custom playbooks to for example + upload and register this CA in the ramdisk at deploy time. + - ``client_cert`` - cert file for client-side SSL authentication. + Takes value of ``[ansible]image_store_certfile`` option if defined. + Currently is not used by default playbooks as it is generally available + since Ansible 2.4 only, + however you can use this value in your custom playbooks. + - ``client_key`` - private key file for client-side SSL authentication. + Takes value of ``[ansible]image_store_keyfile`` option if defined. + Currently is not used by default playbooks as it is generally available + since Ansible 2.4 only, + however you can use this value in your custom playbooks. ``ironic.partiton_info.partitions`` Optional. List of dictionaries defining partitions to create on the node diff --git a/ironic_staging_drivers/ansible/deploy.py b/ironic_staging_drivers/ansible/deploy.py index 968facc..23009b7 100644 --- a/ironic_staging_drivers/ansible/deploy.py +++ b/ironic_staging_drivers/ansible/deploy.py @@ -95,6 +95,33 @@ ansible_opts = [ 'cleaning. Disable it when using custom ramdisk ' 'without callback script. ' 'When callback is disabled, Neutron is mandatory.')), + cfg.BoolOpt('image_store_insecure', + default=False, + help=_('Skip verifying SSL connections to the image store ' + 'when downloading the image. ' + 'Setting it to "True" is only recommended for testing ' + 'environments that use self-signed certificates.')), + cfg.StrOpt('image_store_cafile', + help=_('Specific CA bundle to use for validating ' + 'SSL connections to the image store. ' + 'If not specified, CA available in the ramdisk ' + 'will be used. ' + 'Is not used by default playbooks included with ' + 'the driver. ' + 'Suitable for environments that use self-signed ' + 'certificates.')), + cfg.StrOpt('image_store_certfile', + help=_('Client cert to use for SSL connections ' + 'to image store. ' + 'Is not used by default playbooks included with ' + 'the driver. ' + 'Can be used in custom playbooks and Ansible>=2.4.')), + cfg.StrOpt('image_store_keyfile', + help=_('Client key to use for SSL connections ' + 'to image store. ' + 'Is not used by default playbooks included with ' + 'the driver. ' + 'Can be used in custom playbooks and Ansible>=2.4.')), ] CONF.register_opts(ansible_opts, group='ansible') @@ -331,6 +358,16 @@ def _parse_root_device_hints(node): return root_device_hints +def _add_ssl_image_options(image): + image['validate_certs'] = ('no' if CONF.ansible.image_store_insecure + else 'yes') + if CONF.ansible.image_store_cafile: + image['cafile'] = CONF.ansible.image_store_cafile + if CONF.ansible.image_store_certfile and CONF.ansible.image_store_keyfile: + image['client_cert'] = CONF.ansible.image_store_certfile + image['client_key'] = CONF.ansible.image_store_keyfile + + def _prepare_variables(task): node = task.node i_info = node.instance_info @@ -349,6 +386,7 @@ def _prepare_variables(task): # where API reports checksum as MD5 always. if ':' not in checksum: image['checksum'] = 'md5:%s' % checksum + _add_ssl_image_options(image) variables = {'image': image} configdrive = i_info.get('configdrive') if configdrive: diff --git a/ironic_staging_drivers/ansible/playbooks/library/stream_url.py b/ironic_staging_drivers/ansible/playbooks/library/stream_url.py index 10a1a68..dd750d6 100644 --- a/ironic_staging_drivers/ansible/playbooks/library/stream_url.py +++ b/ironic_staging_drivers/ansible/playbooks/library/stream_url.py @@ -24,13 +24,14 @@ DEFAULT_CHUNK_SIZE = 1024 * 1024 # 1MB class StreamingDownloader(object): - def __init__(self, url, chunksize, hash_algo=None): + def __init__(self, url, chunksize, hash_algo=None, verify=True, + certs=None): if hash_algo is not None: self.hasher = hashlib.new(hash_algo) else: self.hasher = None self.chunksize = chunksize - resp = requests.get(url, stream=True) + resp = requests.get(url, stream=True, verify=verify, certs=certs) if resp.status_code != 200: raise Exception('Invalid response code: %s' % resp.status_code) @@ -47,8 +48,9 @@ class StreamingDownloader(object): return self.hasher.hexdigest() -def stream_to_dest(url, dest, chunksize, hash_algo): - downloader = StreamingDownloader(url, chunksize, hash_algo) +def stream_to_dest(url, dest, chunksize, hash_algo, verify=True, certs=None): + downloader = StreamingDownloader(url, chunksize, hash_algo, + verify=verify, certs=certs) with open(dest, 'wb+') as f: for chunk in downloader: @@ -64,13 +66,25 @@ def main(): dest=dict(required=True, type='str'), checksum=dict(required=False, type='str', default=''), chunksize=dict(required=False, type='int', - default=DEFAULT_CHUNK_SIZE) + default=DEFAULT_CHUNK_SIZE), + validate_certs=dict(required=False, type='bool', default=True), + client_cert=dict(required=False, type='str', default=''), + client_key=dict(required=False, type='str', default='') + )) url = module.params['url'] dest = module.params['dest'] checksum = module.params['checksum'] chunksize = module.params['chunksize'] + validate = module.params['validate_certs'] + client_cert = module.params['client_cert'] + client_key = module.params['client_key'] + if client_cert: + certs = (client_cert, client_key) if client_key else client_cert + else: + certs = None + if checksum == '': hash_algo, checksum = None, None else: @@ -88,7 +102,7 @@ def main(): try: actual_checksum = stream_to_dest( - url, dest, chunksize, hash_algo) + url, dest, chunksize, hash_algo, verify=validate, certs=certs) except Exception as e: module.fail_json(msg=str(e)) else: diff --git a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/configdrive.yaml b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/configdrive.yaml index 13d4fc5..05d469b 100644 --- a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/configdrive.yaml +++ b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/configdrive.yaml @@ -2,6 +2,7 @@ get_url: url: "{{ ironic.configdrive.location }}" dest: /tmp/{{ inventory_hostname }}.gz.base64 + validate_certs: "{{ ironic.image.validate_certs|default(omit) }}" async: 600 poll: 15 when: "{{ ironic.configdrive.type|default('') == 'url' }}" diff --git a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/download.yaml b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/download.yaml index 00d7c9f..87f2501 100644 --- a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/download.yaml +++ b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/download.yaml @@ -8,5 +8,6 @@ url: "{{ ironic.image.url }}" dest: /tmp/{{ inventory_hostname }}.img checksum: "{{ ironic.image.checksum|default(omit) }}" + validate_certs: "{{ ironic.image.validate_certs|default(omit) }}" async: 600 poll: 15 diff --git a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/write.yaml b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/write.yaml index f470fce..9fa0496 100644 --- a/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/write.yaml +++ b/ironic_staging_drivers/ansible/playbooks/roles/deploy/tasks/write.yaml @@ -11,6 +11,7 @@ url: "{{ ironic.image.url }}" dest: "{{ ironic_image_target }}" checksum: "{{ ironic.image.checksum|default(omit) }}" + validate_certs: "{{ ironic.image.validate_certs|default(omit) }}" async: 600 poll: 15 when: "{{ ironic.image.disk_format == 'raw' }}" diff --git a/ironic_staging_drivers/ansible/python-requirements.txt b/ironic_staging_drivers/ansible/python-requirements.txt index 8cc8360..c08887e 100644 --- a/ironic_staging_drivers/ansible/python-requirements.txt +++ b/ironic_staging_drivers/ansible/python-requirements.txt @@ -1 +1 @@ -ansible>=2.1,!=2.2.1.0,!=2.1.4.0 +ansible>=2.1,!=2.2.1.0,!=2.1.4.0,<2.4 diff --git a/ironic_staging_drivers/tests/unit/ansible/test_deploy.py b/ironic_staging_drivers/tests/unit/ansible/test_deploy.py index d73d123..a631530 100644 --- a/ironic_staging_drivers/tests/unit/ansible/test_deploy.py +++ b/ironic_staging_drivers/tests/unit/ansible/test_deploy.py @@ -331,6 +331,7 @@ class TestAnsibleMethods(db_base.DbTestCase): return_value=2000) def test__prepare_variables(self, mem_req_mock): expected = {"image": {"url": "http://image", + "validate_certs": "yes", "source": "fake-image", "mem_req": 2000, "disk_format": "qcow2", @@ -347,6 +348,7 @@ class TestAnsibleMethods(db_base.DbTestCase): self.node.properties = props self.node.save() expected = {"image": {"url": "http://image", + "validate_certs": "yes", "source": "fake-image", "mem_req": 2000, "disk_format": "qcow2", @@ -359,11 +361,13 @@ class TestAnsibleMethods(db_base.DbTestCase): @mock.patch.object(ansible_deploy, '_calculate_memory_req', autospec=True, return_value=2000) def test__prepare_variables_noglance(self, mem_req_mock): + self.config(image_store_insecure=True, group='ansible') i_info = self.node.instance_info i_info['image_checksum'] = 'sha256:checksum' self.node.instance_info = i_info self.node.save() expected = {"image": {"url": "http://image", + "validate_certs": "no", "source": "fake-image", "mem_req": 2000, "disk_format": "qcow2", @@ -380,6 +384,7 @@ class TestAnsibleMethods(db_base.DbTestCase): self.node.instance_info = i_info self.node.save() expected = {"image": {"url": "http://image", + "validate_certs": "yes", "source": "fake-image", "mem_req": 2000, "disk_format": "qcow2", @@ -399,6 +404,7 @@ class TestAnsibleMethods(db_base.DbTestCase): self.node.save() self.config(tempdir='/path/to/tmpfiles') expected = {"image": {"url": "http://image", + "validate_certs": "yes", "source": "fake-image", "mem_req": 2000, "disk_format": "qcow2",