Re: [s...@spacehopper.org: Re: Remove useless line from daemon class in login.conf]

2020-10-15 Thread Kurt Mosiejczuk
On Wed, Oct 14, 2020 at 02:41:51PM +0100, Stuart Henderson wrote:
> Just found this in my local tree still, iirc danj liked it but there
> wasn't much other enthusiasm. Any other comments? Should I just drop
> the diff? Change 'a' to use 2^10 minimum? Change to fixed 2^10 with
> no auto measurement?

I think this makes sense. I like it.

ok kmos

--Kurt

> - Forwarded message from Stuart Henderson  -
> 
> From: Stuart Henderson 
> Date: Sat, 23 May 2020 22:08:11 +0100
> Subject: Re: Remove useless line from daemon class in login.conf
> 
> On 2020/05/22 16:04, Theo de Raadt wrote:
> > Stuart Henderson  wrote:
> > 
> > > On 2020/05/22 17:06, Daniel Jakots wrote:
> > > > Hi,
> > > > 
> > > > We used to have different numbers of blowfish rounds between the
> > > > default and daemon classes in login.conf. On Jun 26, 2016, tedu
> > > > committed "upgrade selected login.conf to use auto rounds for bcrypt"
> > > > for amd64, sparc64, i386, and maccpc [1].
> > > > 
> > > > Since the class daemon inherits from the default class, the 
> > > > :localcipher=blowfish,a:\
> > > > is a duplicate.
> > > > 
> > > > Here's a diff to remove them.
> > > 
> > > I'm OK with unifying these settings, but FWIW I never switched to auto
> > > for these, it doesn't seem all that sensible for somebody with the ability
> > > to generate enough load on the machine to be able to reduce the strength
> > > of bcrypt down to the 64 (2^6) rounds minimum.
> > 
> > Yes, that is problematic.
> > 
> > The minimum should be probably be raised, we should consider if auto
> > should even exist anymore.
> > 
> 
> As long as it doesn't allow weakening things I think auto should still
> exist so that machines can have a stronger bcrypt where it's cheap.
> 
> When this was introduced, login.conf for amd64/i386/macppc/sparc64
> changed from 8 (normal users) and 9 (daemon class i.e. root) to auto.
> Since other, mainly slower, arches stayed with hardcoded 8/9 I don't
> think the current minimum reachable in the code makes sense at all.
> 
> I've gone to a few machines and done:
> 
> - 50 runs of "encrypt -b a" to see what setting was chosen by auto
> 
> for i in `jot 50`; do echo foo | encrypt -b a; sleep .1; done | cut -d'$' -f3 
> | sort | uniq -c
> 
> - 50 runs of "encrypt -b 9" or "encrypt -b 10" and averaged, to see
> how long those two settings take
> 
> time for i in `jot 50`; do echo foo | encrypt -b 10; done
> (divided by 50)
> 
>   Chosen  -b 9-b 10
> Cortex-A53 1.4GHz (pi3)   all 8   0.220.40
> GX-412TC 1GHz (APU2)  all 8   0.160.31
> Cortex-A72 1.5GHz (pi4)   all 9   0.070.14
> L5520 2.27GHz all 9   0.080.16
> E3-1225v3 3.2GHz  12x8 3x9 35x10  0.050.10
> E3-1240v5 3.5GHz  all 10  0.040.08
> E3-1270v6 3.8GHz  all 11  0.030.05
> 
> I think bumping the minimum to 2^9 would be reasonable, there's a more
> noticeable delay on some machines but I think that's fair enough (any
> cracking is likely to be done on a fast machine, and the user can force
> it lower themselves if they want to take the risk).
> 
> With a higher minimum than that the delay starts to get very noticeable
> in some cases, so I'm not sure we're ready for that yet.
> 
> I think it also makes sense to use blowfish,a in login.conf on all
> arches, replacing the old 8/9. Actually -b a is already used in the
> installer for both root and the standard user on all archs, whatever
> they have in login.conf. Resulting in the situation that on some
> archs, the bcrypt created during install for root's password is
> weaker than it would be if reset after boot.
> 
> So maybe this or something like it?
> 
> Index: lib/libc/crypt/bcrypt.c
> ===
> RCS file: /cvs/src/lib/libc/crypt/bcrypt.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 bcrypt.c
> --- lib/libc/crypt/bcrypt.c   26 Aug 2016 08:25:02 -  1.57
> +++ lib/libc/crypt/bcrypt.c   23 May 2020 20:16:46 -
> @@ -237,14 +237,15 @@ bcrypt_checkpass(const char *pass, const
>  DEF_WEAK(bcrypt_checkpass);
>  
>  /*
> - * Measure this system's performance by measuring the time for 8 rounds.
> - * We are aiming for something that takes around 0.1s, but not too much over.
> + * Measure this system's performance by measuring the time for 2^9 rounds.
> + * We are aiming for something that takes around 0.1s, not too much over,
> + * but without allowing it to be too weak.
>   */
>  int
>  _bcrypt_autorounds(void)
>  {
>   struct timespec before, after;
> - int r = 8;
> + int r = 9;
>   char buf[_PASSWORD_LEN];
>   int duration;
>  
> @@ -257,12 +258,12 @@ _bcrypt_autorounds(void)
>   duration += (after.tv_nsec - before.tv_nsec) / 1000;
>  
>   /* too quick? slow it down. */
> - while (r < 16 && duration <= 6) {
> + while (r < 16 && duration <= 75000) {
>   r += 1;
>   

[s...@spacehopper.org: Re: Remove useless line from daemon class in login.conf]

2020-10-14 Thread Stuart Henderson
Just found this in my local tree still, iirc danj liked it but there
wasn't much other enthusiasm. Any other comments? Should I just drop
the diff? Change 'a' to use 2^10 minimum? Change to fixed 2^10 with
no auto measurement?

- Forwarded message from Stuart Henderson  -

From: Stuart Henderson 
Date: Sat, 23 May 2020 22:08:11 +0100
Subject: Re: Remove useless line from daemon class in login.conf

On 2020/05/22 16:04, Theo de Raadt wrote:
> Stuart Henderson  wrote:
> 
> > On 2020/05/22 17:06, Daniel Jakots wrote:
> > > Hi,
> > > 
> > > We used to have different numbers of blowfish rounds between the
> > > default and daemon classes in login.conf. On Jun 26, 2016, tedu
> > > committed "upgrade selected login.conf to use auto rounds for bcrypt"
> > > for amd64, sparc64, i386, and maccpc [1].
> > > 
> > > Since the class daemon inherits from the default class, the 
> > > :localcipher=blowfish,a:\
> > > is a duplicate.
> > > 
> > > Here's a diff to remove them.
> > 
> > I'm OK with unifying these settings, but FWIW I never switched to auto
> > for these, it doesn't seem all that sensible for somebody with the ability
> > to generate enough load on the machine to be able to reduce the strength
> > of bcrypt down to the 64 (2^6) rounds minimum.
> 
> Yes, that is problematic.
> 
> The minimum should be probably be raised, we should consider if auto
> should even exist anymore.
> 

As long as it doesn't allow weakening things I think auto should still
exist so that machines can have a stronger bcrypt where it's cheap.

When this was introduced, login.conf for amd64/i386/macppc/sparc64
changed from 8 (normal users) and 9 (daemon class i.e. root) to auto.
Since other, mainly slower, arches stayed with hardcoded 8/9 I don't
think the current minimum reachable in the code makes sense at all.

I've gone to a few machines and done:

- 50 runs of "encrypt -b a" to see what setting was chosen by auto

for i in `jot 50`; do echo foo | encrypt -b a; sleep .1; done | cut -d'$' -f3 | 
sort | uniq -c

- 50 runs of "encrypt -b 9" or "encrypt -b 10" and averaged, to see
how long those two settings take

time for i in `jot 50`; do echo foo | encrypt -b 10; done
(divided by 50)

Chosen  -b 9-b 10
Cortex-A53 1.4GHz (pi3) all 8   0.220.40
GX-412TC 1GHz (APU2)all 8   0.160.31
Cortex-A72 1.5GHz (pi4) all 9   0.070.14
L5520 2.27GHz   all 9   0.080.16
E3-1225v3 3.2GHz12x8 3x9 35x10  0.050.10
E3-1240v5 3.5GHzall 10  0.040.08
E3-1270v6 3.8GHzall 11  0.030.05

I think bumping the minimum to 2^9 would be reasonable, there's a more
noticeable delay on some machines but I think that's fair enough (any
cracking is likely to be done on a fast machine, and the user can force
it lower themselves if they want to take the risk).

With a higher minimum than that the delay starts to get very noticeable
in some cases, so I'm not sure we're ready for that yet.

I think it also makes sense to use blowfish,a in login.conf on all
arches, replacing the old 8/9. Actually -b a is already used in the
installer for both root and the standard user on all archs, whatever
they have in login.conf. Resulting in the situation that on some
archs, the bcrypt created during install for root's password is
weaker than it would be if reset after boot.

So maybe this or something like it?

Index: lib/libc/crypt/bcrypt.c
===
RCS file: /cvs/src/lib/libc/crypt/bcrypt.c,v
retrieving revision 1.57
diff -u -p -r1.57 bcrypt.c
--- lib/libc/crypt/bcrypt.c 26 Aug 2016 08:25:02 -  1.57
+++ lib/libc/crypt/bcrypt.c 23 May 2020 20:16:46 -
@@ -237,14 +237,15 @@ bcrypt_checkpass(const char *pass, const
 DEF_WEAK(bcrypt_checkpass);
 
 /*
- * Measure this system's performance by measuring the time for 8 rounds.
- * We are aiming for something that takes around 0.1s, but not too much over.
+ * Measure this system's performance by measuring the time for 2^9 rounds.
+ * We are aiming for something that takes around 0.1s, not too much over,
+ * but without allowing it to be too weak.
  */
 int
 _bcrypt_autorounds(void)
 {
struct timespec before, after;
-   int r = 8;
+   int r = 9;
char buf[_PASSWORD_LEN];
int duration;
 
@@ -257,12 +258,12 @@ _bcrypt_autorounds(void)
duration += (after.tv_nsec - before.tv_nsec) / 1000;
 
/* too quick? slow it down. */
-   while (r < 16 && duration <= 6) {
+   while (r < 16 && duration <= 75000) {
r += 1;
duration *= 2;
}
/* too slow? speed it up. */
-   while (r > 6 && duration > 12) {
+   while (r > 10 && duration > 12) {
r -= 1;
duration /= 2;
}
Index: etc/etc.alpha/login.conf
===
RCS file: