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