Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using OVERRIDE_SECURY_POLICY flag causes build to fail #596

Open
bryteise opened this issue Aug 3, 2023 · 18 comments · May be fixed by #600
Open

Using OVERRIDE_SECURY_POLICY flag causes build to fail #596

bryteise opened this issue Aug 3, 2023 · 18 comments · May be fixed by #600

Comments

@bryteise
Copy link

bryteise commented Aug 3, 2023

When running make OVERRIDE_SECURITY_POLICY=1 the build fails as the security_policy.o isn't getting built with this define set (ends up failing in linking with missing function definitions but there might be other functional issues).

If I add the test and set for OVERRIDE_SECURITY_POLICY in the lib's Makefile setting the define it at least builds. Not sure if there are other things that should be set in the lib DEFINES from the top level Makefile DEFINES but at least that one causes problems.

@dennis-tseng99
Copy link
Contributor

Would you please modify Make.defaults like this:
image

A patch will be committed soon. Thanks for your question.

@dennis-tseng99
Copy link
Contributor

After export OVERRIDE_SECURITY_POLIC, I can successfully make files by using the following command:

make clean; make RELEASE=0 SHIMSTEM=shim MMSTEM=MokManager FBSTEM=fbx64 ENABLE_HTTPBOOT=1 OVERRIDE_SECURITY_POLICY=1 DEFAULT_LOADER="\\\\\\\\grub.efi" shim.efi.debug shim.efi MokManager.efi fbx64.efi

@bryteise bryteise linked a pull request Aug 8, 2023 that will close this issue
@bryteise
Copy link
Author

bryteise commented Aug 8, 2023

The above PR is what ended up working for me.

@dennis-tseng99
Copy link
Contributor

I've got that. Thanks for your response indeed.

@mikebeaton
Copy link
Contributor

mikebeaton commented Aug 28, 2023

I suggested a change to @bryteise's helpful PR, but I remain curious about the status of this bug in the first place, and wonder if anyone could clarify? (cc @dennis-tseng99 @bluca - tyvm in advance for any info)

The inability to build with OVERRIDE_SECURITY_POLICY=1 seems longstanding, but the OVERRIDE_SECURITY_POLICY code itself is also longstanding, and there is at least one useful new commit which apparently specifically updates how OVERRIDE_SECURITY_POLICY works (EDIT: or, at least, is a very helpful addition when using it, since a third-party bootloader which perhaps loads other (e.g. fs, audio, etc.) drivers can continue to use the default LoadImage, with the shim cert checks chained into it, even after the first executable is loaded).

What was the supported (unsupported?/undocumented?) way to build this code up to now - and, if I can ask, is it included in most normal/typical shim builds? (EDIT: From a quick further look, I would say that OVERRIDE_SECURITY_POLICY is not used in normal builds: even when grub2 chainloads to another EFI file - and so would require this feature if it was using the default LoadImage - it uses its own image loader by default.)

@julian-klode
Copy link
Collaborator

I've read up a bit and got some feedback and I'm inclined to suggest removing the code for OVERRIDE_SECURITY_POLICY. It's an ugly hack that's not part of UEFI, it hasn't built in ages, and we do not consider the impact of changes we make to its functionality.

Certainly the idea of boot loaders loading drivers using shim keys is not something that has ever crossed my mind, and it's not really something we should be enabling.

The valid use cases are starting executable, the LoadImage/StartImage/UnloadImage functions in the BS, and the goal for shim 16 would be to hook those directly.

@mikebeaton
Copy link
Contributor

Is it not the case that boot loaders loading drivers using shim keys is required, e.g. for a non-grub post-shim boot loader to load MOK signed kernel modules?

@kukrimate
Copy link

@mikebeaton MOK signed kernel modules do not use any boot services code to get loaded, those signatures are verified by the kernel itself. The MOK certificates are availables at runtime in the MokListRT UEFI variable and then runtime things can do their own verification against those.

I am also very much for revmoving OVERRIDE_SECURITY_POLICY, it is an ugly hack based on the implementation details of some firrmwares, and it is definitely not how any bootloaders should use shim.

Your bootloader should also use the shim protocol like everything else to verify its payload, then use its own loader to load said payload. (A PE loader if it happens to be a PE, or a loader for whatever executable format you want to use).

Getting a shim with an embedded CA with such weirdness signed on the MS UEFI 3rd party CA is of course at the discretion of Microsoft, but nonthless it's not stopping you from using the code.

@mikebeaton
Copy link
Contributor

mikebeaton commented Jan 23, 2024

Hi - Thanks for feedback.

MOK certificates are definitely used as additional, allowed validation by boot services LoadImage if OVERRIDE_SECURITY_POLICY is set. I am not at computer and would need to double check, but I am pretty sure these same certs would validate any efi verified using shim lock as well. (So this, at least, would seem to be another supported feature of MOK certs?)

On your other point, or one of them, we are not planning to try to get this signed by Microsoft, so I suppose I am implicitly asking for usable support for own-builds of shim (to package up useful functionality which advanced users might want, for themselves) - I appreciate this might remain an unsupported scenario.

@kukrimate
Copy link

@mikebeaton I appreciate that OVERRIDE_SECURITY_POLICY works on your machine and allows loading MOK signed PEs. However this is because your UEFI uses the protocol it installs, not because this is standard functionality. You will be able to do the same thing after shim hooks LoadImage, UnloadImage, and StartImage without OVERRIDE_SECURITY_POLICY (I believe this is planned for the next major revision of shim).

For now the best practice is to use the shim lock protocol to verify, then use your own image loader to load images. (As a side note, if GPLv3 is a good license for your use case, you could likely adopt the PE loader used in Debian without too much trouble for your boot loader). Or you can obviously keep using OVERRIDE_SECURITY_POLICY but bear in mind it might get removed.

@mikebeaton
Copy link
Contributor

mikebeaton commented Jan 24, 2024

Hi - Many thanks for your feedback.

As you are no doubt aware, and were summarising briefly, it's not exactly correct that OVERRIDE_SECURITY_POLICY installs a protocol. It hooks into existing protocols, if they are present. The protocols themselves EFI_SECURITY_ARCH_PROTOCOL and EFI_SECURITY2_ARCH_PROTOCOL very much are standard. I would have to agree, it is less clear to me whether hooking these is in any way supported, or even whether these protocols have to affect LoadImage in the way that they apparently do when present. (Or whether this is just specific to the EDK-II implementation from which many/most brands of firmware inherit.)

[EDIT: On further reading of the spec doc linked above, it seems to me that these protocols are indeed, by specification, supposed to affect LoadImage in the way that OVERRIDE_SECURITY_POLICY expects that they do, when they are present. I suppose hooking them is as valid or not as hooking, e.g., LoadImage?]

Thanks for the suggestion of a loader; our project already has one, in fact with additional security, plus an own format loader.

In terms of how OVERRIDE_SECURITY_POLICY currently works, and whether it has any advantages, then it seems to me that the advantages are:

  • All drivers loaded are potentially visible to the original firmware loader (therefore to TPM measurement, etc.) automatically
  • Any other solution requires multiple implementations of loader and measurement
    • While TPM measurement is a de facto standard on wintel, it is not logically true that this is the type of measurement which the underlying firmware is using - which seems to be another factor in favour of passing images to the original loader, if possible

I would agree that if shim hooks LoadImage (and UnloadImage and StartImage) then this seems preferable to shim lock in that not every bootloader has to implement its own loader + shim lock protocol support (+ ideally measurement). I wonder what will be supported by this new initiative, out of what is currently supported by OVERRIDE_SECURITY_POLICY and (in a different way) by shim lock protocol:

  • SBAT enforcement
  • Verification of EFI drivers by Machine Owner Keys (MOK)
  • Verification of loaded images against any db and dbx compiled into (or otherwise securely loaded into? cf SBAT revocation metadata updates without re-issuing a new shim binary #521) shim
    • Noting that a non-grub bootloader may wish to load multiple support drivers, before loading one of several different distros, this does still seem convenient and useful to me, although the user certainly could put such distro-specific certs into firmware db and dbx, if they had to

@jsetje
Copy link
Collaborator

jsetje commented Mar 11, 2024

@bryteise would you be willing to share the use case that caused you to fix this?

@bryteise
Copy link
Author

I'll preface this by saying it has been awhile and I wasn't the person who did all of this work originally. Clear Linux builds the shim, uses systemd-boot as the efi bootloader and our goal is to support self signing, signing using the microsoft key (not currently being used however) and an internal to Intel signing process (also not currently used).

@jsetje
Copy link
Collaborator

jsetje commented Mar 12, 2024

Thank you! The fact that this shows up Clear Linux doesn't make me any less confused. Although it certainly looks like it was in that spec file for a very long time now. I wonder if anything would brake if it was removed.

FWIW I'm trying to figure out if we should fix the build breakage or rip this out entirely.

@bryteise
Copy link
Author

I'll see if anybody remembers the original reason for the need to have the option (or a document) but given it is 7 years old I don't expect too much detail if any. My expectation is that the original developer read the code, figured out the path for getting the required behavior at the time and enabled the option to do so. Given we do not currently use 2 of the signing processes we originally designed the configuration around (and I believe self signing is already very robustly supported) I don't think our use case is of much practical importance.

@joeyli
Copy link
Contributor

joeyli commented Mar 15, 2024

As I remember that the OVERRIDE_SECURITY_POLICY is a extra check for grub2 uses load image to verify Linux kernel by MOK or shim embedded key. After shim provides shim_lock protocol to grub2, the OVERRIDE_SECURITY_POLICY hacking approach is useless.

I agree that the OVERRIDE_SECURITY_POLICY and relates code can be removed.

@mikebeaton
Copy link
Contributor

mikebeaton commented Mar 15, 2024

As I remember that the OVERRIDE_SECURITY_POLICY is a extra check for grub2 uses load image to verify Linux kernel by MOK or shim embedded key. After shim provides shim_lock protocol to grub2, the OVERRIDE_SECURITY_POLICY hacking approach is useless.

I agree that the OVERRIDE_SECURITY_POLICY and relates code can be removed.

Why is it hacking? As I mentioned above #596 (comment) it seems that it is to spec that the security policy protocol behaves as expected by the OVERRIDE_SECURITY_POLICY code. It hopefully can't be argued that merely overriding protocols is hacky in itself, or at least not unnaceptably so, since shim does so anyway.

The use-case I am suggesting for OVERRIDE_SECURITY_POLICY is for a non-participating and third-party (i.e. not shipping with the distro) bootloader, which potentially loads various drivers before the Linux kernel (and which wants to do so securely)*, and which wants integration with MOK (for potential driver verification) (this is just a nice-to-have) and with SBAT (there is no other way to get it other than going via shim). It seems desirable to avoid a proliferation of loaders, as each has different behaviour from the others (one example of I am quite sure many), and presumably just expands the attack surface.

*I work on one, which amongst other features is capable of cleanly booting most standard Linux distros (with or without blspec) directly, without chaining via grub. We do have our own secure image loader (actually with additional security features, hence sometimes pop up pointing out not-to-spec issues in the Linux UEFI world). So the issue is not that we don't want to or can't write our own image loader - nor that we couldn't integrate it with shim protocol, if we had to. I guess the issue is really that, going forwards, the only way to get SBAT is via shim, and then, if OVERRIDE_SECURITY_POLICY is removed, the only way to use that is to implement support for shim protocol - which I believe is a shame, even if OVERRIDE_SECURITY_POLICY remains optional and a non-default build mode of shim.

There are other alternatives, we could also implement our own SBAT support; though this still wouldn't get the MOK integration which might be nice to have for Linux users; and it still seems a shame to remove something that already works.

@mikebeaton
Copy link
Contributor

mikebeaton commented Mar 15, 2024

Hmmm... after a little further internal discussion... and assuming that OVERRIDE_SECURITY_POLICY is never going to become the way shim works by default (shame! but I guess that ship has long since sailed, and perhaps not relying on OVERRIDE_SECURITY_POLICY allows shim to work on more, and more buggy, types of firmware?) then perhaps our best bet is indeed to add our own implementation of SBAT support to our bootloader...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants