| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 14:59:50 | gmann | stephenfin: it will pass with project admin too right with ' 'os_compute_api:os-hypervisors:list' policy to project admin or system reader' | |
| 15:00:02 | gmann | it will fail for project member/reader | |
| 15:00:51 | stephenfin | the check will fail (non-fatally) for project admins, which means we'll enter that block | |
| 15:01:29 | gmann | but with this it would fail right?> -https://review.opendev.org/c/openstack/nova/+/765798/6/nova/policies/hypervisors.py#37 | |
| 15:01:33 | gmann | *would not | |
| 15:02:21 | stephenfin | I'm confused. Let's start from the top | |
| 15:02:44 | stephenfin | The policy has been changed from "system reader" to "system reader or project admin" | |
| 15:02:49 | stephenfin | We check that here https://review.opendev.org/c/openstack/nova/+/765798/5..6/nova/api/openstack/compute/hypervisors.py#277 | |
| 15:03:17 | stephenfin | If e.g. a project reader tries to access this API, they'll get a HTTP 403 | |
| 15:03:47 | stephenfin | we then check the *same policy* again here https://review.opendev.org/c/openstack/nova/+/765798/5..6/nova/api/openstack/compute/hypervisors.py#197 | |
| 15:04:13 | stephenfin | only this time we do not provide "target={'project_id': context.project_id}" | |
| 15:04:15 | gmann | yeah 277 protect from project member/reader | |
| 15:04:33 | stephenfin | you must provide that to pass any project checks | |
| 15:04:40 | gmann | ahh, i did not notice the target | |
| 15:04:54 | stephenfin | so that check will return False for project members but true for system admin/member/reader | |
| 15:04:54 | gmann | got it, little confusing to read | |
| 15:05:02 | stephenfin | *project admins | |
| 15:05:08 | gmann | yeah | |
| 15:05:13 | stephenfin | yeah, like I said, it's a bit of hack | |
| 15:05:28 | stephenfin | that's what I was trying to call out with this comment | |
| 15:05:30 | stephenfin | # don't pass project information, forcing the check to be a system | |
| 15:05:32 | stephenfin | # admin check | |
| 15:05:48 | gmann | or simple we can check context.system_scope == 'all' or whatever exact value | |
| 15:05:57 | stephenfin | I should have said "don't provide project information as part of the target" | |
| 15:06:05 | stephenfin | that would be simpler :) | |
| 15:06:28 | stephenfin | what happens if we don't have scoping enabled though? | |
| 15:06:47 | stephenfin | would that result in admins not being able to list all hypervisors? | |
| 15:07:35 | gmann | https://github.com/openstack/oslo.context/blob/0d02866365bc8b779aef9ebd0b79a52c96ae40e5/oslo_context/context.py#L251 | |
| 15:08:23 | gmann | humm that is tricky, in old way we allowed to list all hypervisors | |
| 15:09:36 | gmann | so as long as we support deprecated policy it will pass the 277 and @197 if you can check "if CONF.oslo_policy.enforce_scope and context.system_scope == 'all'" then we can support old way too | |
| 15:09:57 | gmann | i mean only enable that check for CONF.oslo_policy.enforce_scope=true | |
| 15:10:14 | gmann | if false then we do old way to keep compatibility | |
| 15:10:32 | gmann | means return all hyperviros | |
| 15:11:20 | stephenfin | Yup, good idea | |
| 15:11:51 | stephenfin | okay, so back to the response if 'filter_tenant_id' is not set on any aggregate | |
| 15:12:20 | stephenfin | are you suggesting we return all hypervisors for project admins if 'filter_tenant_id=project_id' is not present on any aggregate? | |
| 15:12:38 | stephenfin | that would allow users to know all compute nodes in a given deployment | |
| 15:12:49 | stephenfin | which seems too much | |
| 15:14:12 | gmann | yeah, as filter_tenant_id is not there on aggregate scheduler filter aggregate_multitenancy_isolation allow any project to boot instance, | |
| 15:14:37 | gmann | https://github.com/openstack/nova/blob/0e7cd9d1a95a30455e3c91916ece590454235e0e/nova/scheduler/filters/aggregate_multitenancy_isolation.py#L54 | |
| 15:14:53 | gmann | i mean that is how host is kept open for all project | |
| 15:14:58 | gibi | I project admin in a non tenant isolated cloud should not be allowed to explicitly select any target host, I think | |
| 15:15:23 | stephenfin | there's a big difference between being able to run an instance on any host and being able to get information on all those hosts though | |
| 15:15:33 | gibi | gmann: the filters allows using hosts but for me the question is does the project admin needs to know which host is which? | |
| 15:15:42 | gibi | stephenfin: +1 | |
| 15:15:44 | stephenfin | we don't even expose the host that the instance is on for non-admins | |
| 15:16:47 | stephenfin | as in the 'OS-EXT-SRV-ATTR:hypervisor_hostname' attribute | |
| 15:17:01 | gmann | for this use case, only hstname is needed for create server request | |
| 15:17:30 | gmann | yeah for non-admin we will block list hypervisor or create instance on specific host | |
| 15:18:11 | gmann | gibi's has good point 'I project admin in a non tenant isolated cloud should not be allowed to explicitly select any target host, I think' | |
| 15:18:24 | stephenfin | I just checked. It fails on a DevStack deployment for me | |
| 15:18:30 | gmann | non tenant isolated cloud is anotehr thing to consider | |
| 15:18:35 | stephenfin | . devstack/openrc | |
| 15:18:42 | stephenfin | $ openstack --os-compute-api-version 2.latest server create --flavor m1.tiny --image cirros-0.5.1-x86_64-disk --network adec0800-859b-4aee-bf51-75eecb7aacf2 --hypervisor-hostname devstack-1 --wait test-server | |
| 15:18:48 | stephenfin | Policy doesn't allow compute:servers:create:requested_destination to be performed. (HTTP 403) (Request-ID: req-094bc901-ca54-4c9d-80dc-36f21b9d30ee) | |
| 15:19:55 | stephenfin | So unless an admin changes that policy, this information isn't very helpful | |
| 15:20:05 | stephenfin | even for the tenant isolated cloud | |
| 15:20:30 | stephenfin | Am I missing something? | |
| 15:20:32 | gmann | yeah this one https://github.com/openstack/nova/blob/master/nova/policies/servers.py#L189 | |
| 15:22:15 | stephenfin | Ah, wait, I am. 'source devstack/openrc' won't set things up for a project admin | |
| 15:22:50 | stephenfin | it'll use the demo user | |
| 15:23:14 | stephenfin | so I could wrangle things to use a project admin and it should pass since it's actually using this policy https://github.com/openstack/nova/blob/master/nova/policies/servers.py#L205 | |
| 15:23:30 | stephenfin | but the TODO there indicates that the use of PROJECT_ADMIN is a mistake that we need to fix | |
| 15:23:52 | stephenfin | and we shouldn't really be letting project admins select their host | |
| 15:24:02 | stephenfin | at least that's how I read that TODO | |
| 15:24:03 | gmann | yeah that is the use case end up with hypervors policy change | |
| 15:24:40 | gmann | first we discussed of letting system to create instance for project need vm on specific host as project admin cannot get host info | |
| 15:25:04 | gmann | but later we agreed to do policy change in hypervisor itself and let project admin to list host | |
| 15:26:08 | gmann | In current situation, with default policy, unless user get system and project scope both in their token they cannot boot instance on specific host. | |
| 15:26:29 | gmann | use system scope power to get host info and then project admin power to boot instance with host | |
| 15:27:14 | stephenfin | Okay, so this is an alternative to a new microversion for the 'POST /server' API? | |
| 15:27:31 | gmann | yes | |
| 15:27:46 | stephenfin | Instead of adding e.g. a 'project_id' field to the request body for that | |
| 15:27:52 | stephenfin | Gotcha | |
| 15:27:57 | gmann | exactly | |
| 15:28:48 | stephenfin | So my gut says for this to happen, we'd have to insist on tenant-isolation | |
| 15:29:10 | gmann | gibi: stephenfin but if we want to be more secure, I think returning no host if no matching 'filter_tenant_id' is also fine for me | |
| 15:29:46 | gmann | and if we get any use case to allow non-'filter_tenant_id=project_id' host then we can see | |
| 15:30:07 | gmann | i mean the current way stephenfin doing. | |
| 15:30:15 | stephenfin | I think that's necessary. Listing all hosts in a deployment is too much power | |
| 15:31:01 | gibi | I agree | |
| 15:31:02 | stephenfin | (Continuing to think out loud) Even with that though, it's still odd that we will now allow users to list some hypervisors but we won't show them the 'OS-EXT-SRV-ATTR:hypervisor_hostname' attribute | |
| 15:31:02 | gmann | yeah, and it contradict our new concept of system vs project .. | |
| 15:31:30 | stephenfin | But maybe not as odd as having to ask a system admin to create an instance for you | |
| 15:31:45 | stephenfin | gmann: This is a tricky problem :) | |
| 15:32:34 | gmann | stephenfin: humm, yeah but let it be strict use case of 'you want to create server on host so you can get hypervisor list but no where else we will return host info' | |
| 15:33:13 | gibi | how I see 1) if there is strict tenant isolation and I'm the admin of that tenant then it is OK that I can see the hosts dedicated to my tenant and also it is OK that I can specify which host a VM lands on 2) if there is no strict tenant isolation then a project admin should not see more hypervisor info than a project member and also should not be able to specify which host a VM lands on | |
| 15:33:44 | gmann | +1 | |
| 15:33:52 | gibi | in short if there is isolation then inside an isolated buble the project admin can do whathever | |
| 15:33:59 | gibi | I don't care | |
| 15:34:05 | gibi | but as soon as tenants interact | |
| 15:34:15 | sean-k-mooney | gibi: i can live it that. i was leanign to be more generous and say if ther eis not strict isolation for this tenat | |
| 15:34:16 | gibi | the project admin should no have super power | |
| 15:34:22 | sean-k-mooney | then it can see all host that are not isolated | |
| 15:34:39 | stephenfin | gibi: so should we add another check to 'POST /servers'? | |
| 15:34:40 | gmann | can we add this info in api-ref? this will clarify the usage for this API. | |
| 15:34:50 | sean-k-mooney | since that would be the set of host they could boot on | |
| 15:34:51 | gmann | at lest by seeing how confusing those API combination is | |
| 15:34:59 | stephenfin | if I'm a project admin, the host I request must belong to an aggregate I'm isolated to | |
| 15:35:10 | gibi | agree ^^ | |