libxcrypt icon indicating copy to clipboard operation
libxcrypt copied to clipboard

Consider introducing limits on resource usage by maybe-rogue hash encodings

Open solardiz opened this issue 6 years ago • 6 comments

Historically, hashes were fixed-cost and thus OK for semi-trusted users to be able to specify directly e.g. in Apache httpd .htpasswd files. With tunable-cost hashes, this changes - a semi-trusted user could DoS the service for its other users, and if no adequate resource limits are configured on the service then also DoS the system. musl chose to impose some hard-coded sanity limits on those hashes. With libxcrypt's support for hashes tunable not only for time, but also for memory (scrypt, yescrypt), this may be even more of an issue.

I am undecided on whether we should merely document the issue or also impose limits, and if so whether they should be hard-coded or configurable, and if so compile- or run-time configurable. To avoid duplicate parsing, some logic might need to be added to upstream yescrypt tree and made use of by libxcrypt.

At least we should make an informed decision. Hence opening this issue.

solardiz avatar Nov 01 '18 17:11 solardiz

The crypt.conf format I proposed in #26 could easily be extended to support run-time configurable upper limits.

I think we should certainly document the issue, and I think there's a strong case for doing more than that, but I am not sure I know what that is. Do you have links to the discussion that happened when musl added limits?

zackw avatar Nov 01 '18 18:11 zackw

Here are two relevant messages/threads:

https://www.openwall.com/lists/musl/2012/08/08/20 https://www.openwall.com/lists/oss-security/2013/06/12/1

solardiz avatar Nov 01 '18 19:11 solardiz

If there is a consensus that the problem is worth solving then it must be solved in libxcrypt: since there is no standard scheme to specify the hash parameters then every interested program would have to implement parsing every hash format, thus negating the generality of libxcrypt.

rfc1036 avatar Nov 02 '18 00:11 rfc1036

I just revised the crypt.conf spec so that there are now defcost= and maxcost= keyval arguments for each hashing method. We'd need to add more knobs if we ever allow tuning memory separately from time parameters, but I think this will do for the time being.

Unresolved questions:

  • Does the documentation I wrote explain the issue clearly enough?
  • What should the default upper time limit used by crypt-tune-costs be? One second? Ten?
  • Should there be a mincost= separate from defcost? Right now, the documented behavior is that you can specify costs lower than defcost when calling crypt_gensalt but the result will be flagged as "too cheap" by crypt_checksalt. It might make sense to allow these to be controlled separately; it might also make sense to disallow costs lower than defcost in crypt_gensalt, I am undecided.

zackw avatar Nov 08 '18 17:11 zackw

I haven't reviewed this carefully yet, but conceptually I think a single numeric maxcost= can't fully/directly address the issue of malicious hash encodings manually put into .htpasswd files and the like for hash types that have more than one parameter specified via the setting strings (in current libxcrypt that's scrypt and yescrypt). So perhaps we need something else, or we need some reverse translation layer (a hack) that would convert arbitrary setting strings into single numeric cost values that are at least as resource-demanding as those setting strings would be in terms of max(time, memory) consumption. Besides being a hack, this notion might be difficult for us to explain and for users to grasp.

solardiz avatar Nov 08 '18 18:11 solardiz

conceptually I think a single numeric maxcost= can't fully/directly address the issue of malicious hash encodings manually put into .htpasswd files and the like for hash types that have more than one parameter specified via the setting strings (in current libxcrypt that's scrypt and yescrypt).

You're right. I was thinking only in terms of what can be expressed via the crypt_gensalt interface, not what can be expressed by manual construction of setting strings.

Abstractly what we want is maxcpu= and maxtime= knobs that apply across the board, but I don't see a good way to implement that short of having libxcrypt fork off a child process on every call to crypt so it can apply OS-level resource limits. Maybe that's not a totally crazy idea, tbh, but we'd have to worry about all the hair that comes with calling fork in a library. It feels more of an authentication daemon feature.

zackw avatar Nov 11 '18 15:11 zackw