Don't reveal trust existence to unauthorized users

Without this patch, any user regardless of their relation to a trust can
GET /OS-TRUST/trusts/{trust_id} and find out whether a trust with that
ID exists: if the return code is a 404, it does not exist, whereas if it
is a 403, it exists. This violates security principles and can be used
as part of a privilege escalation attack. This change ensure that the
intended access control is enforced and does not disclose the
non-existence of a trust to unauthorized users.

Since we don't use microversions and don't intend to update the major
API version, this could be read as violating the API SIG's guidelines on
API changes for interoperability[1]. However, the security impact is
severe enough that this change is warranted.

[1] https://specs.openstack.org/openstack/api-sig/guidelines/api_interoperability.html#evaluating-api-changes

Closes-bug: #1840288
Depends-on: https://review.opendev.org/676745

Change-Id: I669cd1675dcc3663aecf0e5882a3e53a3d77c1e0

keystone/api/trusts.py

@@ -44,8 +44,8 @@ TRUST_ID_PARAMETER_RELATION = _build_parameter_relation(
 
 def _trustor_trustee_only(trust):
     user_id = flask.request.environ.get(context.REQUEST_CONTEXT_ENV).user_id
-    if user_id not in [trust.get('trustee_user_id'),
-                       trust.get('trustor_user_id')]:
+    if not trust or (user_id not in [trust.get('trustee_user_id'),
+                                     trust.get('trustor_user_id')]):
         raise exception.ForbiddenAction(
             action=_('Requested user has no relation to this trust'))
 
@@ -147,7 +147,13 @@ class TrustResource(ks_flask.ResourceBase):
 
     def _get_trust(self, trust_id):
         ENFORCER.enforce_call(action='identity:get_trust')
-        trust = PROVIDERS.trust_api.get_trust(trust_id)
+        trust = None
+        try:
+            trust = PROVIDERS.trust_api.get_trust(trust_id)
+        except exception.NotFound:
+            # unauthorized users should not have trust-nonexistence exposed to
+            # them, handle this in the next step
+            pass
         _trustor_trustee_only(trust)
         _normalize_trust_expires_at(trust)
         _normalize_trust_roles(trust)

keystone/tests/unit/test_v3_auth.py

@@ -4377,10 +4377,12 @@ class TrustAPIBehavior(test_v3.RestfulTestCase):
 
     def test_create_one_time_use_trust(self):
         trust = self._initialize_test_consume_trust(1)
-        # No more uses, the trust is made unavailable
+        # No more uses, the trust is made unavailable. Requesting a non-existent
+        # trust will result in a 403 to prevent disclosing its existence or
+        # non-existence.
         self.get(
             '/OS-TRUST/trusts/%(trust_id)s' % {'trust_id': trust['id']},
-            expected_status=http_client.NOT_FOUND)
+            expected_status=http_client.FORBIDDEN)
         # this time we can't get a trust token
         auth_data = self.build_authentication_request(
             user_id=self.trustee_user['id'],
@@ -4900,11 +4902,13 @@ class TestTrustChain(test_v3.RestfulTestCase):
 
             auth_token = self.get_requested_token(auth_data)
 
-            # Assert chained trust have been deleted
+            # Assert chained trust have been deleted. Requesting a non-existent
+            # trust will result in a 403 to prevent disclosing its existence or
+            # non-existence.
             self.get('/OS-TRUST/trusts/%(trust_id)s' % {
                 'trust_id': self.trust_chain[i + 1]['id']},
                 token=auth_token,
-                expected_status=http_client.NOT_FOUND)
+                expected_status=http_client.FORBIDDEN)
 
     def test_trustor_roles_revoked(self):
         self.assert_user_authenticate(self.user_list[0])

keystone/tests/unit/test_v3_trust.py

@@ -140,10 +140,12 @@ class TestTrustOperations(test_v3.RestfulTestCase):
         self.delete(
             '/OS-TRUST/trusts/%(trust_id)s' % {'trust_id': trust['id']})
 
-        # ensure the trust is not found
+        # ensure the trust is no longer available. Requesting a non-existent
+        # trust will result in a 403 to prevent disclosing its existence or
+        # non-existence.
         self.get(
             '/OS-TRUST/trusts/%(trust_id)s' % {'trust_id': trust['id']},
-            expected_status=http_client.NOT_FOUND)
+            expected_status=http_client.FORBIDDEN)
 
     def test_list_trusts(self):
         # create three trusts with the same trustor and trustee
@@ -232,10 +234,12 @@ class TestTrustOperations(test_v3.RestfulTestCase):
         self.delete('/OS-TRUST/trusts/%(trust_id)s' % {
             'trust_id': trust['id']})
 
-        # ensure the trust isn't found
+        # ensure the trust is no longer available. Requesting a non-existent
+        # trust will result in a 403 to prevent disclosing its existence or
+        # non-existence.
         self.get('/OS-TRUST/trusts/%(trust_id)s' % {
             'trust_id': trust['id']},
-            expected_status=http_client.NOT_FOUND)
+            expected_status=http_client.FORBIDDEN)
 
     def test_create_trust_without_trustee_returns_bad_request(self):
         ref = unit.new_trust_ref(
@@ -474,9 +478,10 @@ class TestTrustOperations(test_v3.RestfulTestCase):
         self.delete(
             '/users/%(user_id)s' % {'user_id': trust['trustee_user_id']})
 
+        # requesting a non-existent trust results in an access error
         self.get(
             '/OS-TRUST/trusts/%(trust_id)s' % {'trust_id': trust['id']},
-            expected_status=http_client.NOT_FOUND)
+            expected_status=http_client.FORBIDDEN)
 
         # create another user as the new trustee
         trustee_user = unit.create_user(PROVIDERS.identity_api,

releasenotes/notes/trust-404-8933080f70ed9a55.yaml

@@ -0,0 +1,25 @@
+---
+upgrade:
+  - |
+    Includes a change in the response to the GET /v3/OS-TRUST/trusts/{trust_id}
+    API if a trust is not found: now requesting a non-existent trust will
+    result in a response of HTTP 403 Forbidden to prevent disclosing whether
+    the trust exists.
+security:
+  - |
+    Fixes an oversight that allowed unauthorized users to gain knowledge about
+    the existence or non-existence of a trust. The GET
+    /v3/OS-TRUST/trusts/{trust_id} API will now return an HTTP 403 Forbidden if
+    either a trust does not exist or the requesting user has no association
+    with it (either as a trustor or trustee), where previously it might either
+    return an HTTP 403 Forbidden or HTTP 404 Not Found depending on whether the
+    trust existed and had remaining uses left on it.
+fixes:
+  - |
+    Fixes an oversight that allowed unauthorized users to gain knowledge about
+    the existence or non-existence of a trust. The GET
+    /v3/OS-TRUST/trusts/{trust_id} API will now return an HTTP 403 Forbidden if
+    either a trust does not exist or the requesting user has no association
+    with it (either as a trustor or trustee), where previously it might either
+    return an HTTP 403 Forbidden or HTTP 404 Not Found depending on whether the
+    trust existed and had remaining uses left on it.