On Fri, Aug 11, 2023 at 03:51:38PM +0100, Stuart Henderson wrote:
> On 2023/08/11 16:43, Mark Kettenis wrote:
> > See the recent discussion about _bcrypt_autorounds() in libc.
> > 
> > System performance varies, and even on modern hardware it can provide
> > varying results.  The ramdisk environment is considerably different
> > from the mult-user environment in this respect.
> > 
> > Using a fixed value is way more predictable.  If 16 no longer is a
> > safe default it should be raised.

With this diff and an extra -v, the passphrase through the installer's
question pretty consistently yields 300-350 rounds in my setup, whereas
the same bioctl invocation run in my idle desktop session varies between
ranges from 90 to 350, averaging at 240.

Is that an acceptable raise?

> 
> Agreed. (Re bcrypt, I usually completely ignore auto rounds, I had just
> forgotten to set that up on the machine where I noticed the problem..)
> 
> Also, am I right in thinking that this only affects the time when
> entering the passphrase when mounting or creating the device, i.e. once
> per boot?

Again, yes.

> 
> If so, there's nowhere near as much a downside to that being slow
> as there is for user login. (anyone actually wanting to crack these
> passphrases would be doing it on a fast system rather than whatever
> the device is normally used with, so there are valid reasons for
> picking something that might be a bit slow if it doesn't cause too
> much system impact).

This diff is OK op, who also tested it intensively;  regress/sbin/bioctl
will cover '-r' behaviour soon, too.

I agree with Theo here that bioctl's case is different from passwd(1)/libc.

Default [-r auto] never decreases rounds, only explicit '-r N' can.
16 is the absolute minium.

-P to change passphrases picks MAX(old-rounds, auto).

Feedback? Objection? OK?

Index: bioctl.8
===================================================================
RCS file: /cvs/src/sbin/bioctl/bioctl.8,v
retrieving revision 1.113
diff -u -p -r1.113 bioctl.8
--- bioctl.8    21 Aug 2023 08:33:11 -0000      1.113
+++ bioctl.8    23 Aug 2023 13:22:38 -0000
@@ -282,11 +282,12 @@ passphrase into a key, in order to creat
 passphrase of an existing encrypted volume.
 A larger number of iterations takes more time, but offers increased resistance
 against passphrase guessing attacks.
-If
+By default, or if
 .Ar rounds
-is specified as "auto", the number of rounds will be automatically determined
-based on system performance.
-Otherwise the minimum is 4 rounds and the default is 16.
+is specified as
+.Cm auto ,
+the number of rounds will automatically be based on system performance.
+The minimum is 16 rounds.
 .It Fl s
 Read passphrases from
 .Pa /dev/stdin
Index: bioctl.c
===================================================================
RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
retrieving revision 1.154
diff -u -p -r1.154 bioctl.c
--- bioctl.c    21 Aug 2023 08:33:11 -0000      1.154
+++ bioctl.c    23 Aug 2023 13:22:38 -0000
@@ -89,7 +89,7 @@ int                   devh = -1;
 int                    human;
 int                    verbose;
 u_int32_t              cflags = 0;
-int                    rflag = 0;
+int                    rflag = -1;     /* auto */
 char                   *password;
 
 void                   *bio_cookie;
@@ -182,7 +182,7 @@ main(int argc, char *argv[])
                                rflag = -1;
                                break;
                        }
-                       rflag = strtonum(optarg, 4, 1<<30, &errstr);
+                       rflag = strtonum(optarg, 16, 1<<30, &errstr);
                        if (errstr != NULL)
                                errx(1, "number of KDF rounds is %s: %s",
                                    errstr, optarg);
@@ -979,7 +979,7 @@ bio_kdf_generate(struct sr_crypto_kdfinf
 
        kdfinfo->pbkdf.generic.len = sizeof(kdfinfo->pbkdf);
        kdfinfo->pbkdf.generic.type = SR_CRYPTOKDFT_BCRYPT_PBKDF;
-       kdfinfo->pbkdf.rounds = rflag ? rflag : 16;
+       kdfinfo->pbkdf.rounds = rflag;
 
        kdfinfo->flags = SR_CRYPTOKDF_KEY | SR_CRYPTOKDF_HINT;
        kdfinfo->len = sizeof(*kdfinfo);
@@ -1119,12 +1119,14 @@ bio_changepass(char *dev)
        /* Current passphrase. */
        bio_kdf_derive(&kdfinfo1, &kdfhint, "Old passphrase: ", 0);
 
-       /*
-        * Unless otherwise specified, keep the previous number of rounds as
-        * long as we're using the same KDF.
-        */
-       if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF && !rflag)
-               rflag = kdfhint.rounds;
+       if (rflag == -1) {
+               rflag = bcrypt_pbkdf_autorounds();
+
+               /* Use previous number of rounds for the same KDF if higher. */
+               if (kdfhint.generic.type == SR_CRYPTOKDFT_BCRYPT_PBKDF &&
+                   rflag < kdfhint.rounds)
+                       rflag = kdfhint.rounds;
+       }
 
        /* New passphrase. */
        bio_kdf_generate(&kdfinfo2);
@@ -1328,7 +1330,7 @@ derive_key(u_int32_t type, int rounds, u
            type != SR_CRYPTOKDFT_BCRYPT_PBKDF)
                errx(1, "unknown KDF type %d", type);
 
-       if (rounds < (type == SR_CRYPTOKDFT_PKCS5_PBKDF2 ? 1000 : 4))
+       if (rounds < (type == SR_CRYPTOKDFT_PKCS5_PBKDF2 ? 1000 : 16))
                errx(1, "number of KDF rounds is too small: %d", rounds);
 
        /* get passphrase */

Reply via email to