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

9 files changed, 90 insertions, 2 deletions

diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py
index 281c4803..8aa1f77b 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 2fccb3a2..ad01d582 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 1ba396ea..be9a1952 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 97d82774..823a23a4 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 60c635ca..35d2deeb 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 b76dd596..a57a5532 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 95b5b059..2e47e871 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 487e6aee..0f8e3b85 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 6045f6b4..ddfe25ca 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