From 64a4242454a65df17abc10e13861463a2de71813 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Douglas=20Mendiz=C3=A1bal?= Date: Mon, 27 Sep 2021 14:40:07 -0500 Subject: [PATCH] Fix secret metadata access rules This patch fixes the legacy policy rules for accessing secret metadata by checking that the user making the request is authenticated for the project that owns the secret. Story: 2009253 Task: 43453 Change-Id: Ide37d64dff10d421817bf90b8e2e58bf6ac4f592 (cherry picked from commit 7d270bacbe29a90a10f1855abc3b50dac0f08022) --- barbican/api/controllers/__init__.py | 9 +++++++++ barbican/api/controllers/secretmeta.py | 4 ++-- barbican/api/controllers/secrets.py | 8 +------- barbican/common/policies/base.py | 3 +++ barbican/common/policies/secretmeta.py | 20 ++++++++++++++++---- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/barbican/api/controllers/__init__.py b/barbican/api/controllers/__init__.py index 26977658f..9cc2d2220 100644 --- a/barbican/api/controllers/__init__.py +++ b/barbican/api/controllers/__init__.py @@ -237,3 +237,12 @@ class ACLMixin(object): acl_dict.update(co_dict) return acl_dict + + +class SecretACLMixin(ACLMixin): + + def get_acl_tuple(self, req, **kwargs): + acl = self.get_acl_dict_for_user(req, self.secret.secret_acls) + acl['project_id'] = self.secret.project.external_id + acl['creator_id'] = self.secret.creator_id + return 'secret', acl diff --git a/barbican/api/controllers/secretmeta.py b/barbican/api/controllers/secretmeta.py index 27c24b79a..8215beac7 100644 --- a/barbican/api/controllers/secretmeta.py +++ b/barbican/api/controllers/secretmeta.py @@ -28,7 +28,7 @@ def _secret_metadata_not_found(): pecan.abort(404, u._('Secret metadata not found.')) -class SecretMetadataController(controllers.ACLMixin): +class SecretMetadataController(controllers.SecretACLMixin): """Handles SecretMetadata requests by a given secret id.""" def __init__(self, secret): @@ -106,7 +106,7 @@ class SecretMetadataController(controllers.ACLMixin): return {'key': key, 'value': value} -class SecretMetadatumController(controllers.ACLMixin): +class SecretMetadatumController(controllers.SecretACLMixin): def __init__(self, secret): LOG.debug('=== Creating SecretMetadatumController ===') diff --git a/barbican/api/controllers/secrets.py b/barbican/api/controllers/secrets.py index a74ff40c7..12d6caf37 100644 --- a/barbican/api/controllers/secrets.py +++ b/barbican/api/controllers/secrets.py @@ -71,7 +71,7 @@ def _request_has_twsk_but_no_transport_key_id(): 'transport key id has not been provided.')) -class SecretController(controllers.ACLMixin): +class SecretController(controllers.SecretACLMixin): """Handles Secret retrieval and deletion requests.""" def __init__(self, secret): @@ -81,12 +81,6 @@ class SecretController(controllers.ACLMixin): self.consumer_repo = repo.get_secret_consumer_repository() self.transport_key_repo = repo.get_transport_key_repository() - def get_acl_tuple(self, req, **kwargs): - d = self.get_acl_dict_for_user(req, self.secret.secret_acls) - d['project_id'] = self.secret.project.external_id - d['creator_id'] = self.secret.creator_id - return 'secret', d - @pecan.expose() def _lookup(self, sub_resource, *remainder): if sub_resource == 'acl': diff --git a/barbican/common/policies/base.py b/barbican/common/policies/base.py index debac8fec..a042a7683 100644 --- a/barbican/common/policies/base.py +++ b/barbican/common/policies/base.py @@ -82,6 +82,9 @@ rules = [ name='secret_project_creator', check_str="rule:creator and rule:secret_project_match and " + "rule:secret_creator_user"), + policy.RuleDefault( + name='secret_project_creator_role', + check_str="rule:creator and rule:secret_project_match"), policy.RuleDefault( name='container_project_admin', check_str="rule:admin and rule:container_project_match"), diff --git a/barbican/common/policies/secretmeta.py b/barbican/common/policies/secretmeta.py index 6e064b075..2b629bf99 100644 --- a/barbican/common/policies/secretmeta.py +++ b/barbican/common/policies/secretmeta.py @@ -17,7 +17,10 @@ _MEMBER = "role:member" rules = [ policy.DocumentedRuleDefault( name='secret_meta:get', - check_str=f'rule:all_but_audit or {_MEMBER}', + check_str='rule:secret_non_private_read or ' + + 'rule:secret_project_creator or ' + + 'rule:secret_project_admin or rule:secret_acl_read or ' + + f'{_MEMBER}', scope_types=['project'], description='metadata/: Lists a secrets user-defined metadata. || ' + 'metadata/{key}: Retrieves a secrets user-added metadata.', @@ -34,7 +37,10 @@ rules = [ ), policy.DocumentedRuleDefault( name='secret_meta:post', - check_str=f'rule:admin_or_creator or {_MEMBER}', + check_str='rule:secret_project_admin or ' + + 'rule:secret_project_creator or ' + + '(rule:secret_project_creator_role and ' + + f'rule:secret_non_private_read) or {_MEMBER}', scope_types=['project'], description='Adds a new key/value pair to the secrets user-defined ' + 'metadata.', @@ -47,7 +53,10 @@ rules = [ ), policy.DocumentedRuleDefault( name='secret_meta:put', - check_str=f'rule:admin_or_creator or {_MEMBER}', + check_str='rule:secret_project_admin or ' + + 'rule:secret_project_creator or ' + + '(rule:secret_project_creator_role and ' + + f'rule:secret_non_private_read) or {_MEMBER}', scope_types=['project'], description='metadata/: Sets the user-defined metadata for a secret ' + '|| metadata/{key}: Updates an existing key/value pair ' + @@ -65,7 +74,10 @@ rules = [ ), policy.DocumentedRuleDefault( name='secret_meta:delete', - check_str=f'rule:admin_or_creator or {_MEMBER}', + check_str='rule:secret_project_admin or ' + + 'rule:secret_project_creator or ' + + '(rule:secret_project_creator_role and ' + + f'rule:secret_non_private_read) or {_MEMBER}', scope_types=['project'], description='Delete secret user-defined metadata by key.', operations=[