<div dir="ltr"><div>Add the maillist back, I missed from the previous reply...</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">melanie witt <<a href="mailto:melwittt@gmail.com">melwittt@gmail.com</a>> 于2019年2月16日周六 上午12:22写道：<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks for the reply, Alex. Response is inline.<br>
<br>
On Fri, 15 Feb 2019 15:49:19 +0800, He Jie Xu <<a href="mailto:soulxu@gmail.com" target="_blank">soulxu@gmail.com</a>> wrote:<br>
> We need to ensure all the APIs check the policy with real instance's <br>
> project and user id, like this <br>
> <a href="https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/evacuate.py#L85-L88" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/evacuate.py#L85-L88</a><br>
> Otherwise, any user can get any other tenant's instance.<br>
> <br>
> Some of APIs doesn't check like this, for example:<br>
> <a href="https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L55" rel="noreferrer" target="_blank">https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/attach_interfaces.py#L55</a><br>
<br>
While I agree with this (policy check instance project/user instead of <br>
request context project/user), I'm not sure it's related to the <br>
project_only=True at the database layer. The project_only=True at the <br>
database layer only enforces request context project, which is what the <br>
second example here will also do, if policy contains <br>
'project_id:%(project_id)s'. It seems to me that if we remove <br>
project_only=True from the database layer, we will get proper <br>
enforcement of request context project/user by the existing policy <br>
checks, if the policy is configured as such. And we default policy to <br>
either rule:admin_api or rule:admin_or_owner:<br>
<br>
<a href="https://docs.openstack.org/nova/latest/configuration/sample-policy.html" rel="noreferrer" target="_blank">https://docs.openstack.org/nova/latest/configuration/sample-policy.html</a><br>
<br>
So, it seems to me that changing policy enforcement from request context <br>
project/user => instance project/user would be a separate change. Please <br>
let me know if I'm misunderstanding you.<br>
<br>
One thing I do notice though, in your first example, is that the <br>
get_instance is done _before_ the policy check, which would need to be <br>
moved after the policy check, in the same change that would remove <br>
project_only=True. So I'm glad you pointed that out.<br></blockquote><div><br></div><div>Emm...no, the first example is the right example.</div><div><br></div><div>project_only=True will ensure db call return the instance belong to the project</div><div>in the request context. If project_only=False, the db call may return other project's</div><div>instance.</div><div><br></div><div>Then when the rule is 'project_id:%(project_id)s' and the target is instance's project_id,</div><div>the policy enforcement will ensure the request context's project id match the instance's project_id,</div><div>then the user won't get other project's instance.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> I'm trying to memory why we didn't do that in the beginning, sounds like <br>
> we refused to support user-id based policy. But<br>
> in the end for the backward-campatible for the user like CERN, we <br>
> support user-id based policy for few APIs.<br>
> <a href="https://review.openstack.org/#/q/topic:bp/user_id_based_policy_enforcement+(status:open+OR+status:merged)" rel="noreferrer" target="_blank">https://review.openstack.org/#/q/topic:bp/user_id_based_policy_enforcement+(status:open+OR+status:merged)</a><br>
> <br>
> This is probably why some APIs checks policy with real instance's <br>
> project id and user id and some APIs not.<br>
> <br>
> melanie witt <<a href="mailto:melwittt@gmail.com" target="_blank">melwittt@gmail.com</a> <mailto:<a href="mailto:melwittt@gmail.com" target="_blank">melwittt@gmail.com</a>>> 于2019年2 <br>
> 月15日周五 上午1:23写道：<br>
> <br>
>     Hey all,<br>
> <br>
>     Recently, we had a customer try the following command as a non-admin<br>
>     with a policy role granted in policy.json to allow live migrate:<br>
> <br>
>         "os_compute_api:os-migrate-server:migrate_live": "rule:admin_api or<br>
>     role:Operator"<br>
> <br>
>     The scenario is that they have a server in project A and a user in<br>
>     project B with role:Operator and the user makes a call to live migrate<br>
>     the server.<br>
> <br>
>     But when they call the API, they get the following error response:<br>
> <br>
>         {"itemNotFound": {"message": "Instance <uuid server 1> could not be<br>
>     found.", "code": 404}}<br>
> <br>
>     A superficial look through the code shows that the live migrate should<br>
>     work, because we have appropriate policy checks in the API, and the<br>
>     request makes it past those checks because the policy.json has been set<br>
>     correctly.<br>
> <br>
>     A common pattern in our APIs is that we first compute_api.get() the<br>
>     instance object and then we call the server action (live migrate, stop,<br>
>     start, etc) with it after we retrieve it. In this scenario, the<br>
>     compute_api.get() fails with NotFound.<br>
> <br>
>     And the reason it fails with NotFound is because, much lower level, at<br>
>     the DB layer, we have a keyword arg called 'project_only' which, when<br>
>     True, will scope a database query to the RequestContext.project_id<br>
>     only.<br>
>     We have hard-coded 'project_only=True' for the instance get query.<br>
> <br>
>     So, when the user in project B with role:Operator tries to retrieve the<br>
>     instance record in project A, with appropriate policy rules set, it<br>
>     will<br>
>     fail because 'project_only=True' and the request context is project B,<br>
>     while the instance is in project A.<br>
> <br>
>     My question is: can we get rid of the hard-coded 'project_only=True' at<br>
>     the database layer? This seems like something that should be<br>
>     enforced at<br>
>     the API layer and not at the database layer. It reminded me of an<br>
>     effort<br>
>     we had a few years ago where we removed other hard-coded policy<br>
>     enforcement from the database layer [1][2]. I've uploaded a WIP<br>
>     patch to<br>
>     demonstrate the proposed change [3].<br>
> <br>
>     Can anyone think of any potential problems with doing this? I'd like to<br>
>     be able to remove it so that operators are able use policy to allow<br>
>     non-admin users with appropriately configured roles to run server<br>
>     actions.<br>
> <br>
>     Cheers,<br>
>     -melanie<br>
> <br>
>     [1]<br>
>     <a href="https://blueprints.launchpad.net/nova/+spec/nova-api-policy-final-part" rel="noreferrer" target="_blank">https://blueprints.launchpad.net/nova/+spec/nova-api-policy-final-part</a><br>
>     [2]<br>
>     <a href="https://review.openstack.org/#/q/topic:bp/nova-api-policy-final-part+(status:open+OR+status:merged)" rel="noreferrer" target="_blank">https://review.openstack.org/#/q/topic:bp/nova-api-policy-final-part+(status:open+OR+status:merged)</a><br>
>     [3] <a href="https://review.openstack.org/637010" rel="noreferrer" target="_blank">https://review.openstack.org/637010</a><br>
> <br>
<br>
<br>
<br>
<br>
</blockquote></div></div>