Skip to content
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

[RFC] kdump LUKS support #10

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[RFC] kdump LUKS support #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
00e78be
Get uuid by MAJ:MIN
coiby Mar 4, 2024
757aed9
Support dumping to a LUKS-encrypted target
coiby Aug 7, 2023
be458d8
LUKS: make /usr writable
coiby Jun 4, 2024
9b48a73
LUKS: hotplug support
coiby Jun 4, 2024
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ install: dracut-modules kdump-conf kdump-sysconfig manpages
install -D -m 755 mkdumprd $(DESTDIR)$(sbindir)/mkdumprd
install -D -m 644 kdump.conf $(DESTDIR)$(sysconfdir)
install -D -m 644 kdump.sysconfig $(DESTDIR)$(sysconfdir)/sysconfig/kdump
install -D -m 755 kdump-lib.sh kdump-lib-initramfs.sh kdump-logger.sh -t $(DESTDIR)$(pkglibdir)
install -D -m 755 kdump-lib.sh kdump-lib-initramfs.sh kdump-logger.sh kexec-crypt-setup.sh -t $(DESTDIR)$(pkglibdir)

ifeq ($(ARCH), $(filter ppc64le ppc64,$(ARCH)))
install -m 755 mkfadumprd $(DESTDIR)$(sbindir)
Expand Down
51 changes: 51 additions & 0 deletions dracut-module-setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,55 @@ ForwardToConsole=yes
EOF
}

kdump_check_crypt_targets() {
local _luks_dev _count _devuuid _key_desc
declare -a _luks_devs

_luks_devs=($(get_all_kdump_crypt_dev))
_count=${#_luks_devs[@]}

if [[ $_count -lt 1 ]]; then
Comment on lines +1007 to +1014
Copy link
Collaborator

@prudo1 prudo1 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see the point in having _count. It's only been used once and it's not that much shorter than ${#_luks_devs[@]}. Personaly I would use ${#_luks_devs[@]} directly.

return
fi

# This overrides behaviour of 90crypt
inst_hook cmdline 20 "/usr/lib/kdump/kexec-crypt-setup.sh"

inst cryptsetup
instmods dm_crypt

mkdir -p $hookdir/initqueue/finished

for _luks_dev in "${_luks_devs[@]}"; do
_devuuid=$(maj_min_to_uuid "$_luks_dev")
echo "kdump_luks_uuid=${_devuuid} " >"${initdir}/etc/cmdline.d/62kdump_luks.conf"
Comment on lines +1026 to +1028
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/>/>>/ ?
Or is it intended that you only record one uuid even when you have multiple devices? I'm a little bit confused as for the udev rules you enter one per device.


_key_desc=cryptsetup:$_devuuid

mkdir -p "${initdir}/etc/udev/rules.d/"
{
printf -- 'ENV{ID_FS_UUID}=="%s", ' "$_devuuid"
printf -- 'RUN+="/sbin/initqueue --settled --unique --onetime '
printf -- '--name kdump-crypt-target-%%k %s ' "$(command -v cryptsetup)"
printf -- 'luksOpen --volume-key-keyring %s $env{DEVNAME} %s"\n' "%%user:$_key_desc" "luks-$_devuuid"
} >>"${initdir}/etc/udev/rules.d/70-luks-kdump.rules"
Comment on lines +1030 to +1038
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see the point in having _key_desc. It's only been used once.
Plus I would use a here-doc here instead of a row of printfs. Personally I find that a lot better to read.



done

echo >"$initdir/etc/cmdline.d/90crypt.conf"
echo >"$initdir/etc/crypttab"
Comment on lines +1043 to +1044
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do I understand it correctly, that you are undoing work other dracut modules have done? If so, could you please add a short comment so we still remember it in a couple of years.


# latest systemd makes /usr read-only by default
mkdir -p "${initdir}/etc/systemd/system.conf.d"
{
echo "[Manager]"
echo "ProtectSystem=false"
} >>"${initdir}/etc/systemd/system.conf.d/kdump_luks.conf"
Comment on lines +1047 to +1051
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same like above. Here-doc?


dracut_need_initqueue
}

remove_cpu_online_rule() {
local file=${initdir}/usr/lib/udev/rules.d/40-redhat.rules

Expand Down Expand Up @@ -1065,6 +1114,8 @@ install() {
# at some point of time.
kdump_check_iscsi_targets

kdump_check_crypt_targets

kdump_install_systemd_conf

# nfs/ssh dump will need to get host ip in second kernel and need to call 'ip' tool, see get_host_ip for more detail
Expand Down
4 changes: 4 additions & 0 deletions kdump-lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,10 @@ get_luks_crypt_dev()
done
}

maj_min_to_uuid() {
lsblk -o uuid,MAJ:MIN | grep $1 | cut -d" " -f1
}

# kdump_get_maj_min <device>
# Prints the major and minor of a device node.
# Example:
Expand Down
50 changes: 50 additions & 0 deletions kdumpctl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,8 @@ load_kdump()
chcon -t boot_t "$KDUMP_KERNEL"
fi

prepare_luks

ddebug "$KEXEC $KEXEC_ARGS $standard_kexec_args --command-line=$KDUMP_COMMANDLINE --initrd=$TARGET_INITRD $KDUMP_KERNEL"

# The '12' represents an intermediate temporary file descriptor
Expand Down Expand Up @@ -1030,6 +1032,53 @@ check_final_action_config()
esac
}

LUKS_SYSFS=/sys/kernel/crash_dm_crypt_keys
reuse_loaded_luks_keys()
{
local _luks_dev _count _cmds _cmd _state
declare -a _luks_devs

Comment on lines +1036 to +1040
Copy link
Collaborator

Choose a reason for hiding this comment

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

Most of the variables are unused. And even _state I don't think is useful. IIUC the function could be shortened to

[[ $(< $LUKS_SYSFS ) == loaded ]] && echo -n reuse > $LUKS_SYSFS
return 0

Just for my understanding, what does reuse do? Does it prevent the kernel to discard the stored keys, when unloading the crash kernel?

Copy link
Collaborator

Choose a reason for hiding this comment

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

On second thought, wouldn't it be safer to simply drop this function and always setup the a new key ring when loading the kdump kernel. It's more expensive but otherwise you could end up in a situation like

# dump to luks dev1
$ kdumpctl start
# edit kdump.conf to dump to luks dev2
$ kdumpctl rebuild
$ kdumpctl reload

which, IIUC, should fail as the key of dev1 is now used to decrypt dev2, which most likely should fail.

_state=$(< $LUKS_SYSFS)

if [[ $_state == loaded ]]; then
echo -n reuse > $LUKS_SYSFS
return 0
fi
}

prepare_luks()
{
local _luks_dev _count _cmds _cmd _state
declare -a _luks_devs
Comment on lines +1049 to +1052
Copy link
Collaborator

Choose a reason for hiding this comment

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

_cmds and _cmd are unused.


_state=$(< $LUKS_SYSFS)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens when $LUKS_SYSFS doesn't exist? E.g. when you are running on an old kernel or one where the required configs aren't set?


if [[ $_state == recorded ]]; then
return 0
elif [[ $_state == reuse ]]; then
return 0
elif [[ $_state != fresh ]] && [[ $_state != initialized ]]; then
derror "Unknow state about the LUKS keys"
return 1
fi
Comment on lines +1056 to +1063
Copy link
Collaborator

@prudo1 prudo1 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why not use a switch case, i.e.

case "$_state" in
  fresh|initialized)
    # handled below
   ;;
  recorded|reuse)
    return 0
    ;;
  *)
    derror "Unknow state about the LUKS keys"
    return 1
    ;;
esac

Personally I find that easier to read.


_luks_devs=($(get_all_kdump_crypt_dev))
_count=${#_luks_devs[@]}

if [[ $_count -lt 1 ]]; then
return
fi

if [[ $_state == fresh ]]; then
printf "init %s" "$_count" > $LUKS_SYSFS
fi

for _luks_dev in "${_luks_devs[@]}"; do
_devuuid=$(maj_min_to_uuid "$_luks_dev")
printf "record cryptsetup:%s" "$_devuuid" > $LUKS_SYSFS
done
}

Comment on lines +1072 to +1081
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's no need to use printf here. A simple echo "init $_count" and echo "record cryptsetup:$(maj_min_to_uuid "$_luks_dev")" should work just fine.
Plus _devuuid isn't defined local.

start()
{
check_dump_feasibility || return
Expand Down Expand Up @@ -1702,6 +1751,7 @@ main()
exit $EXIT_CODE
;;
reload)
reuse_loaded_luks_keys
reload
;;
restart)
Expand Down
8 changes: 8 additions & 0 deletions kexec-crypt-setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/bin/sh
#
_devuuid=$(getarg kdump_luks_uuid=)

if [[ -n $_devuuid ]]; then
_key_desc=cryptsetup:$_devuuid
echo -n "$_key_desc" > /sys/kernel/crash_dm_crypt_keys
fi
15 changes: 0 additions & 15 deletions mkdumprd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,6 @@ handle_default_dump_target()
check_size fs "$_target"
}

check_crypt()
{
local _dev

for _dev in $(get_kdump_targets); do
if [[ -n $(get_luks_crypt_dev "$(get_maj_min "$_dev")") ]]; then
derror "Device $_dev is encrypted." && return 1
fi
done
}

if ! check_crypt; then
dwarn "Warning: Encrypted device is in dump path, which is not recommended, see kexec-kdump-howto.txt for more details."
fi

# firstly get right SSH_KEY_LOCATION
keyfile=$(kdump_get_conf_val sshkey)
if [[ -f $keyfile ]]; then
Expand Down