From 253653dfe7fdcaef31feaae36e5c8b473d7e8508 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Tue, 14 Apr 2020 16:47:44 -0700 Subject: [PATCH] Disable altering credential owner attributes Without this patch, an authenticated user can create an EC2 credential for themself for a project they have a role on, then update the credential to target a user and project completely unrelated to them. In the worst case, this could be the admin user and a project the admin user has a role assignment on. A token granted for an altered credential like this would allow the user to masquerade as the victim user. This patch ensures that when updating a credential, the new form of the credential is one the acting user has access to: if the system admin user is changing the credential, the new user ID or project ID could be anything, but regular users may only change the credential to be one that they still own. Change-Id: I39d0d705839fbe31ac518ac9a82959e108cb7c1d Closes-bug: #1872733 (cherry picked from commit 3dbfc90694aa0d1445ce46cde1f9eb97af465c83) (cherry picked from commit a6ad20b68e6945c20a9a3c1920b3ac8a82e6fa38) --- keystone/api/credentials.py | 5 +++ keystone/tests/unit/test_v3_credential.py | 36 +++++++++++++++++++ .../notes/bug-1872733-2377f456a57ad32c.yaml | 16 +++++++++ 3 files changed, 57 insertions(+) create mode 100644 releasenotes/notes/bug-1872733-2377f456a57ad32c.yaml diff --git a/keystone/api/credentials.py b/keystone/api/credentials.py index 08a492c1d..b788bff5a 100644 --- a/keystone/api/credentials.py +++ b/keystone/api/credentials.py @@ -162,6 +162,11 @@ class CredentialResource(ks_flask.ResourceBase): credential = self.request_body_json.get('credential', {}) validation.lazy_validate(schema.credential_update, credential) self._require_matching_id(credential) + # Check that the user hasn't illegally modified the owner or scope + target = {'credential': credential} + ENFORCER.enforce_call( + action='identity:update_credential', target_attr=target + ) ref = PROVIDERS.credential_api.update_credential( credential_id, credential) return self.wrap_member(ref) diff --git a/keystone/tests/unit/test_v3_credential.py b/keystone/tests/unit/test_v3_credential.py index 2538c3a10..5de4d6455 100644 --- a/keystone/tests/unit/test_v3_credential.py +++ b/keystone/tests/unit/test_v3_credential.py @@ -258,6 +258,42 @@ class CredentialTestCase(CredentialBaseTestCase): 'credential_id': credential_id}, body={'credential': update_ref}) + def test_update_credential_non_owner(self): + """Call ``PATCH /credentials/{credential_id}``.""" + alt_user = unit.create_user( + PROVIDERS.identity_api, domain_id=self.domain_id) + alt_user_id = alt_user['id'] + alt_project = unit.new_project_ref(domain_id=self.domain_id) + alt_project_id = alt_project['id'] + PROVIDERS.resource_api.create_project( + alt_project['id'], alt_project) + alt_role = unit.new_role_ref(name='reader') + alt_role_id = alt_role['id'] + PROVIDERS.role_api.create_role(alt_role_id, alt_role) + PROVIDERS.assignment_api.add_role_to_user_and_project( + alt_user_id, alt_project_id, alt_role_id) + auth = self.build_authentication_request( + user_id=alt_user_id, + password=alt_user['password'], + project_id=alt_project_id) + ref = unit.new_credential_ref(user_id=alt_user_id, + project_id=alt_project_id) + r = self.post( + '/credentials', + auth=auth, + body={'credential': ref}) + self.assertValidCredentialResponse(r, ref) + credential_id = r.result.get('credential')['id'] + + # Cannot change the credential to be owned by another user + update_ref = {'user_id': self.user_id, 'project_id': self.project_id} + self.patch( + '/credentials/%(credential_id)s' % { + 'credential_id': credential_id}, + expected_status=403, + auth=auth, + body={'credential': update_ref}) + def test_delete_credential(self): """Call ``DELETE /credentials/{credential_id}``.""" self.delete( diff --git a/releasenotes/notes/bug-1872733-2377f456a57ad32c.yaml b/releasenotes/notes/bug-1872733-2377f456a57ad32c.yaml new file mode 100644 index 000000000..656822c2a --- /dev/null +++ b/releasenotes/notes/bug-1872733-2377f456a57ad32c.yaml @@ -0,0 +1,16 @@ +--- +critical: + - | + [`bug 1872733 `_] + Fixed a critical security issue in which an authenticated user could + escalate their privileges by altering a valid EC2 credential. +security: + - | + [`bug 1872733 `_] + Fixed a critical security issue in which an authenticated user could + escalate their privileges by altering a valid EC2 credential. +fixes: + - | + [`bug 1872733 `_] + Fixed a critical security issue in which an authenticated user could + escalate their privileges by altering a valid EC2 credential. -- 2.26.0