summaryrefslogtreecommitdiff
path: root/keystone/api/users.py
diff options
context:
space:
mode:
authorLance Bragstad <lbragstad@gmail.com>2020-11-02 17:22:49 +0000
committerLance Bragstad <lbragstad@redhat.com>2021-01-22 18:46:32 +0000
commit9c879d46bf40f8a11e8033406bb1b8861baf3f51 (patch)
treede3e8c6529b49f9c1551fd167db92927a327fc1d /keystone/api/users.py
parentb49a465edc907d8b8f863904239b0d6c58293800 (diff)
downloadkeystone-9c879d46bf40f8a11e8033406bb1b8861baf3f51.tar.gz
Use app cred user ID in policy enforcement
The application credential policies use the `rule:owner` policy to allow users to manage their own credentials. The policy engine pulled the user_id attribute from the request path instead of the actual application credential. This allowed for users to exploit the enforcement and view or delete application credentials they don't own. This commit attempts to resolve the issue by updating the flask parameters before they're translated to policy arguments and target data, prior to policy enforcement. This commit also deviates slightly from backports to stable/ussuri, stable/victoria, and master (wallaby). This is because newer branches use `http.client` to assert status codes and in stable/train we are still using `http_client`. This change is functionally the same. Change-Id: I903d20fa41270499ca1c39d296120dd97cef5405 Closes-Bug: 1901207 (cherry picked from commit 2d7bf10a5a145ed3b6e34c3cb95e05fb7e33e0d1) (cherry picked from commit 80832de6ced534bccbf7cfe24a6a01c8262c1779) (cherry picked from commit 661bf6a1dca502f37b5da9995c45ba60581e5e97)
Diffstat (limited to 'keystone/api/users.py')
-rw-r--r--keystone/api/users.py47
1 files changed, 45 insertions, 2 deletions
diff --git a/keystone/api/users.py b/keystone/api/users.py
index 525c5d1cd..a883752f8 100644
--- a/keystone/api/users.py
+++ b/keystone/api/users.py
@@ -121,6 +121,41 @@ def _build_enforcer_target_data_owner_and_user_id_match():
return ref
+def _update_request_user_id_attribute():
+ # This method handles a special case in policy enforcement. The application
+ # credential API is underneath the user path (e.g.,
+ # /v3/users/{user_id}/application_credentials/{application_credential_id}).
+ # The RBAC enforcer thinks the user to evaluate for application credential
+ # ownership comes from the path, but it should come from the actual
+ # application credential reference. By ensuring we pull the user ID from
+ # the application credential, we close a loop hole where users could
+ # effectively bypass authorization to view or delete any application
+ # credential in the system, assuming the attacker knows the application
+ # credential ID of another user. So long as the attacker matches the user
+ # ID in the request path to the user in the token of the request, they can
+ # pass the `rule:owner` policy check. This method protects against that by
+ # ensuring we use the application credential user ID and not something
+ # determined from the client.
+ try:
+ app_cred = (
+ PROVIDERS.application_credential_api.get_application_credential(
+ flask.request.view_args.get('application_credential_id')
+ )
+ )
+ flask.request.view_args['user_id'] = app_cred['user_id']
+
+ # This target isn't really used in the default policy for application
+ # credentials, but we return it since we're using this method as a hook
+ # to update the flask request variables, which are used later in the
+ # keystone RBAC enforcer to populate the policy_dict, which ultimately
+ # turns into target attributes.
+ return {'user_id': app_cred['user_id']}
+ except ks_exception.NotFound: # nosec
+ # Defer existance in the event the application credential doesn't
+ # exist, we'll check this later anyway.
+ pass
+
+
def _format_role_entity(role_id):
role = PROVIDERS.role_api.get_role(role_id)
formatted_entity = role.copy()
@@ -652,7 +687,11 @@ class UserAppCredGetDeleteResource(ks_flask.ResourceBase):
GET/HEAD /v3/users/{user_id}/application_credentials/
{application_credential_id}
"""
- ENFORCER.enforce_call(action='identity:get_application_credential')
+ target = _update_request_user_id_attribute()
+ ENFORCER.enforce_call(
+ action='identity:get_application_credential',
+ target_attr=target,
+ )
ref = PROVIDERS.application_credential_api.get_application_credential(
application_credential_id)
return self.wrap_member(ref)
@@ -663,7 +702,11 @@ class UserAppCredGetDeleteResource(ks_flask.ResourceBase):
DELETE /v3/users/{user_id}/application_credentials/
{application_credential_id}
"""
- ENFORCER.enforce_call(action='identity:delete_application_credential')
+ target = _update_request_user_id_attribute()
+ ENFORCER.enforce_call(
+ action='identity:delete_application_credential',
+ target_attr=target
+ )
token = self.auth_context['token']
_check_unrestricted_application_credential(token)
PROVIDERS.application_credential_api.delete_application_credential(