On Tue, Aug 15, 2023 at 09:08:44AM +0000, Klemens Nanni wrote:
> On Wed, Aug 02, 2023 at 11:51:09AM +0000, Klemens Nanni wrote:
> > 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.
>
> On the other hand, sticking to bioctl's prompt in the installer question
> means consistency across installer, bootloader and userland (change
> passphrase, create/unlock extra disks).
>
> -K makes bioctl(8) similar to passwd(1) and other prompting tools we have,
> although -K is for one very specific and thus very simple, i.e. it retries
> endlessly rather than aborting after N tries like other tools do.
>
> Given it is an extra optional flag and the complexity it avoids, this seems
> like an acceptable solution.
>
> Feedback? Objection? OK?
I prefer this approach, if there is one use for retrying, there are
probably more.
> Index: distrib/miniroot/install.sub
> ===================================================================
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1253
> diff -u -p -r1.1253 install.sub
> --- distrib/miniroot/install.sub 10 Aug 2023 17:09:34 -0000 1.1253
> +++ distrib/miniroot/install.sub 15 Aug 2023 09:07:47 -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 -Cforce -cC -l${_chunk}a softraid0 >/dev/null; do
> - # Most likely botched passphrases, silently retry twice.
> - ((++_tries < 3)) || exit
> - done
> + bioctl -K -Cforce -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 15 Aug 2023 09:06:20 -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 15 Aug 2023 09:06:21 -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 */
--
andrew
Speed matters.
Almost as much as some things, and nowhere near as much as others.
-- Nick Holland
