Update TrenchBoot boot protocol for AMD in GRUB2

[BeataZdunczyk]

Is your feature request related to a problem? Please describe.

Although some work has been done to implement TrenchBoot support for Qubes OS on AMD hardware using GRUB2, the current implementation is not fully aligned with the TrenchBoot boot protocol being upstreamed to GRUB2 and Linux kernel. This issue aims to update the current work and align it with the TrenchBoot boot protocol being upstreamed to GRUB2 and Linux kernel.

Is your feature request related to a new idea or technology that would benefit the project? Please describe.

This task is necessary to ensure that the TrenchBoot support for Qubes OS on AMD hardware using GRUB2 is properly implemented and fully functional, thus improving the security and functionality of the TrenchBoot solution in general.

Describe the solution you'd like

Update the TrenchBoot boot protocol for AMD platforms in GRUB2 to align with the TrenchBoot boot protocol being upstreamed to GRUB2 and Linux kernel. GRUB2 with TrenchBoot support has been added to Qubes building system on 3mdeb fork.

Describe alternatives you've considered

N/A

Additional context

This feature request is part of Phase 4 in TrenchBoot as Anti Evil Maid project, as outlined in the documentation: https://docs.dasharo.com/projects/trenchboot-aem-v2/.

Relevant documentation you've consulted

N/A

[krystian-hebel]

It seems that the most complete code can be found in https://github.com/3mdeb/grub/tree/tmp, it consists of branch used to prepare Linux SKINIT patches for upstreaming with Multiboot2 changes on top of it. Most of https://github.com/3mdeb/grub/commit/d34597ed10e64c6196d02cfc2c51ec4883510a50 would have to be rewritten, it is the implementation of LZ tags that were later extended to SLRT.

There are newer branches (but created from older code base) that were made for specific purposes like building Nix packages or allowing to build EFI binaries that only worked in very specific instances for demo purposes. I haven't seen anything worth taking from them, especially since we now use qubes-builder to produce packages.

[krystian-hebel]

After second look at the commits:

• Everything up to and including i386/slaunch: Add secure launch framework and commands should already be in TXT patches, but code changed since then so expect conflicts when rebasing/cherry-picking the rest.
• i386/slaunch: Add code for searching for DRTM event log in ACPI can be dropped, we only partially implemented it on PC Engines, but even there fallback would work fine, and PC Engines isn't a good platform for Qubes OS anyway.
• i386/skinit: Add AMD SKINIT definitions header file - take as is for now, may need small changes later for newer CPUs.
• i386/skinit: Add AMD SKINIT core implementation - modify for SLRT:
  • drop LZ_TAG_UNAWARE_OS.
  • not sure about LZ_TAG_LZ_HASH, but I'm leaning towards dropping it as well - SKL (previously called LZ) could measure itself (given some changes to the memory layout, which also would reduce amount of data measured by slow TPM) without relying on data passed by untrusted pre-DLE GRUB.
  • everything else should be already in SLRT so we shouldn't have to add anything related to this to the specification, but needs checking in case I missed something.
  • INIT must be sent through APIC some time before SKINIT is executed, this code worked fine with enabled debug output and/or serial enabled, but not when both of those were missing. AMD APM mentioned waiting for 1000 CPU cycles, but I recall that not being enough. For now it would be enough to rely on debug output somewhere after SINIT is sent for delaying, I'm leaving this comment just for reference. Also, use proper MMIO functions instead of handwriting it.
• i386/slaunch: Add support for AMD SKINIT - use this commit, but drop changes in linux.c, not related to AEM, won't be tested here.
• i386/slaunch: Add support for Multiboot2 for AMD SKINIT - use, probably will need a bit of aligning with TXT code for MB2. GRUB_SKINIT_PROTO_* can be dropped, I think everything needed is in SLRT already.

[SergiiDmytruk]

• i386/slaunch: Add support for AMD SKINIT - use this commit, but drop changes in linux.c, not related to AEM, won't be tested here.

I don't see unrelated changes in linux.c there.

everything else should be already in SLRT so we shouldn't have to add anything related to this to the specification, but needs checking in case I missed something.

@krystian-hebel Not entirely clear about boot parameters. Linux takes its structure, MB2 takes MBI. We also need to pass SLRT somehow (to SKL and kernel). TXT has OS2MLE, does AMD have an equivalent? Otherwise we might need to extend SLRT (for SKL) and also link to it from MBI/linux param (to be able to find it in Xen).

[krystian-hebel]

I don't see unrelated changes in linux.c there.

There are changes for TB, but those are not used when booting Xen through MB2, and as such we won't be using them for Qubes OS. During review on https://github.com/QubesOS/qubes-grub2/pull/13 I got a lot of comments about that code, yet it wasn't tested by us in any way, I'd like to avoid that.

Linux takes its structure, MB2 takes MBI.

A pointer is a pointer :slightly_smiling_face: I think it should be moved to SLRT, most likely directly to Dynamic Launch Configuration, but maybe we can use Boot Loader Context for this.

TXT has OS2MLE, does AMD have an equivalent?

AMD is open when it comes to implementation, we could add OS2MLE in exact form as TXT has, but it would be mostly unused, so I think it would be easier to move boot_params_addr to SLRT instead.

As for location of structure with bootloader->SLK data (whether it will be plain SLRT or another structure embedding it), in AMD you start with 64KB block of memory protected against DMA and initially SMM, in that block SLK is loaded and whatever space is left can be used for passing data securely. https://github.com/3mdeb/grub/commit/d34597ed10e64c6196d02cfc2c51ec4883510a50#diff-ee6e12de6eb52b720e2e6f3792ff7228232d905b280bef353649ae579b9a110eR82 is how it is obtained, and size is read as https://github.com/3mdeb/grub/commit/e3ffaf50990b0a25d1a0851b4488499a197009c6#diff-5d70d6dc848c0abd171fd32c2f6b3817be988fe975f95d8de7469557a381a6c5R34 (ptr[1] in that function will be changed, but I don't know yet how, either next array item will be taken or most likely some better defined header structure will be used instead).

Also, I noticed that https://github.com/3mdeb/grub/commit/d34597ed10e64c6196d02cfc2c51ec4883510a50#diff-ee6e12de6eb52b720e2e6f3792ff7228232d905b280bef353649ae579b9a110eR96-R98 is bad. It should depend on chunks allocated by relocator, doing it the way it was done may overwrite another data that isn't yet relocated to its final place.

[SergiiDmytruk]

There are changes for TB, but those are not used when booting Xen through MB2, and as such we won't be using them for Qubes OS. During review on QubesOS/qubes-grub2#13 I got a lot of comments about that code, yet it wasn't tested by us in any way, I'd like to avoid that.

Looks like I missed the comma when reading drop changes in linux.c, not related to AEM :)

I think it should be moved to SLRT, most likely directly to Dynamic Launch Configuration, but maybe we can use Boot Loader Context for this.

I guess it depends whether it's deemed to be platform-specific and whether more data will need to be added in the future. For now Boot Loader Context should do.

Also, I noticed that 3mdeb/grub@d34597e#diff-ee6e12de6eb52b720e2e6f3792ff7228232d905b280bef353649ae579b9a110eR96-R98 is bad. It should depend on chunks allocated by relocator, doing it the way it was done may overwrite another data that isn't yet relocated to its final place.

Isn't it handled by the caller? (This is from rebased version.)

      slparams->dce_base = (grub_uint32_t) get_virtual_current_address (ch);
      slparams->dce_size = grub_skinit_get_sl_size ();

      err = grub_skinit_boot_prepare (slparams, GRUB_SKINIT_PROTO_MB2);

Does it make sense to try booting rebased version? Was it working at some point? I'm wondering because after switching GRUB to SLRT SKL won't work until its updated to use SLRT.

[krystian-hebel]

slparams->dce_base = (grub_uint32_t) get_virtual_current_address (ch);

This part isn't symmetrical to TXT, where dce_base is physical_target address - although they are always the same for TXT, it is used as a target.

Does it make sense to try booting rebased version? Was it working at some point?

It was working, but with one additional patch to SKL to revert a commit that introduced a regression. I don't think it is worth testing until SKL is ready, because that would require also changes in Xen to not depend on SLRT, which wouldn't bring us closer to expected results.

[SergiiDmytruk]

https://github.com/TrenchBoot/grub/pull/17 added support for AMD SKINIT SLaunch to GRUB2.