RBAC Testing Multiple Policies

bp rbac-testing-multiple-policies

Problem Description

Patrole currently RBAC tests an API endpoint by checking whether a policy action is allowed, according to oslo.policy and then executes the API endpoint that does policy enforcement with the role specified under CONF.rbac.rbac_test_role. However, this approach does not account for API endpoints that enforce multiple policy actions, either directly (within the implementation of the API endpoint itself) or indirectly (across different helper functions and API endpoints). The current approach to RBAC testing in Patrole, therefore, does not always provide complete policy coverage. Just like multiple calls are made to oslo.policy by various endpoints, Patrole should do the same.

For example, take an API that enforces 2 policy actions, A and B, where A is admin_api and B is admin_or_owner. Calling the API with rbac_test_role as admin role will necessarily pass, because admin role has permissions to execute policy actions A and B and will also be able to execute the API endpoint. However, the test will fail for non-admin role, with the rbac_rule_validation decorator evaluating only policy action B. This is because a non-admin role (i.e. Member) role has permissions to perform policy action B (which is admin_or_owner) but does not have permissions to execute the API endpoint, since the endpoint enforces an admin_api policy: this results in a Forbidden exception being raised, and the test failing.

Proposed Change

The proposed change is to modify the rbac_rule_validation decorator to be able to take a list of policy actions, rather than just one policy action. For each policy action, a call will be made to oslo.policy to confirm whether the test role is allowed to perform the action. Each result from oslo.policy will be logical-ANDed together. For example, if policy action A evaluates to True and policy action B evaluates to False, then the final outcome is False: therefore, the user should not be able to perform the API call successfully. As such, Patrole can deduce whether a role is allowed to call an API that enforces multiple policies.

To provide a concrete example, the following test:

@rbac_rule_validation.action(
    service="nova",
    rule="os_compute_api:os-lock-server:unlock:unlock_override")
def test_unlock_server_override(self):
    server = self.create_test_server(wait_until='ACTIVE')
    # In order to trigger the unlock:unlock_override policy instead
    # of the unlock policy, the server must be locked by a different
    # user than the one who is attempting to unlock it.
    self.os_admin.servers_client.lock_server(server['id'])
    self.addCleanup(self.servers_client.unlock_server, server['id'])

    self.rbac_utils.switch_role(self, toggle_rbac_role=True)
    self.servers_client.unlock_server(server['id'])

can be changed to:

@rbac_rule_validation.action(
    service="nova",
    rules=["os_compute_api:os-lock-server:unlock",
           "os_compute_api:os-lock-server:unlock:unlock_override"])
def test_unlock_server_override(self):
    server = self.create_test_server(wait_until='ACTIVE')
    self.os_admin.servers_client.lock_server(server['id'])
    self.addCleanup(self.servers_client.unlock_server, server['id'])

    self.rbac_utils.switch_role(self, toggle_rbac_role=True)
    self.servers_client.unlock_server(server['id'])

According to the Nova documentation for locking a server, the “unlock_override” policy is “performed only after the check os_compute_api:os-lock-server:unlock passes”. With this change, Patrole will generate its “expected” result based on whether the test role can perform all the policies passed to rules; otherwise, if the test role cannot perform at least one policy, the expected result will be False. Afterward, the API action will be called with the test role and the outcome of which will be compared with the expected result.

If the expected and actual results match, then the test will pass. Otherwise, Patrole can generate a detailed error message explaining which policies passed to rules caused test failure. For example, in the above example, if the test role has permissions to perform “os_compute_api:os-lock-server:unlock” but not “os_compute_api:os-lock-server:unlock:unlock_override”, then Patrole will emit an error saying that “os_compute_api:os-lock-server:unlock:unlock_override” was responsible for test failure. This will help cloud deployers and developers to determine the source of test failure and to pinpoint inconsistent custom policy configurations.

Alternatives

Currently, there are no other viable alternatives. It is not feasible or desirable to repeatedly call each API endpoint against each policy action that the endpoint enforces, for various fairly obvious reasons:

  1. Code redundancy should be minimized, to make code readability and maintenance easier.

  2. This introduces serious run time concerns in Patrole’s gates.

Security Impact

None.

Notifications Impact

LOG statements will have to be updated to convey multiple policy actions to the user, especially following test failure.

If a Patrole test tests may policies, after test failure, it would be useful for users for Patrole to log which policies caused the test failure. This can be determined by iteratively calling oslo.policy for each policy provided to the rbac_rule_validation decorator and storing the list of policies that are not compatible with the role and the expected test outcome.

Other End User Impact

None.

Performance Impact

The performance impact is negligible. This change will result in barely slower test run time, because multiple calls will be made to oslo.policy rather than just one, per Patrole test.

Other Deployer Impact

None.

Developer Impact

The proposed change requires that developers be prudent about which policy actions they include in the proposed actions parameter. Including an excessively high number of policy actions is not maintainable and is cumbersome from a development standpoint. For example, Cinder enforces volume_extension:volume_host_attribute and volume_extension:volume_mig_status_attribute, along with a number of different policy actions, for many, many endpoints. Repeating these policy actions for every Cinder RBAC test would be redundant and bad design. (If it could be proven that these policy actions are enforced for every Cinder API endpoint, then the policy actions could be auto-injected by the Patrole framework and logical-ANDed with the policy actions explicitly specified in actions. However, this approach goes beyond the scope of this spec).

It is recommended that this enhancement be used judiciously by developers. Only endpoints that enforce multiple relatively unique policy actions should be included in the actions list. Uniqueness can be inferred, for example, from Keystone’s and Nova’s self-documenting in-code policy definitions.

Implementation

Assignee(s)

Primary assignees:
Other contributors:

Work Items

  • Enhance the rbac_rule_validation decorator with the actions parameter and deprecate the rule parameter.

  • Write a helper function in rbac_rule_validation to iteratively call rbac_policy_parser.RbacPolicyParser.allowed for each policy action specified in actions, logically ANDing them together, and returning the result to rbac_rule_validation decorator.

  • Refactoring tests to use actions instead of rule.

  • Writing new unit tests to test the proposed enhancement.

  • Selectively adding multiple policy actions to some tests.

  • Confirming that all API tests work with the proposed enhancement.

  • Updating documentation.

Dependencies

None.

Documentation Impact

Patrole documentation should be updated to convey the new parameter along with intended use, as described in this spec.

References