On Wed, Aug 02, 2023 at 02:38:57PM +0300, Klemens Nanni wrote: > This needs "bioctl: do not confirm new passphrases on stdin" on tech@. > > Current code tries thrice to get matching passphrases before aborting; > simple enough to get the feature going, but also due to code limitations. > > One possible fix is to let the installer (not bioctl) prompt the passphrase > like it does for the root password and pass it to bioctl non-interactively. > > This means > * a familiar question style and endless retry behaviour, not bioctl's prompt > * manual empty string check, bioctl already it > * installer duplicates existing bioctl prompt functionality > > > Setting OpenBSD MBR partition to whole sd0...done. > -New passphrase: > -Re-type passphrase: > +Passphrase for the root disk? (again) > +Passphrase for the root disk? (will not echo) > sd1 at scsibus1 targ 1 lun 0: <OPENBSD, SR CRYPTO, 006>
An alternative approach could be a new bioctl(8)) flag -K Keep prompting until new and re-typed passphrases match. to repeat the prompt (during interactive creation only) until match or ^C: # ./obj/bioctl -K -cC -Cforce -lvnd0a softraid0 New passphrase: Re-type passphrase: bioctl: Passphrases did not match, try again New passphrase: Re-type passphrase: ... softraid0: CRYPTO volume attached as sd3 That means: * straight forward installer code * -K could be extended to unlock (not creation) prompts if deemed useful I see value in both approaches, but do tend towards the first -s one leaving it to the installer, mainly because it touches bioctl.c less and makes the prompt text in line with other questions. Feedback? Index: distrib/miniroot/install.sub =================================================================== RCS file: /cvs/src/distrib/miniroot/install.sub,v retrieving revision 1.1252 diff -u -p -r1.1252 install.sub --- distrib/miniroot/install.sub 2 Aug 2023 08:51:16 -0000 1.1252 +++ distrib/miniroot/install.sub 2 Aug 2023 11:39:24 -0000 @@ -3075,7 +3075,7 @@ do_autoinstall() { } encrypt_root() { - local _chunk _tries=0 + local _chunk [[ $MDBOOTSR == y ]] || return @@ -3097,10 +3097,7 @@ encrypt_root() { md_prep_fdisk $_chunk echo 'RAID *' | disklabel -w -A -T- $_chunk - until bioctl -c C -l ${_chunk}a softraid0 >/dev/null; do - # Most likely botched passphrases, silently retry twice. - ((++_tries < 3)) || exit - done + bioctl -K -cC -l${_chunk}a softraid0 >/dev/null # No volumes existed before asking, but we just created one. ROOTDISK=$(get_softraid_volumes) Index: sbin/bioctl/bioctl.8 =================================================================== RCS file: /cvs/src/sbin/bioctl/bioctl.8,v retrieving revision 1.111 diff -u -p -r1.111 bioctl.8 --- sbin/bioctl/bioctl.8 6 Jul 2023 21:08:50 -0000 1.111 +++ sbin/bioctl/bioctl.8 2 Aug 2023 10:44:16 -0000 @@ -41,7 +41,7 @@ .Ar device .Pp .Nm bioctl -.Op Fl dhiPqsv +.Op Fl dhiKPqsv .Op Fl C Ar flag Ns Op Pf , Ar ... .Op Fl c Ar raidlevel .Op Fl k Ar keydisk @@ -245,6 +245,8 @@ they become part of the array again. .It Fl d Detach volume specified by .Ar device . +.It Fl K +Keep prompting until new and re-typed passphrases match. .It Fl k Ar keydisk Use special device .Ar keydisk Index: sbin/bioctl/bioctl.c =================================================================== RCS file: /cvs/src/sbin/bioctl/bioctl.c,v retrieving revision 1.151 diff -u -p -r1.151 bioctl.c --- sbin/bioctl/bioctl.c 18 Oct 2022 07:04:20 -0000 1.151 +++ sbin/bioctl/bioctl.c 2 Aug 2023 10:44:16 -0000 @@ -90,6 +90,7 @@ int human; int verbose; u_int32_t cflags = 0; int rflag = 0; +int keeptrying = 0; char *password; void *bio_cookie; @@ -114,8 +115,8 @@ main(int argc, char *argv[]) if (argc < 2) usage(); - while ((ch = getopt(argc, argv, "a:b:C:c:dH:hik:l:O:Pp:qr:R:st:u:v")) != - -1) { + while ((ch = getopt(argc, argv, "a:b:C:c:dH:hiKk:l:O:Pp:qr:R:st:u:v")) + != -1) { switch (ch) { case 'a': /* alarm */ func |= BIOC_ALARM; @@ -163,6 +164,9 @@ main(int argc, char *argv[]) case 'i': /* inquiry */ func |= BIOC_INQ; break; + case 'K': /* Keep retrying new passphrase. */ + keeptrying = 1; + break; case 'k': /* Key disk. */ key_disk = optarg; break; @@ -289,7 +293,7 @@ usage(void) "\t[-t patrol-function] " "[-u channel:target[.lun]] " "device\n" - " %s [-dhiPqsv] " + " %s [-dhiKPqsv] " "[-C flag[,...]] [-c raidlevel] [-k keydisk]\n" "\t[-l chunk[,...]] " "[-O device | channel:target[.lun]]\n" @@ -1351,6 +1355,7 @@ derive_key(u_int32_t type, int rounds, u fclose(f); } else { +retry: if (readpassphrase(prompt, passphrase, sizeof(passphrase), rpp_flag) == NULL) err(1, "unable to read passphrase"); @@ -1367,6 +1372,10 @@ derive_key(u_int32_t type, int rounds, u (strcmp(passphrase, verifybuf) != 0)) { explicit_bzero(passphrase, sizeof(passphrase)); explicit_bzero(verifybuf, sizeof(verifybuf)); + if (keeptrying) { + warnx("Passphrases did not match, try again"); + goto retry; + } errx(1, "Passphrases did not match"); } /* forget the re-typed one */