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.

Author: Callum027

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

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

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"}}