From 77b7fbf163a3a62379d7241655b2b56423c80434 Mon Sep 17 00:00:00 2001 From: Callum Dickinson Date: Sun, 22 Mar 2026 12:38:36 +1300 Subject: [PATCH] Return 404 when unauthorised to access resources When a user is not authorised to access a specific resource (e.g. did not create the resource, or not part of the project that owns the resource, or the project that created the resource), they should not be able to enumerate the resource in any way. To prevent such users from inferring the existence of resources they should not have access to, change the policy enforcement for `get resource` such that it returns `404 Not Found` instead of `403 Forbidden` in these cases. --- gnocchi/rest/api.py | 18 +++++-- gnocchi/rest/auth_helper.py | 94 +++++++++++++++++++++++++++++++++++++ gnocchi/tests/test_rest.py | 4 +- 3 files changed, 111 insertions(+), 5 deletions(-) diff --git a/gnocchi/rest/api.py b/gnocchi/rest/api.py index 39d73b362..3b2bebdaf 100644 --- a/gnocchi/rest/api.py +++ b/gnocchi/rest/api.py @@ -801,7 +801,11 @@ def get_all(self): self.resource_type, self.resource_id) if not resource: abort(404, str(indexer.NoSuchResource(self.resource_id))) - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.resource_id, + resource) return pecan.request.indexer.list_metrics( attribute_filter={"=": {"resource_id": self.resource_id}}) @@ -822,7 +826,11 @@ def get(self, **kwargs): if not resource: abort(404, str(indexer.NoSuchResource(self.resource_id))) - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.resource_id, + resource) try: resources = pecan.request.indexer.list_resources( @@ -1080,7 +1088,11 @@ def get(self): resource = pecan.request.indexer.get_resource( self._resource_type, self.id, with_metrics=True) if resource: - enforce("get resource", resource) + pecan.request.auth_helper.enforce_resource_policy( + pecan.request, + "get resource", + self.id, + resource) etag_precondition_check(resource) etag_set_headers(resource) return resource diff --git a/gnocchi/rest/auth_helper.py b/gnocchi/rest/auth_helper.py index 67ab28c23..263df4aa7 100644 --- a/gnocchi/rest/auth_helper.py +++ b/gnocchi/rest/auth_helper.py @@ -19,6 +19,7 @@ import webob import werkzeug.http +from gnocchi import indexer from gnocchi.rest import api @@ -45,6 +46,83 @@ def get_auth_info(request): 'roles': request.headers.get("X-Roles", "").split(","), } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + try: + # Check if the policy allows the user to get any resource + api.enforce(rule, {}) + except webob.exc.HTTPForbidden: + policy_matched = False + auth_info = KeystoneAuthHelper.get_auth_info(request) + project_id = auth_info["project_id"] + user_id = auth_info["user_id"] + + target = {} + if prefix: + target_resource = target[prefix] = {} + else: + target_resource = target + + target_resource["project_id"] = project_id + try: + # Check if the policy allows the user to get resources linked + # to their project + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # User is authenticated using the project that owns the + # resource and the policy allows access by project. + if resource.project_id == project_id: + return + + resource_creator_user_id, _, resource_creator_project_id = ( + resource.creator.partition(":")) + + del target_resource["project_id"] + target_resource["created_by_project_id"] = project_id + try: + # Check if the policy allows the user to get resources linked + # to their created_by_project + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # User is authenticated using the project that created the + # resource and the policy allows access by created_by_project. + if resource_creator_project_id == project_id: + return + + del target_resource["created_by_project_id"] + target_resource["creator"] = user_id + try: + # Check if the policy allows the user to get resources linked + # to their creator + api.enforce(rule, target) + except webob.exc.HTTPForbidden: + pass + else: + policy_matched = True + # The authenticated user is the user that created the resource + # and the policy allows access by creator. + if resource_creator_user_id == user_id: + return + + # If at least one of the policies matched but the user should not + # be allowed access to the resource, return 404 Not Found to + # prevent the user from enumerating the existence of the resource. + if policy_matched: + api.abort(404, str(indexer.NoSuchResource(resource_id))) + + # None of the above policies matched, return 403 Forbidden. + api.abort(403) + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): try: @@ -177,6 +255,14 @@ def get_auth_info(self, request): "system": 'all', } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + pass + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): return None @@ -210,6 +296,14 @@ def get_auth_info(self, request): "system": 'all', } + @staticmethod + def enforce_resource_policy(request, + rule, + resource_id, + resource, + prefix=None): + pass + @staticmethod def get_resource_policy_filter(request, rule, resource_type, prefix=None): return None diff --git a/gnocchi/tests/test_rest.py b/gnocchi/tests/test_rest.py index 93dcc5c79..ef24411ee 100644 --- a/gnocchi/tests/test_rest.py +++ b/gnocchi/tests/test_rest.py @@ -1005,7 +1005,7 @@ def test_get_resource_unauthorized(self): + self.resource_type + "/" + self.attributes['id'], - status=403) + status=404) def test_get_resource_named_metric(self): self.attributes['metrics'] = {'foo': {'archive_policy_name': "high"}} @@ -1026,7 +1026,7 @@ def test_list_resource_metrics_unauthorized(self): self.app.get( "/v1/resource/" + self.resource_type + "/" + self.attributes['id'] + "/metric", - status=403) + status=404) def test_delete_resource_named_metric(self): self.attributes['metrics'] = {'foo': {'archive_policy_name': "high"}}