| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-10-05 | |||
| 18:01:02 | dansmith | is that because that's the token and client that neutron uses to interact with nova or other things in a sensible way, or just because it has no knowledge that a system scoped token would be right? | |
| 18:01:07 | dansmith | artom: ironic, IIRC | |
| 18:01:17 | dansmith | artom: it's not just neutron :) | |
| 18:01:59 | lbragstad | yeah - i think it's using the nova service user, which has the admin role | |
| 18:02:15 | lbragstad | and we haven't really gone through that migration yet, from a policy perspective | |
| 18:03:47 | dansmith | lbragstad: meaning it's just naive and not like that user/client is project-scoped for other stuff? | |
| 18:05:06 | dansmith | even still, if neutron is sending an event for an instance or group of ports or something, having it be project-scoped in that call to the event thing also helps to prevent any missteps in sending events to other instances | |
| 18:05:23 | dansmith | like if there was some port change thing going on, during ownership transfer, or anything else | |
| 18:05:43 | dansmith | this goes back to my "the interface is specific to project-scoped resources, so it feels like it too should be project-scoped" | |
| 18:07:24 | lbragstad | iiuc - it's using the nova service user, which just happens to have the admin role (probably for legacy reasons) and that's why it works | |
| 18:07:52 | lbragstad | so - because the nova user has the 'admin' role on the service project, it can update external server events for instance 'foo' in project bar... | |
| 18:07:53 | dansmith | yeah, well the default policy makes the api admin-only anyway | |
| 18:08:57 | sean-k-mooney | yes so its relyin gon haveing a user with admin rights | |
| 18:09:13 | sean-k-mooney | that can be the nova user or a common service user | |
| 18:09:14 | lbragstad | correct - but it's not enforcing tenancy in any way - from what i can tell | |
| 18:09:33 | sean-k-mooney | i dont think it is either today | |
| 18:09:59 | sean-k-mooney | since this is mainly a service to service api | |
| 18:10:04 | dansmith | the api? | |
| 18:11:44 | dansmith | it uses the context to look up the instances it creates the events for, so it should be scoped to the project of the token | |
| 18:11:48 | sean-k-mooney | the external events api does not assert that the user that created teh event is a has any relationship with the project reosuce that is being modified | |
| 18:12:00 | dansmith | or I guess if you're admin then it lets you see them all, but it should still be tight if you're not admin | |
| 18:12:32 | dansmith | you won't be able to look up instances you don't have access to, and you'll fail with a 404 I think | |
| 18:12:58 | sean-k-mooney | well this has been admin only for as long as i can rememebr it exsiting | |
| 18:13:02 | dansmith | or just not send those events, I guess, I'm not sure, but.. | |
| 18:13:22 | dansmith | sean-k-mooney: by default you mean :) | |
| 18:13:31 | sean-k-mooney | yes by default | |
| 18:13:36 | dansmith | I'm just saying I don't think that the api is not tenant-safe | |
| 18:14:08 | sean-k-mooney | well while you can get a responce form tha tapi when you do a post we dont supprot get request on it correct | |
| 18:14:21 | dansmith | there's nothing to get | |
| 18:14:24 | sean-k-mooney | and the info you get back is pretty limited in general | |
| 18:14:24 | dansmith | there's no persistence | |
| 18:14:40 | dansmith | I'm saying you can't POST events for instances you don't own | |
| 18:14:41 | sean-k-mooney | right so since it most a write only api | |
| 18:14:50 | dansmith | it is entirely write-only | |
| 18:14:50 | sean-k-mooney | sure you can | |
| 18:15:10 | gmann | dansmith: sean-k-mooney I remember the discussion of need of service specific role for such cross service API. system scope in this API was not concluded/discussed permission for this at the time of moving to new defaults. | |
| 18:15:12 | sean-k-mooney | well ok you saying if you change the policy to allow anyone call it | |
| 18:15:15 | sean-k-mooney | then it wont work | |
| 18:15:16 | dansmith | objects.InstanceList.get_by_filters( | |
| 18:15:16 | dansmith | cctxt, {'uuid': instance_uuids_by_cell[cell_uuid]}, | |
| 18:15:32 | dansmith | sean-k-mooney: this ^ will not work for instances you don't own if you are not admin | |
| 18:15:42 | gmann | either project scope or both or need service user specific role for such operation | |
| 18:15:52 | sean-k-mooney | do we have a db level check to enforce that | |
| 18:16:22 | dansmith | sean-k-mooney: that's how that interface works right? | |
| 18:16:35 | dansmith | I mean, it certainly was when the code was written :) | |
| 18:17:25 | sean-k-mooney | in the port bindign case the event are all submited as tyepically the nova user regradesss of who owns the port | |
| 18:17:33 | sean-k-mooney | dansmith: the user token is never used | |
| 18:17:57 | dansmith | ...right, as admin yes? | |
| 18:18:03 | sean-k-mooney | also even if it was the even it decupled form a user action | |
| 18:18:07 | sean-k-mooney | yes as admin | |
| 18:18:17 | dansmith | what are we arguing about again? | |
| 18:19:04 | sean-k-mooney | wether the resouce that is being evented on is own by the user/project assocated with teh keyton token used to send the event to nova i think | |
| 18:19:43 | sean-k-mooney | i kind of got confused by what you were saying would and would not work and im not sure you were following me either | |
| 18:20:18 | dansmith | https://github.com/openstack/nova/blob/7967ad78649a1f8d7ffc34ae28274ee89b0011cf/nova/db/main/api.py#L1413-L1416 | |
| 18:20:32 | dansmith | this ^ is where we enforce you can only see your own instances in the DB layer, if not admin | |
| 18:20:55 | dansmith | which makes that API not allow non-admins (if so granted) able to send events to instances they do not own | |
| 18:21:14 | sean-k-mooney | i see | |
| 18:21:19 | dansmith | if the nova user neutron uses has admin, that's why it can send events to any instance, as expected | |
| 18:21:43 | dansmith | I'm talking about if someone were to drop the admin requirement from the policy, or if we later add a human-initiated event, the API is not insecure | |
| 18:22:09 | sean-k-mooney | right i tought we were trying to remove those low level db enfrocements form the code | |
| 18:22:14 | dansmith | in response to this: | |
| 18:22:14 | dansmith | [11:09:14] <lbragstad> correct - but it's not enforcing tenancy in any way - from what i can tell | |
| 18:22:14 | dansmith | [11:09:33] <sean-k-mooney> i dont think it is either today | |
| 18:22:14 | gmann | dansmith: so your proposal is to allow only project scoped (owner or admin) not system right? | |
| 18:22:47 | dansmith | sean-k-mooney: not those, AFAIK.. tons of stuff would need to change at the top, and always provide a project-level filter down or we'd be querying tons of information all the time that we need to filter in python | |
| 18:22:48 | sean-k-mooney | dansmith: right so its the db laywer not the api that enforcing that | |
| 18:23:16 | gmann | sean-k-mooney: we have that as TODO to move admin-checks from DB to API layer side | |
| 18:23:16 | dansmith | gmann: no I'm saying we allow either and enforce scope type based on the event_type in the event, which are probably all system right now (but need to look) | |
| 18:24:03 | dansmith | gmann: you're not talking about removing the project filtering from the DB queries entirely right? | |
| 18:24:10 | gmann | yeah it is all system-admin for now | |
| 18:24:59 | dansmith | (not that it matters for this conversation, because this API behaves the same as all our other ones at the moment, which rely on db filtering) | |
| 18:25:16 | gmann | dansmith: not filtering but like if admin then we check admin policy and call separate DB interface itself at API layer and do not check is_admin in DB | |
| 18:25:27 | dansmith | gmann: right ++ | |
| 18:27:37 | gmann | on event type: keeping system scope is for human-initiated event ? | |
| 18:28:15 | dansmith | no | |
| 18:28:24 | gmann | making them system-only was not right thing at initial implementation | |
| 18:28:48 | dansmith | honestly, I don't even know what to say anymore: I think neutron declaring a project id when making the call is not bad, | |
| 18:28:50 | sean-k-mooney | i think we just converted admin_api to system_admin_api as an itital step | |
| 18:28:53 | dansmith | which makes it a project-scoped interface It hink | |
| 18:29:11 | gmann | yeah | |
| 18:29:12 | dansmith | I'm just saying I don't want this interface to just be forever tagged as system-only | |
| 18:29:32 | dansmith | if we want to make some events system-only, then let's do that per-event, | |
| 18:29:45 | dansmith | but even in the neutron case, I think scoping it by project is probably not a bad idea anyway | |
| 18:30:05 | sean-k-mooney | dansmith: so currently neutron get the credentails form the config so if we wanted to make it non admin they would need to have some other way to generate the token that is used | |
| 18:30:24 | dansmith | sean-k-mooney: not saying non-admin for network events | |
| 18:30:33 | dansmith | I'm saying probably not system | |
| 18:30:50 | dansmith | there's like a 3D chart here I think :) | |
| 18:30:56 | dansmith | or maybe 7D I dunno | |
| 18:31:05 | gmann | dansmith: agree, and that per event check we can do in code itself as you mentioned earlier. if x_event then context.scope == 'system' | |
| 18:31:14 | dansmith | gmann: exactly | |
| 18:31:36 | dansmith | btw, | |
| 18:32:04 | dansmith | I thought at one point we were supposed to get the ability for neutron to say "here's the user's token for context, and here's my token for auth" | |
| 18:32:18 | sean-k-mooney | right but the network-vif-pluged event for example is sent when but the dhcp server has configured the enttry for the ip and the l2 agent has installed flow rules ectra. so it has to create the token form a config file | |
| 18:32:23 | sean-k-mooney | it cannot use the user token | |
| 18:32:26 | dansmith | so we could scope to the user's project from their token, but elevate the ability to do systemy things with the service's token | |
| 18:32:49 | lbragstad | i think there is a policy check for that? | |
| 18:32:53 | dansmith | sean-k-mooney: it doesn't have to use the user's token | |
| 18:33:47 | dansmith | sean-k-mooney: are you saying it doesn't know anything about the project that owns the port? | |
| 18:33:53 | dansmith | because all it needs is the project_id | |
| 18:34:07 | sean-k-mooney | no the port has a project_id assocatied with it | |