| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-10-05 | |||
| 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 | |
| 18:34:30 | dansmith | right, so it can use an admin token generated from creds from the conf, scoped to that project yeah? | |
| 18:34:34 | sean-k-mooney | but the xproejct_override work that lbragstad has been workign on only works with system tokens | |
| 18:34:44 | dansmith | fine? | |
| 18:35:28 | sean-k-mooney | so its a system admin toke with the project_id also set which is what we started with | |
| 18:35:42 | sean-k-mooney | your just saying the project_id should come form the port | |
| 18:35:48 | sean-k-mooney | not be hardcoded in the config | |
| 18:35:49 | dansmith | what we started with was the assertion that it *has* to be that way | |
| 18:35:54 | dansmith | actuall,y | |
| 18:36:08 | dansmith | I think what we started with was "let's discuss this high-bandwidth at ptg" which is *really* what I'd like | |
| 18:38:44 | sean-k-mooney | ok sure. although this is the second deiscussion i have had on this topic as i started talking to lbragstad about it a few weeks ago | |
| 18:39:36 | dansmith | yeah, clearly he needs to be in said high-bandwidth discussion | |
| 18:39:45 | sean-k-mooney | so i was strating form an assumtion that the neutron config would have the creds for a system scoped token and possibly use the project override mechanium | |
| 18:40:24 | sean-k-mooney | im totally fine with and actully like the idea that neutron would overried the project_id with the ide of the port or network owner | |
| 18:40:48 | lbragstad | override == use the project id pass-through functinality? | |
| 18:41:01 | sean-k-mooney | but we woudl need code change to neutron to do that. today the best we can do via the config would be a system_admin token | |
| 18:41:07 | sean-k-mooney | lbragstad: yes | |
| 18:41:21 | sean-k-mooney | project_id passtough is what i mean by overried | |