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

Drop builtin shell from release UEFI images #1218

Open
macpijan opened this issue Jan 28, 2025 · 14 comments
Open

Drop builtin shell from release UEFI images #1218

macpijan opened this issue Jan 28, 2025 · 14 comments
Labels
bug Something isn't working firmware needs review

Comments

@macpijan
Copy link
Contributor

macpijan commented Jan 28, 2025

Component

Dasharo firmware

Device

other

Dasharo version

All

Dasharo Tools Suite version

No response

Test case ID

No response

Brief summary

Potential to allow for SB bypass.

Resources:

How reproducible

No response

How to reproduce

Enter boot firmware boot manager

Expected behavior

UEFI shell is not present in boot manger

Actual behavior

UEFI shell is present in boot manager

Screenshots

No response

Additional context

No response

Solutions you've tried

No response

@macpijan macpijan added bug Something isn't working needs review labels Jan 28, 2025
@macpijan
Copy link
Contributor Author

@mkopec at some point you have mentioned that perhaps dropping memory access command may also be a way here. Are we aware of others practical bypass methods if we exclude the memory management commands @mkopec @krystian-hebel ?

@krystian-hebel
Copy link
Contributor

There are few commands that allow for installation of UEFI drivers, I don't know if they can load them bypassing SB or not.

@miczyg1
Copy link
Contributor

miczyg1 commented Jan 28, 2025

There are few commands that allow for installation of UEFI drivers, I don't know if they can load them bypassing SB or not.

One can even load an OptionROM.

@mkopec
Copy link
Member

mkopec commented Jan 29, 2025

I don't know if they can load them bypassing SB or not.

The default policy for images from mass storage is to Deny execution when there is security violation. That said, allowing loading arbitrary signed drivers could also be problematic...

I think we can remove the shell pretty safely, if it's needed by users or by us for testing, it can easily be installed to the ESP.

@krystian-hebel
Copy link
Contributor

krystian-hebel commented Jan 29, 2025

I think we can remove the shell pretty safely, if it's needed by users or by us for testing, it can easily be installed to the ESP.

IIRC we're using it also for testing SB itself, so it would have to be signed. Of course, we don't want to have it signed with release keys, so some tests or their order may require changes, so that the shell key is installed for those tests.

@mkopec
Copy link
Member

mkopec commented Jan 29, 2025

SB tests are just trying to boot signed and unsigned images. This can be done from the setup menu itself

@krystian-hebel
Copy link
Contributor

It also compares the output, which may be fine for platforms with UART, but not for manual tests. I recall that in case of error it returns to the setup menu without any delay, but maybe I'm misremembering something.

@mkopec
Copy link
Member

mkopec commented Jan 29, 2025

Hmm I think the last time I ran into this, the error message was followed by Press any key to continue but I might also be misremembering :)

@pietrushnic
Copy link

It retrurn immiedietly, but still I don't see reason why "Active Debug Code" would be absolutely necessary for us to do tests. If that is problem we should change tests. In most cases we want to test system in production state.

Last resort one who would need shell for something have to disable secure boot or sign it plus enroll required keys.

Please let me know if I'm missing something.

@miczyg1
Copy link
Contributor

miczyg1 commented Jan 29, 2025

One can simply plug an USB with signed and unsigned BOOTX64.efi file. The boot manager should still print Access Denied and whether signatures are missing or are invalid: https://github.com/Dasharo/edk2/blob/dasharo/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c#L2027

But it has to be booted from BootManagerMenuApp rather than setup I think.

@krystian-hebel
Copy link
Contributor

What about iPXE? Is version in our releases signed? Does it block attempts to boot anything else than properly signed UEFI binaries?

@macpijan
Copy link
Contributor Author

macpijan commented Jan 30, 2025

IIRC it does not work with SB, but I do not remember on which stage. Probably loading OS?
That is why we used to disable SB in FUM to load DTS, correct?

@m-iwanicki
Copy link

IIRC it does not work with SB, but I do not remember on which stage. Probably loading OS?

@macpijan I managed to make DTS work with SB via iPXE (on QEMU):

  1. Create UKI file (I used kernel and rootfs from https://boot.dasharo.com/dts/v2.2.0/)

    ukify build --linux bzImage-v2.2.0 --initrd dts-base-image-v2.2.0.cpio.gz --output dts.efi
  2. Create DB key and cert

    openssl req -quiet -new -x509 -newkey rsa:2048 -nodes -keyout DB.key -out DB.crt -subj "/C=PL"
    openssl x509 -in DB.crt -out DB.cer -outform DER
  3. Sign dts.efi

    sbsign --key DB.key --cert DB.crt --output dts-signed.efi dts.efi
  4. Create vfat partition file and put DB.crt certificate inside so it can be
    enrolled

    dd if=/dev/zero of=db.img bs=1M count=10
    mkfs.vfat db.img
    mkdir mnt
    sudo mount db.img mnt
    sudo cp DB.crt mnt/
    sudo umount mnt
  5. Start HTTP server

    python -m http.server
  6. Start QEMU in another terminal (I used qemu-run.sh from
    open-source-firmware-validation)

    HDD_PATH="<path to db.img>" ./scripts/ci/qemu-run.sh graphic os
  7. Enroll DB.cer, enable Secure Boot and enable network boot after which
    save and restart

  8. Boot into iPXE shell, use dhcp command after which:

    • Try to boot unsigned dts.efi

      iPXE> chain http://localhost:8000/dts.efi
      http://localhost:8000/dts.efi... ok
      Could not boot: Error 0x7f04818f (https://ipxe.org/7f04818f)
    • Try to boot signed dts-signed.efi

      iPXE> chain http://localhost:8000/dts-signed.efi
      http://localhost:8000/dts-signed.efi... ok
      Error registering initrd: Already started
      Could not boot: Error 0x7f048294 (https://ipxe.org/7f048294)

      Previous attempt likely loaded something already

    • Reboot, reenter iPXE shell and try to boot signed dts-signed.efi
      without first trying to boot dts.efi

      iPXE> chain http://localhost:8000/dts-signed.efi
      http://localhost:8000/dts-signed.efi... ok

      You should be able to boot into DTS

  9. If you managed to boot into DTS:

    bash-5.2# cat /etc/os-release
    ID=dts-distro
    NAME="Dasharo Tools Suite"
    VERSION="2.2.0 (scarthgap)"
    VERSION_ID=2.2.0
    VERSION_CODENAME="scarthgap"
    PRETTY_NAME="Dasharo Tools Suite 2.2.0 (scarthgap)"
    CPE_NAME="cpe:/o:openembedded:dts-distro:2.2.0"
    
    bash-5.2# dmesg | grep -i secure
    [    0.004175] Secure boot enabled
    [    0.331135] sdhci: Secure Digital Host Controller Interface driver
    

I think more advanced use cases could also be made to work e.g. with shim: https://ipxe.org/cmd/shim

It's probably not good issue to discuss this more but maybe we could release UKI EFI binaries in e.g. wic (those would be without rootfs) and on boot.dasharo.com (those would be with kernel/initramfs/rootfs). That would make booting with e.g. SB enabled possible.
In iPXE we could use https://ipxe.org/cfg/platform#test_whether_or_not_current_platform_is_u_efi to decide whether to use EFI file or normal bzImage/cpio.gz

@miczyg1
Copy link
Contributor

miczyg1 commented Feb 10, 2025

This PR drops the SHell globally by default in our builds: Dasharo/coreboot#613
Related PR to allow builds without the Shell: https://github.com/Dasharo/edk2/pull/208/files
Issue for OSFV test updates: Dasharo/open-source-firmware-validation#696

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working firmware needs review
Projects
None yet
Development

No branches or pull requests

6 participants