From: Klemens Nanni <kn@openbsd.org>
Subject: Re: [patch] Autoinstall with disk encryption
To: tech@openbsd.org
Date: Sun,  4 Feb 2024 12:50:01 +0000

On Wed, Jan 31, 2024 at 11:00:03PM +0000, Chris Narkiewicz wrote:
> On Fri, Jan 26, 2024 at 01:48:21AM +0000, Klemens Nanni wrote:
> > Untested idea:  ask_passphrase() question (match interactive bioctl prompt)
> > in unattended install only.
> 
> I'm not sure if it's a good idea to bind bioctl prompt to installer prompt.

Why (not)?

'New Passphrase' is known, unique and noone complained thus far, so I'm
inclined to stick with that.

> I left a custom prompt in my 2nd diff, but if we're really sure it's the
> way, it will be trivial to flip.
> 
> I made the following modifications:
> 1. password renamed to passphrase
> 2. passphrase prompt loops until valid input (same as root pass)
> 3. no more $AI break
> 
> I'm aware that we probably want to re-visit nameing and phrasing.
> 
> Index: distrib/miniroot/install.sub
> ===================================================================
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1257
> diff -u -p -u -p -r1.1257 install.sub
> --- distrib/miniroot/install.sub	24 Oct 2023 18:03:53 -0000	1.1257
> +++ distrib/miniroot/install.sub	31 Jan 2024 22:51:49 -0000
> @@ -3099,14 +3099,20 @@ pick_keydisk() {
>  	KEYDISK=$_disk$_label
>  }
>  
> +ask_disk_encryption_passphrase() {
> +	while :; do
> +		ask_password 'Passphrase for disk encryption?'
> +		[[ -n "$_password" ]] && break
> +		echo "Disk encryption passphrase must be set."
> +	done
> +	(umask 077 && echo "${_password}" > $DISK_PASSPHRASE_FILE)

You need print not echo, as mentioned before:

	$ v='a\rb\nc'
	$ echo "$v"
	b
	c
	$ print -r -- "$v"
	a\rb\nc

> +}
> +
>  encrypt_root() {
>  	local _args _chunk=$ROOTDISK
>  
>  	[[ $MDBOOTSR == y ]] || return
>  
> -	# The interactive bioctl(8) passphrase prompt requires a TTY.
> -	$AI && return
> -
>  	[[ -x /sbin/bioctl ]] || return
>  
>  	# Do not even try if softraid is in use already,
> @@ -3122,8 +3128,11 @@ encrypt_root() {
>  			_args=-k$KEYDISK
>  			break
>  			;;
> -		# Do nothing, bioctl(8) will handle the passphrase.
> -		[pP]*)	break
> +		# Ask for password and store it into a temporary file for bioctl

Passphrase not password, period is missing... I'd just drop the comment.

> +		[pP]*)
> +			ask_disk_encryption_passphrase

Contrary to bigger pick_keydisk() with multiple returns, the new passfile code
is simple enough to be inlined here, imho.

> +			_args=-p$DISK_PASSPHRASE_FILE
> +			break
>  			;;
>  		[nN]*)	return
>  			;;
> @@ -3137,6 +3146,7 @@ encrypt_root() {
>  	echo 'RAID *' | disklabel -w -A -T- $_chunk
>  
>  	bioctl -Cforce -cC -l${_chunk}a $_args softraid0 >/dev/null
> +	rm -f $DISK_PASSPHRASE_FILE
>  
>  	# No volumes existed before asking, but we just created one.
>  	ROOTDISK=$(get_softraid_volumes)
> @@ -3606,6 +3616,7 @@ CGI_INFO=/tmp/i/cgiinfo
>  CGI_METHOD=
>  CGI_TIME=
>  CGI_TZ=
> +DISK_PASSPHRASE_FILE=/tmp/i/disk_passphrase
>  export EDITOR=ed
>  HTTP_DIR=
>  HTTP_LIST=/tmp/i/httplist
> 
> 
> Best regards,
> Chris Narkiewicz
>