Skip to content

Add support of secure warm-boot #2532

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.

Sign up for GitHub

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

Merged
liat-grozovik merged 3 commits into from
Feb 14, 2023
Merged

Add support of secure warm-boot #2532

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8dd8d2
[secure boot]Add support of secure warm-boot by forcing kexec to veri…
davidpil2002 Nov 29, 2022
0ef8677
[secure boot]Add invoke kernel function to avoid duplication of kexec…
davidpil2002 Feb 8, 2023
4cd2598
[secure boot]move bootctl call check inside else condtion to avoid ab…
davidpil2002 Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions scripts/fast-reboot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,13 @@ function load_kernel() {
/sbin/kexec -a -l "$KERNEL_IMAGE" --initrd="$INITRD" --append="$BOOT_OPTIONS"
}

function load_kernel_secure() {
# Load kernel into the memory secure
# -s flag is for enforcing the new load kernel(vmlinuz) to be signed and verify.
# not using -a flag, this flag can fallback to an old kexec load that do not support Secure Boot verification
/sbin/kexec -l "$KERNEL_IMAGE" --initrd="$INITRD" --append="$BOOT_OPTIONS" -s
Copy link
Copy Markdown
Contributor

@stepanblyschak stepanblyschak Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be beneficial if we extend load_kernel with argument whether to perform sign verification instead of copy/paste most of the kexec parameters here?

Copy link
Copy Markdown
Contributor Author

@davidpil2002 davidpil2002 Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my previous implementation, I extended the command, but now it's not possible because was added a new flag that is not secure.
so we need to add and remove flags with the SB use case.

Copy link
Copy Markdown
Contributor

@stepanblyschak stepanblyschak Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about? To avoid duplicating kexec line

function invoke_kexec() {
  /sbin/kexec -l "$KERNEL_IMAGE" --initrd="$INITRD" --append="$BOOT_OPTIONS" $@
}

function load_kernel() {
   invoke_kexec() -a
}

function load_kernel_secure() {
  invoke_kexec() -s
}

Copy link
Copy Markdown
Contributor Author

@davidpil2002 davidpil2002 Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one can work, I will add a commit

}

function unload_kernel()
{
# Unload the previously loaded kernel if any loaded
Expand Down Expand Up @@ -597,9 +604,13 @@ if [[ "$sonic_asic_type" == "mellanox" ]]; then
fi
fi

# check if secure boot is enable in UEFI
SECURE_UPGRADE_ENABLED=$(bootctl status 2>/dev/null | grep -c "Secure Boot: enabled")
Copy link
Copy Markdown
Contributor

@stepanblyschak stepanblyschak Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this var is not needed in aboot secure boot case or it might confuse that we have both SECURE_UPGRADE_ENABLED and is_secureboot

Copy link
Copy Markdown
Contributor Author

@davidpil2002 davidpil2002 Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_secureboot is based on aboot secure boot - "Arista secure boot" was implemented a different process to secure boot the system, not related to our approach and our HLD review to the community.
their flag is_secureboot is based on the information from /proc/cmdline, and in our flow this information it's not including the info about secure boot enabled.

In our generic flow, we are reading the info about secure boot state from bootctl and that it's according to the flag in UEFI/BIOS, it's an accurate way to know that the system is really loaded with SB enabled/disabled.

Copy link
Copy Markdown
Contributor

@stepanblyschak stepanblyschak Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that means SECURE_UPGRADE_ENABLED is relevant only in case the platform is not aboot platform, correct? Or same new approach can be used for aboot? If SECURE_UPGRADE_ENABLED is not relevant to aboot I suggest to set this variable under else clause for non aboot platforms to avoid confusion.

Copy link
Copy Markdown
Contributor Author

@davidpil2002 davidpil2002 Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, if aboot its not with secure boot they still need load kernel regular, and I dont like to create a complex flow or duplicate code.
I moved now the bootctl call in the else case, so its a practical and easy solution and should be a robust solutions for all the vendors.
pls approve or suggest other solution
thanks


if is_secureboot && grep -q aboot_machine= /host/machine.conf; then
load_aboot_secureboot_kernel
elif [ ${SECURE_UPGRADE_ENABLED} -eq 1 ]; then
load_kernel_secure
else
load_kernel
fi
Expand Down