MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PATCH alpine-conf v8] Support encrypted root in setup-disk
From: =?utf-8?q?Krystian_Chachu=C5=82a?= <krystian@krystianch.com>
To: "Drew DeVault" <sir@cmpwn.com>, <~alpine/devel@lists.alpinelinux.org>
Date: Sun, 10 Oct 2021 19:55:32 +0200
Message-Id: <CEVX59MUE9QH.342GEHM50UU3N@userpc>
In-Reply-To: <CEO8ZQHLY4SN.3UPI2TMBB02BQ@taiga>

Thank you Natanael for the review.

On Fri Oct 1, 2021 at 6:40 PM CEST, Natanael Copa wrote:
> > * 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: settin=
g
> >   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.

Ok, I'll address it in v9.

> What happens if user provides wrong password to `cryptsetup open` in
> setup_crypt? Shouldn't we handle error here?

I'd add `--tries 1` to make `cryptsetup open` exit on wrong password and pr=
ompt
user what to do:

Could not unlock disk: [REASON FROM EXIT CODE]. What would you like to do?
('retry', 'reformat', 'skip' or '?' for help)

Help would say:

You can select between 'retry', 'reformat', or 'skip'.

retry:
  Try unlocking disk again.

reformat:
  Discard current password and set a new one.

skip:
  Continue without encrypting the root partition.

> >  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?

I'd make `setup_crypt` echo nothing when user skips encryption and, if it
happens, skip overwriting `root_dev` here.

> I wonder how much extra work it would be to add support for encrypted
> disk under lvm?

I'd say not that much. For LVM on LUKS:

(1) `native_disk_install_lvm` would need to call `setup_crypt` to create
	`/dev/mapper/cryptlvm` before `setup_lvm_volume_group` and
(2) `setup_lvm_volume_group` would need to set up LVM on `/dev/mapper/crypt=
lvm`.

If this looks ok I'll implement this in v9.

> 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.

I think we need an init script that runs before localmount and opens the
encrypted var partition by asking for password interactively.