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
2020-10-28 21:05:43 +00:00 · 2020-10-28 21:05:43 +00:00 · 73203a91b6
parent f9f6599472
commit 73203a91b6
7 changed files with 520 additions and 28 deletions
--- 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

--- 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
--- 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),
 ]


--- 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()
--- 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
--- a/placement/tests/functional/gabbits/resource-provider-legacy-rbac.yaml
+++ 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
--- a/placement/tests/functional/gabbits/resource-provider-secure-rbac.yaml
+++ 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