ShellPkg: Disable the shell when UEFI Secure Boot is on

[mediouni-m]

When UEFI Secure Boot is on, we should not allow for the UEFI shell to be used. As such, disabling it in that scenario.

• [ ] Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • Examples: Does it add a new library class or move a module to a different repo.
• [x] Impacts security?
  • The UEFI shell is known as insecure.
• [ ] Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

Has been on AWS since a while, see https://github.com/aws/uefi/blob/5c3ac896feea3923a96944dc23e808385939c0a1/edk2-stable202211/0032-edk2-stable202211-uefi-shell-Disable-the-shell-when-UEFI-Secure-Boot-is-on.patch

Integration Instructions

N/A

[leiflindholm]

I agree 100% with the aim, but I'm not sure this is the best way to solve this upstream.

First, as @samimujawar points out, this is a real problem for development. Second, I should be able to build and sign a standalone shell if that is something I want to do on my own machine.

What we really want to prevent is the lazy inclusion of the shell in the FV during development and then leaving it there when shipping. I ... don't know the best way of doing that.

[NikolajSchlej]

I think we also need to prevent OEMs from signing a shell and using it as a part of their workflows, especially ones facing the user. UEFI shell should be considered a developer-only tool that should never be a part of secure boot chain, and should never be released as a part of such.

I do think the approach done in this patch is sound, and while an option is fine, the default for it should rather be "see SecureBoot enabled => loudly complain and hang", not just exit quietly.

If a developer actually knows what they are doing, it would be trivial for them to flip a define in a header file (I'm against a PCD because that will immediately be set to "make the issue go away" and copy-pasted into the future forever) saying YES_I_M_AWARE_THAT_MIXING_SECUREBOOT_WITH_A_SHELL_IS_REALLY_BAD_IDEA.

[xpahos]

For dsc, there is already an option DEFINE BUILD_SHELL = FALSE. Perhaps it would be sufficient to set its default value to FALSE?

[mediouni-m]

UEFI Shim allows to load an untrusted by UEFI Secure Boot image if it's authorised by the user.

Wonder if disabling startup.nsh and argument parsing when loading Shell.efi would be considered as the right thing to do then (when considering that threat model)

[leiflindholm]

Why cannot Shell be launched when Secure Boot is on?

It is designed to be a debug tool, not a part of a secured boot flow.

As a result, it is pretty much an unbounded attack surface. The mm command on its own rules out any serious CA from considering signing it. But many include it in the FV during development, bypassing Secure Boot checks, and some neglect to delete it for deployment.

[kukrimate]

Why is this being considered now (besides the new media campaigns)? Canonical has reported the same problem and proposed a similar fix that was rejected years ago.

The conclusion there was to disable including the Shell in Secure Boot compatible builds of OVMF, but a similar patch to the Shell itself that would just make it exit with a security violation was rejected. Also note that my patch was somewhat better too because it took Setup mode into account as well.

[JohnAZoidberg]

It is designed to be a debug tool, not a part of a secured boot flow. As a result, it is pretty much an unbounded attack surface. The mm command on its own rules out any serious CA from considering signing it.

The shell can be built with those commands excluded. Is it inconceivable to have a secure shell that is only used for simple scripts and does not allow dangerous operations? Launching applications, evaluating their return codes or output, setting UEFI variables and launching other apps?

Is it more secure to have people learn the intricacies of doing those things in the UEFI C environment?

[kukrimate]

setting UEFI variables

you have already evaluated one of the footguns... being able to set boot services variables will bypass some security features like SkuSiPolicy and SBAT

[leiflindholm]

The shell can be built with those commands excluded.

Which ones? And who will continuously monitor it and make sure changes in UEFI itself doesn't turn previously safe operations unsafe?

But this isn't really about technical details. UEFI Shell isn't a part of UEFI. Unlike open firmware, u-boot, etc, UEFI isn't designed with a command shell in mind. The UEFI Shell is a (very handy) development tool that escaped and started being used in CI, factory testing etc.

[Wack0]

My thought here: disable running startup.nsh automatically when Secure Boot is enabled and Setup Mode is off.

This would prevent usage in a bootkit style scenario with automatically running commands, which seems to be the main issue here.

[NikolajSchlej]

@Wack0, that will not be enough, as a spec-compliant UEFI shell can take a script to run as a command-line option from either ShellOpt variable, or as part of BootXXXX variable it had been executed from, both of which as normal NV vars settable by OS-level attacker (see chapter 3.2 of the the spec linked above for details).

We could disable scripting entirely, making the shell into interactive-only tool, but I'm still in favor of disabling the shell entirely, and explicitly telling the user that it's not compatible with SecureBoot in any way, shape or form.

[Wack0]

a spec-compliant UEFI shell can take a script to run as a command-line option from either ShellOpt variable, or as part of BootXXXX variable it had been executed from, both of which as normal NV vars settable by OS-level attacker (see chapter 3.2 of the the spec linked above for details).

I was unaware of that part of the spec (although I have seen shells in the past implementing the latter, I was unaware it was part of the spec). In that case my thought can easily be revised to "disable any way a script or command can automatically run on shell start if Secure Boot is enabled". That's what is causing exploitability issues here; that "dangerous" commands can automatically execute without confirmation by a physically present user.

[leiflindholm]

I was unaware of that part of the spec (although I have seen shells in the past implementing the latter, I was unaware it was part of the spec). In that case my thought can easily be revised to "disable any way a script or command can automatically run on shell start if Secure Boot is enabled". That's what is causing exploitability issues here; that "dangerous" commands can automatically execute without confirmation by a physically present user.

The Shell is inherently not secure. I don't think there is value, and I absolutely see risk, in trying to retrofit a "securable" sub-configuration.

[mikebeaton]

	NOOPT/DEBUG	RELEASE
Secure Boot	Build and sign shell, and include in FV	Build and sign shell, but don't include in FV (can be manually loaded)
Not Secure Boot	Build and include shell	Build and include shell

Is there any way we could build the shell and include in FV in DEBUG, build and sign but not include in FV in RELEASE? Then developers get the shell 'for free' in DEBUG, and can save it somewhere, navigate to it and launch it, in RELEASE.

[Flickdm]

NOOPT/DEBUG RELEASE Secure Boot Build and sign shell, and include in FV Build and sign shell, but don't include in FV (can be manually loaded) Not Secure Boot Build and include shell Build and include shell Is there any way we could build the shell and include in FV in DEBUG, build and sign but not include in FV in RELEASE? Then developers get the shell 'for free' in DEBUG, and can save it somewhere, navigate to it and launch it, in RELEASE.

If the binary is in the FV it does not need to be signed (FV Apps are implicitly trusted) that being said at Microsoft, our DEBUG firmware (never released to the public and will not work on a fused platform once it leaves the factory) is allowed to include shell simply because the reduced security posture during firmware development.

In RELEASE firmware, shell is not built nor is it included in the FV.

When I need shell in RELEASE firmware with secure boot enabled I do the following:

1. Disable secureboot
2. Enter Setup Mode
3. Add a certificate to the DB
4. Sign my shell with the following command:

& 'signtool.exe' sign /fd sha256 /f DB.pfx /p <password> shell.efi

There should be no reason why in RELEASE firmware shell needs to be included. As @NikolajSchlej said Shell and Secure boot are not compatible.

I'm happy to document the exact steps at https://github.com/microsoft/secureboot_objects

[mikebeaton]

If the binary is in the FV it does not need to be signed (FV Apps are implicitly trusted) that being said at Microsoft, our DEBUG firmware (never released to the public and will not work on a fused platform once it leaves the factory) is allowed to include shell simply because the reduced security posture during firmware development.

In RELEASE firmware, shell is not built nor is it included in the FV.

When I need shell in RELEASE firmware with secure boot enabled I do the following:

1. Disable secureboot
2. Enter Setup Mode
3. Add a certificate to the DB
4. Sign my shell with the following command:

& 'signtool.exe' sign /fd sha256 /f DB.pfx /p <password> shell.efi

There should be no reason why in RELEASE firmware shell needs to be included. As @NikolajSchlej said Shell and Secure boot are not compatible.

I'm happy to document the exact steps at https://github.com/microsoft/secureboot_objects

Fair point that a binary in FV does not even need to be signed. All I was saying was, could the shell not be built, but not placed into any FV, in RELEASE build with secure boot enabled? Even better if there is no question of it being signed in this process - you are quite correct that any developer should know various ways to load the built binary (by signing it, or disabling secure boot) if they need it.