Default `target={}` value leaks into subsequent `policy.check()` calls

[Timur Sufiev]

Due to mutable dictionary being used as the default `target` argument value the first target calculated from scratch in POLICY_CHECK function will be used for all subsequent calls to POLICY_CHECK with 2 arguments. The wrong `target` can either lead to a reduced set of operations on an entity for a given user, or to enlarged one. The latter case poses a security breach from an cloud operators' point of view.

[Julie Pichon (jpichon)]

I'm not sure if this can be intentionally exploited, and after chatting with Timur some more on IRC I suspect this mainly creates a huge UX hassle (that we definitely want to fix asap) and the security implications may be limited, because in the end the other services' policies will take over and prevent the end-user from accessing data or performing actions they're not allowed to. I could be missing something though.

Also, applying the patch (which looks fine to me and gets my +2) appears to resolve the random access issues highlighted in bug 1382316.

We will definitely want to backport this to Juno and Icehouse as well (the current patch only applies cleanly onto master at the moment).

[Jeremy Stanley (fungi)]

I've left this bug private, subscribed the Horizon core security reviewers and added an incomplete security advisory task until we can determine if this is a class A or class D. https://wiki.openstack.org/wiki/Vulnerability_Management#Incident_report_taxonomy

[Paul McMillan (paul-mcmillan)]

+1 on the patch, it looks good.

As Julie said, Horizon doesn't have special access, these policies are enforced elsewhere, so I'm fine with calling this a class D. It's unfortunate that that may complicate people's backport policies, but this doesn't deserve a CVE.

[Julie Pichon (jpichon)]

Does class D mean we can go ahead and work on the patch in the open?

[Thierry Carrez (ttx)]

@Julie, if we confirm it, yes.
We'll have a meeting in a few hours to decide if we open this up. Please keep it private until we switch the bug to Public.

[Julie Pichon (jpichon)]

Understood, thanks for the update!

[Thierry Carrez (ttx)]

Confirmed class D

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/138097
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=dab964d781699d07883a659750c6913b649fed38
Submitter: Jenkins
Branch: master

commit dab964d781699d07883a659750c6913b649fed38
Author: Timur Sufiev <email address hidden>
Date: Wed Nov 26 13:11:27 2014 +0300

    Prevent leaking `target` info into subsequent `policy.check()` calls

    Due to mutable dictionary being used as the default `target` argument
    value the first target calculated from scratch in POLICY_CHECK
    function will be used for all subsequent calls to POLICY_CHECK with 2
    arguments. The wrong `target` can either lead to a reduced set of
    operations on an entity for a given user, or to enlarged one. Due to
    independent policy checks at each service's side this doesn't pose a
    serious security breach, but can lead to weird UX behaviour.

    Change-Id: I744fac28de0fb7060b50c5db689e74631a628c88
    Closes-Bug: #1396544

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/138313

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/138314

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/138313
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=13b0c0e5d7b1ccf311e09d6f2c35ed9127814a0c
Submitter: Jenkins
Branch: stable/juno

commit 13b0c0e5d7b1ccf311e09d6f2c35ed9127814a0c
Author: Timur Sufiev <email address hidden>
Date: Wed Nov 26 13:11:27 2014 +0300

    Prevent leaking `target` info into subsequent `policy.check()` calls

    Due to mutable dictionary being used as the default `target` argument
    value the first target calculated from scratch in POLICY_CHECK
    function will be used for all subsequent calls to POLICY_CHECK with 2
    arguments. The wrong `target` can either lead to a reduced set of
    operations on an entity for a given user, or to enlarged one. Due to
    independent policy checks at each service's side this doesn't pose a
    serious security breach, but can lead to weird UX behaviour.

    Change-Id: I744fac28de0fb7060b50c5db689e74631a628c88
    Closes-Bug: #1396544
    (cherry picked from commit dab964d781699d07883a659750c6913b649fed38)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/138314
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=33f2b939fd20dbeb4555640937db645104c2a0fc
Submitter: Jenkins
Branch: stable/icehouse

commit 33f2b939fd20dbeb4555640937db645104c2a0fc
Author: Timur Sufiev <email address hidden>
Date: Wed Nov 26 13:11:27 2014 +0300

    Prevent leaking `target` info into subsequent `policy.check()` calls

    Due to mutable dictionary being used as the default `target` argument
    value the first target calculated from scratch in POLICY_CHECK
    function will be used for all subsequent calls to POLICY_CHECK with 2
    arguments. The wrong `target` can either lead to a reduced set of
    operations on an entity for a given user, or to enlarged one. Due to
    independent policy checks at each service's side this doesn't pose a
    serious security breach, but can lead to weird UX behaviour.

    Change-Id: I744fac28de0fb7060b50c5db689e74631a628c88
    Closes-Bug: #1396544
    (cherry picked from commit dab964d781699d07883a659750c6913b649fed38)