Re: crypt* files in crypt directory

[Rich Felker <dalias@aerifal.cx>]

On Wed, Aug 08, 2012 at 09:06:23AM -0400, Rich Felker wrote:
> Actually this brings up a HUGE DoS vuln in blowfish crypt: with tcb
> passwords, a malicious user can put a password with count=31 (it's
> logarithmic, so this means 2^31) in their tcb shadow file. This will
> cause a root-owned process to eat 100% cpu for hours if not days.
> Perform a few simultaneous login attempts and the whole server becomes
> unusable.
> 
> I don't know how to solve it, but in musl I think we'll have to put a
> low limit on count if we're going to support blowfish. Unfortunately I
> don't see a good way to make it runtime configurable without
> hard-coding additional non-standard config paths, but letting the DoS
> bug slip in is not acceptable.

OK, here's my proposed direction for a fix. I definitely don't want to
be _unconditionally_ reading a config file for every crypt() call,
since that would adversely affect even calls with sane iteration
counts and perhaps dominate the run time or at least the cache
thrashing. Instead, I propose we come up with a range of iteration
counts that are "remotely sane" in the sense of taking (for example)
less than 250ms on a very low-end system, and always allowing any
count in this range. Initially we can just forbid higher counts, and
later we can extend the code to probe some sort of configuration when
the default limit is exceeded to see if there's a configuration
extending the limit.

On the other hand, for self-contained static binaries, it might be
desirable the have the limit be configured into the binary. This could
be achieved by making a weak symbol whose address is used as the
limit, and then -Wl,--defsym=__crypt_iter_max=20 or similar could be
used to configure it at link time.

In any case, if the default limit is sufficiently large, I think we
can get away without taking any action on the configurability right
away..

Rich