| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-05-20 | |||
| 19:05:03 | sean-k-mooney | it could be a rbd volume mounted with dmcypt no? | |
| 19:05:27 | melwitt | so the only thing COMPUTE_EPHEMERAL_ENCRYPTION_PLAIN could mean is lvm and [ephemeral_storage_encryption]enabled=true or whatever | |
| 19:05:47 | sean-k-mooney | melwitt: so to me COMPUTE_EPHEMERAL_ENCRYPTION_PLAIN imples using the new feature which also impleis storign seperate secrets for each disk in barbican ectra | |
| 19:06:14 | melwitt | that is not my understanding that we would be supporting rbd with dm-crypt plain | |
| 19:06:30 | sean-k-mooney | the spec does not say you will implment it | |
| 19:06:47 | sean-k-mooney | but takeing a step back and looking at the trait defintion that woudl be a vaild implmeation in nova | |
| 19:06:57 | melwitt | I think (and I could be wrong because I never discussed this with Lee) but I think PLAIN was intended to be for retrofitting legacy (lvm) encryption into the new model | |
| 19:07:08 | melwitt | and no longer use the config options for lvm encryption | |
| 19:08:02 | sean-k-mooney | perhaps but i would not consider the legacy lvm supprot to be a valid impletation with regards to reporting COMPUTE_EPHEMERAL_ENCRYPTION | |
| 19:08:16 | melwitt | the spec has always said "deprecate legacy lvm config and make lvm encrypt work with the flavor/image model" | |
| 19:08:23 | sean-k-mooney | perhaps we would need to granfather it in for upgrade reasons | |
| 19:08:50 | sean-k-mooney | but today if you use the legacy feature its not reflected in the bdms right | |
| 19:08:51 | melwitt | yeah I think the intent was/is to consolidate the way in which local encryption is requested and scheduled and all that | |
| 19:09:04 | melwitt | to make it all work the same way | |
| 19:09:24 | sean-k-mooney | yes but my understaindg is that it was replacing and "adopting" the existing lvm volumes | |
| 19:09:24 | melwitt | I'm not sure about lvm in BDMs, I have to double check | |
| 19:09:46 | sean-k-mooney | i.e. we would back populate the bdms with the encyption flags and format info | |
| 19:10:14 | melwitt | my understanding is that it also provides the mechanism for requesting lvm going forward | |
| 19:10:35 | sean-k-mooney | so lvm encyption is not somethign you request today its config drivven only | |
| 19:10:48 | melwitt | I think it's supposed to be all of that. replace/adopt + support creating new instances with lvm using flavor/image and no more lvm encryption config options | |
| 19:11:01 | melwitt | right.. | |
| 19:11:13 | sean-k-mooney | but the new lvm instance would not use the config options | |
| 19:11:24 | melwitt | right | |
| 19:11:30 | sean-k-mooney | we would use the flavor/iamge properties | |
| 19:11:36 | sean-k-mooney | and unlike the adoptied ones | |
| 19:11:44 | sean-k-mooney | each lvm disk would have its own key | |
| 19:11:49 | sean-k-mooney | just like with qcow | |
| 19:12:14 | melwitt | yes, that is what I thought the spec was intending | |
| 19:12:18 | melwitt | to not have two separate ways of doing local encryption | |
| 19:12:33 | sean-k-mooney | right | |
| 19:12:45 | melwitt | and in the process lvm gains some new user friendly abilities as a result | |
| 19:13:00 | sean-k-mooney | but that would only result in host tha tuse lvm reporting COMPUTE_EPHEMERAL_ENCRYPTION | |
| 19:13:10 | sean-k-mooney | after we supprot upgrading the exiting instnaces | |
| 19:13:32 | melwitt | ok. so all that said, my understanding was that for the current chunk we're trying to do now in dalmatian we are only doing luks and leaving lvm as the legacy config options at this time | |
| 19:13:50 | sean-k-mooney | yep | |
| 19:13:54 | sean-k-mooney | thats what i tought too | |
| 19:14:08 | sean-k-mooney | so supporting rbd,raw/flat,qcow | |
| 19:14:17 | sean-k-mooney | but not lvm | |
| 19:14:26 | melwitt | ok. so it seems then that we would hold off on introducing hw_ephemeral_encryption_format until we work on the lvm support | |
| 19:14:31 | melwitt | right | |
| 19:14:37 | sean-k-mooney | and the exisitng lvm fature will be deprecate but not removed until we can adopt them into the new framework | |
| 19:15:23 | sean-k-mooney | yes we would only need hw_ephemeral_encryption_format if we supprot somethign other hten luksv1 | |
| 19:16:11 | sean-k-mooney | which also means if os_glance_encrypt_format is set to anything other then luks we shoudl reject that too right | |
| 19:16:31 | melwitt | ok, I think we all agree about that. so I'll update the spec to explain this in a lot of detail 😂 and that hw_ephemeral_encryption_format will not be part of this first chunk and that it would come into play only when we work on adding support for lvm | |
| 19:17:02 | sean-k-mooney | and do we agree the same is true for the config option | |
| 19:17:22 | melwitt | I think it would be reasonable to reject that yes, if os_glance_encrypt_format is not luks. we won't be able to honor it | |
| 19:17:40 | sean-k-mooney | ok | |
| 19:18:15 | melwitt | I think I made the config option a choices= where the only choice is luks | |
| 19:18:17 | melwitt | but I'll double check and make it that if it isn't already | |
| 19:18:30 | sean-k-mooney | ok | |
| 19:18:40 | sean-k-mooney | if its only luks then i dont think we need it personally | |
| 19:20:13 | melwitt | that's a fair point. I suppose all it could potentially be helping with transparency but I could see the argument not to have it | |
| 19:20:23 | opendevreview | Merged openstack/nova master: Enable OCaaS for several nova jobs https://review.opendev.org/c/openstack/nova/+/919738 | |
| 19:20:26 | sean-k-mooney | i think what we are setteling on is we only need 1 extra spec/image property the boolean hw:ephemeral_encryption/hw_ephemeral_encryption | |
| 19:20:40 | melwitt | that is also my understanding | |
| 19:20:44 | sean-k-mooney | and if that is unset and the image in encyped then the vm would be encypted too | |
| 19:20:53 | sean-k-mooney | meaning you dont have to opt in for encypted images | |
| 19:21:04 | melwitt | yes, that's what I understand as well | |
| 19:21:20 | sean-k-mooney | so there is only one last bit i want to check | |
| 19:22:00 | sean-k-mooney | will we allow an opt out via the flavor i.e. hw:ephemeral_encryption=false and os_glance_encrypt_format=luks | |
| 19:22:06 | sean-k-mooney | should that be a 409 conflict | |
| 19:22:09 | sean-k-mooney | or decypt the image | |
| 19:22:15 | melwitt | the main thing is I think we want to avoid an unpleasant surprise on that. similar to the encrypted rescue image, not to just decrypt it | |
| 19:22:47 | melwitt | I think the desire based on current code review comments is that it would decrypt in that case | |
| 19:22:51 | sean-k-mooney | personally im feeling like hw:ephemeral_encryption=false and os_glance_encrypt_format=luks woudl be a 409 conflict at least for now | |
| 19:23:09 | melwitt | yeah, I think it would be fair to start with that | |
| 19:23:50 | sean-k-mooney | the reason i hesitate there is it would allow anyoen in the same project (realy that has access to the image secret) to trivially/acidentaly decypt the image | |
| 19:24:14 | melwitt | and tack on a separate patch that enables the ability to request decryption | |
| 19:24:18 | melwitt | because it's like a user experience thing. not really "required" but nice to have to give users that ability | |
| 19:24:23 | sean-k-mooney | its not uncommon for peopel to query flavors by size and select the first one that they find | |
| 19:24:30 | melwitt | yeah. that would be my only concern with it as well | |
| 19:25:03 | sean-k-mooney | to be fair if thte image had os_glance_encrypt_format=luks hw_ephemeral_encryption=ture | |
| 19:25:27 | sean-k-mooney | i think we all agree it should be a 409 conflict if the flavor has hw:ephemeral_encryption=false | |
| 19:26:00 | sean-k-mooney | but since the arguement is being made that os_glance_encrypt_format=luks effectivly implies hw_ephemeral_encryption=ture | |
| 19:26:33 | sean-k-mooney | im wonderin if we shoudl also imply it when comparing ot a flavor with hw:ephemeral_encryption=false | |
| 19:26:35 | melwitt | I expect we all agree that's an ok first step. we've been trying to keep patches smaller in this thing as well so I have been splitting smaller things like that into their own patch | |
| 19:26:49 | sean-k-mooney | ok | |
| 19:27:28 | sean-k-mooney | so before this converstaion i was tending to say "ok ill approve the spec and we can take it to the implemeation" no i feel like we were not on the same page but have got much closer | |
| 19:27:45 | sean-k-mooney | so my stomach say i shoudl have breakfast/dinner | |
| 19:27:55 | sean-k-mooney | but i also dont really want to leave this open | |
| 19:28:04 | sean-k-mooney | so melwitt how do you want to proceed | |
| 19:28:18 | melwitt | yeah so I was thinking if os_glance_encrypt_format=luks (and really mainly just the presence of os_glance_encrypt_key_id) then if hw:ephemeral_encryption=false is in direct conflict and would act the same as the image conflict. until we offer decryption later if we do that | |
| 19:29:29 | sean-k-mooney | ack i think that is logicaly consitent | |
| 19:30:03 | sean-k-mooney | i feel like os_glance_encrypt_key_id is only valid if all the other os_glance_encrypt_* are dfeind | |
| 19:30:15 | melwitt | sean-k-mooney: I think it would probably be best to update the spec now. either way I will update it -- if it gets approved as-is I will still have an amendment proposed shortly. to not forget all this stuff | |
| 19:30:17 | melwitt | so either way is fine with me | |
| 19:30:26 | melwitt | I agree. I think that is the intent in the standardized properties anyway | |
| 19:30:29 | sean-k-mooney | so its not clear to me if we shoudl be checking any spefici one of those keys or if any of them exist | |
| 19:32:16 | melwitt | yeah.. I'll probably add validation for that just to make a better experience. I'm not sure if they are all required to be set from a glance perspective. like, will glance reject upload of an image that only sets one or a subset of them? or maybe if it does allow it, it will fill it defaults itself | |
| 19:32:29 | melwitt | so either way on the nova side we would expect/require them all to be set if one is set | |
| 19:32:38 | sean-k-mooney | so the glance spec used ot have a seperate nova spec https://review.opendev.org/c/openstack/nova-specs/+/608696/11/specs/train/approved/image-encryption.rst | |
| 19:32:43 | sean-k-mooney | for how we would use those | |
| 19:33:27 | melwitt | from my perspective at a minimum if os_glance_encrypt_key_id is set, then os_glance_encrypt_format is required | |
| 19:33:31 | sean-k-mooney | in that they explictly talk about adding supprot for decypting the encyped image when booting the instnace | |
| 19:33:59 | sean-k-mooney | melwitt: ya i think those two are requried and likely the type as well i.e. symetic | |
| 19:34:15 | melwitt | ah ok | |
| 19:34:17 | sean-k-mooney | having said that i think we are only going to support symmetric encption | |
| 19:34:26 | melwitt | ah right | |
| 19:34:30 | sean-k-mooney | so i guess if its single valued | |
| 19:34:34 | sean-k-mooney | it does not matter for now | |