Add an option to require TLS for agent callback_url

Change-Id: Idf85dfd110de6181c6592644fd57e109ba87b971
Story: #2007214
Task: #40822

4 files changed, 45 insertions, 0 deletions

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 96754ab42..70baafc1c 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -3136,6 +3136,14 @@ class ConductorManager(base_manager.BaseConductorManager):
                 raise exception.InvalidParameterValue(
                     'Invalid or missing agent token received.')
 
+            if (CONF.agent.require_tls and callback_url
+                    and not callback_url.startswith('https://')):
+                LOG.error('Rejecting callback_url %(url)s for node '
+                          '%(node)s because it does not use TLS',
+                          {'url': callback_url, 'node': task.node.uuid})
+                raise exception.InvalidParameterValue(
+                    _('TLS is required by configuration'))
+
             task.spawn_after(
                 self._spawn_worker, task.driver.deploy.heartbeat,
                 task, callback_url, agent_version)
diff --git a/ironic/conf/agent.py b/ironic/conf/agent.py
index 85b0fc4e0..aed04ea53 100644
--- a/ironic/conf/agent.py
+++ b/ironic/conf/agent.py
@@ -144,6 +144,11 @@ opts = [
                default=10,
                help=_('Wait time in seconds between attempts for validating '
                       'Neutron agent status.')),
+    cfg.BoolOpt('require_tls',
+                default=False,
+                mutable=True,
+                help=_('If set to True, callback URLs without https:// will '
+                       'be rejected by the conductor.')),
 ]
 
 
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 1ed3ec855..bd390ac41 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -7431,6 +7431,33 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
         self.assertFalse(mock_heartbeat.called)
 
+    @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat',
+                autospec=True)
+    @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker',
+                autospec=True)
+    def test_heartbeat_tls_required(self, mock_spawn, mock_heartbeat):
+        """Heartbeat fails when it does not match."""
+        self.config(require_tls=True, group='agent')
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.DEPLOYING,
+            target_provision_state=states.ACTIVE,
+            driver_internal_info={'agent_secret_token': 'a secret'})
+
+        self._start_service()
+
+        mock_spawn.reset_mock()
+
+        mock_spawn.side_effect = self._fake_spawn
+
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.heartbeat, self.context,
+                                node.uuid, 'http://callback',
+                                agent_token='a secret')
+        self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
+        self.assertIn('TLS is required', str(exc.exc_info[1]))
+        self.assertFalse(mock_heartbeat.called)
+
 
 @mgr_utils.mock_record_keepalive
 class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin,
diff --git a/releasenotes/notes/require-tls-3880e6bec3075f4d.yaml b/releasenotes/notes/require-tls-3880e6bec3075f4d.yaml
new file mode 100644
index 000000000..ea37f8ef8
--- /dev/null
+++ b/releasenotes/notes/require-tls-3880e6bec3075f4d.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    A new configuration option ``[agent]require_tls`` allows rejecting
+    ramdisk callback URLs that don't use the ``https://`` schema.