| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-04 | |||
| 08:32:12 | johnthetubaguy | this is implementation detail | |
| 08:33:19 | kashyap | johnthetubaguy: In your copious free time, mind having a look at the Secure Boot spec, please? -- https://review.openstack.org/#/c/506720/ | |
| 08:34:01 | johnthetubaguy | its spec review day, so totally will take a look at that | |
| 08:34:02 | kashyap | Before I respin to rephrase some text, just want to check if there's anything else anyone has. Near as I see, I've addressed all the feedback | |
| 08:34:09 | kashyap | johnthetubaguy: Most excellent | |
| 08:34:27 | johnthetubaguy | that and I have customers who will want this | |
| 08:36:58 | kashyap | johnthetubaguy: Cool! | |
| 08:37:41 | kashyap | johnthetubaguy: A good chunk of stuff is in Work Items | |
| 08:38:05 | kashyap | johnthetubaguy: The thid point, that shows the use 'UefiShell.iso' is info-only; no need for us to run any of that. | |
| 08:38:42 | kashyap | Because, all distributions that matter, ship OVMF "variables" (or "vars") files that have default UEFI keys enrolled. | |
| 08:38:52 | kashyap | Anyway, it's a detail. We'll get to it when you're reviewing. | |
| 08:40:51 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: WIP: Add new default roles in servers API policies https://review.opendev.org/662971 | |
| 08:42:09 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Change the default of notification_format to unversioned https://review.opendev.org/603079 | |
| 08:57:10 | johnthetubaguy | kashyap: I see what you mean about the work items thing... its normally like three lines | |
| 08:57:17 | kashyap | :D | |
| 08:57:43 | kashyap | johnthetubaguy: I wanted to remove some of the background stuff, to reduce text. As most of the stuff is automated for us by libvirt's new interface | |
| 08:57:59 | kashyap | And I've working with distributions to ensure they're shipping the variables files | |
| 09:05:04 | johnthetubaguy | kashyap: yeah, I think you could, finding it interesting, but don't feel qualified to review it | |
| 09:05:38 | kashyap | johnthetubaguy: Okido; I'll leave it as is. I made sure to write clear sentences that add some value :D | |
| 09:07:17 | johnthetubaguy | kashyap: I am putting some comments on the bits I would trim, like that list of distro details, I think just two distros to illustrate the complexity would do | |
| 09:07:41 | kashyap | johnthetubaguy: Sure. Comment away | |
| 09:07:58 | kashyap | johnthetubaguy: Aside: If you want to try a SecureBoot-enabled guest, here's an automated script I wrote for it: https://kashyapc.fedorapeople.org/Create-a-SecureBoot-enabled-VM.bash | |
| 09:08:17 | johnthetubaguy | the ovmf loader paths in the xml... I thought libvirt did all that for us/ | |
| 09:08:57 | kashyap | johnthetubaguy: Are you referring to point-2 in the Work Items section? | |
| 09:09:07 | kashyap | johnthetubaguy: Yes, the new libvirt does handle that for us. | |
| 09:09:19 | johnthetubaguy | yeah, that is the bit | |
| 09:09:35 | kashyap | I have a TODO to trim that second point to reflect that. | |
| 09:19:06 | johnthetubaguy | kashyap: so this sounds harsh... but I think you need to turn your spec upside down, given the new libvirt version, and I guess that is what you are thinking too | |
| 09:19:35 | kashyap | johnthetubaguy: Sorry, what do you mean upside down? | |
| 09:19:49 | kashyap | johnthetubaguy: You mean, push the content from Work Items a bit above? | |
| 09:20:12 | kashyap | Noting clearly that the new libvirt (and QEMU, OVMF / EDK2) version handles most of the work for us? | |
| 09:20:46 | kashyap | (All harsh feedback is welcome :-) After spending so much time on my text, I need other eyes to help edit it.) | |
| 09:20:56 | kashyap | s/other/others'/ | |
| 09:21:32 | johnthetubaguy | kashyap: trying to work out how to describe what I am thinking :) | |
| 09:22:09 | kashyap | johnthetubaguy: Sure. Whatever brings clarity. | |
| 09:22:12 | johnthetubaguy | so overall I think we are largely there, libvirt does all the work, it clearly is something useful | |
| 09:22:43 | kashyap | "No mind is ever willingly deprived of the truth" — Plato | |
| 09:22:51 | kashyap | johnthetubaguy: Yeah, indeed. | |
| 09:23:18 | johnthetubaguy | so I think this spec was needed before libvirt did all the hard work | |
| 09:23:37 | johnthetubaguy | and we still need a bunch of the detail for sure | |
| 09:23:46 | johnthetubaguy | just wondering first about what is missing | |
| 09:23:51 | johnthetubaguy | ... so the last two work items | |
| 09:24:37 | johnthetubaguy | actually, that is back to front, maybe I just say what I think I was expecting... which doesn't make it the right or only way, its just a data point | |
| 09:25:01 | kashyap | Yeah, noted. | |
| 09:25:10 | kashyap | Also, it's not just the last two points, surely? | |
| 09:25:48 | kashyap | Also the point on making Nova use the firmware auto-selection feature? | |
| 09:25:49 | kashyap | </os> | |
| 09:25:49 | kashyap | <loader secure='yes'/> | |
| 09:25:49 | kashyap | <os firmware='efi'> | |
| 09:25:56 | johnthetubaguy | yeah, totally | |
| 09:26:14 | johnthetubaguy | lets go through each bit of the spec quickly, I think its close | |
| 09:26:22 | kashyap | Certainly. | |
| 09:26:54 | johnthetubaguy | so problem and use cases, I think you could simplify it a little bit, line 51-52 and line 42-44 are good | |
| 09:27:47 | kashyap | Right, I'll reword the "or other kernel code ..." thing, if you prefer. | |
| 09:28:05 | johnthetubaguy | well, I mean those lines are vital... i.e. you need this for guests to protect against certain kinds of maleware, and by the way we already added this for hyper-v | |
| 09:28:08 | kashyap | (It's just saying: either guest side malware or malware from kernel modules) | |
| 09:28:25 | johnthetubaguy | the hypervisor kernel modules? | |
| 09:28:28 | kashyap | johnthetubaguy: Added what? (The SecureBoot feature? Sure) | |
| 09:28:31 | kashyap | Yes | |
| 09:28:58 | johnthetubaguy | hmm, OK, I guess it does... :/ not sure | |
| 09:29:21 | johnthetubaguy | anyways, the guest malware seems the key thing for the user | |
| 09:29:29 | johnthetubaguy | sweet | |
| 09:30:38 | johnthetubaguy | the proposed change thing: (1) copy hyper-v interface, (2) libvirt does all the work when we add the above XML, (3) scheduling to make sure we get on a capable host | |
| 09:30:56 | johnthetubaguy | is there anything else from the "changes to nova" sense? | |
| 09:31:23 | kashyap | johnthetubaguy: For the (2) part, we need to expand that: introduce libvirt config classes, etc? | |
| 09:32:20 | kashyap | johnthetubaguy: No, from the list of three, I don't think there's any further, from "changes to Nova" sense. | |
| 09:32:29 | johnthetubaguy | I would do the reverse, this is the magic we put in the XML... PS this is what that really means | |
| 09:33:18 | johnthetubaguy | from the scheduling, I think in the comments it noted that the libvirt driver would add a secure_boot capability style trait if its capable | |
| 09:33:19 | kashyap | Right, noted. | |
| 09:33:32 | johnthetubaguy | I guess we need to ask libvirt if we can do that, once libvirt is the correct version? | |
| 09:34:31 | kashyap | Yes, we need to query via `capabilities` | |
| 09:34:41 | kashyap | To look if it supports the "secure" flag for 'efi' | |
| 09:34:46 | johnthetubaguy | cool | |
| 09:34:54 | johnthetubaguy | so its "a little bit" like this: https://github.com/openstack/nova/blob/2ea6e6f8db9fc6cecf389cacdd0d82d8226b99fb/nova/virt/libvirt/driver.py#L334 | |
| 09:35:07 | kashyap | s/capabilities/getDomainCapabilities/ | |
| 09:35:09 | johnthetubaguy | in that its a conditional capability | |
| 09:36:00 | kashyap | johnthetubaguy: (Aside: While implementing, I could use some help on the scheduling bits, it's my weak area.) | |
| 09:36:22 | johnthetubaguy | so I fear the spec needs to cover the rough details of what is going to happen there | |
| 09:36:24 | johnthetubaguy | https://github.com/openstack/nova/blob/2ea6e6f8db9fc6cecf389cacdd0d82d8226b99fb/nova/virt/libvirt/driver.py#L4959 | |
| 09:36:33 | johnthetubaguy | I guess its an extension of that | |
| 09:37:07 | kashyap | johnthetubaguy: Yeah, indeed: Need to write: _has_uefi_secure_boot_support() | |
| 09:37:34 | johnthetubaguy | how to we schedule for uefi today... I don't rightly remember | |
| 09:37:55 | kashyap | We don't do any scheduling decisions for UEFI, IIRC | |
| 09:40:05 | kashyap | johnthetubaguy: Do you see anything contrary to what I say in the code? | |
| 09:41:18 | johnthetubaguy | kashyap: not so far... just re-reading the hyper-v spec | |
| 09:42:10 | johnthetubaguy | do we not need os_secure_boot_signature? | |
| 09:42:17 | johnthetubaguy | ref: https://specs.openstack.org/openstack/nova-specs/specs/ocata/implemented/hyper-v-uefi-secureboot.html | |
| 09:42:37 | openstackgerrit | Ghanshyam Mann proposed openstack/nova-specs master: Policy Default Refresh spec https://review.opendev.org/547850 | |
| 09:44:50 | kashyap | johnthetubaguy: I _think_ for the first iteration, it should be optional. | |
| 09:45:03 | kashyap | `os_secure_boot_signature` allows specifying bootloader's signature | |
| 09:45:17 | kashyap | I need to play a bit more to see how strongly we need it | |
| 09:45:47 | kashyap | johnthetubaguy: Because, the OVMF maintainer says: if you don't trust the default UEFI keys, then it is almost the same as you're not trusting the filesystem where your Compute node is running | |
| 09:46:08 | johnthetubaguy | so you know... I think this was before the distros shipped trusted default keys | |
| 09:46:27 | kashyap | Yeah, very true. | |
| 09:46:29 | johnthetubaguy | lets just add in the spec that we only support using the default keys in the first implementation? | |
| 09:46:44 | kashyap | johnthetubaguy: Yes, very much worth it to spell it out | |
| 09:54:06 | johnthetubaguy | kashyap: so I just added an extra note to give a summary of what I was thinking for the proposed changes section | |
| 09:54:22 | johnthetubaguy | I am tempted to say this first version follows UEFI and simple errors out if not supported? | |
| 09:55:00 | johnthetubaguy | then in the alternatives but that in the future a request spec filter can be added similar to the existing image type filter | |
| 09:56:06 | johnthetubaguy | kashyap, the release note for hyper-v is quite nice: https://github.com/openstack/nova/blob/c6218428e9b29a2c52808ec7d27b4b21aadc0299/releasenotes/notes/hyperv-uefi-secure-boot-a2a617ac2c313afd.yaml | |