| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-10-22 | |||
| 10:11:12 | sean-k-mooney | although on master yo uwoudl now do that via pyproject.toml i think | |
| 10:12:10 | zigo | python3 -m oslo_utils.imageutils -i /path/to/disk <--- Shows nothing, just exists. This means validated in Flamingo ? | |
| 10:13:08 | sean-k-mooney | zigo: anyway my hunch is the mbr is subtlly invalid and not fully complient with the gpt spec | |
| 10:13:19 | sean-k-mooney | am thatm mean our error handleing sucks :) | |
| 10:13:25 | sean-k-mooney | try adding -v | |
| 10:13:53 | zigo | # python3 -m oslo_utils.imageutils -v -i /path/to/disk | |
| 10:13:53 | zigo | SAFETY_CHECK_PASSED=False | |
| 10:13:53 | zigo | VIRTUAL_SIZE=262144 | |
| 10:13:53 | zigo | ACTUAL_SIZE=262144 | |
| 10:13:55 | zigo | IMAGE_FORMAT="gpt" | |
| 10:13:57 | zigo | OSLO_UTILS_VERSION="9.1.0" | |
| 10:14:00 | zigo | FAILURE_REASONS='mbr: GPT MBR has invalid start CHS' | |
| 10:14:14 | sean-k-mooney | there you go | |
| 10:14:23 | zigo | Great, that was very helpful sean! | |
| 10:14:26 | sean-k-mooney | lookign at the code it exits 0 on success and 1 on failure by default | |
| 10:15:09 | sean-k-mooney | so that is form here https://github.com/openstack/oslo.utils/blob/stable/2025.2/oslo_utils/imageutils/format_inspector.py#L1267-L1270 | |
| 10:19:05 | sean-k-mooney | so that path means that the os type is 0xEE a protective MBR https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html#os-types | |
| 10:25:15 | sean-k-mooney | the check that is failign is "(starth, starts, startt) != (0x00, 0x02, 0x00):" thats StartHead StartSector and StartTrack | |
| 10:25:47 | sean-k-mooney | https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html#protective-mbr-partition-record-protecting-the-entire-disk | |
| 10:26:09 | sean-k-mooney | StartingCHS is 3 bytes Set to 0x000200, corresponding to the Starting LBA field. | |
| 10:42:31 | zigo | I probably will propose a patch to accept more valid values. | |
| 10:43:33 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove legacy placement tables from API DB https://review.opendev.org/c/openstack/nova/+/889086 | |
| 10:43:34 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove legacy tables from main DB https://review.opendev.org/c/openstack/nova/+/889087 | |
| 10:43:34 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove legacy columns from main DB https://review.opendev.org/c/openstack/nova/+/889088 | |
| 10:49:12 | opendevreview | Stephen Finucane proposed openstack/nova master: Remove unused 'shutdown_terminate' flag https://review.opendev.org/c/openstack/nova/+/773156 | |
| 10:49:47 | zigo | stephenfin: Gosh, thanks for the placement db cleanup. It's not like if it was a 5 years old technical dept ... :/ | |
| 10:58:14 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove custom comparison methods https://review.opendev.org/c/openstack/nova/+/472285 | |
| 11:05:14 | sean-k-mooney | zigo: that is unlikely to be accpeted as there arent more _valid_ vlaues | |
| 11:05:43 | zigo | There sure is! | |
| 11:05:52 | zigo | https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html | |
| 11:05:55 | sean-k-mooney | if the os type is declared as 0xEE then it must be that value or its not a valid GPT partion table acrodign to the spec | |
| 11:05:56 | zigo | Search for CHS. | |
| 11:06:25 | sean-k-mooney | no the other value are only valid if the mbr is a legacy mbr not a protective mbr | |
| 11:06:50 | zigo | In the case of "Protective MBR Partition Record" their are some other valid values. | |
| 11:07:17 | zigo | Right, but then why would OpenStack prevent using "Protective MBR" ? | |
| 11:07:37 | sean-k-mooney | we dont protective mbr requeis it to be exactly 0x000200 | |
| 11:08:16 | zigo | My image is perfectly valid and has 0x00, 0xee, 0xfe | |
| 11:08:33 | zigo | (starth, starts, startt) | |
| 11:08:38 | sean-k-mooney | legacy mbr allows other value but you cant mix thet 2 | |
| 11:09:17 | zigo | I believe the Debian image is mixing them to allow both MBR *AND* uefi. | |
| 11:09:32 | sean-k-mooney | this is somtihng that is often done incorectly when trying to create a image that is both bootable as uefi and bios | |
| 11:09:57 | zigo | This is maybe incorrect, but existing in the wild at least. | |
| 11:10:19 | sean-k-mooney | yep we have encounedre that in other cases too. | |
| 11:10:28 | sean-k-mooney | those are not techinially allowed by the gpt spec | |
| 11:10:33 | sean-k-mooney | there is a way to do it | |
| 11:10:44 | sean-k-mooney | but its often not done correctly | |
| 11:11:05 | sean-k-mooney | we had this dicussion with at leat one other disto | |
| 11:11:13 | zigo | Ok, but then, what our format_inspector is trying ot acheive is *not* checking if some images aren't respecting the GPT specs. We're trying to check if nova/cinder/glance is being hacked... | |
| 11:12:12 | zigo | I wouldn't mind fixing the Debian if it's broken, that's probably the case, but that wont fix the "I cannot migrate my VMs from my old cloud" use case... | |
| 11:12:31 | zigo | (and that's what I'm currently trying to fix) | |
| 11:12:32 | sean-k-mooney | we are tryign to require "valid" boot image and currently that includes a valid GPT partion table | |
| 11:12:51 | sean-k-mooney | let me see if i can find the perio bug | |
| 11:13:00 | sean-k-mooney | i have forgotten much of the context ath this point | |
| 11:13:30 | zigo | Right. We aren't trying to prevent border-line wrongly not respecting the spec images. | |
| 11:13:49 | zigo | Just something that Qemu can boot safely. | |
| 11:14:38 | zigo | It'd be ok to be more respecting the spec if we were writing a spec validation tool for qemu, but that's not our goal ... | |
| 11:14:47 | sean-k-mooney | https://bugs.launchpad.net/oslo.utils/+bug/2091114 | |
| 11:15:03 | zigo | Thanks, reading. | |
| 14:36:01 | Uggla | Does someone knows about https://review.opendev.org/c/openstack/nova-specs/+/964444 ? I mean do we have the owners of this patch around ? | |
| 14:43:23 | sean-k-mooney | i just saw the propal come in yesterday | |
| 14:43:38 | sean-k-mooney | so i dont know anything more beyond what is in the review | |
| 15:30:47 | zigo | dansmith: I do not agree with your comment in the review. From your review: | |
| 15:30:47 | zigo | "Removing checks from a security layer because Debian's images have been wrong for a long time is not a good plan, IMHO." | |
| 15:30:47 | zigo | It is wrong that these magic byte checks are doing any security improvements. Plus I'm not sure where this image is comming from, I'm not sure it's from Debian, but it's been there in our older cloud that I'm trying to migrate. | |
| 15:31:17 | dansmith | zigo: understood :) | |
| 15:31:19 | zigo | IMO, we should re-open the topic during the vPTG. | |
| 15:31:49 | zigo | I do agree some kind of checking is good, and thanks for writing these. | |
| 15:32:34 | zigo | Though blocking existing (customer) VMs that's been there for so long isn't helpful. | |
| 15:32:56 | zigo | If I'm being ignored, I'll cary this as Debian specific patches ... :P | |
| 15:33:04 | dansmith | these things are 100% security related btw.. the configuration of the MBR is read and interpreted by the hypervisor, the emulated BIOS/firmware, etc.. so it's entirely possible there's an attack vector there | |
| 15:33:41 | zigo | At what stage the hypervisor reads it?!? | |
| 15:34:00 | zigo | Shouldn't this be done by the virtual BIOS when booting? | |
| 15:34:08 | zigo | (ie: not even in Qemu) | |
| 15:34:24 | dansmith | the hypervisor is running the virtual bios :) | |
| 15:34:40 | zigo | Well in qemu, no? | |
| 15:35:02 | dansmith | yes.. you know qemu is the thing that was hackable with the previous image attack right? | |
| 15:35:24 | zigo | If there was a way to escape when Qemu boots an HDD, that would be a serious security issue in Qemu. | |
| 15:35:43 | dansmith | escape is not the only attack mode, of course | |
| 15:35:45 | zigo | Well, the thing that was hackable, was the image conversion. | |
| 15:36:06 | dansmith | no it wasnt.. you might want to re-read the bug :) | |
| 15:36:11 | zigo | That's very different from executing sea-BIOS instructions. | |
| 15:36:15 | dansmith | it was literally qemu itself | |
| 15:37:25 | opendevreview | Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130 | |
| 15:37:34 | zigo | When reading the bootloader stuff?!? | |
| 15:38:42 | dansmith | when reading the image and executing the guest.. not the bootloader specifically, but again, we're talking about being defensive for the *next* thing here | |
| 15:39:33 | dansmith | anyway, re-open the discussion of supporting spec-violating images in the vPTG if you want, and carry debian patches to remove these matches-the-spec image security checks if you think that's the best plan | |
| 15:40:19 | zigo | IMO, here, we're talking about being deffensive against the sea-BIOS being executed and doing weird thing, which is *not* our job, and which is hopefully harmless. | |
| 15:40:52 | zigo | But fine ... :P | |
| 15:41:07 | dansmith | well, I thought that inspecting the details of even things like qcow2 data structures was completely not our job until qemu told us it was | |
| 15:41:13 | zigo | BTW, my first iteration got things wrong, what I added wasn't the boot signature I've found. | |
| 15:41:28 | zigo | I've found (0x00, 0x01, 0x00) | |
| 15:42:02 | zigo | dansmith: Right. It probably deserves a discussion. :) | |
| 15:42:28 | zigo | Thanks for this begining of a chat already. | |
| 15:43:27 | dansmith | personally, I think we've already had the discussion, and we've already had a prior art bug that shows another distro realized they were wrong and posted the easy "make the image match the spec" commands, which we could put in the docs | |
| 15:44:34 | zigo | I probably can also patch the MBR of these old VMs... | |
| 15:44:43 | zigo | I'll see if that path works and let you know. | |
| 15:46:40 | dansmith | maybe glance could have a feature in image conversion to fix these things specifically with parted.. that's something I'd be on board with and probably would address your migration issue | |
| 15:50:56 | sean-k-mooney | zigo: (0x00, 0x01, 0x00) is because of a bug in parted | |
| 15:51:09 | sean-k-mooney | acording to the the flatcar image fix pr anyway | |
| 15:51:22 | zigo | Ah... thanks a lot! :) | |