From 73203a91b63488cf649d430f75c76e027a5fc717 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Wed, 28 Oct 2020 21:05:43 +0000 Subject: [PATCH] Implement secure RBAC for resource providers This commit updates the policies for the resource providers in placement to support read-only roles. This is part of a broader community effort to support read-only roles and implement secure, consistent default policies. This commit also introduces some testing infrastructure and plumbing that is useful for implementing protection tests with gabbi. Including testing for deprecated policies. Change-Id: Icc904bf325eaa60377171a22a86932135c384e6a --- placement/auth.py | 12 +- placement/policies/base.py | 6 + placement/policies/resource_provider.py | 92 +++++--- placement/policy.py | 2 + .../tests/functional/fixtures/gabbits.py | 24 ++ .../resource-provider-legacy-rbac.yaml | 210 ++++++++++++++++++ .../resource-provider-secure-rbac.yaml | 202 +++++++++++++++++ 7 files changed, 520 insertions(+), 28 deletions(-) create mode 100644 placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml create mode 100644 placement/tests/functional/gabbits/resource-provider-secure-rbac.yaml diff --git a/placement/auth.py b/placement/auth.py index df7dd7b1a..e5f263713 100644 --- a/placement/auth.py +++ b/placement/auth.py @@ -28,7 +28,10 @@ class Middleware(object): self.application = application -# NOTE(cdent): Only to be used in tests where auth is being faked. +# NOTE(cdent): Only to be used in tests where auth is being faked. This +# middleware can be used to mimic keystonemiddleware auth_token middleware, +# which is important for building API protection tests without an external +# dependency on keystone. class NoAuthMiddleware(Middleware): """Require a token if one isn't present.""" @@ -46,12 +49,15 @@ class NoAuthMiddleware(Middleware): token = req.headers['X-Auth-Token'] user_id, _sep, project_id = token.partition(':') project_id = project_id or user_id - if user_id == 'admin': + if 'HTTP_X_ROLES' in req.environ.keys(): + roles = req.headers['X_ROLES'].split(',') + elif user_id == 'admin': roles = ['admin'] else: roles = [] req.headers['X_USER_ID'] = user_id - req.headers['X_TENANT_ID'] = project_id + if not req.headers.get('OPENSTACK_SYSTEM_SCOPE'): + req.headers['X_TENANT_ID'] = project_id req.headers['X_ROLES'] = ','.join(roles) return self.application diff --git a/placement/policies/base.py b/placement/policies/base.py index 1e728a37f..4d2bf5f29 100644 --- a/placement/policies/base.py +++ b/placement/policies/base.py @@ -13,6 +13,12 @@ from oslo_policy import policy RULE_ADMIN_API = 'rule:admin_api' +# NOTE(lbragstad): We might consider converting these generic checks into +# RuleDefaults or DocumentedRuleDefaults, but we need to thoroughly vet the +# approach in oslo.policy and consume a new version. Until we have that done, +# let's continue using generic check strings. +SYSTEM_ADMIN = 'role:admin and system_scope:all' +SYSTEM_READER = 'role:reader and system_scope:all' rules = [ # "placement" is the default rule (action) used for all routes that do diff --git a/placement/policies/resource_provider.py b/placement/policies/resource_provider.py index e751b6fde..8c1a48cf2 100644 --- a/placement/policies/resource_provider.py +++ b/placement/policies/resource_provider.py @@ -11,6 +11,7 @@ # under the License. +from oslo_log import versionutils from oslo_policy import policy from placement.policies import base @@ -23,62 +24,103 @@ SHOW = PREFIX % 'show' UPDATE = PREFIX % 'update' DELETE = PREFIX % 'delete' +DEPRECATED_REASON = """ +The resource provider API now supports a read-only role by default. +""" + +deprecated_list_resource_providers = policy.DeprecatedRule( + name=LIST, + check_str=base.RULE_ADMIN_API +) +deprecated_show_resource_provider = policy.DeprecatedRule( + name=SHOW, + check_str=base.RULE_ADMIN_API +) +deprecated_create_resource_provider = policy.DeprecatedRule( + name=CREATE, + check_str=base.RULE_ADMIN_API +) +deprecated_update_resource_provider = policy.DeprecatedRule( + name=UPDATE, + check_str=base.RULE_ADMIN_API +) +deprecated_delete_resource_provider = policy.DeprecatedRule( + name=DELETE, + check_str=base.RULE_ADMIN_API +) + + rules = [ policy.DocumentedRuleDefault( - LIST, - base.RULE_ADMIN_API, - "List resource providers.", - [ + name=LIST, + check_str=base.SYSTEM_READER, + description="List resource providers.", + operations=[ { 'method': 'GET', 'path': '/resource_providers' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=deprecated_list_resource_providers, + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY), policy.DocumentedRuleDefault( - CREATE, - base.RULE_ADMIN_API, - "Create resource provider.", - [ + name=CREATE, + check_str=base.SYSTEM_ADMIN, + description="Create resource provider.", + operations=[ { 'method': 'POST', 'path': '/resource_providers' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=deprecated_create_resource_provider, + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY), policy.DocumentedRuleDefault( - SHOW, - base.RULE_ADMIN_API, - "Show resource provider.", - [ + name=SHOW, + check_str=base.SYSTEM_READER, + description="Show resource provider.", + operations=[ { 'method': 'GET', 'path': '/resource_providers/{uuid}' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=deprecated_show_resource_provider, + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY), policy.DocumentedRuleDefault( - UPDATE, - base.RULE_ADMIN_API, - "Update resource provider.", - [ + name=UPDATE, + check_str=base.SYSTEM_ADMIN, + description="Update resource provider.", + operations=[ { 'method': 'PUT', 'path': '/resource_providers/{uuid}' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=deprecated_update_resource_provider, + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY), policy.DocumentedRuleDefault( - DELETE, - base.RULE_ADMIN_API, - "Delete resource provider.", - [ + name=DELETE, + check_str=base.SYSTEM_ADMIN, + description="Delete resource provider.", + operations=[ { 'method': 'DELETE', 'path': '/resource_providers/{uuid}' } ], - scope_types=['system']), + scope_types=['system'], + deprecated_rule=deprecated_delete_resource_provider, + deprecated_reason=DEPRECATED_REASON, + deprecated_since=versionutils.deprecated.WALLABY), ] diff --git a/placement/policy.py b/placement/policy.py index 4701468c2..96ca66634 100644 --- a/placement/policy.py +++ b/placement/policy.py @@ -107,6 +107,8 @@ def authorize(context, action, target, do_raise=True): except policy.PolicyNotRegistered: with excutils.save_and_reraise_exception(): LOG.exception('Policy not registered') + except policy.InvalidScope: + raise exception.PolicyNotAuthorized(action) except Exception: with excutils.save_and_reraise_exception(): credentials = context.to_policy_values() diff --git a/placement/tests/functional/fixtures/gabbits.py b/placement/tests/functional/fixtures/gabbits.py index 53a8413e9..79e3d6075 100644 --- a/placement/tests/functional/fixtures/gabbits.py +++ b/placement/tests/functional/fixtures/gabbits.py @@ -50,6 +50,9 @@ def setup_app(): class APIFixture(fixture.GabbiFixture): """Setup the required backend fixtures for a basic placement service.""" + # TODO(stephenfin): Remove this once we drop the deprecated policy rules + _secure_rbac = False + def start_fixture(self): global CONF # Set up stderr and stdout captures by directly driving the @@ -72,6 +75,11 @@ class APIFixture(fixture.GabbiFixture): self.conf_fixture.setUp() conf.register_opts(self.conf_fixture.conf) self.conf_fixture.config(group='api', auth_strategy='noauth2') + self.conf_fixture.config( + group='oslo_policy', + enforce_scope=self._secure_rbac, + enforce_new_defaults=self._secure_rbac, + ) self.placement_db_fixture = fixtures.Database( self.conf_fixture, set_config=True) @@ -748,3 +756,19 @@ class OpenPolicyFixture(APIFixture): def stop_fixture(self): super(OpenPolicyFixture, self).stop_fixture() + + +class SecureRBACPolicyFixture(APIFixture): + """An APIFixture that enforce secure default policies and scope.""" + + _secure_rbac = True + + +# Even though this just configures the defaults for enforce_scope and +# enforce_new_default, it's useful because it's explicit in saying we're +# testing old policy behavior. We can remove this once placement removes its +# deprecated policies. +class LegacyRBACPolicyFixture(APIFixture): + """An APIFixture that enforce deprecated policies.""" + + _secure_rbac = False diff --git a/placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml b/placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml new file mode 100644 index 000000000..d9de51e0f --- /dev/null +++ b/placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml @@ -0,0 +1,210 @@ +--- +fixtures: + - LegacyRBACPolicyFixture + +vars: + - &project_id $ENVIRON['PROJECT_ID'] + - &system_admin_headers + x-auth-token: user + x-roles: admin,member,reader + accept: application/json + content-type: application/json + openstack-api-version: placement latest + openstack-system-scope: all + - &system_reader_headers + x-auth-token: user + x-roles: reader + accept: application/json + content-type: application/json + openstack-api-version: placement latest + openstack-system-scope: all + - &project_admin_headers + x-auth-token: user + x-roles: admin,member,reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + - &project_member_headers + x-auth-token: user + x-roles: member,reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + - &project_reader_headers + x-auth-token: user + x-roles: reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + +tests: + +- name: system admin can list resource providers + GET: /resource_providers + request_headers: *system_admin_headers + response_json_paths: + $.resource_providers: [] + +- name: system reader can list resource providers + GET: /resource_providers + request_headers: *system_reader_headers + response_json_paths: + $.resource_providers: [] + +- name: project admin can list resource providers + GET: /resource_providers + request_headers: *project_admin_headers + response_json_paths: + $.resource_providers: [] + +- name: project member cannot list resource providers + GET: /resource_providers + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot list resource providers + GET: /resource_providers + request_headers: *project_reader_headers + status: 403 + +- name: system admin can create resource providers + POST: /resource_providers + request_headers: *system_admin_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 200 + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader cannot create resource providers + POST: /resource_providers + request_headers: *system_reader_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: system admin can delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + status: 204 + +- name: project admin can create resource providers + POST: /resource_providers + request_headers: *project_admin_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: project member cannot create resource providers + POST: /resource_providers + request_headers: *project_member_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: project reader cannot create resource providers + POST: /resource_providers + request_headers: *project_reader_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: system admin can show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader can show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: project admin can show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: project member cannot show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + status: 403 + +- name: system admin can update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + data: + name: new name + status: 200 + response_json_paths: + $.name: new name + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + data: + name: new name + status: 403 + +- name: project admin can update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + data: + name: new name + status: 200 + response_json_paths: + $.name: new name + $.uuid: $ENVIRON['RP_UUID'] + +- name: project member cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + data: + name: new name + status: 403 + +- name: project reader cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + data: + name: new name + status: 403 + +- name: system reader cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + status: 403 + +- name: project member cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + status: 403 + +- name: project admin can delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + status: 204 + +# We tested that system admins can delete resource providers above diff --git a/placement/tests/functional/gabbits/resource-provider-secure-rbac.yaml b/placement/tests/functional/gabbits/resource-provider-secure-rbac.yaml new file mode 100644 index 000000000..0013a291b --- /dev/null +++ b/placement/tests/functional/gabbits/resource-provider-secure-rbac.yaml @@ -0,0 +1,202 @@ +--- +fixtures: + - SecureRBACPolicyFixture + +vars: + - &project_id $ENVIRON['PROJECT_ID'] + - &system_admin_headers + x-auth-token: user + x-roles: admin,member,reader + accept: application/json + content-type: application/json + openstack-api-version: placement latest + openstack-system-scope: all + - &system_reader_headers + x-auth-token: user + x-roles: reader + accept: application/json + content-type: application/json + openstack-api-version: placement latest + openstack-system-scope: all + - &project_admin_headers + x-auth-token: user + x-roles: admin,member,reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + - &project_member_headers + x-auth-token: user + x-roles: member,reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + - &project_reader_headers + x-auth-token: user + x-roles: reader + x-project-id: *project_id + accept: application/json + content-type: application/json + openstack-api-version: placement latest + +tests: + +- name: system admin can list resource providers + GET: /resource_providers + request_headers: *system_admin_headers + response_json_paths: + $.resource_providers: [] + +- name: system reader can list resource providers + GET: /resource_providers + request_headers: *system_reader_headers + response_json_paths: + $.resource_providers: [] + +- name: project admin cannot list resource providers + GET: /resource_providers + request_headers: *project_admin_headers + status: 403 + +- name: project member cannot list resource providers + GET: /resource_providers + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot list resource providers + GET: /resource_providers + request_headers: *project_reader_headers + status: 403 + +- name: system admin can create resource providers + POST: /resource_providers + request_headers: *system_admin_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 200 + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader cannot create resource providers + POST: /resource_providers + request_headers: *system_reader_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: project admin cannot create resource providers + POST: /resource_providers + request_headers: *project_admin_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: project member cannot create resource providers + POST: /resource_providers + request_headers: *project_member_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: project reader cannot create resource providers + POST: /resource_providers + request_headers: *project_reader_headers + data: + name: $ENVIRON['RP_NAME'] + uuid: $ENVIRON['RP_UUID'] + status: 403 + +- name: system admin can show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader can show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + response_json_paths: + $.uuid: $ENVIRON['RP_UUID'] + +- name: project admin cannot show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + status: 403 + +- name: project member cannot show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot show resource provider + GET: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + status: 403 + +- name: system admin can update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + data: + name: new name + status: 200 + response_json_paths: + $.name: new name + $.uuid: $ENVIRON['RP_UUID'] + +- name: system reader cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + data: + name: new name + status: 403 + +- name: project admin cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + data: + name: new name + status: 403 + +- name: project member cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + data: + name: new name + status: 403 + +- name: project reader cannot update resource provider + PUT: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + data: + name: new name + status: 403 + +- name: system reader cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_reader_headers + status: 403 + +- name: project admin cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_admin_headers + status: 403 + +- name: project member cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_member_headers + status: 403 + +- name: project reader cannot delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *project_reader_headers + status: 403 + +- name: system admin can delete resource provider + DELETE: /resource_providers/$ENVIRON['RP_UUID'] + request_headers: *system_admin_headers + status: 204