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

Reply via email to