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