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?


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 */

Reply via email to