#1385 PR `merged`: Do not recreate BOOTX64.efi, if Secure boot is enabled.¶

Labels: fixed / solved / done, minor bug

gozora opened issue at 2017-06-19 13:32:¶

If server have Secure Boot enabled, ReaR should not try to create boot loader (BL) for recovery system (which is handy in other cases). As we are not able to sign it, it will be refused by Secure Boot.

This patch just skips ReaRs attempt to create BL and uses original shipped with OS.

c.f. https://github.com/rear/rear/issues/1374

jsmeix commented at 2017-06-20 08:19:¶

@gozora
a side note to be perhaps more future proof:
Currently (also in output/ISO/Linux-i386/250_populate_efibootimg.sh)
the following test is used

if [[ $(basename ${UEFI_BOOTLOADER}) = shim.efi ]]; then

to do "the right things" in case of Secure Boot.
I wonder about the hardcoded fixed file name 'shim.efi'.
Can we assume with sufficiently high probability that
the properly signed bootloader for Secure Boot will
be in a file named 'shim.efi' or should we do some
guesswork to find it under different file names too
or even add a config variable like
SECURE_BOOT_BOOTLOADER="shim.efi"
so that the user can specify - if needed - his actually
used bootloader for Secure Boot?
( Hint: "Arch Linux" ;-)

gozora commented at 2017-06-20 09:15:¶

Hello @jsmeix,

Reading 850_save_sysfs_uefi_vars.sh, shim.efi just can't be guessed by ReaR so user must set UEFI_BOOTLOADER variable (otherwise Secure Boot will not work).
At this stage I'm not sure how guessing could be done, because as you've mentioned shim.efi is not a constant and can be arbitrary named.

I personally would let user to decide ...

V.

gdha commented at 2017-06-20 09:31:¶

@gozora I fully agree with you

schlomo commented at 2017-06-20 09:37:¶

Why can't we read the current boot configuration? Like this:

$ sudo efibootmgr -v
BootCurrent: 0000
Timeout: 2 seconds
BootOrder: 0000,001A,001B,001C,0017,0018,0019
Boot0000* ubuntu    HD(1,GPT,f8dd62b6-355c-4843-b6f0-3fc9b07f29f2,0x800,0x1e8000)/File(\EFI\ubuntu\shimx64.efi)
Boot0010  Setup FvFile(721c8b66-426c-4e86-8e99-3457c46ab0b9)
Boot0011  Boot Menu FvFile(126a762d-5758-4fca-8531-201a7f57f850)
Boot0012  Diagnostic Splash Screen  FvFile(a7d8d9a6-6ab0-4aeb-ad9d-163e59a7a380)
Boot0013  Lenovo Diagnostics    FvFile(3f7e615b-0d45-4f80-88dc-26b234958560)
Boot0014  Startup Interrupt Menu    FvFile(f46ee6f4-4785-43a3-923d-7f786c3c8479)
Boot0015  Rescue and Recovery   FvFile(665d3f60-ad3e-4cad-8e26-db46eee9f1b5)
Boot0016  MEBx Hot Key  FvFile(ac6fd56a-3d41-4efd-a1b9-870293811a28)
Boot0017* USB CD    VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,86701296aa5a7848b66cd49dd3ba6a55)
Boot0018* USB FDD   VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,6ff015a28830b543a8b8641009461e49)
Boot0019* NVMe0 VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,001c199932d94c4eae9aa0b6e98eb8a400)
Boot001A* ATA HDD0  VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,91af625956449f41a7b91f4f892ab0f600)
Boot001B* USB HDD   VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,33e821aaaf33bc4789bd419f88c50803)
Boot001C* PCI LAN   VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,78a84aaf2b2afc4ea79cf5cc8f3d3803)
Boot001D* IDER BOOT CDROM   PciRoot(0x0)/Pci(0x16,0x2)/Ata(0,1,0)
Boot001E* IDER BOOT Floppy  PciRoot(0x0)/Pci(0x16,0x2)/Ata(0,0,0)
Boot001F* ATA HDD   VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,91af625956449f41a7b91f4f892ab0f6)
Boot0020* ATAPI CD  VenMsg(bc7838d2-0f82-4d60-8316-c068ee79d25b,aea2090adfde214e8b3a5e471856a354)

And from there we see which file is used on the source system and simply use that also for ReaR and put it back after recovery.

gozora commented at 2017-06-20 10:29:¶

@schlomo I was thinking about this approach in the past.
However, UEFI is so variable that even efibootmgr don't need to display right values.
Problem is that your system might be booted from EFI shell (either automatically or with help of startup.nsh) so that would mean additional parsing or searching UEFI firmware for correct values.
IMHO this could become quite a hell to maintain.

V.

jsmeix commented at 2017-06-20 11:14:¶

Perhaps I confuse something here but assume
for a particular user the bootloader for Secure Boot
is in a file named 'mysignedbootloader'
and the user specified that with
UEFI_BOOTLOADER=/boot/efi/EFI/mysignedbootloader
then I wonder how the test for the hardcoded 'shim.efi'
would work?

gdha commented at 2017-06-20 11:16:¶

@jsmeix if you know how to sign your own UEFI bootloader then you can do what you want - it is not that hard, but is IMHO out-of-scope for ReaR recovery

gdha commented at 2017-06-20 11:20:¶

@schlomo the efibootmgr is a crappy piece of software - years ago when I implemented the UEFI boot stuff it gave me lost of issues.
I fully agree with @gozora that sometimes the user knows best what to do. The only thing that we might foresee is a better explanation in the default.conf file perhaps?

jsmeix commented at 2017-06-20 11:21:¶

@gdha
look at what @schlomo wrote above in his
https://github.com/rear/rear/pull/1385#issuecomment-309700064
(excerpt)

Boot0000* ubuntu ... /File(\EFI\ubuntu\shimx64.efi)

which seems to indicate that on Ubuntu the
bootloader for Secure Boot is in a file named 'shimx64.efi'
so that - if I understand @gozora
https://github.com/rear/rear/pull/1385#issuecomment-309694405
correctly the user would have to specify it like

UEFI_BOOTLOADER=/boot/EFI/ubuntu/shimx64.efi

and then

if [[ $(basename ${UEFI_BOOTLOADER}) = shim.efi ]]; then

would not match.

gdha commented at 2017-06-20 11:26:¶

@jsmeix I think that rear will pick this up automatically? @gozora Can you confirm this? I don't use UEFI anymore myself.

gozora commented at 2017-06-20 11:39:¶

Actually @jsmeix is right!
If something else than shim.efi is used, problems might occur again.
So introducing SECURE_BOOT_BOOTLOADER and removing "shim.efi" from code is a good idea!

V.

schlomo commented at 2017-06-20 11:47:¶

Good point about the fact that UEFI allows also other boot methods. IMHO we should try our best guess that works automagically for systems there where set up without manual customizing (like my laptop here). Additionally we should allow manual configuration via variables for those cases where the automagic mode did not work.

gozora commented at 2017-06-20 12:03:¶

@schlomo

IMHO we should try our best guess that works automagically for systems there where set up without manual customizing (like my laptop here). Additionally we should allow manual configuration via variables for those cases where the automagic mode did not work.

I guess this is something we already have in place ;-). I personally like current UEFI implementation.

@schlomo, @jsmeix, @gdha
So everybody OK with introducing SECURE_BOOT_BOOTLOADER in default.conf ?
If so, I'd update my PR later today ...

V.

schlomo commented at 2017-06-20 12:04:¶

👍

jsmeix commented at 2017-06-20 13:01:¶

I always like to have config variables for basically everything, cf.
https://github.com/rear/rear/pull/1204#issuecomment-282252947
https://github.com/rear/rear/issues/1213#issuecomment-285618629

jsmeix commented at 2017-06-21 07:53:¶

@gozora
I fear I am a pain in your neck when I write this
but I think it is better when I tell about my thoughts.
In the end it is your code and you make the final decision
what the best way is how to implement it.

For me without background knowledge plain looking at

UEFI_BOOTLOADER="/boot/efi/EFI/BOOT/shim.efi"
SECURE_BOOT_BOOTLOADER="shim.efi"

looks somewhat intricate and/or overcomplicated
so that I wonder if it really must be this way.

When I see that I think something like
"Why does one need to specify 'shim.efi' twice?
Why does not only something like this

SECURE_BOOT_BOOTLOADER="/boot/efi/EFI/BOOT/shim.efi"

work?
Why is UEFI_BOOTLOADER a full path but
SECURE_BOOT_BOOTLOADER is only a file name?"

I would expect that when SECURE_BOOT_BOOTLOADER
is non-empty, it means Secure Boot is used and
then SECURE_BOOT_BOOTLOADER specifies the
full path to the right (signed) Secure Boot bootloader.

In this case the tests could be simplified to something like

if test -e "$SECURE_BOOT_BOOTLOADER" ; then

and the build_bootx86_efi call might be even simplified to

test -e "$SECURE_BOOT_BOOTLOADER" || build_bootx86_efi

Perhaps additionally something like

if test -e "$SECURE_BOOT_BOOTLOADER" ; then
    UEFI_BOOTLOADER="$SECURE_BOOT_BOOTLOADER"
fi

is also needed because Secure Boot implies UEFI.

Or alternatively when SECURE_BOOT_BOOTLOADER is
specified UEFI_BOOTLOADER can be used to specify
the payload of the Secure Boot bootloader?

Because in the curent code I notice another semi-hardcoded
filename 'grub*.efi':

    # if shim is used, bootloader can be actually anything
    # named as grub*.efi (follow-up loader is shim compile time option)
    # http://www.rodsbooks.com/efi-bootloaders/secureboot.html#initial_shim
    cp $v $(dirname ${UEFI_BOOTLOADER})/grub*.efi $TMP_DIR/mnt/EFI/BOOT/ >&2

I guess "follow-up loader" means the payload of shim.
As far as I know the payload of shim can be anything
so that it might be good when ReaR is prepared to be able
to copy any shim payload into the recovery system
via something like

if test -e "$SECURE_BOOT_BOOTLOADER" ; then
    # When SECURE_BOOT_BOOTLOADER is specified
    # the Secure Boot bootloader is shim.
    # When shim is used the shim payload
    # must be copied into the recovery system.
    # The shim payload is a shim compile-time setting, see
    # http://www.rodsbooks.com/efi-bootloaders/secureboot.html#initial_shim
    # Usually the shim payload is a follow-up bootloader
    # which is usually a file named grub*.efi
    # so that we use this as default for the shim payload:
    shim_payload=$( dirname ${SECURE_BOOT_BOOTLOADER} )/grub*.efi
    # When SECURE_BOOT_BOOTLOADER is specified
    # UEFI_BOOTLOADER can be used to explicitly
    # specify the shim payload file:
    test -e "$UEFI_BOOTLOADER" && shim_payload=$UEFI_BOOTLOADER
    cp $v $shim_payload $TMP_DIR/mnt/EFI/BOOT/ >&2
fi

gozora commented at 2017-06-21 08:24:¶

@jsmeix

I fear I am a pain in your neck when I write this

No, you are not!

I like your idea. At the end we would have basically two *_BOOTLOADER variables.

UEFI_BOOTLOADER
SECURE_BOOT_BOOTLOADER

If SECURE_BOOT_BOOTLOADER is set, UEFI_BOOTLOADER will be overwritten by value of SECURE_BOOT_BOOTLOADER (hope I've understood it right)

That is indeed much simpler to understand!

Thanks for your inputs

I'll update PR later today.

V.

schlomo commented at 2017-06-21 08:39:¶

Maybe I missed a point somewhere. After reading all this I have one main question:

Why do we need two variables? In the end, we can only configure one EFI program to be loaded by the EFI firmware, so why two inputs leading to one output?

Specifically, if we faithfully replicate the boot scripts, EFI programs etc. of the source system then it should be - in my simple world view - enough to put the original EFI program back into the firmware and everything else should be as before.

So if the source OS used shim.efi or shimx64,efi then we put that back. If it used grubx86.efi or systemd-bootx64.efi then we put that back into the EFI firmware. As long as we restored the EFI partition to its original state everything else should also work.

So the question is, why not simply have one variable named UEFI_BOOTLOADER which also supports secure boot if needed. As different distros use different signed boot files (Arch uses PreLoader.efi, Ubuntu and others shim ...) we probably have to keep our code sufficiently generic in any case.

If I understand this PR in its original intention it was to stop recreating boot files and using the existing ones that where part of the source system. I also think that this is the right approach wherever possible.

Maybe the original author (@gdha ?) of the secure boot implementation can fill in why we needed to recreate EFI files in the beginning and what prevented using the existing files.

Another aspect we should keep in mind: Custom Machine Owner Keys. I am not sure if we can properly automate that and I wonder who in the real world (e.g. large setups) actually does that. In any case I can easily imagine that prepping the replacement hardware with custom MoKs is out of scope for ReaR.

With regard to merging this PR: IMHO those with access to secure boot enabled hardware and the time to test it should have a say, which unfortunately excludes myself ATM.

jsmeix commented at 2017-06-21 09:09:¶

I think for Secure Boot we always need two config variables:

One config variable to specify the initial bootloader which is
usually GRUB2 for UEFI without Secure Boot
and shim for UEFI with Secure Boot.

For Secure Boot we need a second config variable
to specify the payload file for shim which
also must be copied into the recovery system.

It is crucial that many config variables do not necessarily mean
that the user must specify all of them in any case.

Ideally all "just works" automatically "out of the box".

In this case ReaR would automatically find out
what the initial bootloader is
whether or not Secure Boot is used and
when Secure Boot is used what the shim payload is.

For me the main purpose and advantage of config variables
in contrast to hardcoded values hidden somewhere in the code
is that when things do not work automatically the user can
specify the config variable values to make things work for him.

This results two advantages for the ReaR developers
and for the ReaR users:

Initially the implementation can be simple because
no automatisms need to be implemented right from the start.
This way ReaR can support things initially in a manual way
where the user must specify the config variable values
to make it work.
Over time when we learn more and more how to do things
automatically more and more automatisms can be added
to make thinks more and more work automatically.

When things do not work automatically (i.e. when a user
submits a GitHub issue) we can easily and quickly help
the user by explaining how he can manually specify
appropriate config variable values to make things
work for him.

schlomo commented at 2017-06-21 09:16:¶

Thank you @jsmeix for pointing out the part I missed: This is all about how the rescue system can boot.

Maybe this is yet another case where the code would be much simpler if we could limit support to systemd booting. I know, I keep dreaming about a new rear major version that does away with all the old stuff. As only recent distros support Secure Boot this actually does sound like a great candidate.

Nevertheless, I stay by my own words. Those who know and can test should decide on the PR.

gdha commented at 2017-06-21 09:21:¶

@schlomo As far as I can remember (and it has been a while ago - years) the original code main concern was to get the ISO image UEFI aware and therefore, I needed to recreate the EFI image (I think). For the rest it was a pure restore of the /boot/efi vfat directory.
Furthermore, afterwards the local grub entry came along (which was wrong to implement IMHO), which made things only complicated.
We should stick to the essence and bare minimum: recover the OS and nothing more and hopefully get it bootable again.
My brains are not big enough to understand why we need 2 variables, but I'm not the authority on UEFI anymore (as I removed it from all my systems).

jsmeix commented at 2017-06-21 09:37:¶

Only a side note FYI:
Because I am also dreaming about a new ReaR major version
that does away with all the old stuff so that I did now
https://github.com/rear/rear/issues/1390

jsmeix commented at 2017-06-21 09:54:¶

@gozora
could it perhaps avoid most issues if by default the whole
/boot/efi/ content gets copied into the recovery system
plus a config variable to specify the initial bootloader
(or more general: the file to be loaded by the EFI firmware)?

But even then I think we still need two config variables:
One to specify the file to be loaded by the EFI firmware and
another one (probably an array) to specify what EFI files
should be copied into the recovery system (by default /boot/efi/).

gozora commented at 2017-06-21 10:59:¶

@jsmeix

could it perhaps avoid most issues if by default the whole
/boot/efi/ content gets copied into the recovery system
plus a config variable to specify the initial bootloader
(or more general: the file to be loaded by the EFI firmware)?

We basically could. But we would need to count on fact that users might need to have some basic UEFI knowledge to manually trigger boot loader.
Reason why current UEFI code might look complicated is because we are trying to ensure that boot loader is always launched and all the user have to do, is press enter to boot.
If you ask me, this is what first 'R' stands for in ReaR ;-)

@schlomo
What do you mean by systemd booting? I've heard about it here somewhere but not sure what is suppose to be?

V.

schlomo commented at 2017-06-21 11:10:¶

@gozora https://wiki.archlinux.org/index.php/systemd-boot has a good introduction. I tried it and loved on first sight. Reduced to the absolute minimum needed for booting on EFI. Also handles the EFI firmware. Really nice CLI interface. Basically the quality you know from systemd applied to EFI booting.

gozora commented at 2017-06-21 11:22:¶

@schlomo it basically looks like kernel efistub.
Since I works with UEFI I still did not decided if omitting grub is good or bad idea ...

schlomo commented at 2017-06-21 11:46:¶

Yes, systemd-boot can only load other EFI programs. It provides a simple menu that you can configure in Linux and that allows you to add kernel parameters on the fly. If you never need to do that then you can also use efistub directly.

[Export of Github issue for rear/rear.]

#1385 PR merged: Do not recreate BOOTX64.efi, if Secure boot is enabled.¶

gozora opened issue at 2017-06-19 13:32:¶

jsmeix commented at 2017-06-20 08:19:¶

gozora commented at 2017-06-20 09:15:¶

gdha commented at 2017-06-20 09:31:¶

schlomo commented at 2017-06-20 09:37:¶

gozora commented at 2017-06-20 10:29:¶

jsmeix commented at 2017-06-20 11:14:¶

gdha commented at 2017-06-20 11:16:¶

gdha commented at 2017-06-20 11:20:¶

jsmeix commented at 2017-06-20 11:21:¶

gdha commented at 2017-06-20 11:26:¶

gozora commented at 2017-06-20 11:39:¶

schlomo commented at 2017-06-20 11:47:¶

gozora commented at 2017-06-20 12:03:¶

schlomo commented at 2017-06-20 12:04:¶

jsmeix commented at 2017-06-20 13:01:¶

jsmeix commented at 2017-06-21 07:53:¶

gozora commented at 2017-06-21 08:24:¶

schlomo commented at 2017-06-21 08:39:¶

jsmeix commented at 2017-06-21 09:09:¶

schlomo commented at 2017-06-21 09:16:¶

gdha commented at 2017-06-21 09:21:¶

jsmeix commented at 2017-06-21 09:37:¶

jsmeix commented at 2017-06-21 09:54:¶

gozora commented at 2017-06-21 10:59:¶

schlomo commented at 2017-06-21 11:10:¶

gozora commented at 2017-06-21 11:22:¶

schlomo commented at 2017-06-21 11:46:¶

#1385 PR `merged`: Do not recreate BOOTX64.efi, if Secure boot is enabled.¶