From 1154292d46645ea380baf8ba93b938495eac9933 Mon Sep 17 00:00:00 2001 From: vmud213 Date: Fri, 24 Jul 2020 08:41:35 +0000 Subject: [PATCH] Allow HttpImageService to accept custom certificate While validating and downloading image references, allow HttpImageService to use config parameters to enable/disable TLS verification and to use custom certificates on the secured connections. Change-Id: I5f308271004a24203ecbbc1718ba9070ed65b960 Story: #2007939 Task: #40404 --- ironic/common/image_service.py | 38 ++- ironic/conf/default.py | 21 ++ .../tests/unit/common/test_image_service.py | 232 ++++++++++++++++-- ...rtificate_validation-8ba00759ed79e429.yaml | 5 + 4 files changed, 271 insertions(+), 25 deletions(-) create mode 100644 releasenotes/notes/allow_custom_certificate_validation-8ba00759ed79e429.yaml diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py index e67aca83c2..aec53c8aef 100644 --- a/ironic/common/image_service.py +++ b/ironic/common/image_service.py @@ -23,6 +23,7 @@ import shutil from urllib import parse as urlparse from oslo_log import log +from oslo_utils import strutils from oslo_utils import uuidutils import requests import sendfile @@ -31,6 +32,7 @@ from ironic.common import exception from ironic.common.glance_service.image_service import GlanceImageService from ironic.common.i18n import _ from ironic.common import utils +from ironic.conf import CONF IMAGE_CHUNK_SIZE = 1024 * 1024 # 1mb # NOTE(kaifeng) Image will be truncated to 2GiB by sendfile, @@ -88,14 +90,23 @@ class HttpImageService(BaseImageService): :returns: Response to HEAD request. """ output_url = 'secreturl' if secret else image_href + try: - response = requests.head(image_href) + verify = strutils.bool_from_string(CONF.webserver_verify_ca, + strict=True) + except ValueError: + verify = CONF.webserver_verify_ca + + try: + response = requests.head(image_href, verify=verify) if response.status_code != http_client.OK: raise exception.ImageRefValidationFailed( image_href=output_url, - reason=_("Got HTTP code %s instead of 200 in response to " - "HEAD request.") % response.status_code) - except requests.RequestException as e: + reason=_("Got HTTP code %s instead of 200 in response " + "to HEAD request.") % response.status_code) + + except (OSError, requests.ConnectionError, + requests.RequestException) as e: raise exception.ImageRefValidationFailed(image_href=output_url, reason=str(e)) return response @@ -111,16 +122,27 @@ class HttpImageService(BaseImageService): * IOError happened during file write; * GET request failed. """ + try: - response = requests.get(image_href, stream=True) + verify = strutils.bool_from_string(CONF.webserver_verify_ca, + strict=True) + except ValueError: + verify = CONF.webserver_verify_ca + + try: + response = requests.get(image_href, stream=True, + verify=verify) if response.status_code != http_client.OK: raise exception.ImageRefValidationFailed( image_href=image_href, - reason=_("Got HTTP code %s instead of 200 in response to " - "GET request.") % response.status_code) + reason=_("Got HTTP code %s instead of 200 in response " + "to GET request.") % response.status_code) + with response.raw as input_img: shutil.copyfileobj(input_img, image_file, IMAGE_CHUNK_SIZE) - except (requests.RequestException, IOError) as e: + + except (OSError, requests.ConnectionError, requests.RequestException, + IOError) as e: raise exception.ImageDownloadFailed(image_href=image_href, reason=str(e)) diff --git a/ironic/conf/default.py b/ironic/conf/default.py index b799208f03..870dea5530 100644 --- a/ironic/conf/default.py +++ b/ironic/conf/default.py @@ -372,6 +372,26 @@ utils_opts = [ 'dir.')), ] +cert_verify_opts = [ + cfg.StrOpt('webserver_verify_ca', + default='True', + mutable=True, + help=_('CA certificates to be used for certificate ' + 'verification. This can be either a Boolean value ' + 'or a path to a CA_BUNDLE file.' + 'If set to True, the certificates present in the ' + 'standard path are used to verify the host ' + 'certificates.' + 'If set to False, the conductor will ignore verifying ' + 'the SSL certificate presented by the host.' + 'If it"s a path, conductor uses the specified ' + 'certificate for SSL verification. If the path does ' + 'not exist, the behavior is same as when this value ' + 'is set to True i.e the certificates present in the ' + 'standard path are used for SSL verification.' + 'Defaults to True.')), +] + def register_opts(conf): conf.register_opts(api_opts) @@ -386,3 +406,4 @@ def register_opts(conf): conf.register_opts(portgroup_opts) conf.register_opts(service_opts) conf.register_opts(utils_opts) + conf.register_opts(cert_verify_opts) diff --git a/ironic/tests/unit/common/test_image_service.py b/ironic/tests/unit/common/test_image_service.py index d59c1a4bc8..aff875a08f 100644 --- a/ironic/tests/unit/common/test_image_service.py +++ b/ironic/tests/unit/common/test_image_service.py @@ -18,6 +18,7 @@ import os import shutil from unittest import mock +from oslo_config import cfg from oslo_utils import uuidutils import requests import sendfile @@ -32,14 +33,17 @@ class HttpImageServiceTestCase(base.TestCase): def setUp(self): super(HttpImageServiceTestCase, self).setUp() self.service = image_service.HttpImageService() - self.href = 'http://127.0.0.1:12345/fedora.qcow2' + self.href = 'https://127.0.0.1:12345/fedora.qcow2' + @mock.patch.object(os.path, 'exists', autospec=True) @mock.patch.object(requests, 'head', autospec=True) - def test_validate_href(self, head_mock): + def test_validate_href_http_scheme(self, head_mock, path_mock): + self.href = 'http://127.0.0.1:12345/fedora.qcow2' response = head_mock.return_value response.status_code = http_client.OK self.service.validate_href(self.href) - head_mock.assert_called_once_with(self.href) + path_mock.assert_not_called() + head_mock.assert_called_once_with(self.href, verify=True) response.status_code = http_client.NO_CONTENT self.assertRaises(exception.ImageRefValidationFailed, self.service.validate_href, @@ -50,21 +54,110 @@ class HttpImageServiceTestCase(base.TestCase): self.href) @mock.patch.object(requests, 'head', autospec=True) - def test_validate_href_error_code(self, head_mock): - head_mock.return_value.status_code = http_client.BAD_REQUEST + def test_validate_href_verify_false(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'False') + + response = head_mock.return_value + response.status_code = http_client.OK + self.service.validate_href(self.href) + head_mock.assert_called_once_with(self.href, verify=False) + response.status_code = http_client.NO_CONTENT self.assertRaises(exception.ImageRefValidationFailed, - self.service.validate_href, self.href) - head_mock.assert_called_once_with(self.href) + self.service.validate_href, + self.href) + response.status_code = http_client.BAD_REQUEST + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, + self.href) @mock.patch.object(requests, 'head', autospec=True) - def test_validate_href_error(self, head_mock): + def test_validate_href_verify_false_error(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'False') head_mock.side_effect = requests.ConnectionError() self.assertRaises(exception.ImageRefValidationFailed, self.service.validate_href, self.href) - head_mock.assert_called_once_with(self.href) + head_mock.assert_called_once_with(self.href, verify=False) + head_mock.side_effect = requests.RequestException() + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_true(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + response = head_mock.return_value + response.status_code = http_client.OK + self.service.validate_href(self.href) + head_mock.assert_called_once_with(self.href, verify=True) + response.status_code = http_client.NO_CONTENT + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, + self.href) + response.status_code = http_client.BAD_REQUEST + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, + self.href) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_true_error(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + + head_mock.side_effect = requests.ConnectionError() + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + head_mock.assert_called_once_with(self.href, verify=True) + head_mock.side_effect = requests.RequestException() + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_valid_path(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + + response = head_mock.return_value + response.status_code = http_client.OK + + self.service.validate_href(self.href) + head_mock.assert_called_once_with(self.href, verify='/some/path') + response.status_code = http_client.NO_CONTENT + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, + self.href) + response.status_code = http_client.BAD_REQUEST + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, + self.href) + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_connect_error(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + response = mock.Mock() + response.status_code = http_client.OK + head_mock.side_effect = requests.ConnectionError() + + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + head_mock.assert_called_once_with(self.href, verify='/some/path') + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_error(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + head_mock.side_effect = requests.RequestException() + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + head_mock.assert_called_once_with(self.href, verify='/some/path') + + @mock.patch.object(requests, 'head', autospec=True) + def test_validate_href_verify_os_error(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + head_mock.side_effect = OSError() + self.assertRaises(exception.ImageRefValidationFailed, + self.service.validate_href, self.href) + head_mock.assert_called_once_with(self.href, verify='/some/path') @mock.patch.object(requests, 'head', autospec=True) def test_validate_href_error_with_secret_parameter(self, head_mock): + cfg.CONF.set_override('webserver_verify_ca', 'False') head_mock.return_value.status_code = 204 e = self.assertRaises(exception.ImageRefValidationFailed, self.service.validate_href, @@ -72,7 +165,7 @@ class HttpImageServiceTestCase(base.TestCase): True) self.assertIn('secreturl', str(e)) self.assertNotIn(self.href, str(e)) - head_mock.assert_called_once_with(self.href) + head_mock.assert_called_once_with(self.href, verify=False) @mock.patch.object(requests, 'head', autospec=True) def _test_show(self, head_mock, mtime, mtime_date): @@ -82,7 +175,7 @@ class HttpImageServiceTestCase(base.TestCase): 'Last-Modified': mtime } result = self.service.show(self.href) - head_mock.assert_called_once_with(self.href) + head_mock.assert_called_once_with(self.href, verify=True) self.assertEqual({'size': 100, 'updated_at': mtime_date, 'properties': {}}, result) @@ -104,11 +197,12 @@ class HttpImageServiceTestCase(base.TestCase): head_mock.return_value.headers = {} self.assertRaises(exception.ImageRefValidationFailed, self.service.show, self.href) - head_mock.assert_called_with(self.href) + head_mock.assert_called_with(self.href, verify=True) @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_success(self, req_get_mock, shutil_mock): + def test_download_success_http_scheme(self, req_get_mock, shutil_mock): + self.href = 'http://127.0.0.1:12345/fedora.qcow2' response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK response_mock.raw = mock.MagicMock(spec=io.BytesIO) @@ -118,10 +212,65 @@ class HttpImageServiceTestCase(base.TestCase): response_mock.raw.__enter__(), file_mock, image_service.IMAGE_CHUNK_SIZE ) - req_get_mock.assert_called_once_with(self.href, stream=True) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify=True) + @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_connerror(self, req_get_mock): + def test_download_success_verify_false( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', 'False') + response_mock = req_get_mock.return_value + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + file_mock = mock.Mock(spec=io.BytesIO) + self.service.download(self.href, file_mock) + shutil_mock.assert_called_once_with( + response_mock.raw.__enter__(), file_mock, + image_service.IMAGE_CHUNK_SIZE + ) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify=False) + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_success_verify_true( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', 'True') + response_mock = req_get_mock.return_value + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + file_mock = mock.Mock(spec=io.BytesIO) + self.service.download(self.href, file_mock) + shutil_mock.assert_called_once_with( + response_mock.raw.__enter__(), file_mock, + image_service.IMAGE_CHUNK_SIZE + ) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify=True) + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_success_verify_path( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + response_mock = req_get_mock.return_value + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + file_mock = mock.Mock(spec=io.BytesIO) + self.service.download(self.href, file_mock) + shutil_mock.assert_called_once_with( + response_mock.raw.__enter__(), file_mock, + image_service.IMAGE_CHUNK_SIZE + ) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify='/some/path') + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_fail_verify_false_connerror( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', False) req_get_mock.side_effect = requests.ConnectionError() file_mock = mock.Mock(spec=io.BytesIO) self.assertRaises(exception.ImageDownloadFailed, @@ -129,7 +278,9 @@ class HttpImageServiceTestCase(base.TestCase): @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) - def test_download_fail_ioerror(self, req_get_mock, shutil_mock): + def test_download_fail_verify_false_ioerror( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', False) response_mock = req_get_mock.return_value response_mock.status_code = http_client.OK response_mock.raw = mock.MagicMock(spec=io.BytesIO) @@ -137,7 +288,54 @@ class HttpImageServiceTestCase(base.TestCase): shutil_mock.side_effect = IOError self.assertRaises(exception.ImageDownloadFailed, self.service.download, self.href, file_mock) - req_get_mock.assert_called_once_with(self.href, stream=True) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify=False) + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_success_verify_true_connerror( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + response_mock = mock.Mock() + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + req_get_mock.side_effect = requests.ConnectionError + + file_mock = mock.Mock(spec=io.BytesIO) + self.assertRaises(exception.ImageDownloadFailed, + self.service.download, self.href, file_mock) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify='/some/path') + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_fail_verify_true_ioerror( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + response_mock = req_get_mock.return_value + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + file_mock = mock.Mock(spec=io.BytesIO) + shutil_mock.side_effect = IOError + self.assertRaises(exception.ImageDownloadFailed, + self.service.download, self.href, file_mock) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify='/some/path') + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) + @mock.patch.object(requests, 'get', autospec=True) + def test_download_fail_verify_true_oserror( + self, req_get_mock, shutil_mock): + cfg.CONF.set_override('webserver_verify_ca', '/some/path') + response_mock = req_get_mock.return_value + response_mock.status_code = http_client.OK + response_mock.raw = mock.MagicMock(spec=io.BytesIO) + file_mock = mock.Mock(spec=io.BytesIO) + shutil_mock.side_effect = OSError() + self.assertRaises(exception.ImageDownloadFailed, + self.service.download, self.href, file_mock) + req_get_mock.assert_called_once_with(self.href, stream=True, + verify='/some/path') class FileImageServiceTestCase(base.TestCase): diff --git a/releasenotes/notes/allow_custom_certificate_validation-8ba00759ed79e429.yaml b/releasenotes/notes/allow_custom_certificate_validation-8ba00759ed79e429.yaml new file mode 100644 index 0000000000..bf02ab98f5 --- /dev/null +++ b/releasenotes/notes/allow_custom_certificate_validation-8ba00759ed79e429.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds a configuration option ``webserver_verify_ca`` to support + custom certificates to validate URLs hosted on a HTTPS webserver.