Glance doesn't send correctly authorization request to Oslo policy

[WuKong]

We have an OpenStack/Mitaka installed with Keystone, Nova and Glance.
In /etc/glance/policy.json, we have modified the following lines to test the http_check function of Oslo policy:

    ...
    "add_image": "http://moon:8081/authz/wrapper",
    "delete_image": "http://moon:8081/authz/wrapper",
    "get_image": "http://moon:8081/authz/wrapper",
    "get_images": "http://moon:8081/authz/wrapper",
    "modify_image": "http://moon:8081/authz/wrapper",
    ...
Then, when we run:
$ openstack image list
+--------------------------------------+--------+--------+
| ID | Name | Status |
+--------------------------------------+--------+--------+
| 6e31cdd2-4968-4a80-aebc-fcdd6f213091 | cirros | active |
+--------------------------------------+--------+--------+
with no problem, but if we run:
$ openstack image create --disk-format qcow2 --file /vagrant/cirros-0.3.5-x86_64-disk.img --container-format bare cirros4

400 Bad Request
cannot deepcopy this pattern object
    (HTTP 400)

The Oslo_Policy code doesn't raise an error but stop when trying to deepcopying the target variable in oslo.policy/oslo.policy/oslo_policy/_checks.py (line ~241) and we have the following event in Glance API logs:
2017-09-25 12:48:16.044 16600 DEBUG oslo_policy._cache_handler [req-e98eef44-d01b-401f-8e69-c78adf5310d3 - - - - -] Reloading cached file /etc/glance/policy.json read_cached_file /usr/local/lib/python2.7/dist-packages/oslo_policy/_cache_handler.py:40
2017-09-25 12:48:16.047 16600 DEBUG oslo_policy.policy [req-e98eef44-d01b-401f-8e69-c78adf5310d3 - - - - -] Reloaded policy file: /etc/glance/policy.json _load_policy_file /usr/local/lib/python2.7/dist-packages/oslo_policy/policy.py:682
2017-09-25 12:48:16.075 16600 DEBUG glance.api.v2.images [req-e98eef44-d01b-401f-8e69-c78adf5310d3 - - - - -] cannot deepcopy this pattern object create /usr/lib/python2.7/dist-packages/glance/api/v2/images.py:85
2017-09-25 12:48:16.084 16600 INFO eventlet.wsgi.server [req-e98eef44-d01b-401f-8e69-c78adf5310d3 - - - - -] 127.0.0.1 - - [25/Sep/2017 12:48:16] "POST /v2/images HTTP/1.1" 400 273 0.053245

An other problem is that we have not enough information in the target variable (in oslo.policy/oslo.policy/oslo_policy/_checks.py). Because most of the information have the 'object' type, they are deleted from the temp_target variable (line ~244).

We believe that this is due to the Glance part since it doesn't well prepare the authorization request (body) to Oslo policy.

[Cyril Roelandt (cyril-roelandt)]

Looking at the code from oslo_policy/_checks.py, three things come to mind:

1) copy.deepcopy is called on a glance.api.policy.ImageTarget, and this class does not seem to define a __deepcopy__ method, which we might want to add since deep copying seems tricky.
2) the keys() method is called on the glance.api.policy.ImageTarget object, and that is definitely not going to work, since this is not a dict, and no keys() method is defined.
3) jsonutils.dumps() is called on temp_target, which may not be JSON-serializable.

I'm not sure whether these issues should be fixed by modifying the ImageTarget class, or whether we should actually pass a dict to oslo.policy. If we want to fix the ImageTarget class, we should probably start with something along those lines: http://paste.debian.net/989066/

Does anyone know how this feature is supposed to work? Has it ever worked?

[Abhishek Kekane (abhishek-kekane)]

If you specify something like below in policy file then it will work,

"restricted": "not ('test_key':%(test_key)s and role:_member_)"
"download_image": "role:admin or rule:restricted"

Where test_key is extra property for image.

ImageTarget class was added to search this property specified in json either in core properties or in extra properties.

[Doug Hellmann (doug-hellmann)]

It sounds like the policy library is not clear enough that it expects the thing passed to it to be a dictionary, or at least to fully act like one.

@Cyril, I think your pastebin from comment #1 is going in the right direction. Would it fill out the API of ImageTarget so that it is fully compliant with the Mapping abstract base class? https://docs.python.org/2/library/collections.html#collections.Mapping

[OpenStack Infra (hudson-openstack)]

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

[Brian Rosmaita (brian-rosmaita)]

Comment from Doug in IRC (#openstack-glance):
[16:23:25] <dhellmann> rosmaita, jokke_ : regarding the http check policy thing raised in the glance meeting -- the issue with the current ImageTarget class is that it seems to support looking up a single key, but doesn't provide a way to get all of the properties at once
[16:23:46] <dhellmann> the http check needs that because it ships all of the properties to the remote service so that service can enforce the policy
[16:24:00] <dhellmann> and the generic http check doesn't know what properties (a) exist or (b) are needed
[16:25:18] <dhellmann> the API for the enforcer wasn't clear about the expectations for the "target" argument -- we really did assume it was a dict instance, I think. So ImageTarget is behaving sort of like a dict, but not enough to work for all cases

[Brian Rosmaita (brian-rosmaita)]

The glance patch for this is: https://review.openstack.org/#/c/512020/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/512020
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=3134ee07b2ab25f63183c699df163b5f8dfd0918
Submitter: Zuul
Branch: master

commit 3134ee07b2ab25f63183c699df163b5f8dfd0918
Author: Cyril Roelandt <email address hidden>
Date: Fri Oct 13 23:22:16 2017 +0200

    Make ImageTarget behave like a dictionary

    This is required because oslo_policy's 'enforce' method expects a dict-like
    object as its second argument.

    Change-Id: I9187b6805d3b2cd351189e34dd2f9db3158f6b8d
    Closes-Bug: #1720354

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/525127

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/525133

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/glance 16.0.0.0b2 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/510569
Committed: https://git.openstack.org/cgit/openstack/oslo.policy/commit/?id=8710f6338d0596ebc7c0d8a69675d9333631504b
Submitter: Zuul
Branch: master

commit 8710f6338d0596ebc7c0d8a69675d9333631504b
Author: Doug Hellmann <email address hidden>
Date: Mon Oct 9 09:31:08 2017 -0400

    expand type documentation for Enforcer arguments

    As part of bug #1720354 we discovered that arguments being passed to
    the enforcer were not always dictionaries and did not always support
    the full API needed. Expand the documentation to make the requirements
    clearer.

    Change-Id: I6c940d825cf72777e2a7946ab7489a1ed5359235
    Closes-Bug: #1720354
    Signed-off-by: Doug Hellmann <email address hidden>

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/oslo.policy 1.32.2 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/525127
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=2eb2e0b17ec09b4d17f10724aae52d5deaf61bde
Submitter: Zuul
Branch: stable/pike

commit 2eb2e0b17ec09b4d17f10724aae52d5deaf61bde
Author: Cyril Roelandt <email address hidden>
Date: Fri Oct 13 23:22:16 2017 +0200

    Make ImageTarget behave like a dictionary

    This is required because oslo_policy's 'enforce' method expects a dict-like
    object as its second argument.

    Change-Id: I9187b6805d3b2cd351189e34dd2f9db3158f6b8d
    Closes-Bug: #1720354
    (cherry-picked from commit 3134ee07b2ab25f63183c699df163b5f8dfd0918)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/525133
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=2ae4541f3e462343f84effae01e9dcdfdaefa991
Submitter: Zuul
Branch: stable/ocata

commit 2ae4541f3e462343f84effae01e9dcdfdaefa991
Author: Cyril Roelandt <email address hidden>
Date: Fri Oct 13 23:22:16 2017 +0200

    Make ImageTarget behave like a dictionary

    This is required because oslo_policy's 'enforce' method expects a dict-like
    object as its second argument.

    Change-Id: I9187b6805d3b2cd351189e34dd2f9db3158f6b8d
    Closes-Bug: #1720354
    (cherry-picked from commit 3134ee07b2ab25f63183c699df163b5f8dfd0918)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/glance 15.0.2 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/glance ocata-eol release.