From b792d03fee5a821b5660ff501a445543ce806083 Mon Sep 17 00:00:00 2001 From: Nuutti Hakala Date: Wed, 24 Jun 2026 08:46:33 +0300 Subject: [PATCH] fix broken project scoping An issue was discovered where users can list resources without correct authorization. Volume connectors and targets connected to certain node could be listed when knowing the node ID, even if the user is not authorized to see the node. Similar problem was also in ports, detailed port information related to all nodes belonging to a given shard could be listed, even if the user is not authorized to see the nodes. Related-Bug: 2150461 Change-Id: I9dd1072ee14b2005020da60d3d2226f5c04bd781 Signed-off-by: Nuutti Hakala (cherry picked from commit dbea2cca344b2b457e637cd4af54d86e20a76a0d) --- ironic/db/api.py | 11 +++++++++-- ironic/db/sqlalchemy/api.py | 9 ++++++--- ironic/objects/port.py | 3 ++- ironic/tests/unit/db/test_ports.py | 12 ++++++++++-- ironic/tests/unit/db/test_volume_connectors.py | 17 +++++++++++++++++ ironic/tests/unit/db/test_volume_targets.py | 11 +++++++++++ ...broken-project-scoping-86c5d19912158085.yaml | 8 ++++++++ 7 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/fix-broken-project-scoping-86c5d19912158085.yaml diff --git a/ironic/db/api.py b/ironic/db/api.py index 77e109ced9..59e6d6f443 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -298,10 +298,17 @@ def get_port_list(self, limit=None, marker=None, @abc.abstractmethod def get_ports_by_shards(self, shards, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): """Return a list of ports contained in the provided shards. - :param shard_ids: A list of shards to filter ports by. + :param shards: A list of shards to filter ports by. + :param limit: Maximum number of ports to return. + :param marker: the last item of the previous page; we return the next + result set. + :param sort_key: Attribute by which results should be sorted. + :param sort_dir: direction in which results should be sorted. + (asc, desc) + :param project: a node owner or lessee to match against """ @abc.abstractmethod diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index be0af10fce..3478c3eb27 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -1080,12 +1080,15 @@ def get_port_list(self, limit=None, marker=None, sort_key, sort_dir, query) def get_ports_by_shards(self, shards, limit=None, marker=None, - sort_key=None, sort_dir=None): + sort_key=None, sort_dir=None, project=None): shard_node_ids = sa.select(models.Node) \ .where(models.Node.shard.in_(shards)) \ .with_only_columns(models.Node.id) query = sa.select(models.Port) \ .where(models.Port.node_id.in_(shard_node_ids)) + + if project: + query = add_port_filter_by_node_project(query, project) return _paginate_query( models.Port, limit, marker, sort_key, sort_dir, query) @@ -1687,7 +1690,7 @@ def get_volume_connectors_by_node_id(self, node_id, limit=None, query = sa.select(models.VolumeConnector).where( models.VolumeConnector.node_id == node_id) if project: - add_volume_conn_filter_by_node_project(query, project) + query = add_volume_conn_filter_by_node_project(query, project) return _paginate_query(models.VolumeConnector, limit, marker, sort_key, sort_dir, query) @@ -1778,7 +1781,7 @@ def get_volume_targets_by_node_id(self, node_id, limit=None, marker=None, query = sa.select(models.VolumeTarget).where( models.VolumeTarget.node_id == node_id) if project: - add_volume_target_filter_by_node_project(query, project) + query = add_volume_target_filter_by_node_project(query, project) return _paginate_query(models.VolumeTarget, limit, marker, sort_key, sort_dir, query) diff --git a/ironic/objects/port.py b/ironic/objects/port.py index d961a6e3eb..b6fe48555c 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -317,7 +317,8 @@ def list_by_node_shards(cls, context, shards, limit=None, marker=None, db_ports = cls.dbapi.get_ports_by_shards(shards, limit=limit, marker=marker, sort_key=sort_key, - sort_dir=sort_dir) + sort_dir=sort_dir, + project=project) return cls._from_db_object_list(context, db_ports) # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable diff --git a/ironic/tests/unit/db/test_ports.py b/ironic/tests/unit/db/test_ports.py index 0284ee0d00..d74dd13996 100644 --- a/ironic/tests/unit/db/test_ports.py +++ b/ironic/tests/unit/db/test_ports.py @@ -22,10 +22,11 @@ from ironic.tests.unit.db import utils as db_utils -def _create_test_port_with_shard(shard, address): +def _create_test_port_with_shard(shard, address, owner='12345', + lessee='54321'): node = db_utils.create_test_node( uuid=uuidutils.generate_uuid(), - owner='12345', lessee='54321', shard=shard) + owner=owner, lessee=lessee, shard=shard) pg = db_utils.create_test_portgroup( name='pg-%s' % shard, uuid=uuidutils.generate_uuid(), @@ -241,6 +242,13 @@ def test_get_ports_by_shard_with_match_multi(self): self.assertNotEqual('port-shard3', res[0].name) self.assertNotEqual('port-shard3', res[1].name) + def test_get_ports_by_shard_with_invalid_project(self): + _create_test_port_with_shard('shard1', 'aa:bb:cc:dd:ee:ff') + _create_test_port_with_shard('shard2', 'ff:ee:dd:cc:bb:aa', + owner='67890', lessee='09876') + res = self.dbapi.get_ports_by_shards(['shard2'], project='12345') + self.assertEqual(0, len(res)) + def test_destroy_port(self): self.dbapi.destroy_port(self.port.id) self.assertRaises(exception.PortNotFound, diff --git a/ironic/tests/unit/db/test_volume_connectors.py b/ironic/tests/unit/db/test_volume_connectors.py index 005933fa48..4dffe5fe2d 100644 --- a/ironic/tests/unit/db/test_volume_connectors.py +++ b/ironic/tests/unit/db/test_volume_connectors.py @@ -94,6 +94,23 @@ def test_get_volume_connectors_by_node_id(self): self.assertEqual(self.connector.type, res[0].type) self.assertEqual(self.connector.connector_id, res[0].connector_id) + def test_get_volume_connectors_by_node_id_with_invalid_project(self): + node1 = db_utils.create_test_node(owner='12345') + db_utils.create_test_volume_connector( + uuid=uuidutils.generate_uuid(), + node_id=node1.id, + type='iqn', + connector_id='iqn.test-%s' % 1) + node2 = db_utils.create_test_node(owner='54321') + db_utils.create_test_volume_connector( + uuid=uuidutils.generate_uuid(), + node_id=node2.id, + type='iqn', + connector_id='iqn.test-%s' % 2) + res = self.dbapi.get_volume_connectors_by_node_id(node2.id, + project='12345') + self.assertEqual(0, len(res)) + def test_get_volume_connectors_by_node_id_that_does_not_exist(self): self.assertEqual([], self.dbapi.get_volume_connectors_by_node_id(99)) diff --git a/ironic/tests/unit/db/test_volume_targets.py b/ironic/tests/unit/db/test_volume_targets.py index 200addf83b..82c8d70df7 100644 --- a/ironic/tests/unit/db/test_volume_targets.py +++ b/ironic/tests/unit/db/test_volume_targets.py @@ -115,6 +115,17 @@ def test_get_volume_targets_by_node_id(self): def test_get_volume_targets_by_node_id_that_does_not_exist(self): self.assertEqual([], self.dbapi.get_volume_targets_by_node_id(99)) + def test_get_volume_targets_by_node_id_with_invalid_project(self): + node1 = db_utils.create_test_node(owner='12345') + db_utils.create_test_volume_target( + uuid=uuidutils.generate_uuid(), node_id=node1.id) + node2 = db_utils.create_test_node(owner='54321') + db_utils.create_test_volume_target( + uuid=uuidutils.generate_uuid(), node_id=node2.id) + res = self.dbapi.get_volume_targets_by_node_id(node2.id, + project='12345') + self.assertEqual(0, len(res)) + def test_get_volume_targets_by_volume_id(self): # Create two volume_targets. They'll have the same volume_id. uuids = self._create_list_of_volume_targets(2) diff --git a/releasenotes/notes/fix-broken-project-scoping-86c5d19912158085.yaml b/releasenotes/notes/fix-broken-project-scoping-86c5d19912158085.yaml new file mode 100644 index 0000000000..f0097394be --- /dev/null +++ b/releasenotes/notes/fix-broken-project-scoping-86c5d19912158085.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes a security issue where user can list volume connectors, volume + targets and ports of nodes that they are not authorized to access. Volume + connectors and target could be listed when specifying the node ID and ports + could be listed by giving a valid shard. Related bug report + `bug 2150461 `_.