Skip to content

Rework handling of the Secure Boot keys and certificates #3044

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions build_library/build_image_util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,6 @@ EOF
# calculated. Only for unofficial builds as official builds get signed later.
if [[ ${COREOS_OFFICIAL:-0} -ne 1 ]]; then
do_sbsign --output "${root_fs_dir}/boot/flatcar/vmlinuz-a"{,}
cleanup_sbsign_certs
fi

if [[ -n "${image_kernel}" ]]; then
Expand Down Expand Up @@ -904,7 +903,7 @@ sbsign_image() {

"${BUILD_LIBRARY_DIR}/disk_util" --disk_layout="${disk_layout}" \
mount "${disk_img}" "${root_fs_dir}"
trap "cleanup_mounts '${root_fs_dir}'; cleanup_sbsign_certs" EXIT
trap "cleanup_mounts '${root_fs_dir}'" EXIT

# Sign the kernel with the shim-embedded key.
do_sbsign --output "${root_fs_dir}/boot/flatcar/vmlinuz-a"{,}
Expand Down Expand Up @@ -934,7 +933,6 @@ sbsign_image() {
fi

cleanup_mounts "${root_fs_dir}"
cleanup_sbsign_certs
trap - EXIT

if [[ -n "${pcr_policy}" ]]; then
Expand Down
5 changes: 2 additions & 3 deletions build_library/grub_install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ ESP_DIR=
LOOP_DEV=

cleanup() {
cleanup_sbsign_certs
if [[ -d "${ESP_DIR}" ]]; then
if mountpoint -q "${ESP_DIR}"; then
sudo umount "${ESP_DIR}"
Expand Down Expand Up @@ -200,8 +199,8 @@ case "${FLAGS_target}" in

# Unofficial build: Sign shim with our development key.
sudo sbsign \
--key /usr/share/sb_keys/DB.key \
--cert /usr/share/sb_keys/DB.crt \
--key /usr/share/sb_keys/unofficial/DB.key \
--cert /usr/share/sb_keys/unofficial/DB.pem \
--output "${ESP_DIR}/EFI/boot/boot${EFI_ARCH}.efi" \
"${BOARD_ROOT}/usr/lib/shim/shim${EFI_ARCH}.efi"
else
Expand Down
31 changes: 5 additions & 26 deletions build_library/sbsign_util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,23 @@
# found in the LICENSE file.

if [[ ${COREOS_OFFICIAL:-0} -ne 1 ]]; then
SBSIGN_KEY="/usr/share/sb_keys/shim.key"
SBSIGN_CERT="/usr/share/sb_keys/shim.pem"
SBSIGN_KEY="/usr/share/sb_keys/unofficial/shim.key"
SBSIGN_CERT="/usr/share/sb_keys/unofficial/shim.pem"
else
SBSIGN_KEY="pkcs11:token=flatcar-sb-dev-hsm-sign-2025"
unset SBSIGN_CERT
SBSIGN_CERT="/usr/share/sb_keys/official/signing.pem"
Copy link
Member

Choose a reason for hiding this comment

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

Since we can fetch it from Keyvault on demand, and this certificate is needed while signing, why store it in the repo. Better keep the way it's done.

fi

PKCS11_MODULE_PATH="/usr/$(get_sdk_libdir)/pkcs11/azure-keyvault-pkcs11.so"
PKCS11_MODULE_PATH="$(pkg-config p11-kit-1 --variable p11_module_path)/azure-keyvault-pkcs11.so"

PKCS11_ENV=(
AZURE_CORE_COLLECT_TELEMETRY=no
Copy link
Member

Choose a reason for hiding this comment

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

I think we should leave this at the default, though you may want to provide an option for rebuilders to easily override.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured the telemetry might not be that useful when it is coming from an ephemeral container, but I don't feel strongly about it.

AZURE_KEYVAULT_URL="https://flatcar-sb-dev-kv.vault.azure.net/"
PKCS11_MODULE_PATH="${PKCS11_MODULE_PATH}"
AZURE_KEYVAULT_PKCS11_DEBUG=1
)

get_sbsign_cert() {
if [[ ${SBSIGN_KEY} != pkcs11:* || -s ${SBSIGN_CERT-} ]]; then
return
fi

SBSIGN_CERT=$(mktemp -t signing-cert.XXXXXXXXXX.pem)
info "Fetching ${SBSIGN_KEY} from Azure"

# Needs Key Vault Reader role.
env "${PKCS11_ENV[@]}" p11-kit export-object \
--provider "${PKCS11_MODULE_PATH}" \
"${SBSIGN_KEY};type=cert" \
| tee "${SBSIGN_CERT}"
}

cleanup_sbsign_certs() {
if [[ ${SBSIGN_CERT-} == "${TMPDIR-/tmp}"/* ]]; then
rm -f -- "${SBSIGN_CERT}"
fi
}

do_sbsign() {
get_sbsign_cert
info "Signing ${@:$#} with ${SBSIGN_KEY}"

if [[ ${SBSIGN_KEY} == pkcs11:* ]]; then
Expand Down
4 changes: 2 additions & 2 deletions build_library/vm_image_util.sh
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ _write_qemu_uefi_secure_conf() {
local flash_rw="$(_dst_name "_efi_vars.qcow2")"
local flash_ro="$(_dst_name "_efi_code.qcow2")"
local script="$(_dst_dir)/$(_dst_name ".sh")"
local owner="00000000-0000-0000-0000-000000000000"
local owner=$(</usr/share/sb_keys/owner.txt)
local flash_in

_write_qemu_uefi_conf
Expand All @@ -886,7 +886,7 @@ _write_qemu_uefi_secure_conf() {
virt-fw-vars \
--input "${flash_in}" \
--output "$(_dst_dir)/${flash_rw}" \
--add-db "${owner}" /usr/share/sb_keys/DB.crt \
--add-db "${owner}" /usr/share/sb_keys/unofficial/DB.pem \
--add-db "${owner}" "${BUILD_LIBRARY_DIR}/flatcar-sb-dev-shim-2025.cert"

sed -e "s%^SECURE_BOOT=.*%SECURE_BOOT=1%" -i "${script}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,4 @@ 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,60 @@
# 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="https://www.flatcar.org/"
S="${WORKDIR}"

LICENSE="BSD"
SLOT="0"
KEYWORDS="amd64 arm64"

BDEPEND="
app-emulation/virt-firmware
dev-libs/openssl
"

# Arbitrary value created for Flatcar.
OWNER_GUID="4a974879-bf65-4eb8-b404-ac3a6141121e"

src_compile() {
local TYPE
for TYPE in unofficial official; do
mkdir "${TYPE}" || die

# Gather all the shim vendor PEM certs into an array.
local FILES=( "${FILESDIR}/${TYPE}"/shim-*.pem )

# Rewrite the newest shim vendor PEM cert in a consistent PEM format,
# checking its validity. Only the newest is needed in PEM format for
# inserting into the kernel to verify the verity root hash at boot time.
openssl x509 -in "${FILES[-1]}" -inform PEM -out "${TYPE}"/shim.pem || die

local ARGS=() FILE
for FILE in "${FILES[@]}"; do
# Add each shim vendor PEM cert to the DER ESL creation below.
ARGS+=( --add-cert "${OWNER_GUID}" "${FILE}" )
done

# This ingests shim vendor PEM certs and outputs a combined DER ESL.
virt-fw-sigdb "${ARGS[@]}" --output "${TYPE}"/shim.esl || die
done

# Rewrite the official signing PEM cert in a consistent PEM format, checking
# its validity. Only the newest is needed in PEM format to sign the
# bootloader and the kernel image. Unofficial builds are already covered
# above because the shim vendor cert /is/ the signing cert, not a CA.
openssl x509 -in "${FILESDIR}"/official/signing.pem -inform PEM -out official/signing.pem || die
Copy link
Member

Choose a reason for hiding this comment

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

This certificate is only needed during signing - storing it in the repo means we need to update more places. Fetching it from Keyvault when signing seems like the better choice.

}

src_install() {
insinto /usr/share/sb_keys
newins - owner.txt <<< "${OWNER_GUID}"
doins -r unofficial official

insinto /usr/share/sb_keys/unofficial
doins "${FILESDIR}"/unofficial/{DB.{key,pem},shim.key}
}
Copy link
Member

Choose a reason for hiding this comment

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

  1. If this is the official one, why is the issuer: "Flatcar Container Linux Secure Boot Development CA"
  2. This key is RSA 2K while the signing one is RSA 4K. The CA key must not be weaker than the signing key.
  3. Naming would be clearer if shim-20241107.pem were called "flatcar-sb-ca.pem"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. That's what @sayanchowdhury called it. 🤷‍♂️ This CA certificate matches the issuer of flatcar-sb-dev-hsm-sign-2025 in AKV, which is the one we've been signing official releases with. I don't think this is the actual certificate we will use in the shim review, which would explain the name, but I've never been entirely clear on this.
  2. Again, @sayanchowdhury created it, so I'll leave him to answer this point.
  3. It is the shim vendor certificate, but yeah, that name might be clearer.

Copy link
Member

Choose a reason for hiding this comment

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

Since the current solution was a stop-gap solution, my intention was to keep the name as development. Going foward, I'll post the metadata to be used and once vouched for, I'll proceed.
I'll look into the keysize.

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
-----BEGIN CERTIFICATE-----
MIIDaDCCAlCgAwIBAgIRAINN27bWbE7vq1xOCROIuyMwDQYJKoZIhvcNAQELBQAw
PTE7MDkGA1UEAxMyRmxhdGNhciBDb250YWluZXIgTGludXggU2VjdXJlIEJvb3Qg
RGV2ZWxvcG1lbnQgQ0EwHhcNMjQxMTA3MTkwMjIyWhcNMzcwMTE5MDMxNDA3WjA9
MTswOQYDVQQDEzJGbGF0Y2FyIENvbnRhaW5lciBMaW51eCBTZWN1cmUgQm9vdCBE
ZXZlbG9wbWVudCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAJ5q
pvNmeh7nGFp5k4WnsOnBct1DtRcgCPbKVNOs6tvmSiygkrnw4l6mR2FJr7JPl5IE
IDttPQnotGYjKJhhEkYIcLUn91w8UAuq68iKfHcr3RKfD0u3ZzxqXzGrPaIcqbGl
GQwFxtoFxwf8MOskyM3R1zIKKYKABpEZj8Nq7Y/1tYdqip63KygJ4NGs+qYkaPcX
TlCh2rSZsc39kiGdASzHn0LsSU4IFFSLUQvNOO3DXwI0RCOrvl4KoyXtPRB33dUN
H80A/bPXUe67GBhlZyzctJ+YDSXzkPZftlAgMB/+/rtxHCSJXgGFmm5smICW4yRT
byKpnlp5apcc4x2TbQUCAwEAAaNjMGEwHwYDVR0jBBgwFoAUbfTa4PZ82LTaSzln
iD0LyFf+h7AwDgYDVR0PAQH/BAQDAgGGMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0O
BBYEFG302uD2fNi02ks5Z4g9C8hX/oewMA0GCSqGSIb3DQEBCwUAA4IBAQBPx4C1
z5/eVwMTaDT2zo1qe+pqscMjZ1+TClQ+wK33tkHrvzfHtDnjgPDEM/rpHKucSoeS
xAdFlXqnJja0MEWdCh4503/KWOxl4quQdQclVFlV6A6/f8kbCB2hzb2k5NMUPE2b
Vg+wTOd22dm7HSeCsxkvAbtAQPDXLaBJ6dxGS7KEk3akHHIYqBSBbXEuhjgIcdJ2
5mzUCAXJEpZ1J/nqQZsZD+Ugp7iMB3JV9xC01JNWMuMlGoeI1F6orssR6jU3aCjk
jZjSnyqDINcqddkevpYWg+ffvg3ewQ4lMcFPwjGDxqPOCPZmAw/l7xd6LqFTq//C
iAQ/ssvyzfqSgqWN
-----END CERTIFICATE-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
-----BEGIN CERTIFICATE-----
MIIELTCCAxWgAwIBAgICCSkwDQYJKoZIhvcNAQELBQAwPTE7MDkGA1UEAxMyRmxh
dGNhciBDb250YWluZXIgTGludXggU2VjdXJlIEJvb3QgRGV2ZWxvcG1lbnQgQ0Ew
HhcNMjUwMzIwMTE0ODIyWhcNMjgwMzIwMTE0ODIyWjBCMUAwPgYDVQQDEzdGbGF0
Y2FyIENvbnRhaW5lciBMaW51eCBTZWN1cmUgQm9vdCBEZXZlbG9wbWVudCBTaWdu
aW5nMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEAi4DQeJdbATc+6bXu
fC45iOj+fuA9p/L7WvY0FoT4ohmynoaYgBzUXMV333H7Pzy1qknZ1Kz8A8c8nlp0
BQOr0cnvoJhPt2lOP7EofCdylOBOfWPqP9l73pATp2/jJnUi+288eSTHBX6GgLqk
PYceTjIxQWP6OzjP94ZCOBQnB6Ib4TaMbuylkz7vrog1JMPoGOdrSmZvaWqHebx5
GaPwNOQzJAw6d4rFrphqso51r/Q4vxps+fMQZoJAMP4xT1SCIQO1kFt+3Q6MFi3m
ztte5Y/Q46IvKEZaWU8K5umFnBaOWCDeFelV268TsvyrBBrCBB4Q5ooo2NiZESh4
7fS1lT3Jcr8Unw3b2E91Y/hmGRDwrC+yp+7f51gRC3UIP5AsF1oX/nF+kD+mrhhg
3gPtCXdx88ttaJBdOCMfdgoONvY8Yl1WlbjbC5pCEMK3/gV5SGCbvlAaqBkh6gvY
dsFKMCbSHg/eD6PS3bgRtfcb1jCsIXGrzJRVi/Fgu0u9D9UylNkGkrvHQ0r1ffKM
O7hlgooo9SLloU+4MLMHCujOQo978XIdsYy2DCiRQdD33LXEgQMGEcZvENIXkmFU
Iujq32PMKc31P84JPGzyeUN3VkdUPww4HRwEcsfUFJ8b4xEEmAUi/+zqz5XCo3m1
nh62zX/ByIyCmY56rgWFWmSON4UCAwEAAaMyMDAwCQYDVR0TBAIwADATBgNVHSUE
DDAKBggrBgEFBQcDAzAOBgNVHQ8BAf8EBAMCBaAwDQYJKoZIhvcNAQELBQADggEB
AGKZATREaf6bdjfYmrM0cKwlPcd9Bo95gX/uucRzLmAvgo5KftgFABhqNxQeb6g2
mkz9EKCtYt8WPH7j914yXQLLes/nrqARPIV2n0IP5SxRDmDxhSl3jhStp97idGvv
zS07tM7oHqPycwitfrJl0kbrIYXr5f2PyG2kANjPgoYHcl5KLnffv95XSokNhWFK
17asGpHUrtOlOJm1mCgjpmiotYgMIGtKFMiKB8yFQRFRnyMb7uowkq/G9btDjW/7
kVJwppmwXXL5qiAAzvFQcbBbWmiIwVXjb8OLO43dsB0znt+IMDf7itUiUCLSOjqQ
z3S4t9QNWU6jp0RBwEQfI6g=
-----END CERTIFICATE-----
Binary file not shown.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright 2015 CoreOS, Inc.
# Distributed under the terms of the GNU General Public License v2

EAPI=8

DESCRIPTION="UEFI Shim loader"
HOMEPAGE="https://github.com/rhboot/shim"
SRC_URI="https://github.com/rhboot/shim/releases/download/${PV}/shim-${PV}.tar.bz2"
KEYWORDS="amd64 arm64"

LICENSE="BSD"
SLOT="0"
IUSE="official"

# TODO: Would be ideal to depend on sys-boot/gnu-efi package, but
# currently the shim insists on using the bundled copy. This will need
# to be addressed by patching this check out after making sure that
# our copy of gnu-efi is as usable as the bundled one.
DEPEND="
dev-libs/openssl
"
BDEPEND="
coreos-base/coreos-sb-keys
"

PATCHES=(
"${FILESDIR}/0001-Fix-parallel-build-of-gnu-efi.patch"
)

src_compile() {
sed -e "s/@@VERSION@@/${PVR}/" "${FILESDIR}"/sbat.csv.in >"${WORKDIR}/sbat.csv" || die

unset ARCH
emake \
CROSS_COMPILE="${CHOST}-" \
ENABLE_SBSIGN=1 \
SBATPATH="${WORKDIR}"/sbat.csv \
VENDOR_DB_FILE="${BROOT}"/usr/share/sb_keys/$(usex official official unofficial)/shim.esl
}

src_install() {
insinto /usr/lib/shim
doins shim?*.efi mm?*.efi
}