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

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

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)

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

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.