Unable to distinguish between not is_admin_project and feature not enabled

[Jamie Lennox]

The is_admin_project flag is used in a token to indicate that the current token is scoped to a project that is indicated as the admin project. Currently this is only added to the token when the admin_project is True and nothing added when False.

From a policy perspective we need to write policy files that are backwards compatible, however we cannot determine the difference in policy between is_admin_project == False and the admin project not being set from config because both result in no flag being set in the token.

If we were to enforce is_admin_project == True on a deployment that did not use it this would completely break backwards compatibility as the is_admin_project check would never pass.

To fix this we need to make is_admin_project a required field of a token at least when the admin project is set.

Because the current default is that every project can be the admin project, the default value of is_admin_project must be True. For deployments that do not configure an admin project we can either set is_admin_project=True for every project, or we can exclude the field from the token. I think exclude makes sense because the check from a policy perspective is going to have to default to True for backwards compatibility and you can then tell from a token whether the admin_project concept is in use (i'm not sure if this is an information leakage problem - please comment on preference).

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/312323

[Jamie Lennox (jamielennox)]

Adding keystoneauth and keystonemiddleware because we need to plumb this information through to a service so that it can actually use it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/314409

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/314409
Committed: https://git.openstack.org/cgit/openstack/keystoneauth/commit/?id=ed7586380732842a0b52236a59499d191033ef11
Submitter: Jenkins
Branch: master

commit ed7586380732842a0b52236a59499d191033ef11
Author: Jamie Lennox <email address hidden>
Date: Tue May 10 14:10:52 2016 +1000

    Expose is_admin_project in AccessInfo

    There is currently incomplete is_admin_project information in the token.
    We can expose this already via keystoneauth because we have to handle
    the default case where there is nothing in the token.

    The default feels backwards but to handle the historical situation where
    a deployment has not got the admin_project set all projects were in the
    admin project so it must default to true for policy enforcement.

    Adds the fixture handling as well for testing with this enabled.

    Change-Id: I58db52427a2bac6cd56794429559771499dc7f5a
    Closes-Bug: #1577996

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/312323
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ed634e8cdcdf385b749bbb9e951104989a020277
Submitter: Jenkins
Branch: master

commit ed634e8cdcdf385b749bbb9e951104989a020277
Author: Jamie Lennox <email address hidden>
Date: Wed May 4 14:30:56 2016 +1000

    Always add is_admin_project if admin project defined

    By only setting is_admin_project in the token if it is true we are
    unable to distinguish in policy enforcement if the admin project is not
    defined in configuration or if the current scope is not the admin
    project.

    If the admin project is defined in config we should always set the
    is_admin_project in the token either true or false so we can provide
    backwards compatible policy files in projects.

    Change-Id: Icdfc4f4792422af9d844004c2c92993c9065134d
    Closes-Bug: #1577996

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/keystoneauth 2.7.0 release.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/keystone 10.0.0.0b1 development milestone.

[Jamie Lennox (jamielennox)]

requirements bump: https://review.openstack.org/#/c/330884/

then we can start work on keystonemiddleware, then oslo.context, then oslo.policy. fun.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/331374

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/331374
Committed: https://git.openstack.org/cgit/openstack/keystonemiddleware/commit/?id=0562670d4e56c257aec8db5a2bb651b5e59fddb2
Submitter: Jenkins
Branch: master

commit 0562670d4e56c257aec8db5a2bb651b5e59fddb2
Author: Jamie Lennox <email address hidden>
Date: Sat Jun 18 09:14:26 2016 +1000

    Pass X_IS_ADMIN_PROJECT header from auth_token

    To do policy enforcement around admin projects we need for auth_token
    middleware to pass this information down to context objects.

    Closes-Bug: #1577996
    Change-Id: Ic680e6eaa683926914cf4b2152ec3bb67c6601ff

[Jamie Lennox (jamielennox)]

oslo.context review: https://review.openstack.org/#/c/331916/2

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/keystonemiddleware 4.6.0 release.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/oslo.context 2.6.0 release.

[Lance Bragstad (lbragstad)]

Hi Jamie,

Any update on the bits needed for oslo.policy?

[Jamie Lennox (jamielennox)]

Lance:

So the plan was to maybe add a new rule to oslo.policy like just is_admin_project or something - however as part of the oslo.context refactoring and making that common between projects we now have {'is_admin_project': True/False} as part of most context dicts. This means that you can enforce policy just as is_admin_project:True in your policy files.

I guess it would be slightly neater to have this as a policy rule because it's unlikely you'd ever check for False but it hasn't been a priority.

Do you think it worth chasing?

[Lance Bragstad (lbragstad)]

Thanks for the update Jamie. I certainly wouldn't be opposed to seeing a WIP patch or anything like that. With how many folks are working on policy across the various projects, we could find a way to address the new rule.

[Lance Bragstad (lbragstad)]

With the work on system scope, I'm wondering how much more needs to be done here. Unfortunately, it looks like some of this was done prior to the original system scope discussions held in Denver (Sept, 2017).

[Jamie Lennox (jamielennox)]

There are a couple of projects already enforcing on is_admin_project, they just do it manually and the outstanding oslo.policy bit was just a helper to make that a bit neater.

I'm not sure what the migration plan is from is_admin_project -> system scope. My guess would be the admin_project is the same as system for a while? If so it would probably be worth waiting to see what happens in oslo.policy to check system scope and just include the is_admin_project compatibility in that. No point giving a new feature under an old name.

[Adam Young (ayoung)]

I've been letting Lance drive it since it is his choice to drive on with
system scoping. What projects are checking is-admin-project so far?

On Wed, Jun 13, 2018, 10:40 PM Jamie Lennox <email address hidden> wrote:

> There are a couple of projects already enforcing on is_admin_project,
> they just do it manually and the outstanding oslo.policy bit was just a
> helper to make that a bit neater.
>
> I'm not sure what the migration plan is from is_admin_project -> system
> scope. My guess would be the admin_project is the same as system for a
> while? If so it would probably be worth waiting to see what happens in
> oslo.policy to check system scope and just include the is_admin_project
> compatibility in that. No point giving a new feature under an old name.
>
> --
> You received this bug notification because you are subscribed to
> OpenStack Identity (keystone).
> Matching subscriptions: keystone-bugs
> https://bugs.launchpad.net/bugs/1577996
>
> Title:
> Unable to distinguish between not is_admin_project and feature not
> enabled
>
> Status in OpenStack Identity (keystone):
> Fix Released
> Status in keystoneauth:
> Fix Released
> Status in keystonemiddleware:
> Fix Released
> Status in oslo.context:
> Fix Released
> Status in oslo.policy:
> Confirmed
>
> Bug description:
> The is_admin_project flag is used in a token to indicate that the
> current token is scoped to a project that is indicated as the admin
> project. Currently this is only added to the token when the
> admin_project is True and nothing added when False.
>
> From a policy perspective we need to write policy files that are
> backwards compatible, however we cannot determine the difference in
> policy between is_admin_project == False and the admin project not
> being set from config because both result in no flag being set in the
> token.
>
> If we were to enforce is_admin_project == True on a deployment that
> did not use it this would completely break backwards compatibility as
> the is_admin_project check would never pass.
>
> To fix this we need to make is_admin_project a required field of a
> token at least when the admin project is set.
>
> Because the current default is that every project can be the admin
> project, the default value of is_admin_project must be True. For
> deployments that do not configure an admin project we can either set
> is_admin_project=True for every project, or we can exclude the field
> from the token. I think exclude makes sense because the check from a
> policy perspective is going to have to default to True for backwards
> compatibility and you can then tell from a token whether the
> admin_project concept is in use (i'm not sure if this is an
> information leakage problem - please comment on preference).
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/keystone/+bug/1577996/+subscriptions
>

[Jamie Lennox (jamielennox)]

Heat was the early adopter, they were passing the whole token to policy if I remember right.

I know there wasn't conversations around other projects after that but I don't remember which or if they went ahead.