Skip to content

Commit 95b2bbe

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "Fix security issues with EC2 credentials" into stable/stein
2 parents 02447bf + 206392a commit 95b2bbe

File tree

8 files changed

+608
-62
lines changed

8 files changed

+608
-62
lines changed

keystone/api/_shared/EC2_S3_Resource.py

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ def handle_authenticate(self):
115115
project_id=cred.get('project_id'),
116116
access=loaded.get('access'),
117117
secret=loaded.get('secret'),
118-
trust_id=loaded.get('trust_id')
118+
trust_id=loaded.get('trust_id'),
119+
app_cred_id=loaded.get('app_cred_id'),
120+
access_token_id=loaded.get('access_token_id')
119121
)
120122

121123
# validate the signature
@@ -137,8 +139,34 @@ def handle_authenticate(self):
137139
sys.exc_info()[2])
138140

139141
self._check_timestamp(credentials)
140-
roles = PROVIDERS.assignment_api.get_roles_for_user_and_project(
141-
user_ref['id'], project_ref['id'])
142+
143+
trustee_user_id = None
144+
auth_context = None
145+
if cred_data['trust_id']:
146+
trust = PROVIDERS.trust_api.get_trust(cred_data['trust_id'])
147+
roles = [r['id'] for r in trust['roles']]
148+
# NOTE(cmurphy): if this credential was created using a
149+
# trust-scoped token with impersonation, the user_id will be for
150+
# the trustor, not the trustee. In this case, issuing a
151+
# trust-scoped token to the trustor will fail. In order to get a
152+
# trust-scoped token, use the user ID of the trustee. With
153+
# impersonation, the resulting token will still be for the trustor.
154+
# Without impersonation, the token will be for the trustee.
155+
if trust['impersonation'] is True:
156+
trustee_user_id = trust['trustee_user_id']
157+
elif cred_data['app_cred_id']:
158+
ac_client = PROVIDERS.application_credential_api
159+
app_cred = ac_client.get_application_credential(
160+
cred_data['app_cred_id'])
161+
roles = [r['id'] for r in app_cred['roles']]
162+
elif cred_data['access_token_id']:
163+
access_token = PROVIDERS.oauth_api.get_access_token(
164+
cred_data['access_token_id'])
165+
roles = jsonutils.loads(access_token['role_ids'])
166+
auth_context = {'access_token_id': cred_data['access_token_id']}
167+
else:
168+
roles = PROVIDERS.assignment_api.get_roles_for_user_and_project(
169+
user_ref['id'], project_ref['id'])
142170

143171
if not roles:
144172
raise ks_exceptions.Unauthorized(_('User not valid for project.'))
@@ -149,7 +177,14 @@ def handle_authenticate(self):
149177

150178
method_names = ['ec2credential']
151179

180+
if trustee_user_id:
181+
user_id = trustee_user_id
182+
else:
183+
user_id = user_ref['id']
152184
token = PROVIDERS.token_provider_api.issue_token(
153-
user_id=user_ref['id'], method_names=method_names,
154-
project_id=project_ref['id'])
185+
user_id=user_id, method_names=method_names,
186+
project_id=project_ref['id'],
187+
trust_id=cred_data['trust_id'],
188+
app_cred_id=cred_data['app_cred_id'],
189+
auth_context=auth_context)
155190
return token

keystone/api/credentials.py

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# This file handles all flask-restful resources for /v3/credentials
1414

1515
import hashlib
16+
import six
1617

1718
import flask
1819
from oslo_serialization import jsonutils
@@ -60,30 +61,41 @@ def _blob_to_json(ref):
6061
ref['blob'] = jsonutils.dumps(blob)
6162
return ref
6263

63-
def _assign_unique_id(self, ref, trust_id=None):
64+
def _validate_blob_json(self, ref):
65+
try:
66+
blob = jsonutils.loads(ref.get('blob'))
67+
except (ValueError, TabError):
68+
raise exception.ValidationError(
69+
message=_('Invalid blob in credential'))
70+
if not blob or not isinstance(blob, dict):
71+
raise exception.ValidationError(attribute='blob',
72+
target='credential')
73+
if blob.get('access') is None:
74+
raise exception.ValidationError(attribute='access',
75+
target='credential')
76+
return blob
77+
78+
def _assign_unique_id(
79+
self, ref, trust_id=None, app_cred_id=None, access_token_id=None):
6480
# Generates an assigns a unique identifier to a credential reference.
6581
if ref.get('type', '').lower() == 'ec2':
66-
try:
67-
blob = jsonutils.loads(ref.get('blob'))
68-
except (ValueError, TabError):
69-
raise exception.ValidationError(
70-
message=_('Invalid blob in credential'))
71-
if not blob or not isinstance(blob, dict):
72-
raise exception.ValidationError(attribute='blob',
73-
target='credential')
74-
if blob.get('access') is None:
75-
raise exception.ValidationError(attribute='access',
76-
target='credential')
77-
82+
blob = self._validate_blob_json(ref)
7883
ref = ref.copy()
7984
ref['id'] = hashlib.sha256(
8085
blob['access'].encode('utf8')).hexdigest()
81-
# update the blob with the trust_id, so credentials created with
82-
# a trust scoped token will result in trust scoped tokens when
83-
# authentication via ec2tokens happens
86+
# update the blob with the trust_id or app_cred_id, so credentials
87+
# created with a trust- or app cred-scoped token will result in
88+
# trust- or app cred-scoped tokens when authentication via
89+
# ec2tokens happens
8490
if trust_id is not None:
8591
blob['trust_id'] = trust_id
8692
ref['blob'] = jsonutils.dumps(blob)
93+
if app_cred_id is not None:
94+
blob['app_cred_id'] = app_cred_id
95+
ref['blob'] = jsonutils.dumps(blob)
96+
if access_token_id is not None:
97+
blob['access_token_id'] = access_token_id
98+
ref['blob'] = jsonutils.dumps(blob)
8799
return ref
88100
else:
89101
return super(CredentialResource, self)._assign_unique_id(ref)
@@ -146,23 +158,47 @@ def post(self):
146158
)
147159
validation.lazy_validate(schema.credential_create, credential)
148160
trust_id = getattr(self.oslo_context, 'trust_id', None)
161+
app_cred_id = getattr(
162+
self.auth_context['token'], 'application_credential_id', None)
163+
access_token_id = getattr(
164+
self.auth_context['token'], 'access_token_id', None)
149165
ref = self._assign_unique_id(
150-
self._normalize_dict(credential), trust_id=trust_id)
151-
ref = PROVIDERS.credential_api.create_credential(ref['id'], ref,
152-
initiator=self.audit_initiator)
166+
self._normalize_dict(credential),
167+
trust_id=trust_id, app_cred_id=app_cred_id,
168+
access_token_id=access_token_id)
169+
ref = PROVIDERS.credential_api.create_credential(
170+
ref['id'], ref, initiator=self.audit_initiator)
153171
return self.wrap_member(ref), http_client.CREATED
154172

173+
def _validate_blob_update_keys(self, credential, ref):
174+
if credential.get('type', '').lower() == 'ec2':
175+
new_blob = self._validate_blob_json(ref)
176+
old_blob = credential.get('blob')
177+
if isinstance(old_blob, six.string_types):
178+
old_blob = jsonutils.loads(old_blob)
179+
# if there was a scope set, prevent changing it or unsetting it
180+
for key in ['trust_id', 'app_cred_id', 'access_token_id']:
181+
if old_blob.get(key) != new_blob.get(key):
182+
message = _('%s can not be updated for credential') % key
183+
raise exception.ValidationError(message=message)
184+
155185
def patch(self, credential_id):
156186
# Update Credential
157187
ENFORCER.enforce_call(
158188
action='identity:update_credential',
159189
build_target=_build_target_enforcement
160190
)
161-
PROVIDERS.credential_api.get_credential(credential_id)
191+
current = PROVIDERS.credential_api.get_credential(credential_id)
162192

163193
credential = self.request_body_json.get('credential', {})
164194
validation.lazy_validate(schema.credential_update, credential)
195+
self._validate_blob_update_keys(current.copy(), credential.copy())
165196
self._require_matching_id(credential)
197+
# Check that the user hasn't illegally modified the owner or scope
198+
target = {'credential': dict(current, **credential)}
199+
ENFORCER.enforce_call(
200+
action='identity:update_credential', target_attr=target
201+
)
166202
ref = PROVIDERS.credential_api.update_credential(
167203
credential_id, credential)
168204
return self.wrap_member(ref)

keystone/api/users.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,25 @@ def _normalize_role_list(app_cred_roles):
568568
role['name']))
569569
return roles
570570

571+
def _get_roles(self, app_cred_data, token):
572+
if app_cred_data.get('roles'):
573+
roles = self._normalize_role_list(app_cred_data['roles'])
574+
# NOTE(cmurphy): The user is not allowed to add a role that is not
575+
# in their token. This is to prevent trustees or application
576+
# credential users from escallating their privileges to include
577+
# additional roles that the trustor or application credential
578+
# creator has assigned on the project.
579+
token_roles = [r['id'] for r in token.roles]
580+
for role in roles:
581+
if role['id'] not in token_roles:
582+
detail = _('Cannot create an application credential with '
583+
'unassigned role')
584+
raise ks_exception.ApplicationCredentialValidationError(
585+
detail=detail)
586+
else:
587+
roles = token.roles
588+
return roles
589+
571590
def get(self, user_id):
572591
"""List application credentials for user.
573592
@@ -603,8 +622,7 @@ def post(self, user_id):
603622
app_cred_data['secret'] = self._generate_secret()
604623
app_cred_data['user_id'] = user_id
605624
app_cred_data['project_id'] = project_id
606-
app_cred_data['roles'] = self._normalize_role_list(
607-
app_cred_data.get('roles', token.roles))
625+
app_cred_data['roles'] = self._get_roles(app_cred_data, token)
608626
if app_cred_data.get('expires_at'):
609627
app_cred_data['expires_at'] = utils.parse_expiration_date(
610628
app_cred_data['expires_at'])

keystone/tests/unit/test_v3_application_credential.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,37 @@ def test_create_application_credential_with_application_credential(self):
166166
expected_status_code=http_client.FORBIDDEN,
167167
headers={'X-Auth-Token': token})
168168

169+
def test_create_application_credential_with_trust(self):
170+
second_role = unit.new_role_ref(name='reader')
171+
PROVIDERS.role_api.create_role(second_role['id'], second_role)
172+
PROVIDERS.assignment_api.add_role_to_user_and_project(
173+
self.user_id, self.project_id, second_role['id'])
174+
with self.test_client() as c:
175+
pw_token = self.get_scoped_token()
176+
# create a self-trust - only the roles are important for this test
177+
trust_ref = unit.new_trust_ref(
178+
trustor_user_id=self.user_id,
179+
trustee_user_id=self.user_id,
180+
project_id=self.project_id,
181+
role_ids=[second_role['id']])
182+
resp = c.post('/v3/OS-TRUST/trusts',
183+
headers={'X-Auth-Token': pw_token},
184+
json={'trust': trust_ref})
185+
trust_id = resp.json['trust']['id']
186+
trust_auth = self.build_authentication_request(
187+
user_id=self.user_id,
188+
password=self.user['password'],
189+
trust_id=trust_id)
190+
trust_token = self.v3_create_token(
191+
trust_auth).headers['X-Subject-Token']
192+
app_cred = self._app_cred_body(roles=[{'id': self.role_id}])
193+
# only the roles from the trust token should be allowed, even if
194+
# the user has the role assigned on the project
195+
c.post('/v3/users/%s/application_credentials' % self.user_id,
196+
headers={'X-Auth-Token': trust_token},
197+
json=app_cred,
198+
expected_status_code=http_client.BAD_REQUEST)
199+
169200
def test_create_application_credential_allow_recursion(self):
170201
with self.test_client() as c:
171202
roles = [{'id': self.role_id}]

0 commit comments

Comments
 (0)