From 6095b4017d64573abdb202fedc7fb1aede56ce86 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 4 Jun 2024 11:00:37 -0700 Subject: [PATCH] Change [agent]require_tls to True by default While looking at the overall heartbeat/agent workflow, it seemed like the [agent]require_tls setting should likely be True by default, as we are well past the initial phase where operators might not have the TLS capability when upgrading. Change-Id: Id526e948e6c5ed032d7542232b1c1a31cb285b26 --- ironic/conf/agent.py | 8 ++++--- ironic/tests/unit/conductor/test_manager.py | 24 +++++++++---------- ...agent-to-require-tls-08a9571793e75943.yaml | 5 ++++ 3 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/set-agent-to-require-tls-08a9571793e75943.yaml diff --git a/ironic/conf/agent.py b/ironic/conf/agent.py index 85341e0ae5..bfa7e25c90 100644 --- a/ironic/conf/agent.py +++ b/ironic/conf/agent.py @@ -153,10 +153,12 @@ opts = [ help=_('Wait time in seconds between attempts for validating ' 'Neutron agent status.')), cfg.BoolOpt('require_tls', - default=False, + default=True, mutable=True, - help=_('If set to True, callback URLs without https:// will ' - 'be rejected by the conductor.')), + help=_('If set to False, callback URLs without https:// will ' + 'be permitted by the conductor, which may be needed ' + 'for backwards compatibility outside of the supported ' + 'version window.')), cfg.StrOpt('certificates_path', default='/var/lib/ironic/certificates', help=_('Path to store auto-generated TLS certificates used to ' diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 2917ef8e0d..27c4addd04 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -7669,11 +7669,11 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn - self.service.heartbeat(self.context, node.uuid, 'http://callback', + self.service.heartbeat(self.context, node.uuid, 'https://callback', agent_version=None, agent_token='magic', agent_status='start') mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', None, + 'https://callback', None, None, 'start', None) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @@ -7682,6 +7682,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): autospec=True) def test_heartbeat_with_agent_version(self, mock_spawn, mock_heartbeat): """Test heartbeating.""" + self.config(require_tls=False, group='agent') node = obj_utils.create_test_node( self.context, driver='fake-hardware', provision_state=states.DEPLOYING, @@ -7743,10 +7744,10 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn - self.service.heartbeat(self.context, node.uuid, 'http://callback', + self.service.heartbeat(self.context, node.uuid, 'https://callback', '6.1.0', agent_token='a secret') mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', '6.1.0', None, + 'https://callback', '6.1.0', None, None, None) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @@ -7768,10 +7769,10 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.side_effect = self._fake_spawn - self.service.heartbeat(self.context, node.uuid, 'http://callback', + self.service.heartbeat(self.context, node.uuid, 'https://callback', '6.1.0', agent_token='a secret') mock_heartbeat.assert_called_with(mock.ANY, mock.ANY, - 'http://callback', '6.1.0', None, + 'https://callback', '6.1.0', None, None, None) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.heartbeat', @@ -7795,7 +7796,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.heartbeat, self.context, - node.uuid, 'http://callback', + node.uuid, 'https://callback', agent_token='evil', agent_version='5.0.0b23') self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertFalse(mock_heartbeat.called) @@ -7822,7 +7823,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): # occurs. exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.heartbeat, self.context, - node.uuid, 'http://callback', + node.uuid, 'https://callback', agent_token='evil', agent_version='4.0.0') self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertFalse(mock_heartbeat.called) @@ -7847,7 +7848,7 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.heartbeat, self.context, - node.uuid, 'http://callback', + node.uuid, 'https://callback', agent_token=None, agent_version='6.1.5') self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) self.assertFalse(mock_heartbeat.called) @@ -7858,7 +7859,6 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): 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, @@ -7900,11 +7900,11 @@ class HeartbeatTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): mock_spawn.reset_mock() mock_spawn.side_effect = self._fake_spawn - self.service.heartbeat(self.context, node.uuid, 'http://callback', + self.service.heartbeat(self.context, node.uuid, 'https://callback', agent_version='6.1.0', agent_token='a secret', agent_verify_ca='abcd') mock_heartbeat.assert_called_with( - mock.ANY, mock.ANY, 'http://callback', '6.1.0', '/path/to/crt', + mock.ANY, mock.ANY, 'https://callback', '6.1.0', '/path/to/crt', None, None) diff --git a/releasenotes/notes/set-agent-to-require-tls-08a9571793e75943.yaml b/releasenotes/notes/set-agent-to-require-tls-08a9571793e75943.yaml new file mode 100644 index 0000000000..022c110090 --- /dev/null +++ b/releasenotes/notes/set-agent-to-require-tls-08a9571793e75943.yaml @@ -0,0 +1,5 @@ +--- +security: + - | + Agent communication now requires an HTTPS url by default. This can be + changed using the ``[agent]require_tls`` setting.