Skip to content

Commit 40cbb7b

Browse files
Zuulopenstack-gerrit
authored andcommitted
Merge "Fix security issues with EC2 credentials" into stable/train
2 parents d028dfb + 5459054 commit 40cbb7b

File tree

8 files changed

+606
-62
lines changed

8 files changed

+606
-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
@@ -559,6 +559,25 @@ def _normalize_role_list(app_cred_roles):
559559
role['name']))
560560
return roles
561561

562+
def _get_roles(self, app_cred_data, token):
563+
if app_cred_data.get('roles'):
564+
roles = self._normalize_role_list(app_cred_data['roles'])
565+
# NOTE(cmurphy): The user is not allowed to add a role that is not
566+
# in their token. This is to prevent trustees or application
567+
# credential users from escallating their privileges to include
568+
# additional roles that the trustor or application credential
569+
# creator has assigned on the project.
570+
token_roles = [r['id'] for r in token.roles]
571+
for role in roles:
572+
if role['id'] not in token_roles:
573+
detail = _('Cannot create an application credential with '
574+
'unassigned role')
575+
raise ks_exception.ApplicationCredentialValidationError(
576+
detail=detail)
577+
else:
578+
roles = token.roles
579+
return roles
580+
562581
def get(self, user_id):
563582
"""List application credentials for user.
564583
@@ -594,8 +613,7 @@ def post(self, user_id):
594613
app_cred_data['secret'] = self._generate_secret()
595614
app_cred_data['user_id'] = user_id
596615
app_cred_data['project_id'] = project_id
597-
app_cred_data['roles'] = self._normalize_role_list(
598-
app_cred_data.get('roles', token.roles))
616+
app_cred_data['roles'] = self._get_roles(app_cred_data, token)
599617
if app_cred_data.get('expires_at'):
600618
app_cred_data['expires_at'] = utils.parse_expiration_date(
601619
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
@@ -169,6 +169,37 @@ def test_create_application_credential_with_application_credential(self):
169169
expected_status_code=http_client.FORBIDDEN,
170170
headers={'X-Auth-Token': token})
171171

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

0 commit comments

Comments
 (0)