From 1fb7f8e15e19fbe61230b70203b0c35fca54f0a0 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 25 May 2022 17:39:14 +0200 Subject: [PATCH 1/5] test: cover initrd->sysroot transition in TEST-24 This should cover cases regarding devices with `OPTIONS+="db_persist"` during initrd->sysroot transition. See: * https://github.com/systemd/systemd/issues/23429 * https://github.com/systemd/systemd/pull/23218 * https://github.com/systemd/systemd/pull/23489 * https://bugzilla.redhat.com/show_bug.cgi?id=2087225 --- test/TEST-24-CRYPTSETUP/test.sh | 57 ++++++++++++++++----------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/test/TEST-24-CRYPTSETUP/test.sh b/test/TEST-24-CRYPTSETUP/test.sh index 96d255dd96b..83f4d65b1da 100755 --- a/test/TEST-24-CRYPTSETUP/test.sh +++ b/test/TEST-24-CRYPTSETUP/test.sh @@ -10,6 +10,13 @@ TEST_FORCE_NEWIMAGE=1 # shellcheck source=test/test-functions . "${TEST_BASE_DIR:?}/test-functions" +PART_UUID="deadbeef-dead-dead-beef-000000000000" +DM_NAME="test24_varcrypt" +# Mount the keyfile only in initrd (hence rd.luks.key), since it resides on +# the rootfs and we would get a (harmless) error when trying to mount it after +# switching root (since rootfs is already mounted) +KERNEL_APPEND+=" rd.luks=1 luks.name=$PART_UUID=$DM_NAME rd.luks.key=$PART_UUID=/etc/varkey:LABEL=systemd_boot" + check_result_qemu() { local ret=1 @@ -17,13 +24,13 @@ check_result_qemu() { [[ -e "${initdir:?}/testok" ]] && ret=0 [[ -f "$initdir/failed" ]] && cp -a "$initdir/failed" "${TESTDIR:?}" - cryptsetup luksOpen "${LOOPDEV:?}p2" varcrypt <"$TESTDIR/keyfile" - mount /dev/mapper/varcrypt "$initdir/var" + cryptsetup luksOpen "${LOOPDEV:?}p2" "${DM_NAME:?}" <"$TESTDIR/keyfile" + mount "/dev/mapper/$DM_NAME" "$initdir/var" save_journal "$initdir/var/log/journal" check_coverage_reports "${initdir:?}" || ret=5 _umount_dir "$initdir/var" _umount_dir "$initdir" - cryptsetup luksClose /dev/mapper/varcrypt + cryptsetup luksClose "/dev/mapper/$DM_NAME" [[ -f "$TESTDIR/failed" ]] && cat "$TESTDIR/failed" echo "${JOURNAL_LIST:-No journals were saved}" @@ -36,45 +43,35 @@ test_create_image() { create_empty_image_rootdir echo -n test >"${TESTDIR:?}/keyfile" - cryptsetup -q luksFormat --pbkdf pbkdf2 --pbkdf-force-iterations 1000 "${LOOPDEV:?}p2" "$TESTDIR/keyfile" - cryptsetup luksOpen "${LOOPDEV}p2" varcrypt <"$TESTDIR/keyfile" - mkfs.ext4 -L var /dev/mapper/varcrypt + cryptsetup -q luksFormat --uuid="$PART_UUID" --pbkdf pbkdf2 --pbkdf-force-iterations 1000 "${LOOPDEV:?}p2" "$TESTDIR/keyfile" + cryptsetup luksOpen "${LOOPDEV}p2" "${DM_NAME:?}" <"$TESTDIR/keyfile" + mkfs.ext4 -L var "/dev/mapper/$DM_NAME" mkdir -p "${initdir:?}/var" - mount /dev/mapper/varcrypt "$initdir/var" + mount "/dev/mapper/$DM_NAME" "$initdir/var" - # Create what will eventually be our root filesystem onto an overlay - ( - LOG_LEVEL=5 - # shellcheck source=/dev/null - source <(udevadm info --export --query=env --name=/dev/mapper/varcrypt) - # shellcheck source=/dev/null - source <(udevadm info --export --query=env --name="${LOOPDEV}p2") + LOG_LEVEL=5 - setup_basic_environment - mask_supporting_services + setup_basic_environment + mask_supporting_services - install_dmevent - generate_module_dependencies - cat >"$initdir/etc/crypttab" <"$initdir/etc/varkey" - ddebug <"$initdir/etc/crypttab" + install_dmevent + generate_module_dependencies - cat >>"$initdir/etc/fstab" <"$initdir/etc/varkey" + + cat >>"$initdir/etc/fstab" <> "$initdir/etc/systemd/journald.conf" - ) + # Forward journal messages to the console, so we have something + # to investigate even if we fail to mount the encrypted /var + echo ForwardToConsole=yes >> "$initdir/etc/systemd/journald.conf" } cleanup_root_var() { ddebug "umount ${initdir:?}/var" mountpoint "$initdir/var" && umount "$initdir/var" - [[ -b /dev/mapper/varcrypt ]] && cryptsetup luksClose /dev/mapper/varcrypt + [[ -b "/dev/mapper/${DM_NAME:?}" ]] && cryptsetup luksClose "/dev/mapper/$DM_NAME" } test_cleanup() { From b22d90e59438481b421b1eb2449e6efdfb7f2118 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 26 May 2022 13:19:11 +0200 Subject: [PATCH 2/5] test: generate a custom initrd for TEST-24 if $INITRD is unset Co-Authored-By: Yu Watanabe --- test/TEST-24-CRYPTSETUP/test.sh | 24 ++++++++++++++++++++++++ test/test-functions | 5 +++++ 2 files changed, 29 insertions(+) diff --git a/test/TEST-24-CRYPTSETUP/test.sh b/test/TEST-24-CRYPTSETUP/test.sh index 83f4d65b1da..bdf630d9120 100755 --- a/test/TEST-24-CRYPTSETUP/test.sh +++ b/test/TEST-24-CRYPTSETUP/test.sh @@ -66,6 +66,30 @@ EOF # Forward journal messages to the console, so we have something # to investigate even if we fail to mount the encrypted /var echo ForwardToConsole=yes >> "$initdir/etc/systemd/journald.conf" + + # If $INITRD wasn't provided explicitly, generate a custom one with dm-crypt + # support + if [[ -z "$INITRD" ]]; then + INITRD="${TESTDIR:?}/initrd.img" + dinfo "Generating a custom initrd with dm-crypt support in '${INITRD:?}'" + + if command -v dracut >/dev/null; then + dracut --force --verbose --add crypt "$INITRD" + elif command -v mkinitcpio >/dev/null; then + mkinitcpio --addhooks sd-encrypt --generate "$INITRD" + elif command -v mkinitramfs >/dev/null; then + # The cryptroot hook is provided by the cryptsetup-initramfs package + if ! dpkg-query -s cryptsetup-initramfs; then + derror "Missing 'cryptsetup-initramfs' package for dm-crypt support in initrd" + return 1 + fi + + mkinitramfs -o "$INITRD" + else + dfatal "Unrecognized initrd generator, can't continue" + return 1 + fi + fi } cleanup_root_var() { diff --git a/test/test-functions b/test/test-functions index 06a06e706ad..daed481a292 100644 --- a/test/test-functions +++ b/test/test-functions @@ -337,6 +337,11 @@ qemu_min_version() { # Return 0 if qemu did run (then you must check the result state/logs for actual # success), or 1 if qemu is not available. run_qemu() { + # If the test provided its own initrd, use it (e.g. TEST-24) + if [[ -z "$INITRD" && -f "${TESTDIR:?}/initrd.img" ]]; then + INITRD="$TESTDIR/initrd.img" + fi + if [ -f /etc/machine-id ]; then read -r MACHINE_ID Date: Thu, 26 May 2022 14:52:52 +0200 Subject: [PATCH 3/5] test: store the key on a separate device --- test/TEST-24-CRYPTSETUP/test.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/TEST-24-CRYPTSETUP/test.sh b/test/TEST-24-CRYPTSETUP/test.sh index bdf630d9120..b81b811654f 100755 --- a/test/TEST-24-CRYPTSETUP/test.sh +++ b/test/TEST-24-CRYPTSETUP/test.sh @@ -12,10 +12,8 @@ TEST_FORCE_NEWIMAGE=1 PART_UUID="deadbeef-dead-dead-beef-000000000000" DM_NAME="test24_varcrypt" -# Mount the keyfile only in initrd (hence rd.luks.key), since it resides on -# the rootfs and we would get a (harmless) error when trying to mount it after -# switching root (since rootfs is already mounted) -KERNEL_APPEND+=" rd.luks=1 luks.name=$PART_UUID=$DM_NAME rd.luks.key=$PART_UUID=/etc/varkey:LABEL=systemd_boot" +KERNEL_APPEND+=" rd.luks=1 luks.name=$PART_UUID=$DM_NAME luks.key=$PART_UUID=/keyfile:LABEL=varcrypt_keydev" +QEMU_OPTIONS+=" -drive format=raw,cache=unsafe,file=${STATEDIR:?}/keydev.img" check_result_qemu() { local ret=1 @@ -57,7 +55,13 @@ test_create_image() { install_dmevent generate_module_dependencies - echo -n test >"$initdir/etc/varkey" + # Create a keydev + dd if=/dev/zero of="${STATEDIR:?}/keydev.img" bs=1M count=16 + mkfs.ext4 -L varcrypt_keydev "$STATEDIR/keydev.img" + mkdir -p "$STATEDIR/keydev" + mount "$STATEDIR/keydev.img" "$STATEDIR/keydev" + echo -n test >"$STATEDIR/keydev/keyfile" + umount "$STATEDIR/keydev" cat >>"$initdir/etc/fstab" < Date: Wed, 25 May 2022 12:01:00 +0200 Subject: [PATCH 4/5] core/device: device_coldplug(): don't set DEVICE_DEAD dm-crypt device units generated by systemd-cryptsetup-generator habe BindsTo= dependencies on their backend devices. The dm-crypt devices have the db_persist flag set, and thus survive the udev db cleanup while switching root. But backend devices usually don't survive. These devices are neither mounted nor used for swap, thus they will seen as DEVICE_NOT_FOUND after switching root. The BindsTo dependency will cause systemd to schedule a stop job for the dm-crypt device, breaking boot: [ 68.929457] krypton systemd[1]: systemd-cryptsetup@cr_root.service: Unit is stopped because bound to inactive unit dev-disk-by\x2duuid-3bf91f73\x2d1ee8\x2d4cfc\x2d9048\x2d93ba349b786d.device. [ 68.945660] krypton systemd[1]: systemd-cryptsetup@cr_root.service: Trying to enqueue job systemd-cryptsetup@cr_root.service/stop/replace [ 69.473459] krypton systemd[1]: systemd-cryptsetup@cr_root.service: Installed new job systemd-cryptsetup@cr_root.service/stop as 343 Avoid this by not setting the state of the backend devices to DEVICE_DEAD. Fixes the LUKS setup issue reported in #23429. --- src/core/device.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 4c261ec554d..87286305234 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -205,8 +205,6 @@ static int device_coldplug(Unit *u) { found &= ~DEVICE_FOUND_UDEV; /* ignore DEVICE_FOUND_UDEV bit */ if (state == DEVICE_PLUGGED) state = DEVICE_TENTATIVE; /* downgrade state */ - if (found == DEVICE_NOT_FOUND) - state = DEVICE_DEAD; /* If nobody sees the device, downgrade more */ } if (d->found == found && d->state == state) From 4fc69e8a0949c2537019466f839d9b7aee5628c9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 20 May 2022 10:25:12 +0200 Subject: [PATCH 5/5] core/device: do not downgrade device state if it is already enumerated On switching root, a device may have a persistent databse. In that case, Device.enumerated_found may have DEVICE_FOUND_UDEV flag, and it is not necessary to downgrade the Device.deserialized_found and Device.deserialized_state. Otherwise, the state of the device unit may be changed plugged -> dead -> plugged, if the device has not been mounted. Fixes #23429. [mwilck: cherry-picked from #23437] --- src/core/device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/device.c b/src/core/device.c index 87286305234..fcde8a420e6 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -201,7 +201,8 @@ static int device_coldplug(Unit *u) { * Of course, deserialized parameters may be outdated, but the unit state can be adjusted later by * device_catchup() or uevents. */ - if (!m->honor_device_enumeration && !MANAGER_IS_USER(m)) { + if (!m->honor_device_enumeration && !MANAGER_IS_USER(m) && + !FLAGS_SET(d->enumerated_found, DEVICE_FOUND_UDEV)) { found &= ~DEVICE_FOUND_UDEV; /* ignore DEVICE_FOUND_UDEV bit */ if (state == DEVICE_PLUGGED) state = DEVICE_TENTATIVE; /* downgrade state */