Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160720210647.GA24898@openwall.com>
Date: Thu, 21 Jul 2016 00:06:47 +0300
From: Solar Designer <solar@...nwall.com>
To: owl-dev@...ts.openwall.com
Subject: Re: passwdqc code quality

On Wed, Jul 20, 2016 at 06:57:43PM +0300, Solar Designer wrote:
> For now, I am going to switch to size_t only for local variables holding
> strlen() return value, so that our ABI is preserved.

Speaking of ABI, I added _passwdqc_memzero() and exported it from
libpasswdqc and made use of it in pwqcheck and pwqgen and pam_passwdqc,
all without bumping soname.  Dmitry, do you think this is acceptable?
Not bumping soname means package managers won't catch this, and if
someone upgrades only some of the subpackages (in case a distro splits
passwdqc into subpackages), but not the library, then those upgraded
subpackages will fail with an unknown symbol error.  Not great, but
hardly reason enough to bump soname, right?

> I am also going to introduce a hard-coded length limit of 100000, after
> which REASON_LONG is always returned.  Checking a password this long
> takes a second.  Checking a password 10 times longer (1000000 chars)
> takes a minute.  (Neither makes much sense.  Normally, "max" would be
> set way lower.  By default, it is 40.)
> 
> Not to break existing configs (in a minor version update like this), max
> up to INT_MAX will still be accepted, but will be capped at 100000
> internally.

I came up with a deliberately costly to process password that hits one
second at 10000, so I chose that as the cap.

The main reason for things being slow with unreasonably long passwords
is that our is_simple() only returns a Boolean, so the is_based() checks
don't know how much margin they have.  With passwords this long, it is
usually possible to fully skip the word-based check, but the current
code doesn't know.  This is something for us to optimize in a non-minor
update later.  (I am treating the current update as minor, triggered by
a bug report.)

Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.