The trust API doesn't use default roles

Bug #1818846 reported by Lance Bragstad
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Low
Colleen Murphy

Bug Description

In Rocky, keystone implemented support to ensure at least three default roles were available [0]. The trust API doesn't incorporate these defaults into its default policies [1], but it should.

It would be useful for system members and readers to diagnose issues with trusts, instead of requiring system administrators to do everything.

[0] http://specs.openstack.org/openstack/keystone-specs/specs/keystone/rocky/define-default-roles.html
[1] http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/policies/trust.py?id=6e3f1f6e46787ed4542609c935c13cb85e91d7fc

tags: added: default-roles policy
Changed in keystone:
status: New → Triaged
importance: Undecided → Low
Colleen Murphy (krinkle)
Changed in keystone:
assignee: nobody → Colleen Murphy (krinkle)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

Fix proposed to branch: master
Review: https://review.opendev.org/675720

Changed in keystone:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/675807

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676277

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676283

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676284

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676287

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676847

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/676995

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Fix proposed to branch: master
Review: https://review.opendev.org/677004

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.opendev.org/675720
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=5086709ae2d673716653cd8812247ea5a1cb5e69
Submitter: Zuul
Branch: master

commit 5086709ae2d673716653cd8812247ea5a1cb5e69
Author: Colleen Murphy <email address hidden>
Date: Fri Aug 9 21:37:23 2019 -0700

    Add protection tests for trusts API

    Currently, the majority of access control enforcement for the trusts API
    is not done in policy, but hardcoded in the controller logic. The
    default policy check strings for these routes are empty. Before we can
    enable system scope and default roles through the trusts policy, we need
    to replicate the existing access control in policy. To do that, we
    should test how it currently works. This patch adds those tests.

    The trusts API is mostly only useable by the trustor or trustee. Mostly,
    admins can't perform trust actions on behalf of the trustor or trustee.
    The exception is for the delete action, but only when the is_admin
    context is set. This change also fixes a minor regression where the
    is_admin admin could not perform this action due to the auth_context not
    being populated.

    Change-Id: I6a5eca8240aa905e02fbf9bec335996c3a4f1c79
    Partial-bug: #1818846
    Partial-bug: #1818850

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/675807
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=0df8d0e2e1519154ec76100f7a4c2fdc8c9c04bb
Submitter: Zuul
Branch: master

commit 0df8d0e2e1519154ec76100f7a4c2fdc8c9c04bb
Author: Colleen Murphy <email address hidden>
Date: Sun Aug 11 21:06:36 2019 -0700

    Move list_trusts enforcement to default policies

    Without this change, policy enforcement for the GET /OS-TRUST/trusts API
    is hardcoded in the flask dispatcher code. This is a problem because
    this enforcement can't be controlled by the operator, as is the norm.
    Moreover, it makes the transition to system-scope and
    default-roles-aware policies more difficult because there's no sensible
    migration from "" to a logical role-based check string.

    This patch starts the conversion from hardcoded enforcement to
    enforcement via default policies for GET /OS-TRUST/trusts. To do this,
    we add two new policy rules, "identity:list_trusts_for_trustor" and
    "identity:list_trusts_for_trustee". We need to do this so that we can
    keep backwards compatibility with the bizarre behavior that an admin can
    list all trusts (GET /OS-TRUST/trusts) but not list trusts for a trustor
    or trustee (GET /OS-TRUST/trusts?trustor_user_id={} and
    GET/OS-TRUST/trusts?trustee_user_id={}). The tricky part is that it's
    plausible that operators may have incorporated the hardcoded empty
    default for "identity:list_trusts" into their on-disk policy
    configuration, either by never removing the old default policy file that
    used to come packaged with keystone, or by generating a sample file and
    applying that to disk (we don't recommend that but we don't expressly
    forbid or discourage it either). To overcome
    this, the trust API code checks whether the "identity:list_trusts" rule
    is "" and re-applies the enforcement with a warning. We don't need to do
    this for the two new policies because they are initially enforced
    in-code and an operator would have to take explicit action on upgrade to
    override them.

    This change does not use the formal oslo.policy deprecation system
    because "" OR'd with the new default is entirely useless as a policy.

    Partial-bug: #1818850
    Partial-bug: #1818846

    Change-Id: I6c1a4ecd756519f7f807c9d28960482e7f0d235b

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676277
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a09163a3202c32f05cf636559a95fe45c6ea272b
Submitter: Zuul
Branch: master

commit a09163a3202c32f05cf636559a95fe45c6ea272b
Author: Colleen Murphy <email address hidden>
Date: Tue Aug 13 13:09:41 2019 -0700

    Move delete_trust enforcement to default policies

    Without this change, policy enforcement for the DELETE
    /OS-TRUST/trusts/{trust_id} API is hardcoded in the flask dispatcher
    code. This is a problem because this enforcement can't be controlled by
    the operator, as is the norm. Moreover, it makes the transition to
    system-scope and default-roles-aware policies more difficult because
    there's no sensible migration from "" to a logical role-based check
    string.

    This converts the hardcoded enforcement to enforcement via default
    policies for DELETE /OS-TRUST/trusts/{trust_id}. Currently only the
    trustor or the is_admin user can access this API (since the is_admin
    user bypasses the policy loading). This behavior will be changed in a
    future patch that will allow the system admin to access this API.

    This change does not use the formal oslo.policy deprecation system
    because "" OR'd with the new default is entirely useless as a policy.

    Change-Id: I1aaba72b69b389ffbfcf7d5b8cc70453ffa59e73
    Partial-bug: #1818850
    Partial-bug: #1818846

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676283
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=b5617eee416d168a674283e220f67e2a9f174a19
Submitter: Zuul
Branch: master

commit b5617eee416d168a674283e220f67e2a9f174a19
Author: Colleen Murphy <email address hidden>
Date: Tue Aug 13 14:44:44 2019 -0700

    Move get_trust enforcement to default policies

    Without this change, policy enforcement for the GET
    /OS-TRUST/trusts/{trust_id} API is hardcoded in the flask dispatcher
    code. This is a problem because this enforcement can't be controlled by
    the operator, as is the norm. Moreover, it makes the transition to
    system-scope and default-roles-aware policies more difficult because
    there's no sensible migration from "" to a logical role-based check
    string.

    This converts the hardcoded enforcement to enforcement via default
    policies for GET /OS-TRUST/trusts/{trust_id}. The API specifically
    blocks the is_admin user from using it, and since policies aren't loaded
    for the is_admin user we need to continue explicitly blocking it.

    This change does not use the formal oslo.policy deprecation system
    because "" OR'd with the new default is entirely useless as a policy.

    Change-Id: I3c0718330d5a18c0c79e8f12509200fd97a55913
    Partial-bug: #1818850
    Partial-bug: #1818846

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676284
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=b100825a03920e59700dba7c45b2317344924867
Submitter: Zuul
Branch: master

commit b100825a03920e59700dba7c45b2317344924867
Author: Colleen Murphy <email address hidden>
Date: Tue Aug 13 15:24:33 2019 -0700

    Move list_roles_for_trust enforcement to policies

    Without this change, policy enforcement for the GET
    /OS-TRUST/trusts/{trust_id}/roles API is hardcoded in the flask
    dispatcher code. This is a problem because this enforcement can't be
    controlled by the operator, as is the norm. Moreover, it makes the
    transition to system-scope and default-roles-aware policies more
    difficult because there's no sensible migration from "" to a logical
    role-based check string.

    This converts the hardcoded enforcement to enforcement via default
    policies for GET /OS-TRUST/trusts/{trust_id}/roles. The API specifically
    blocks the is_admin user from using it, and since policies aren't loaded
    for the is_admin user we need to continue explicitly blocking it.

    This change does not use the formal oslo.policy deprecation system
    because "" OR'd with the new default is entirely useless as a policy.

    Change-Id: Ib339852c9d619b8cbf7a00d45da461377991ba6f
    Partial-bug: #1818850
    Partial-bug: #1818846

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676287
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=09e699baba89b94a020682ab7d916d67360f4481
Submitter: Zuul
Branch: master

commit 09e699baba89b94a020682ab7d916d67360f4481
Author: Colleen Murphy <email address hidden>
Date: Tue Aug 13 15:49:30 2019 -0700

    Move get_role_for_trust enforcement to policies

    Without this change, policy enforcement for the GET
    /OS-TRUST/trusts/{trust_id}/roles/{role_id} API is hardcoded in the
    flask dispatcher code. This is a problem because this enforcement can't
    be controlled by the operator, as is the norm. Moreover, it makes the
    transition to system-scope and default-roles-aware policies more
    difficult because there's no sensible migration from "" to a logical
    role-based check string.

    This converts the hardcoded enforcement to enforcement via default
    policies for GET /OS-TRUST/trusts/{trust_id}/roles/{role_id}. The API
    specifically blocks the is_admin user from using it, and since policies
    aren't loaded for the is_admin user we need to continue explicitly
    blocking it.

    This change does not use the formal oslo.policy deprecation system
    because "" OR'd with the new default is entirely useless as a policy.

    Change-Id: Ib5a6a87313aa7b2a73211f512b8a8c675a21b52f
    Partial-bug: #1818850
    Partial-bug: #1818846

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676847
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=ea7acd80362e27c44a299c70504c21fdc7953e21
Submitter: Zuul
Branch: master

commit ea7acd80362e27c44a299c70504c21fdc7953e21
Author: Colleen Murphy <email address hidden>
Date: Thu Aug 15 18:39:41 2019 -0700

    Implement system reader role for trusts API

    Currently, the trusts API only allows the "project" scope type, and
    moreover inconsistently enforces different actions based on admin status
    or trustor/trustee relationship: for example, an "admin" can list all
    trusts but not filter by trustor or trustee and cannot get details for a
    single trust, not can they list or get trust roles. This patch changes
    the behavior of the trusts API to allow a system reader to list and get
    details for trusts and trust roles, where previously only a trustor or
    trustee could do so. This helps make the different actions in the trusts
    API consistent with one another and makes the API more useful to a
    deployment auditor. A subsequent patch will add system admin
    functionality.

    This change does not use the oslo.policy deprecation feature for the
    'identity:list_trusts_for_trustor' or 'identity:list_trusts_for_trustee'
    policies as those are new policies introduced in 7717ed3.

    Change-Id: I4e1482643e18fd46e937ffae8b3623cea2d2dd62
    Partial-bug: #1818850
    Partial-bug: #1818846
    Related-Bug: #968696

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/676995
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=6aebf179b8b1b48909ea57666f61bbc080bd6888
Submitter: Zuul
Branch: master

commit 6aebf179b8b1b48909ea57666f61bbc080bd6888
Author: Colleen Murphy <email address hidden>
Date: Fri Aug 16 10:38:43 2019 -0700

    Add tests for system member for trusts

    For trusts, a system member is essentially the same as a system reader:
    system members should not be able to create or delete trusts. This
    change adds tests to assert that, but no policy changes are required to
    account for the member role.

    Change-Id: I0acd55f4428708430740bf2c305f664e199dd304
    Partial-bug: #1818846
    Related-Bug: #968696

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/677004
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=9be1caff97355099d25170fe390dd15d6f592d56
Submitter: Zuul
Branch: master

commit 9be1caff97355099d25170fe390dd15d6f592d56
Author: Colleen Murphy <email address hidden>
Date: Fri Aug 16 11:14:16 2019 -0700

    Implement system admin for trusts API

    This change enables a system admin to delete trusts. Previously, only
    the trustor or the is_admin admin could delete a trust. This changes
    makes the trusts API more useful to system administrators who need to
    clean up trusts and makes the API consistent with others.

    This does not enable system admins to create trusts. A trust can only be
    scoped to a project, so creating one is inherently a project-scoped
    action. If trusts later gain the ability to be scoped to the system or
    domains, we can add those scopes to the create_trust scope_types.

    Change-Id: Idf13b862f345388bb2372609787947eb43d7ba75
    Closes-bug: #1818846
    Closes-bug: #1818850
    Related-Bug: #968696

Changed in keystone:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/keystone 16.0.0.0rc1

This issue was fixed in the openstack/keystone 16.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.