Policy does not work for trusts

Bug #1717847 reported by Adrian Turjak on 2017-09-18
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Low
Unassigned

Bug Description

see: http://lists.openstack.org/pipermail/openstack-dev/2017-September/122115.html

In short, the trusts APIs handle their policy in code rather than from the policy file.

This is rather confusing seeing as we have policies for trusts in the policy json file which do nothing:
https://github.com/openstack/keystone/blob/master/etc/policy.v3cloudsample.json#L137-L142

We should set better default policies, and change the code to respect the policy files rather than handle the policy checking based on hardcoded values.

This change needs to be handled carefully (and made very obvious in release notes), because anyone using an older policy file once the change to respect the policy file is part of a release, will mean any authed user can list trusts because of the existing (and incorrect) default policy rules.

Matthew Edmonds (edmondsw) wrote :

The keystone.common.controller.protected decorator is used on the create_token, etc. methods in keystone.trust.controllers.TrustV3, so it certainly looks like policy is being checked. If you customize those rules to require a specific role (e.g. "role:admin" instead of "" for list_trusts) are you still able to perform the action regardless of your role?

I think what you're indicating is that we have hard-coded *additional* checks, and that is intentional. In fact the default for create_trust is the only one that's wrong here. That should be "", with the user_id restriction hard-coded rather than left to policy.

Scope checks (e.g. project of token matches project of target resource) should always be hard-coded. This is because there is only one correct scope check per action. Allowing scope checks to be configurable only makes policy more complicated for operators, with no benefits since there is only one secure answer. Keystone (and other projects) have not been consistent about it in the past, which we're trying to address, but for everything except create_trust it appears that keystone has done better than usual when it comes to trusts.

Adrian Turjak (adriant-y) wrote :

I don't really think this is much of an issue. I was only checking because by default the policy is what controls most of the APIs in keystone, so seeing a "" policy made me worry enough to check.

While I get that chances are you probably don't want to be do anything too different, and sensible defaults are important, it feels weird to hardcode this API when very few of the other Keystone APIs have any such hardcoding (apart from a few exceptions). For a lot of them, you can make the policy blank and suddenly your whole cloud is exposed. That is bad, but it also means deployers have more power to tweak their policy.

Hardcoding of policy makes it harder to use different names for roles for one, and having this API be an outlier that isn't really affected by policy is just odd.

For my purposes, this API is secure, and I don't really care beyond that too much, so it's up to the Keystone team if they feel that hardcoding policy in this case really is a good idea or not. I lean towards not especially when dealing with roles, but things like APIs returning things based on scope by default is fine.

What I'm curious of is are any of these hard coded policies documented somewhere? I tried looking but didn't find anything.

Lance Bragstad (lbragstad) wrote :

To the best of my knowledge, this is highly specific to trusts. For trusts, certain scope-like things were hardcoded into the actual API because we didn't really have a good way to do it in policy.json format (which probably ended up being a good thing).

As of today, we are working on tools that will help us move to sane default policies in a way operators can consume [0].

If we keep this open, we could target this specific bug to leveraging that work.

[0] https://specs.openstack.org/openstack/oslo-specs/specs/queens/policy-deprecation.html

Changed in keystone:
status: New → Confirmed
importance: Undecided → Low
Morgan Fainberg (mdrnstm) wrote :

I'm going to mark this as opinion. It likely will get better with scope-types and policy-in-code, but this bug in itself isn't relevant due to how trusts were architected.

Changed in keystone:
status: Confirmed → Opinion
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers