| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-04-17 | |||
| 12:46:58 | bauzas | Uggla: let's discuss this directly by gmeet if you want ;) | |
| 12:47:08 | Uggla | ok | |
| 12:47:17 | Uggla | cleaning my env and calling you | |
| 12:48:13 | bauzas | songwenping: honestly, nova just adds a mdev on the libvirt XML https://libvirt.org/drvnodedev.html#mediated-devices-mdevs | |
| 12:48:32 | bauzas | Uggla: ok | |
| 13:22:03 | artom | Uggla, oh, the privsep mount thing | |
| 13:22:09 | artom | Yeah, it's super weird, I am indeed a witness | |
| 13:41:30 | opendevreview | Dan Smith proposed openstack/nova master: Stop ignoring missing compute nodes in claims https://review.opendev.org/c/openstack/nova/+/880633 | |
| 13:41:30 | opendevreview | Dan Smith proposed openstack/nova master: Remove silent failure to find a node on rebuild https://review.opendev.org/c/openstack/nova/+/880632 | |
| 13:41:41 | dansmith | bauzas: I pulled the RT stuff out into a separate set^ | |
| 13:42:02 | bauzas | dansmith: on a long call with Uggla but okay, I'll try | |
| 13:42:07 | dansmith | and fixed another silent failure we ignore in evacuate | |
| 14:21:10 | bauzas | gibi: dansmith: sean-k-mooney: fyi, we discussed with Uggla about his series and we discovered a potential large leak for ACLs in Manila access rights | |
| 14:21:32 | gibi | ack | |
| 14:21:39 | bauzas | gibi: dansmith: sean-k-mooney: when adding an access-allow for the share, we pass the compute IP address to Manila | |
| 14:21:55 | dansmith | which is visible by the user? | |
| 14:21:57 | bauzas | then the IP address can be seen by any user in the same project, and also any user can delete this ACL | |
| 14:22:19 | dansmith | that'd be bad | |
| 14:22:42 | bauzas | so we need to tell the Manila folks to somehow hide those details | |
| 14:22:57 | dansmith | yeah | |
| 14:23:22 | bauzas | if the ACL is done by a service, it shouldn't be seen by an enduser, neither be able to delete it | |
| 14:23:46 | bauzas | if we ask Manila to lock a share, that's a different concern | |
| 14:23:55 | sean-k-mooney | bauzas: this is partly why i wanted to use the cert auth method not the ip one | |
| 14:23:57 | bauzas | because users can delete ACLs without unlocking | |
| 14:24:09 | dansmith | ...yup | |
| 14:24:38 | bauzas | sean-k-mooney: it should also leak the certificate, right? | |
| 14:24:49 | bauzas | and users could also delete the ACL | |
| 14:25:23 | bauzas | there are two problems to resolve : 1/ we need to hide the ACL, 2/ we need to make sure it's not possible to delete it by an user | |
| 14:25:30 | Uggla | sean-k-mooney, also today you can do a manila show share that will reveal IP + export location | |
| 14:25:32 | sean-k-mooney | the cert would be a per vm cert generted for that insntace | |
| 14:25:33 | bauzas | we == Manila API | |
| 14:25:40 | sean-k-mooney | and it woudl only be the public key | |
| 14:25:50 | sean-k-mooney | so i dont think we care if that is visable | |
| 14:26:31 | dansmith | does nova use only the user's token to talk to mania? | |
| 14:26:32 | bauzas | sean-k-mooney: then we would need to extend the lock mechanism to deny any ACL modification | |
| 14:26:35 | dansmith | *manila | |
| 14:26:53 | sean-k-mooney | https://docs.openstack.org/api-ref/shared-file-system/?expanded=grant-access-detail#grant-access | |
| 14:26:58 | bauzas | dansmith: we tested this also with admin | |
| 14:27:02 | dansmith | I'm asking | |
| 14:27:08 | sean-k-mooney | i would prefer to use cert or user for auth then ip | |
| 14:27:20 | bauzas | dansmith: if you're an admin, you can generate a share and add an ACL | |
| 14:27:36 | bauzas | dansmith: but any user from the same project can both see the share and delete the ACL you created | |
| 14:27:40 | dansmith | bauzas: can you answer my question? | |
| 14:28:01 | dansmith | does nova use the user's token (only) to talk to manila? | |
| 14:28:13 | sean-k-mooney | dansmith: yes i belive we use only the user token in the current proposal | |
| 14:28:18 | bauzas | dansmith: IIRC today yes but Uggla knows better about it | |
| 14:28:24 | dansmith | ack | |
| 14:28:30 | sean-k-mooney | but we could add an admin token if we needed too. i dont think we use any admin api today | |
| 14:28:38 | dansmith | we have a lot of different places where we've done that in the past and it has come back to bite us | |
| 14:28:39 | sean-k-mooney | but we might for the lock api | |
| 14:28:42 | bauzas | we discussed this at the PTG, we could use a service token if Manila adds it | |
| 14:28:51 | bauzas | but again, we missed at the PTG the ACL problem | |
| 14:28:59 | dansmith | cinder, glance, etc.. so perhaps we should not replicate the same thing here | |
| 14:29:21 | sean-k-mooney | bauzas: the probelm is leakign the ip/subnet of the nova comptue host | |
| 14:29:25 | bauzas | since anyone from the same project can read anything about shares, then a service token needs to be related to a specific different project | |
| 14:29:30 | dansmith | I know that even if nova uses admin or service, manila probably needs extra work to not allow a user to delete/see that thing | |
| 14:29:30 | sean-k-mooney | the fix for that is to use a differnt grant type | |
| 14:29:43 | dansmith | but I'm saying.. using the user's token puts us at a disadvantage for actually filtering those things | |
| 14:30:15 | dansmith | sean-k-mooney: that may be better anyway, but I'm saying it might be a good idea to not go down this same road again | |
| 14:30:16 | gibi | so when nova adds the compute IP to a request sent to manila with the user token, nova basically leaked the IP of the compute to that user (and project) | |
| 14:30:17 | bauzas | sean-k-mooney: that's not only an IP leak problem, this is also a security problem since a malicious user can block the mount by deleting the ACL | |
| 14:30:39 | Uggla | sean-k-mooney, we need to take care even with certs, I'm not sure that manila will not expose the IP in the export location as well. | |
| 14:30:40 | dansmith | even if we use cert auth, we're leaking something about the compute node | |
| 14:30:48 | sean-k-mooney | bauzas: im aware but if we have locked the share you shoudl not be able to do that | |
| 14:30:56 | sean-k-mooney | so we can include that in the spec for the new lock api | |
| 14:31:02 | dansmith | two users can conspire to compare certs about their hosts if they can see them and determine if they're on the same compute node, for example | |
| 14:31:04 | bauzas | yup, + again, we allow anyone to block a mount | |
| 14:31:10 | dansmith | the cert probably has the hostname in it too right? | |
| 14:31:28 | sean-k-mooney | dansmith not if the cert is per instance | |
| 14:31:31 | bauzas | I think sean was proposing an instance-based cert | |
| 14:31:32 | sean-k-mooney | but otherwise yes | |
| 14:31:42 | bauzas | but that doesn't solve the ACL delete case | |
| 14:31:48 | dansmith | ack | |
| 14:31:51 | dansmith | right | |
| 14:31:56 | bauzas | and it adds a lot of complexity for little benefits | |
| 14:32:16 | bauzas | + I'm unsure that the export location doesn't show the IP address, even with certr | |
| 14:32:27 | dansmith | I think manila is going to need finer granularity for this sort of thing, and nova is going to have to not use the user's token (alone) to do this stuff if we're going to distinguish between the user and nova | |
| 14:34:16 | bauzas | dansmith: yup, and we need to round it back to the Manila team | |
| 14:34:28 | dansmith | yeah | |
| 14:38:02 | bauzas | thinking it more, the real problem is the fact that Manila exposes the export_location on the API for any user | |
| 14:38:35 | dansmith | well, for standalone usage it probably has to | |
| 14:38:36 | bauzas | sean-k-mooney: even with certs, the compute IP address would be shown in the export_location field of the manila share | |
| 14:38:42 | bauzas | tryue | |
| 14:38:47 | dansmith | however, I wonder if it should always only show a user their *own* exports | |
| 14:38:53 | bauzas | that's absolutely understandable from a standalone perspective | |
| 14:38:55 | sean-k-mooney | im not sure about that | |
| 14:38:59 | sean-k-mooney | but im in another meeting | |
| 14:39:01 | dansmith | that way, if nova uses its own user to create the export, the user would not be able to see it | |
| 14:39:04 | sean-k-mooney | so ill check back with this after | |
| 14:41:52 | bauzas | dansmith: another user, another project, but yeah | |
| 14:42:06 | bauzas | dansmith: it looks to me the granularity on CRUDs on per project | |
| 14:42:07 | dansmith | bauzas: well, either really | |
| 14:42:09 | dansmith | bauzas: we have per-user resources | |
| 14:42:15 | dansmith | but we need something | |
| 14:42:43 | dansmith | project would mean that the neutron user could see exports created by the nova user, which is probably not ever necessary | |
| 14:42:56 | dansmith | also not terrible, but.. | |
| 14:42:58 | bauzas | dansmith: well, we tested with Uggla, a devstack admin can create a share and add an access ACL, a devstack demo user can both delete the ACL and the share | |
| 14:43:16 | dansmith | wait, what? | |
| 14:43:22 | bauzas | that yeah | |
| 14:43:29 | dansmith | but why? | |