diff --git a/ironic/api/controllers/v1/ramdisk.py b/ironic/api/controllers/v1/ramdisk.py
index 47b320b24c..9ce0608f21 100644
--- a/ironic/api/controllers/v1/ramdisk.py
+++ b/ironic/api/controllers/v1/ramdisk.py
@@ -13,6 +13,7 @@
# under the License.
from http import client as http_client
+from urllib import parse as urlparse
from oslo_config import cfg
from oslo_log import log
@@ -225,6 +226,20 @@ class HeartbeatController(rest.RestController):
rpc_node = api_utils.get_rpc_node_with_suffix(node_ident)
dii = rpc_node['driver_internal_info']
agent_url = dii.get('agent_url')
+ try:
+ # NOTE(TheJulia): Use of urllib.urlparse is not a security
+ # guard, but detecting oddities and incorrect formatting
+ # https://docs.python.org/3/library/urllib.parse.html#url-parsing-security # noqa
+ parsed_url = urlparse.urlparse(callback_url)
+ # Check if http (compatibility), or https (much newer agents)
+ if 'http' not in parsed_url.scheme:
+ raise ValueError
+ callback_url = parsed_url.geturl()
+ except ValueError:
+ raise exception.InvalidParameterValue(
+ _('An issue with the supplied "callback_url" has been '
+ 'detected.'))
+
# If we have an agent_url on file, and we get something different
# we should fail because this is unexpected behavior of the agent.
if agent_url is not None and agent_url != callback_url:
diff --git a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
index 70a918fde6..0eb4fe426f 100644
--- a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
+++ b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py
@@ -204,7 +204,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
def test_old_api_version(self):
response = self.post_json(
'/heartbeat/%s' % uuidutils.generate_uuid(),
- {'callback_url': 'url'},
+ {'callback_url': 'https://url'},
headers={api_base.Version.string: str(api_v1.min_version())},
expect_errors=True)
self.assertEqual(http_client.NOT_FOUND, response.status_int)
@@ -212,7 +212,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
def test_node_not_found(self):
response = self.post_json(
'/heartbeat/%s' % uuidutils.generate_uuid(),
- {'callback_url': 'url'},
+ {'callback_url': 'https://url'},
headers={api_base.Version.string: str(api_v1.max_version())},
expect_errors=True)
self.assertEqual(http_client.NOT_FOUND, response.status_int)
@@ -222,14 +222,14 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'x'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None, 'x',
- None, None, None,
+ node.uuid, 'https://url', None,
+ 'x', None, None, None,
topic='test-topic')
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
@@ -238,14 +238,14 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s.json' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'maybe some magic'},
headers=headers)
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
- 'maybe some magic',
+ node.uuid, 'https://url',
+ None, 'maybe some magic',
None, None, None,
topic='test-topic')
@@ -254,13 +254,13 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context, name='test.1')
response = self.post_json(
'/heartbeat/%s' % node.name,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'token'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'https://url', None,
'token', None, None, None,
topic='test-topic')
@@ -269,14 +269,15 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_version': '1.4.1',
'agent_token': 'meow'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', '1.4.1',
+ node.uuid, 'https://url',
+ '1.4.1',
'meow',
None, None, None,
topic='test-topic')
@@ -286,20 +287,31 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_version': '1.4.1'},
headers={api_base.Version.string: '1.35'},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+ @mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
+ def test_heartbeat_rejects_file_url(self, mock_heartbeat):
+ node = obj_utils.create_test_node(
+ self.context)
+ response = self.post_json(
+ '/heartbeat/%s' % node.uuid,
+ {'callback_url': 'file:///path/to/the/wizzard'},
+ headers={api_base.Version.string: str(api_v1.max_version())},
+ expect_errors=True)
+ self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+
@mock.patch.object(rpcapi.ConductorAPI, 'heartbeat', autospec=True)
def test_heartbeat_rejects_different_callback_url(self, mock_heartbeat):
node = obj_utils.create_test_node(
self.context,
- driver_internal_info={'agent_url': 'url'})
+ driver_internal_info={'agent_url': 'https://url'})
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url2'},
+ {'callback_url': 'https://url2'},
headers={api_base.Version.string: str(api_v1.max_version())},
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
@@ -309,13 +321,13 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'http://url',
'agent_token': 'abcdef1'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'http://url', None,
'abcdef1', None, None, None,
topic='test-topic')
@@ -324,14 +336,14 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'meow',
'agent_verify_ca': 'abcdef1'},
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'https://url', None,
'meow', 'abcdef1', None, None,
topic='test-topic')
@@ -340,7 +352,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'meow',
'agent_status': 'start',
'agent_status_message': 'woof',
@@ -349,7 +361,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(b'', response.body)
mock_heartbeat.assert_called_once_with(mock.ANY, mock.ANY,
- node.uuid, 'url', None,
+ node.uuid, 'https://url', None,
'meow', 'abcdef1', 'start',
'woof', topic='test-topic')
@@ -358,7 +370,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'meow',
'agent_status': 'invalid_state',
'agent_status_message': 'woof',
@@ -372,7 +384,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'meow',
'agent_verify_ca': 'abcd'},
headers={api_base.Version.string: '1.67'},
@@ -384,7 +396,7 @@ class TestHeartbeat(test_api_base.BaseApiTest):
node = obj_utils.create_test_node(self.context)
response = self.post_json(
'/heartbeat/%s' % node.uuid,
- {'callback_url': 'url',
+ {'callback_url': 'https://url',
'agent_token': 'meow',
'agent_verify_ca': 'abcd',
'agent_status': 'wow',
diff --git a/releasenotes/notes/additional-agent-url-validation-97271ce72b0b1a9d.yaml b/releasenotes/notes/additional-agent-url-validation-97271ce72b0b1a9d.yaml
new file mode 100644
index 0000000000..2dbcb21cdf
--- /dev/null
+++ b/releasenotes/notes/additional-agent-url-validation-97271ce72b0b1a9d.yaml
@@ -0,0 +1,10 @@
+---
+features:
+ - |
+ Adds additional validation to the agent ``callback_url``.
+security:
+ - |
+ Additional validation of the ``callback_url`` which is supplied to Ironic
+ by the agent has been added. In addition to any standardized formatting
+ checks included in Python urllib, we will also reject requests which
+ have an invalid URL schema formatting.