MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 01 Oct 2021 19:31:12 +0200
Message-Id: <CEO8ZQHLY4SN.3UPI2TMBB02BQ@taiga>
Cc: =?utf-8?q?Krystian_Chachu=C5=82a?= <krystian@krystianch.com>
Subject: Re: [PATCH alpine-conf v8] Support encrypted root in setup-disk
From: "Drew DeVault" <sir@cmpwn.com>
To: <~alpine/devel@lists.alpinelinux.org>
In-Reply-To: <20210814191142.11043-1-krystian@krystianch.com>

Some feedback from ncopa:

From: Natanael Copa <ncopa@alpinelinux.org>
To: Krystian Chachu*a <krystian@krystianch.com>
Cc: ~alpine/devel@lists.alpinelinux.org, Drew DeVault <sir@cmpwn.com>
Subject: Re: [PATCH alpine-conf v8] Support encrypted root in setup-disk
Date: Fri, 1 Oct 2021 18:40:51 +0200
X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.30; x86_64-alpine-linux-musl)

Hi!

Thank you for working on this.

On Sat, 14 Aug 2021 21:11:42 +0200
Krystian Chachu=C5=82a <krystian@krystianch.com> wrote:

> Co-authored-by: Drew DeVault <sir@cmpwn.com>
> ---
> Major changes since v7 are:
> * setup_grub and setup_root: when using EFI and encryption, the boot
>   partition is unencrypted and mounted to /boot instead of /boot/efi
>   (like current behavior with syslinux); this solves two issues: setting
>   up GRUB with luks2 encrypted /boot is difficult, installing GRUB with
>   encrypted /boot needs chroot or a workaround (see v7 discussion) =20

I think it would be nice with an explanation in the commit message why
it is done this way.

> * install_mounted_root: cryptroot kernel param is now set to UUID
>   instead of device name
> * init_progs: blkid is explicitly installed, because crypt_required
>   needs it
>=20
> I tested sys and cryptsys installation with syslinux/BIOS,
> GRUB/BIOS and GRUB/EFI on x86_64. =20

great! thanks!
>=20
> See
> https://git.sr.ht/~krystianch/alpine-conf/commit/70aaffca3d4685f3a8d78dea=
ca7b479521b13ab8
> for full diff between v7 and v8.
>=20
> I'd appreciate a review / feedback.
>=20
>  setup-disk.in | 135 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 106 insertions(+), 29 deletions(-)
>=20
> diff --git a/setup-disk.in b/setup-disk.in
> index 656b5bc..46c8068 100644
> --- a/setup-disk.in
> +++ b/setup-disk.in
> @@ -82,6 +82,26 @@ enumerate_fstab() {
>  		done
>  }
> =20
> +# given an fstab on stdin, determine if any of the mountpoints are encry=
pted
> +crypt_required() {
> +	while read -r devname mountpoint fstype mntops freq passno; do
> +		if [ -z "$devname" ] || [ "${devname###}" !=3D "$devname" ]; then
> +			continue
> +		fi
> +		uuid=3D"${devname##UUID=3D}"
> +		if [ "$uuid" !=3D "$devname" ]; then
> +			devname=3D"$(blkid --uuid "$uuid")"
> +		fi
> +		mapname=3D"${devname##/dev/mapper/}"
> +		if [ "$mapname" !=3D "$devname" ]; then
> +			if cryptsetup status "$mapname" >&1 >/dev/null; then
> +				return 0
> +			fi
> +		fi
> +	done
> +	return 1
> +}
> +
>  is_vmware() {
>  	grep -q VMware /proc/scsi/scsi 2>/dev/null \
>  		|| grep -q VMware /proc/ide/hd*/model 2>/dev/null
> @@ -283,19 +303,23 @@ setup_grub() {
>  	# install GRUB efi mode
>  	if [ -n "$USE_EFI" ]; then
>  		local target fwa
> +		local efi_directory=3D"$mnt"/boot/efi
>  		case "$ARCH" in
>  			x86_64)		target=3Dx86_64-efi ; fwa=3Dx64 ;;
>  			x86)		target=3Di386-efi ; fwa=3Dia32 ;;
>  			arm*)		target=3Darm-efi ; fwa=3Darm ;;
>  			aarch64)	target=3Darm64-efi ; fwa=3Daa64 ;;
>  		esac
> +		if [ -n "$USE_CRYPT" ]; then
> +			efi_directory=3D"$mnt"/boot
> +		fi
>  		# currently disabling nvram so grub doesnt call efibootmgr
>  		# installing to alpine directory so other distros dont overwrite it
> -		grub-install --target=3D$target --efi-directory=3D"$mnt"/boot/efi \
> +		grub-install --target=3D$target --efi-directory=3D"$efi_directory" \
>  			--bootloader-id=3Dalpine --boot-directory=3D"$mnt"/boot --no-nvram
>  		# fallback mode will use boot/boot${fw arch}.efi
> -		install -D "$mnt"/boot/efi/EFI/alpine/grub$fwa.efi \
> -			"$mnt"/boot/efi/EFI/boot/boot$fwa.efi
> +		install -D "$efi_directory"/EFI/alpine/grub$fwa.efi \
> +			"$efi_directory"/EFI/boot/boot$fwa.efi
>  	# install GRUB for ppc64le
>  	elif [ "$ARCH" =3D "ppc64le" ]; then
>  		shift 5
> @@ -427,7 +451,7 @@ setup_raspberrypi_bootloader() {
>  install_mounted_root() {
>  	local mnt=3D"$1"
>  	shift 1
> -	local disks=3D"${@}" mnt_boot=3D boot_fs=3D root_fs=3D
> +	local disks=3D"${@}" mnt_boot=3D boot_fs=3D root_fs=3D use_crypt=3D
>  	local initfs_features=3D"ata base ide scsi usb virtio"
>  	local pvs=3D dev=3D rootdev=3D bootdev=3D extlinux_raidopt=3D root=3D m=
odules=3D
>  	local kernel_opts=3D"$KERNELOPTS"
> @@ -490,7 +514,6 @@ install_mounted_root() {
>  		esac
>  	done
> =20
> -
>  	if [ -n "$VERBOSE" ]; then
>  		echo "Root device:     $rootdev"
>  		echo "Root filesystem: $root_fs"
> @@ -513,6 +536,28 @@ install_mounted_root() {
>  	# we should not try start modloop on sys install
>  	rm -f "$mnt"/etc/runlevels/*/modloop
> =20
> +	# generate the fstab
> +	if [ -f "$mnt"/etc/fstab ]; then
> +		mv "$mnt"/etc/fstab "$mnt"/etc/fstab.old
> +	fi
> +	enumerate_fstab "$mnt" >> "$mnt"/etc/fstab
> +	if [ -n "$SWAP_DEVICES" ]; then
> +		local swap_dev
> +		for swap_dev in $SWAP_DEVICES; do
> +			echo -e "$(uuid_or_device ${swap_dev})\tswap\tswap\tdefaults\t0 0" \
> +				>> "$mnt"/etc/fstab
> +		done
> +	fi
> +	cat >>"$mnt"/etc/fstab <<-__EOF__
> +		/dev/cdrom	/media/cdrom	iso9660	noauto,ro 0 0
> +		/dev/usbdisk	/media/usb	vfat	noauto	0 0
> +	__EOF__
> +
> +	if crypt_required <"$mnt"/etc/fstab; then
> +		use_crypt=3D1
> +		initfs_features=3D"$initfs_features cryptsetup"
> +	fi
> +
>  	# generate mkinitfs.conf
>  	mkdir -p "$mnt"/etc/mkinitfs/features.d
>  	echo "features=3D\"$initfs_features\"" > "$mnt"/etc/mkinitfs/mkinitfs.c=
onf
> @@ -530,24 +575,15 @@ install_mounted_root() {
>  	if [ -n "$(get_bootopt nomodeset)" ]; then
>  		kernel_opts=3D"nomodeset $kernel_opts"
>  	fi
> +	if [ "$use_crypt" ] && cryptsetup status "$rootdev" 2>&1 >/dev/null; th=
en
> +		# Boot to encrypted root
> +		root=3D$(cryptsetup status "$rootdev" | awk '/device:/ { print $2 }')
> +		root=3D$(uuid_or_device $root)
> +		kernel_opts=3D"cryptroot=3D$root cryptdm=3Droot"
> +		root=3D/dev/mapper/root
> +	fi
>  	modules=3D"sd-mod,usb-storage,${root_fs}${raidmod}"
> =20
> -	# generate the fstab
> -	if [ -f "$mnt"/etc/fstab ]; then
> -		mv "$mnt"/etc/fstab "$mnt"/etc/fstab.old
> -	fi
> -	enumerate_fstab "$mnt" >> "$mnt"/etc/fstab
> -	if [ -n "$SWAP_DEVICES" ]; then
> -		local swap_dev
> -		for swap_dev in $SWAP_DEVICES; do
> -			echo -e "$(uuid_or_device ${swap_dev})\tswap\tswap\tdefaults\t0 0" \ =
=20
> -				>> "$mnt"/etc/fstab   =20
> -		done
> -	fi
> -	cat >>"$mnt"/etc/fstab <<-__EOF__
> -		/dev/cdrom	/media/cdrom	iso9660	noauto,ro 0 0
> -		/dev/usbdisk	/media/usb	vfat	noauto	0 0
> -	__EOF__
>  	# remove the installed db in case its there so we force re-install
>  	rm -f "$mnt"/var/lib/apk/installed "$mnt"/lib/apk/db/installed
>  	echo "Installing system on $rootdev:"
> @@ -595,6 +631,10 @@ unmount_partitions() {
> =20
>  	# unmount the partitions
>  	umount $(awk '{print $2}' /proc/mounts | egrep "^$mnt(/|\$)" | sort -r)
> +
> +	if [ "$USE_CRYPT" ]; then
> +		cryptsetup close /dev/mapper/root
> +	fi
>  }
> =20
>  # figure out decent default swap size in mega bytes
> @@ -697,9 +737,10 @@ select_bootloader_pkg() {
> =20
>  # install needed programs
>  init_progs() {
> -	local raidpkg=3D lvmpkg=3D fs=3D fstools=3D grub=3D
> +	local raidpkg=3D lvmpkg=3D cryptpkg=3D fs=3D fstools=3D grub=3D
>  	[ -n "$USE_RAID" ] && raidpkg=3D"mdadm"
>  	[ -n "$USE_LVM" ] && lvmpkg=3D"lvm2"
> +	[ -n "$USE_CRYPT" ] && cryptpkg=3D"cryptsetup blkid"
>  	for fs in $BOOTFS $ROOTFS $VARFS; do
>  		# we need load btrfs module early to avoid the error message:
>  		# 'failed to open /dev/btrfs-control'
> @@ -714,7 +755,7 @@ init_progs() {
>  		vfat) fstools=3D"$fstools dosfstools";;
>  		esac
>  	done
> -	apk add --quiet sfdisk $lvmpkg $raidpkg $fstools $@
> +	apk add --quiet sfdisk $cryptpkg $lvmpkg $raidpkg $fstools $@
>  }
> =20
>  show_disk_info() {
> @@ -1018,11 +1059,11 @@ setup_root() {
>  	mkfs.$ROOTFS $MKFS_OPTS_ROOT $mkfs_args "$root_dev"
>  	mkdir -p "$SYSROOT"
>  	mount -t $ROOTFS $root_dev "$SYSROOT" || return 1
> -	if [ -n "$boot_dev" ] && [ -z "$USE_EFI" ]; then
> +	if [ -n "$boot_dev" ] && ([ -z "$USE_EFI" ] || [ -n "$USE_CRYPT" ]); th=
en
>  		mkdir -p "$SYSROOT"/boot
>  		mount -t $BOOTFS $boot_dev "$SYSROOT"/boot || return 1
>  	fi
> -	if [ -n "$boot_dev" ] && [ -n "$USE_EFI" ]; then
> +	if [ -n "$boot_dev" ] && [ -n "$USE_EFI" ] && [ -z "$USE_CRYPT" ]; then
>  		mkdir -p "$SYSROOT"/boot/efi
>  		mount -t $BOOTFS $boot_dev "$SYSROOT"/boot/efi || return 1
>  	fi
> @@ -1082,6 +1123,18 @@ native_disk_install_lvm() {
>  	setup_root $root_dev $BOOT_DEV
>  }
> =20
> +setup_crypt() {
> +	local dev=3D"$1" local dmname=3D"$2"
> +	mkdir -p /run/cryptsetup
> +	echo "Preparing root partition for encryption." >&2
> +	echo "You will be prompted for your password at boot." >&2
> +	echo "If you forget your password, your data will be lost." >&2
> +	cryptsetup luksFormat --type luks2 "$dev" >&2
> +	echo "Enter password again to unlock disk for installation." >&2
> +	cryptsetup open "$dev" "$dmname" >&2 =20

What happens if user provides wrong password to `cryptsetup open` in
setup_crypt? Shouldn't we handle error here?

> +	echo "/dev/mapper/$dmname"
> +}
> +
>  native_disk_install() {
>  	local prep_part_type=3D$(partition_id prep)
>  	local root_part_type=3D$(partition_id linux)
> @@ -1156,6 +1209,10 @@ native_disk_install() {
>  		root_dev=3D$(find_nth_non_boot_parts $index "$root_part_type" $@)
>  	fi
> =20
> +	if [ "$USE_CRYPT" ]; then
> +		root_dev=3D$(setup_crypt $root_dev root) =20

What should happen here if `cryptsetup open` failed?

> +	fi
> +
>  	[ $SWAP_SIZE -gt 0 ] && setup_swap_dev $swap_dev
>  	setup_root $root_dev $BOOT_DEV $@
>  }
> @@ -1176,7 +1233,7 @@ diskselect_help() {
>  diskmode_help() {
>  	cat <<-__EOF__
> =20
> -		You can select between 'sys', 'data', 'lvm', 'lvmsys' or 'lvmdata'.
> +		You can select between 'sys', 'cryptsys', 'data', 'lvm', 'lvmsys' or '=
lvmdata'.
> =20
>  		sys:
>  		  This mode is a traditional disk install. The following partitions wi=
ll be
> @@ -1184,6 +1241,12 @@ diskmode_help() {
> =20
>  		  This mode may be used for development boxes, desktops, virtual serve=
rs, etc.
> =20
> +		cryptsys:
> +		  This mode is equivalent to sys, except that the root filesystem will=
 be
> +		  encrypted with cryptsetup. You will be prompted to enter a decryptio=
n
> +		  password, and will need to use this password to boot up the operatin=
g
> +		  system after installation.
> +
>  		data:
>  		  This mode uses your disk(s) for data storage, not for the operating =
system.
>  		  The system itself will run from tmpfs (RAM).
> @@ -1233,7 +1296,7 @@ ask_disk() {
> =20
>  usage() {
>  	cat <<-__EOF__
> -		usage: setup-disk [-hLqrv] [-k kernelflavor] [-m MODE] [-o apkovl] [-s=
 SWAPSIZE]
> +		usage: setup-disk [-hLqrve] [-k kernelflavor] [-m MODE] [-o apkovl] [-=
s SWAPSIZE]
>  		                  [MOUNTPOINT | DISKDEV...]
> =20
>  		Install alpine on harddisk.
> @@ -1247,6 +1310,7 @@ usage() {
> =20
>  		options:
>  		 -h  Show this help
> +		 -e  Encrypt disk
>  		 -m  Use disk for MODE without asking, where MODE is either 'data' or =
'sys'
>  		 -o  Restore system from given apkovl file
>  		 -k  Use kernelflavor instead of $KERNEL_FLAVOR
> @@ -1286,11 +1350,13 @@ case $kver in
>  	*) KERNEL_FLAVOR=3Dlts;;
>  esac
> =20
> +USE_CRYPT=3D
>  DISK_MODE=3D
>  USE_LVM=3D
>  # Parse args
> -while getopts "hk:Lm:o:qrs:v" opt; do
> +while getopts "hek:Lm:o:qrs:v" opt; do
>  	case $opt in
> +		e) USE_CRYPT=3D1;;
>  		m) DISK_MODE=3D"$OPTARG";;
>  		k) KERNEL_FLAVOR=3D"$OPTARG";;
>  		L) USE_LVM=3D"_lvm";;
> @@ -1319,6 +1385,12 @@ fi
>  reset_var
>  swapoff -a
> =20
> +if [ $USE_CRYPT -eq 1 ] && [ -n "$USE_LVM" ]; then
> +	echo "cryptsys and lvm are currently mutually incompatible."
> +	echo "Please run $0 again with only one of these options selected."
> +	exit 1
> +fi
> + =20

I wonder how much extra work it would be to add support for encrypted
disk under lvm?

>  # stop all volume groups in use
>  vgchange --ignorelockingfailure -a n >/dev/null 2>&1
> =20
> @@ -1381,9 +1453,14 @@ if [ -n "$diskdevs" ] && [ -z "$DISK_MODE" ]; then
>  		echo "The following $disk_is_or_disks_are selected${USE_LVM:+ (with LV=
M)}:"
>  		show_disk_info $diskdevs
>  		_lvm=3D${USE_LVM:-", 'lvm'"}
> -		ask "How would you like to use $it_them? ('sys', 'data'${_lvm#_lvm} or=
 '?' for help)" "?"
> +		ask "How would you like to use $it_them? ('sys', 'cryptsys', 'data'${_=
lvm#_lvm} or '?' for help)" "?"
>  		case "$resp" in
>  		'?') diskmode_help;;
> +		cryptsys)
> +			resp=3Dsys
> +			USE_CRYPT=3D1
> +			break
> +			;; =20

What would be needed to be able to select 'crypt' and then select
either 'sys' or 'data'? similar to what we do with lvm. So we could
have data on cryptsetup as well.

>  		sys|data) break;;
>  		lvm) USE_LVM=3D"_lvm" ;;
>  		nolvm) USE_LVM=3D"";;