Merge "Use app cred user ID in policy enforcement" into stable/ussuri
This commit is contained in:
commit
891d06c4bb
|
@ -121,6 +121,41 @@ def _build_enforcer_target_data_owner_and_user_id_match():
|
||||||
return ref
|
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):
|
def _format_role_entity(role_id):
|
||||||
role = PROVIDERS.role_api.get_role(role_id)
|
role = PROVIDERS.role_api.get_role(role_id)
|
||||||
formatted_entity = role.copy()
|
formatted_entity = role.copy()
|
||||||
|
@ -652,7 +687,11 @@ class UserAppCredGetDeleteResource(ks_flask.ResourceBase):
|
||||||
GET/HEAD /v3/users/{user_id}/application_credentials/
|
GET/HEAD /v3/users/{user_id}/application_credentials/
|
||||||
{application_credential_id}
|
{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(
|
ref = PROVIDERS.application_credential_api.get_application_credential(
|
||||||
application_credential_id)
|
application_credential_id)
|
||||||
return self.wrap_member(ref)
|
return self.wrap_member(ref)
|
||||||
|
@ -663,7 +702,11 @@ class UserAppCredGetDeleteResource(ks_flask.ResourceBase):
|
||||||
DELETE /v3/users/{user_id}/application_credentials/
|
DELETE /v3/users/{user_id}/application_credentials/
|
||||||
{application_credential_id}
|
{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']
|
token = self.auth_context['token']
|
||||||
_check_unrestricted_application_credential(token)
|
_check_unrestricted_application_credential(token)
|
||||||
PROVIDERS.application_credential_api.delete_application_credential(
|
PROVIDERS.application_credential_api.delete_application_credential(
|
||||||
|
|
|
@ -418,6 +418,72 @@ class OwnerTests(_TestAppCredBase,
|
||||||
def test_owner_can_delete_application_credential(self):
|
def test_owner_can_delete_application_credential(self):
|
||||||
self._test_delete_application_credential()
|
self._test_delete_application_credential()
|
||||||
|
|
||||||
|
def test_user_cannot_lookup_application_credential_for_another_user(self):
|
||||||
|
# create another user
|
||||||
|
another_user = unit.new_user_ref(
|
||||||
|
domain_id=CONF.identity.default_domain_id
|
||||||
|
)
|
||||||
|
another_user_id = PROVIDERS.identity_api.create_user(
|
||||||
|
another_user
|
||||||
|
)['id']
|
||||||
|
|
||||||
|
auth = self.build_authentication_request(
|
||||||
|
user_id=another_user_id,
|
||||||
|
password=another_user['password']
|
||||||
|
)
|
||||||
|
|
||||||
|
# authenticate for a token as a completely different user with
|
||||||
|
# completely different authorization
|
||||||
|
with self.test_client() as c:
|
||||||
|
r = c.post('/v3/auth/tokens', json=auth)
|
||||||
|
another_user_token = r.headers['X-Subject-Token']
|
||||||
|
|
||||||
|
# create an application credential as the self.user_id user on a
|
||||||
|
# project that the user above doesn't have any authorization on
|
||||||
|
app_cred = self._create_application_credential()
|
||||||
|
|
||||||
|
# attempt to lookup the application credential as another user
|
||||||
|
with self.test_client() as c:
|
||||||
|
c.get(
|
||||||
|
'/v3/users/%s/application_credentials/%s' % (
|
||||||
|
another_user_id,
|
||||||
|
app_cred['id']),
|
||||||
|
expected_status_code=http.client.FORBIDDEN,
|
||||||
|
headers={'X-Auth-Token': another_user_token})
|
||||||
|
|
||||||
|
def test_user_cannot_delete_application_credential_for_another_user(self):
|
||||||
|
# create another user
|
||||||
|
another_user = unit.new_user_ref(
|
||||||
|
domain_id=CONF.identity.default_domain_id
|
||||||
|
)
|
||||||
|
another_user_id = PROVIDERS.identity_api.create_user(
|
||||||
|
another_user
|
||||||
|
)['id']
|
||||||
|
|
||||||
|
auth = self.build_authentication_request(
|
||||||
|
user_id=another_user_id,
|
||||||
|
password=another_user['password']
|
||||||
|
)
|
||||||
|
|
||||||
|
# authenticate for a token as a completely different user with
|
||||||
|
# completely different authorization
|
||||||
|
with self.test_client() as c:
|
||||||
|
r = c.post('/v3/auth/tokens', json=auth)
|
||||||
|
another_user_token = r.headers['X-Subject-Token']
|
||||||
|
|
||||||
|
# create an application credential as the self.user_id user on a
|
||||||
|
# project that the user above doesn't have any authorization on
|
||||||
|
app_cred = self._create_application_credential()
|
||||||
|
|
||||||
|
# attempt to delete the application credential as another user
|
||||||
|
with self.test_client() as c:
|
||||||
|
c.delete(
|
||||||
|
'/v3/users/%s/application_credentials/%s' % (
|
||||||
|
another_user_id,
|
||||||
|
app_cred['id']),
|
||||||
|
expected_status_code=http.client.FORBIDDEN,
|
||||||
|
headers={'X-Auth-Token': another_user_token})
|
||||||
|
|
||||||
|
|
||||||
class DomainAdminTests(_TestAppCredBase,
|
class DomainAdminTests(_TestAppCredBase,
|
||||||
common_auth.AuthTestMixin,
|
common_auth.AuthTestMixin,
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
security:
|
||||||
|
- |
|
||||||
|
[`bug 1901207 <https://bugs.launchpad.net/keystone/+bug/1901207>`_]
|
||||||
|
Policy enforcement for application credentials has been updated to protect
|
||||||
|
against invalid ownership checks resulting in unauthorized users being able
|
||||||
|
to get and delete application credentials for other users.
|
Loading…
Reference in New Issue