~alpine/devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
2 2

[PATCH v5] Support encrypted root in setup-disk

Details
Message ID
<20200121184924.99939-1-sir@cmpwn.com>
DKIM signature
missing
Download raw message
Patch: +80 -21
---
This should be the one. It addresses Nathaniel's feedback from IRC and
has been tested on new installs.

 setup-disk.in | 101 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 21 deletions(-)

diff --git a/setup-disk.in b/setup-disk.in
index 5eb8638..71c2602 100644
--- a/setup-disk.in
+++ b/setup-disk.in
@@ -79,6 +79,26 @@ enumerate_fstab() {
		done
}

# given an fstab on stdin, determine if any of the mountpoints are encrypted
crypt_required() {
	while read devname mountpoint fstype mntops freq passno; do
		if [ -z "$devname" ] || [ "${devname###}" != "$devname" ]; then
			continue
		fi
		uuid="${devname##UUID=}"
		if [ "$uuid" != "$devname" ]; then
			devname="$(blkid --uuid "$uuid")"
		fi
		mapname="${devname##/dev/mapper/}"
		if [ "$mapname" != "$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
@@ -343,7 +363,7 @@ setup_syslinux() {
install_mounted_root() {
	local mnt="$1"
	shift 1
	local disks="${@}" mnt_boot= boot_fs= root_fs=
	local disks="${@}" mnt_boot= boot_fs= root_fs= use_crypt=
	local initfs_features="ata base ide scsi usb virtio"
	local pvs= dev= rootdev= bootdev= extlinux_raidopt= root= modules=
	local kernel_opts="quiet"
@@ -402,7 +422,6 @@ install_mounted_root() {
		esac
	done


	if [ -n "$VERBOSE" ]; then
		echo "Root device:     $rootdev"
		echo "Root filesystem: $root_fs"
@@ -425,6 +444,28 @@ install_mounted_root() {
	# we should not try start modloop on sys install
	rm -f "$mnt"/etc/runlevels/*/modloop

	# 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=1
		initfs_features="${initfs_features% cryptsetup} cryptsetup"
	fi

	# generate mkinitfs.conf
	mkdir -p "$mnt"/etc/mkinitfs/features.d
	echo "features=\"$initfs_features\"" > "$mnt"/etc/mkinitfs/mkinitfs.conf
@@ -442,24 +483,14 @@ install_mounted_root() {
	if [ -n "$(get_bootopt nomodeset)" ]; then
		kernel_opts="nomodeset $kernel_opts"
	fi
	if [ "$use_crypt" ] && cryptsetup status "$rootdev" 2>&1 >/dev/null; then
		# Boot to encrypted root
		root=$(cryptsetup status "$rootdev" | awk '/device:/ { print $2 }')
		kernel_opts="cryptroot=$root cryptdm=root"
		root=/dev/mapper/root
	fi
	modules="sd-mod,usb-storage,${root_fs}${raidmod}"

	# 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__
	# 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:"
@@ -503,6 +534,10 @@ unmount_partitions() {

	# unmount the partitions
	umount $(awk '{print $2}' /proc/mounts | egrep "^$mnt(/|\$)" | sort -r)

	if [ "$USE_CRYPT" ]; then
		cryptsetup close /dev/mapper/root
	fi
}

# figure out decent default swap size in mega bytes
@@ -994,6 +1029,18 @@ native_disk_install_lvm() {
	setup_root $root_dev $BOOT_DEV
}

setup_crypt() {
	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 "$1" >&2
	echo "Enter password again to unlock disk for installation." >&2
	cryptsetup open "$1" root >&2
	cryptroot="$1"
	echo "/dev/mapper/root"
}

native_disk_install() {
	local prep_part_type=$(partition_id prep)
	local root_part_type=$(partition_id linux)
@@ -1065,6 +1112,10 @@ native_disk_install() {
		root_dev=$(find_nth_non_boot_parts $index "$root_part_type" $@)
	fi

	if [ "$USE_CRYPT" ]; then
		root_dev=$(setup_crypt $root_dev)
	fi

	[ $SWAP_SIZE -gt 0 ] && setup_swap_dev $swap_dev
	setup_root $root_dev $BOOT_DEV $@
}
@@ -1143,7 +1194,7 @@ ask_disk() {

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...]

		Install alpine on harddisk.
@@ -1157,6 +1208,7 @@ usage() {

		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
@@ -1193,11 +1245,13 @@ case $kver in
	*) KERNEL_FLAVOR=vanilla;;
esac

USE_CRYPT=
DISK_MODE=
USE_LVM=
# 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=1;;
		m) DISK_MODE="$OPTARG";;
		k) KERNEL_FLAVOR="$OPTARG";;
		L) USE_LVM="_lvm";;
@@ -1275,10 +1329,15 @@ if [ -n "$diskdevs" ] && [ -z "$DISK_MODE" ]; then
		echo "The following $disk_is_or_disks_are selected${USE_LVM:+ (with LVM)}:"
		show_disk_info $diskdevs
		_lvm=${USE_LVM:-", 'lvm'"}
		echon "How would you like to use $it_them? ('sys', 'data'${_lvm#_lvm} or '?' for help) [?] "
		echon "How would you like to use $it_them? ('sys', 'cryptsys', 'data'${_lvm#_lvm} or '?' for help) [?] "
		default_read answer '?'
		case "$answer" in
		'?') diskmode_help;;
		cryptsys)
			answer=sys
			USE_CRYPT=1
			break
			;;
		sys|data) break;;
		lvm) USE_LVM="_lvm" ;;
		nolvm) USE_LVM="";;
-- 
2.25.0
Details
Message ID
<20200416133308.0f6b27b7@ncopa-desktop.copa.dup.pw>
In-Reply-To
<20200121184924.99939-1-sir@cmpwn.com> (view parent)
DKIM signature
missing
Download raw message
Hi!

Finally having a look at this.

We also need add some help test in diskmode_help that explains what cryptsys does.

I think we should also keep in mind that we may want support encrypted
data or encrypted lvm in the future.

See also comments inline below.


-nc


On Tue, 21 Jan 2020 13:49:24 -0500
Drew DeVault <sir@cmpwn.com> wrote:

> ---
> This should be the one. It addresses Nathaniel's feedback from IRC and
> has been tested on new installs.
> 
>  setup-disk.in | 101 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 80 insertions(+), 21 deletions(-)
> 
> diff --git a/setup-disk.in b/setup-disk.in
> index 5eb8638..71c2602 100644
> --- a/setup-disk.in
> +++ b/setup-disk.in
> @@ -79,6 +79,26 @@ enumerate_fstab() {
>  		done
>  }
>  
> +# given an fstab on stdin, determine if any of the mountpoints are encrypted
> +crypt_required() {
> +	while read devname mountpoint fstype mntops freq passno; do
> +		if [ -z "$devname" ] || [ "${devname###}" != "$devname" ]; then
> +			continue
> +		fi
> +		uuid="${devname##UUID=}"
> +		if [ "$uuid" != "$devname" ]; then
> +			devname="$(blkid --uuid "$uuid")"
> +		fi
> +		mapname="${devname##/dev/mapper/}"
> +		if [ "$mapname" != "$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
> @@ -343,7 +363,7 @@ setup_syslinux() {
>  install_mounted_root() {
>  	local mnt="$1"
>  	shift 1
> -	local disks="${@}" mnt_boot= boot_fs= root_fs=
> +	local disks="${@}" mnt_boot= boot_fs= root_fs= use_crypt=
>  	local initfs_features="ata base ide scsi usb virtio"
>  	local pvs= dev= rootdev= bootdev= extlinux_raidopt= root= modules=
>  	local kernel_opts="quiet"
> @@ -402,7 +422,6 @@ install_mounted_root() {
>  		esac
>  	done
>  
> -
>  	if [ -n "$VERBOSE" ]; then
>  		echo "Root device:     $rootdev"
>  		echo "Root filesystem: $root_fs"
> @@ -425,6 +444,28 @@ install_mounted_root() {
>  	# we should not try start modloop on sys install
>  	rm -f "$mnt"/etc/runlevels/*/modloop
>  
> +	# 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

Maybe use sed to remove comments here?

  sed 's/#.*//' "$mnt"/etc/fstab | crypt_required

> +		use_crypt=1
> +		initfs_features="${initfs_features% cryptsetup} cryptsetup"

initfs_features should only be modified if the rootdev is encrypted and
not if any other mounts are.

> +	fi
> +
>  	# generate mkinitfs.conf
>  	mkdir -p "$mnt"/etc/mkinitfs/features.d
>  	echo "features=\"$initfs_features\"" > "$mnt"/etc/mkinitfs/mkinitfs.conf
> @@ -442,24 +483,14 @@ install_mounted_root() {
>  	if [ -n "$(get_bootopt nomodeset)" ]; then
>  		kernel_opts="nomodeset $kernel_opts"
>  	fi
> +	if [ "$use_crypt" ] && cryptsetup status "$rootdev" 2>&1 >/dev/null; then
> +		# Boot to encrypted root
> +		root=$(cryptsetup status "$rootdev" | awk '/device:/ { print $2 }')
> +		kernel_opts="cryptroot=$root cryptdm=root"
> +		root=/dev/mapper/root
> +	fi
>  	modules="sd-mod,usb-storage,${root_fs}${raidmod}"
>  
> -	# 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__
>  	# 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:"
> @@ -503,6 +534,10 @@ unmount_partitions() {
>  
>  	# unmount the partitions
>  	umount $(awk '{print $2}' /proc/mounts | egrep "^$mnt(/|\$)" | sort -r)
> +
> +	if [ "$USE_CRYPT" ]; then
> +		cryptsetup close /dev/mapper/root
> +	fi
>  }
>  
>  # figure out decent default swap size in mega bytes
> @@ -994,6 +1029,18 @@ native_disk_install_lvm() {
>  	setup_root $root_dev $BOOT_DEV
>  }
>  
> +setup_crypt() {
> +	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 "$1" >&2
> +	echo "Enter password again to unlock disk for installation." >&2
> +	cryptsetup open "$1" root >&2
> +	cryptroot="$1"
> +	echo "/dev/mapper/root"
> +}

I think we should pass "root" as an argument so it can be reused for a
encrypted data partition in the future.

setup_crypt() {
	local dev="$1" local dmname="$2"
	...
	echo "Preparing your $dmname partition for encryption."
	...
	cryptsetup open "$dev" "$dmname"
}

That way we also don't need send the prompt to stderr.

> +
>  native_disk_install() {
>  	local prep_part_type=$(partition_id prep)
>  	local root_part_type=$(partition_id linux)
> @@ -1065,6 +1112,10 @@ native_disk_install() {
>  		root_dev=$(find_nth_non_boot_parts $index "$root_part_type" $@)
>  	fi
>  
> +	if [ "$USE_CRYPT" ]; then
> +		root_dev=$(setup_crypt $root_dev)
> +	fi

Maybe something like if we do the above setup_crypt adjustment:

	if [ "$USE_CRYPT" ]; then
		setup_crypt $root_dev root
		root_dev=/dev/mapper/root
	fi
	

> +
>  	[ $SWAP_SIZE -gt 0 ] && setup_swap_dev $swap_dev
>  	setup_root $root_dev $BOOT_DEV $@
>  }
> @@ -1143,7 +1194,7 @@ ask_disk() {
>  
>  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...]
>  
>  		Install alpine on harddisk.
> @@ -1157,6 +1208,7 @@ usage() {
>  
>  		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
> @@ -1193,11 +1245,13 @@ case $kver in
>  	*) KERNEL_FLAVOR=vanilla;;
>  esac
>  
> +USE_CRYPT=
>  DISK_MODE=
>  USE_LVM=
>  # 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=1;;
>  		m) DISK_MODE="$OPTARG";;
>  		k) KERNEL_FLAVOR="$OPTARG";;
>  		L) USE_LVM="_lvm";;
> @@ -1275,10 +1329,15 @@ if [ -n "$diskdevs" ] && [ -z "$DISK_MODE" ]; then
>  		echo "The following $disk_is_or_disks_are selected${USE_LVM:+ (with LVM)}:"
>  		show_disk_info $diskdevs
>  		_lvm=${USE_LVM:-", 'lvm'"}
> -		echon "How would you like to use $it_them? ('sys', 'data'${_lvm#_lvm} or '?' for help) [?] "
> +		echon "How would you like to use $it_them? ('sys', 'cryptsys', 'data'${_lvm#_lvm} or '?' for help) [?] "
>  		default_read answer '?'
>  		case "$answer" in
>  		'?') diskmode_help;;
> +		cryptsys)
> +			answer=sys
> +			USE_CRYPT=1
> +			break
> +			;;
>  		sys|data) break;;
>  		lvm) USE_LVM="_lvm" ;;
>  		nolvm) USE_LVM="";;

What happens if you enable bot encryption and lvm (setup-disk -e -L)? should we error out and say its not yet supported?

Thanks!

-nc
Details
Message ID
<C2HZGIQXJBH5.5GKVB3WJ6CZE@homura>
In-Reply-To
<20200416133308.0f6b27b7@ncopa-desktop.copa.dup.pw> (view parent)
DKIM signature
missing
Download raw message
Thanks for the review. Are you okay with receiving v6 here on the
mailing list or has alpine-conf been moved to GitLab yet?

On Thu Apr 16, 2020 at 9:33 AM PST, Natanael Copa wrote:
> > +	if crypt_required <"$mnt"/etc/fstab; then
>
> Maybe use sed to remove comments here?
>
> sed 's/#.*//' "$mnt"/etc/fstab | crypt_required

crypt_required strips out comments itself, which seems more robust imo.

> > +setup_crypt() {
> > +	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 "$1" >&2
> > +	echo "Enter password again to unlock disk for installation." >&2
> > +	cryptsetup open "$1" root >&2
> > +	cryptroot="$1"
> > +	echo "/dev/mapper/root"
> > +}
>
> I think we should pass "root" as an argument so it can be reused for a
> encrypted data partition in the future.
>
> setup_crypt() {
> local dev="$1" local dmname="$2"
> ...
> echo "Preparing your $dmname partition for encryption."
> ...
> cryptsetup open "$dev" "$dmname"
> }
>
> That way we also don't need send the prompt to stderr.

This is a good idea, but I'm not sure how it saves us from sending the
prompt to stderr.

> Maybe something like if we do the above setup_crypt adjustment:
>
> if [ "$USE_CRYPT" ]; then
> setup_crypt $root_dev root
> root_dev=/dev/mapper/root
> fi

I just adjusted setup_crypt to echo the final dev name instead, so e.g.

setup_crypt /dev/sda1 data

Now echos "/dev/mapper/data". Similarly to crypt_required, it seems more
robust to give the onus of correctness over here.

> What happens if you enable bot encryption and lvm (setup-disk -e -L)?
> should we error out and say its not yet supported?

I can make it do so, for sure.