Skip to content

Commit

Permalink
Merge pull request #1589 from flatcar/sayan/secureboot-changes
Browse files Browse the repository at this point in the history
Initial implementation for Secure boot support
  • Loading branch information
pothos authored Feb 26, 2024
2 parents dbd40fb + 7db81c2 commit d35414a
Show file tree
Hide file tree
Showing 34 changed files with 473 additions and 136 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/portage-stable-packages-list
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ eclass/python-utils-r1.eclass
eclass/readme.gentoo-r1.eclass
eclass/ruby-single.eclass
eclass/ruby-utils.eclass
eclass/rpm.eclass
eclass/savedconfig.eclass
eclass/selinux-policy-2.eclass
eclass/strip-linguas.eclass
Expand Down Expand Up @@ -507,6 +508,7 @@ sys-block/parted
sys-block/thin-provisioning-tools

sys-boot/efibootmgr
sys-boot/mokutil
# Updating to 3.0.17 breaks building of sys-boot/shim.
#
# sys-boot/gnu-efi
Expand Down
4 changes: 2 additions & 2 deletions build_library/build_image_util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -828,8 +828,8 @@ EOF

# Sign the kernel after /usr is in a consistent state and verity is calculated
if [[ ${COREOS_OFFICIAL:-0} -ne 1 ]]; then
sudo sbsign --key /usr/share/sb_keys/DB.key \
--cert /usr/share/sb_keys/DB.crt \
sudo sbsign --key /usr/share/sb_keys/shim.key \
--cert /usr/share/sb_keys/shim.pem \
"${root_fs_dir}/boot/flatcar/vmlinuz-a"
sudo mv "${root_fs_dir}/boot/flatcar/vmlinuz-a.signed" \
"${root_fs_dir}/boot/flatcar/vmlinuz-a"
Expand Down
12 changes: 0 additions & 12 deletions build_library/grub.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,6 @@ set linux_append=""

set secure_boot="0"

if [ "$grub_platform" = "efi" ]; then
getenv -e SecureBoot -g 8be4df61-93ca-11d2-aa0d-00e098032b8c -b sb
getenv -e SetupMode -g 8be4df61-93ca-11d2-aa0d-00e098032b8c -b setupmode
if [ "$sb" = "01" -a "$setupmode" = "00" ]; then
set secure_boot="1"
getenv -e NetBootVerificationKey -g b8ade7d5-d400-4213-8d15-d47be0a621bf -b gpgpubkey
if [ "$gpgpubkey" != "" ]; then
trust_var gpgpubkey
fi
fi
fi

if [ "$net_default_server" != "" ]; then
smbios --type 1 --get-uuid 8 --set uuid
smbios --type 1 --get-string 7 --set serial
Expand Down
45 changes: 30 additions & 15 deletions build_library/grub_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,19 @@ CORE_NAME=

# Whether the SDK's grub or the board root's grub is used. Once amd64 is
# fixed up the board root's grub will always be used.
BOARD_GRUB=0
BOARD_GRUB=1

SBAT_ARG=()

case "${FLAGS_target}" in
i386-pc)
CORE_MODULES+=( biosdisk serial )
CORE_NAME="core.img"
;;
x86_64-efi)
CORE_MODULES+=( serial efi_gop efinet pgp http tftp )
CORE_MODULES+=( serial linux efi_gop efinet pgp http tftp )
CORE_NAME="core.efi"
SBAT_ARG=( --sbat "${BOARD_ROOT}/usr/share/grub/sbat.csv" )
;;
x86_64-xen)
CORE_NAME="core.elf"
Expand All @@ -68,6 +71,7 @@ case "${FLAGS_target}" in
CORE_MODULES+=( serial linux efi_gop efinet pgp http tftp )
CORE_NAME="core.efi"
BOARD_GRUB=1
SBAT_ARG=( --sbat "${BOARD_ROOT}/usr/share/grub/sbat.csv" )
;;
*)
die_notrace "Unknown GRUB target ${FLAGS_target}"
Expand Down Expand Up @@ -164,7 +168,7 @@ if [[ ! -f "${ESP_DIR}/flatcar/grub/grub.cfg.tar" ]]; then
fi

sudo tar cf "${ESP_DIR}/flatcar/grub/grub.cfg.tar" \
-C "${GRUB_TEMP_DIR}" "grub.cfg"
-C "${GRUB_TEMP_DIR}" "grub.cfg"
fi

info "Generating ${GRUB_DIR}/${CORE_NAME}"
Expand All @@ -174,6 +178,7 @@ sudo grub-mkimage \
--directory "${GRUB_SRC}" \
--config "${ESP_DIR}/${GRUB_DIR}/load.cfg" \
--memdisk "${ESP_DIR}/flatcar/grub/grub.cfg.tar" \
"${SBAT_ARG[@]}" \
--output "${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}" \
"${CORE_MODULES[@]}"

Expand All @@ -192,26 +197,36 @@ case "${FLAGS_target}" in
x86_64-efi)
info "Installing default x86_64 UEFI bootloader."
sudo mkdir -p "${ESP_DIR}/EFI/boot"
# Use the test keys for signing unofficial builds
if [[ ${COREOS_OFFICIAL:-0} -ne 1 ]]; then
sudo sbsign --key /usr/share/sb_keys/DB.key \
--cert /usr/share/sb_keys/DB.crt \
"${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}"
# Use the test keys for signing unofficial builds
if [[ ${COREOS_OFFICIAL:-0} -ne 1 ]]; then
# Sign the GRUB with the shim-embedded key
sudo sbsign --key /usr/share/sb_keys/shim.key \
--cert /usr/share/sb_keys/shim.pem \
"${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}"
sudo cp "${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}.signed" \
"${ESP_DIR}/EFI/boot/grub.efi"
"${ESP_DIR}/EFI/boot/grubx64.efi"
# Sign the mokmanager(mm) with the shim-embedded key
sudo sbsign --key /usr/share/sb_keys/shim.key \
--cert /usr/share/sb_keys/shim.pem \
"/usr/lib/shim/mmx64.efi"
sudo cp "/usr/lib/shim/mmx64.efi.signed" \
"${ESP_DIR}/EFI/boot/mmx64.efi"

sudo sbsign --key /usr/share/sb_keys/DB.key \
--cert /usr/share/sb_keys/DB.crt \
--output "${ESP_DIR}/EFI/boot/bootx64.efi" \
"/usr/lib/shim/shim.efi"
--cert /usr/share/sb_keys/DB.crt \
--output "${ESP_DIR}/EFI/boot/bootx64.efi" \
"/usr/lib/shim/shim.efi"
else
sudo cp "${ESP_DIR}/${GRUB_DIR}/${CORE_NAME}" \
"${ESP_DIR}/EFI/boot/grub.efi"
"${ESP_DIR}/EFI/boot/grubx64.efi"
sudo cp "/usr/lib/shim/shim.efi" \
"${ESP_DIR}/EFI/boot/bootx64.efi"
fi
sudo cp "/usr/lib/shim/mmx64.efi" \
"${ESP_DIR}/EFI/boot/mmx64.efi"
fi
# copying from vfat so ignore permissions
if [[ -n "${FLAGS_copy_efi_grub}" ]]; then
cp --no-preserve=mode "${ESP_DIR}/EFI/boot/grub.efi" \
cp --no-preserve=mode "${ESP_DIR}/EFI/boot/grubx64.efi" \
"${FLAGS_copy_efi_grub}"
fi
if [[ -n "${FLAGS_copy_shim}" ]]; then
Expand Down
12 changes: 8 additions & 4 deletions build_library/qemu_template.sh
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,14 @@ if [ "${SAFE_ARGS}" -eq 1 ]; then
else
case "${VM_BOARD}+$(uname -m)" in
amd64-usr+x86_64)
set -- -global ICH9-LPC.disable_s3=1 \
-global driver=cfi.pflash01,property=secure,value=on \
"$@"
# Emulate the host CPU closely in both features and cores.
set -- -machine accel=kvm:hvf:tcg -cpu host -smp "${VM_NCPUS}" "$@" ;;
set -- -machine q35,accel=kvm:hvf:tcg,smm=on -cpu host -smp "${VM_NCPUS}" "$@"
;;
amd64-usr+*)
set -- -machine pc-q35-2.8 -cpu kvm64 -smp 1 -nographic "$@" ;;
set -- -machine q35 -cpu kvm64 -smp 1 -nographic "$@" ;;
arm64-usr+aarch64)
set -- -machine virt,accel=kvm,gic-version=3 -cpu host -smp "${VM_NCPUS}" -nographic "$@" ;;
arm64-usr+*)
Expand Down Expand Up @@ -215,8 +219,8 @@ fi

if [ -n "${VM_PFLASH_RO}" ] && [ -n "${VM_PFLASH_RW}" ]; then
set -- \
-drive if=pflash,file="${SCRIPT_DIR}/${VM_PFLASH_RO}",format=raw,readonly=on \
-drive if=pflash,file="${SCRIPT_DIR}/${VM_PFLASH_RW}",format=raw "$@"
-drive if=pflash,unit=0,file="${SCRIPT_DIR}/${VM_PFLASH_RO}",format=raw,readonly=on \
-drive if=pflash,unit=1,file="${SCRIPT_DIR}/${VM_PFLASH_RW}",format=raw "$@"
fi

if [ -n "${IGNITION_CONFIG_FILE}" ]; then
Expand Down
16 changes: 8 additions & 8 deletions build_library/vm_image_util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -807,14 +807,10 @@ _write_qemu_uefi_conf() {
# Get edk2 files into local build workspace.
info "Updating edk2 in /build/${BOARD}"
emerge-${BOARD} --nodeps --select --verbose --update --getbinpkg --newuse sys-firmware/edk2-aarch64
# Create 64MiB flash device image files.
dd if=/dev/zero bs=1M count=64 of="$(_dst_dir)/${flash_rw}" \
status=none
cp "/build/${BOARD}/usr/share/edk2-aarch64/QEMU_EFI.fd" \
"$(_dst_dir)/${flash_ro}.work"
truncate --reference="$(_dst_dir)/${flash_rw}" \
"$(_dst_dir)/${flash_ro}.work"
mv "$(_dst_dir)/${flash_ro}.work" "$(_dst_dir)/${flash_ro}"
cp "${BOARD_ROOT}/usr/share/AAVMF/AAVMF_CODE.fd" "$(_dst_dir)/${flash_ro}"
cp "${BOARD_ROOT}/usr/share/AAVMF/AAVMF_VARS.fd" "$(_dst_dir)/${flash_rw}"
truncate -s 64M "$(_dst_dir)/${flash_ro}"
truncate -s 64M "$(_dst_dir)/${flash_rw}"
;;
esac

Expand All @@ -825,14 +821,18 @@ _write_qemu_uefi_conf() {

_write_qemu_uefi_secure_conf() {
local flash_rw="$(_dst_name "_efi_vars.fd")"
local flash_ro="$(_dst_name "_efi_code.fd")"
local script="$(_dst_dir)/$(_dst_name ".sh")"

_write_qemu_uefi_conf
cp "/usr/share/edk2-ovmf/OVMF_CODE.secboot.fd" "$(_dst_dir)/${flash_ro}"
cert-to-efi-sig-list "/usr/share/sb_keys/PK.crt" "${VM_TMP_DIR}/PK.esl"
cert-to-efi-sig-list "/usr/share/sb_keys/KEK.crt" "${VM_TMP_DIR}/KEK.esl"
cert-to-efi-sig-list "/usr/share/sb_keys/DB.crt" "${VM_TMP_DIR}/DB.esl"
flash-var "$(_dst_dir)/${flash_rw}" "PK" "${VM_TMP_DIR}/PK.esl"
flash-var "$(_dst_dir)/${flash_rw}" "KEK" "${VM_TMP_DIR}/KEK.esl"
flash-var "$(_dst_dir)/${flash_rw}" "db" "${VM_TMP_DIR}/DB.esl"
sed -e "s%^SECURE_BOOT=.*%SECURE_BOOT=1%" -i "${script}"
}

_write_pxe_conf() {
Expand Down
1 change: 1 addition & 0 deletions changelog/changes/2024-01-25-shim-secureboot-update.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- A new format `qemu_uefi_secure` is introduced to test Flatcar for SecureBoot-enabled features. The format will be later merged into `qemu_uefi`.
1 change: 1 addition & 0 deletions changelog/updates/2024-01-25-shim-15.8.ebuild.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- shim ([15.8](https://github.com/rhboot/shim/releases/tag/15.8))
1 change: 1 addition & 0 deletions ci-automation/ci-config.env
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ QEMU_BIOS="/usr/share/qemu/bios-256k.bin"
# UEFI bios filename on build cache.
# Published by vms.sh as part of the qemu vendor build.
QEMU_UEFI_BIOS="${QEMU_UEFI_BIOS:-flatcar_production_qemu_uefi_efi_code.fd}"
QEMU_UEFI_SECURE_BIOS="${QEMU_UEFI_SECURE_BIOS:-flatcar_production_qemu_uefi_secure_efi_code.fd}"

# Update payload for the qemu_update.sh test.
# The default path set below is relative to TEST_WORK_DIR
Expand Down
7 changes: 7 additions & 0 deletions ci-automation/vendor-testing/qemu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ fi
bios="${QEMU_BIOS}"
if [ "${CIA_TESTSCRIPT}" = "qemu_uefi.sh" ] ; then
bios="${QEMU_UEFI_BIOS}"
fi

if [ "${CIA_TESTSCRIPT}" = "qemu_uefi_secure.sh" ] ; then
bios="${QEMU_UEFI_SECURE_BIOS}"
fi

if [ "${CIA_TESTSCRIPT}" = "qemu_uefi.sh" ] || [ "${CIA_TESTSCRIPT}" = "qemu_uefi_secure.sh" ] ; then
if [ -f "${bios}" ] ; then
echo "++++ ${CIA_TESTSCRIPT}: Using existing ${bios} ++++"
else
Expand Down
1 change: 1 addition & 0 deletions ci-automation/vendor-testing/qemu_uefi_secure.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Keys & Certificates

- PK (Platform Key): The Platform Key is the key to the platform.
- KEK (Key Exchange Key): The Key Exchange Key is used to update the signature database.
- DB (Signature Database): The signature database is used to validate signed EFI binaries.
- Shim Certificates: Our set of certificates


## Generation of Keys & Certificates


Generate the our shim certificates:

```
openssl genrsa -out "shim.key" 2048
openssl req -new -x509 -sha256 -subj "/CN=shim/" -key "shim.key" -out "shim.pem" -days 7300
openssl x509 -in "shim.pem" -inform PEM -out "shim.der" -outform DER
```

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright (c) 2015 CoreOS Inc.
# Copyright (c) 2024 The Flatcar Maintainers.
# Distributed under the terms of the GNU General Public License v2

EAPI=8

DESCRIPTION="Flatcar Secure Boot keys"
HOMEPAGE=""
SRC_URI=""
LICENSE="BSD"
SLOT="0"
KEYWORDS="amd64 arm64"
IUSE=""

S="${WORKDIR}"

src_install() {
insinto /usr/share/sb_keys
newins "${FILESDIR}/PK.key" PK.key
newins "${FILESDIR}/PK.crt" PK.crt
newins "${FILESDIR}/KEK.key" KEK.key
newins "${FILESDIR}/KEK.crt" KEK.crt
newins "${FILESDIR}/DB.key" DB.key
newins "${FILESDIR}/DB.crt" DB.crt

# shim keys
newins "${FILESDIR}/shim.key" shim.key
newins "${FILESDIR}/shim.der" shim.der
newins "${FILESDIR}/shim.pem" shim.pem
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDpPGgXHDI8K9Th
CzVTNPyKZqVAvgUKZE+Wzvnuj6Bsghud//17MFUcLIjrrOl3o+hYUzK8dbdQl2Mw
zq1gpPDs+bEe0+AFoyLU1LrPZVrZxRRXhRrAsGinkOOsApjMlikSEBrevqvbVElU
0hONyj4mvSaVof6AqVObJyslYerxZVoMkbIIm5gfsGu05xBgdVs5cnYUYpQxNmPy
LK1ImwFVXZSg0ZxdsEIdLDbWaAFVxBmezv+7U7UZaGi1fFZv6m8LxSMvGtxPFyh2
Mx3NXFKShgr/QhuAATcMNsYWASgp5tQetOBBlZ8wNefLWtKTdhMDF5Ni88brpuls
MQO/dpRJAgMBAAECggEAIbJpBYG83kWk5XillSZwIBzRXke12bkBaLPxlx5oGpU3
oT21ZSFoAoCKraYXOwJS1MP8bg8B06Jzob8SfIaICmzOwrnwwU++/gnYDZPCqvjW
xghEg7dY/3Cm/BiJ8/Dz8RijkS/yC2ejip4pVhB0p0snsnGrn/IW0rE3ghiiBYsM
971GSgbGp6o25rhA8/yx5+OOFvGoDX2nIymfFASSPmxiAbXcb4DmdMlrRZ6P4z51
8WJ8gXiTYvALFVWMNtv8GJZCQFi2fHcat/mWiVzg28J4Mzz9n79E0MrZ+4pxXLFT
lbtI6OvcjRgvsyxPwkExCsBTKnOeAdgKXKwiczBdMwKBgQD4u5NSEpx98GxiWVZX
DtT7WuCN257S0KztWzAYpTI5SZIRv4jylZPo+JnSrCvNt4hVs0Jz/aQQXhRIzVSj
4VrkhlxXGnJpZz1DkICIoFQLi9maazgj1aB9Y6lZeGxAlzCnDHP7pR7dxUj4FF2p
G6udyGhb3qfsevbSdykZ7DsHMwKBgQDwDOvheT71dNlcNuKrHi89sT5SoD4A2yTv
pyzBCvh2a+UFxveFa6l+/VgxR8AkX9z37hQxi++QFrBHnTD/NZcLijLnPI1V0pIQ
uNym6dx1PfuCtulZ24i2Fn5zrNUiNnTLBR31Fa1RJcyJv50IoTMK6F+0Bz4Qxan1
0Um+xgDGkwKBgAb32ky2UMQGdELdFdoihDz2cswGlxB44B9WKqbGGf4Y3Yq5vvBs
2FPygvyv7ho5RgyAlSACvxHmUNMpTXG54n38daHLD+F8Du9RoQgy1aftJw94aX43
geOBY0Eqan30vlwvsSAfpBm6aSzqBSWzrL8i2imYt0OcvkVvKSucvpqZAoGAWoXk
5dAdJ976oMWp0LG/StpuECaRey0ozp8SR3HlpHKnmPghG1UwQ80x1tOh55Wm9G/5
eX21x3Zm33qtoXAKF7Xz4DN7cOPJZTjxLJiAJE5NbEuhz9rzwQbWhLSmYxJ6FJ1H
YMbd5v4EFeYGR9zSLMjYXkFk7Fo9748O6jwsyrUCgYEApBlTWbna9BoxiVElEmvT
u/NgdKZIEBbeX/NWJz8BJWiBVRg5WaAeuriga/1tMhiX8dgo7z7uGm3moEsXGlVD
IhZiJeAgMmamr1yqII1q9RTBcA7iPqKmAgto+7zwcVxRmXCMRM/daJ04uqGine+K
dM/o7gBtadQHJ1KPftM8SqQ=
-----END PRIVATE KEY-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIC/zCCAeegAwIBAgIUbWirlHd6eCJi2JtP3Z0GEGWTWTMwDQYJKoZIhvcNAQEL
BQAwDzENMAsGA1UEAwwEc2hpbTAeFw0yMzExMjMyMzAxNTBaFw00MzExMTgyMzAx
NTBaMA8xDTALBgNVBAMMBHNoaW0wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK
AoIBAQDpPGgXHDI8K9ThCzVTNPyKZqVAvgUKZE+Wzvnuj6Bsghud//17MFUcLIjr
rOl3o+hYUzK8dbdQl2Mwzq1gpPDs+bEe0+AFoyLU1LrPZVrZxRRXhRrAsGinkOOs
ApjMlikSEBrevqvbVElU0hONyj4mvSaVof6AqVObJyslYerxZVoMkbIIm5gfsGu0
5xBgdVs5cnYUYpQxNmPyLK1ImwFVXZSg0ZxdsEIdLDbWaAFVxBmezv+7U7UZaGi1
fFZv6m8LxSMvGtxPFyh2Mx3NXFKShgr/QhuAATcMNsYWASgp5tQetOBBlZ8wNefL
WtKTdhMDF5Ni88brpulsMQO/dpRJAgMBAAGjUzBRMB0GA1UdDgQWBBSAVx8cxySJ
XcuJa6P2jBwOxJTNpDAfBgNVHSMEGDAWgBSAVx8cxySJXcuJa6P2jBwOxJTNpDAP
BgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCaj3785ElsU/QkPB3B
25xaCz23R2079ir0I6p91Zb9QM+n4fOLvEhhrb0tia1X6xaBHBtGk1kpCMP/JTQ2
ZNW43HuVLieiQnp+oSPGVZ52HnL4keptRr4Dvm+d7K6DDcn8Lcov4euDCsVzgBKE
EQcjIhAjKdc+nbI51cSoaDhtbBxNsF+ErsWi6+VIyBZ1ATsO6AbSZdKiE2o/3CDv
il7KIEEJsG43bTdeeuM1d/NLOoZjAnXUPizP0BGJtEE4GljYkN7PHr3czETsRIQ0
d5JUeoW3b2lYOf85n0ru+fCudk0NSSUyF4LEW6pLmCZCtCAb2GDQ5jeVmFF7BIFl
M8F2
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ RDEPEND="${RDEPEND}
amd64? (
app-emulation/xenserver-pv-version
app-emulation/xenstore
sys-boot/mokutil
)"

# sys-devel/gettext: it embeds 'envsubst' binary which is useful for simple file templating.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ RDEPEND="
coreos-base/nova-agent-container
coreos-base/nova-agent-watcher
)
arm64? (
sys-boot/grub
sys-firmware/edk2-ovmf-bin
)
sys-boot/grub
app-containers/containerd
app-containers/docker
app-containers/docker-cli
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,6 @@

# Accept unstable host Rust compilers.
=virtual/rust-1.76.0 ~amd64 ~arm64

# Upgrade to latest version for secureboot
=sys-boot/mokutil-0.6.0 ~amd64
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,4,Free Software Foundation,grub,@@UPSTREAM_VERSION@@,https://www.gnu.org/software/grub/
grub.flatcar,1,Flatcar,grub2,@@VERSION@@,https://github.com/flatcar/flatcar
Loading

0 comments on commit d35414a

Please sign in to comment.