Research increasing pbkdf2 iterations

[mrjones-plip]

Describe the issue Best practices for hashing passwords is to use the highest number of iterations possible, with out impacting server performance (too much ; ). Currently our iterations is set to 10. This is considered very low, it looks like CouchDB default is 10k and minimum is 100.

Describe the improvement you'd like Increase the iterations to the highest number reasonable for the server specs we set.

Describe alternatives you've considered NA

[fardarter]

Would bcrypt not be a better option in general?

[garethbowen]

@fardarter Somewhat, yes. pbkdf2 with high iterations should be good enough though.

We hand authentication off to couchdb rather than implementing it ourselves and the configuration option to change hash algorithms was only added in v3.3.0. So it's not possible currently. We should evaluate this after #6289 lands.

[garethbowen]

Bumping to 4.4.0 as we're almost ready to release 4.3.0 and this has more work needed yet.

[fardarter]

See: Public Comments on SP 800-132, Recommendation for Password-Based Key Derivation: Part 1: Storage Applications via: https://docs.python.org/3/library/hashlib.html#key-derivation

Looks like recommended min for sha1 is 1300000 iterations.

[garethbowen]

One significant downside to increasing this number is it will seriously impact anywhere that uses a lot of basic auth, which at the moment is api, sentinel, and cht-conf in particular. What this means is all of these will need to be changed to use session based auth which is not a trivial amount of work.

[fardarter]

@garethbowen This seems like a fairly critical thing to address, though?

I would take the NIST recommendation seriously.

[garethbowen]

This seems like a fairly critical thing to address, though?

Definitely.

[mrjones-plip]

@garethbowen

these will need to be changed to use session based auth

Can you explain the need for this change? I would have thought that the only change would be increased load on the server:

"A higher number [of hash iterations] provides better hash durability, but comes at a cost in performance for each request that requires authentication. When using hundreds of thousands of iterations, use session cookies, or the performance hit will be huge." - couchd docs

[garethbowen]

Yes the load will be on the server, and the iterations need to be increased until it's a noticeable load. Taking cht-conf as an example, we send hundreds of basic auth requests to update the config. Each of these would incur significantly higher load on the server which would make cht-conf much slower if done serially, and risks DOSing the server if done in parallel.

It's the same with the sentinel backlog where we make a range of requests for every doc change using basic auth.

So the load is on the couchdb server. To avoid this the client needs to be updated to log in once and use the session cookie for future requests. Each change is not particularly hard but it will be an issue in lots of places.

[fardarter]

@mrjones-plip The purpose of the higher iteration is to make it take longer to check the password value. It's so that it's expensive and time consuming to crack the passwords in the DB if it is leaked.

I didn't realise that session authentication wasn't being used for the services, but the idea there is that once you acquire the token you are no longer having to run the expensive check.

At the NIST recommended level with basic rather than session auth you'd be placing about a second's worth of delay on every request even if the server stayed up.

Moving to session auth would be the only viable way of using a high iteration value.

[mrjones-plip]

Ah ha! Yes, that makes a lot of sense. Thanks you two for taking a sec to walk me through it.

[fardarter]

Ah ha! Yes, that makes a lot of sense. Thanks you two for taking a sec to walk me through it.

Nice resource: https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#work-factors

[fardarter]

@garethbowen Am I correct in saying at present only the admin password needs to be low iterations for perf? Users are all using cookie auth?

For example, would this set the user iterations to a higher value while allowing a performant basic auth for the services (assuming I'm pre-computing the admin string)?

[couch_httpd_auth]
iterations = 1300000
min_iterations = 100
max_iterations = 5000000

[garethbowen]

Am I correct in saying at present only the admin password needs to be low iterations for perf?

Usually that's the case. You could create different users with permissions and use those in cht-conf for example, and this would be best practice instead of everyone sharing a single admin user, but out of the box there's a single admin that's used in api, sentinel, and cht-conf.

assuming I'm pre-computing the admin string

Do you mean, once the admin has been created? Yes that would be a good workaround - create the admin with low iterations, then increase the iterations in couch and create all the other users. The downside is the most important account is the least secure, but at least the bulk of users have strong hashing.

[fardarter]

@garethbowen

Usually that's the case. You could create different users with permissions and use those in cht-conf for example, and this would be best practice instead of everyone sharing a single admin user, but out of the box there's a single admin that's used in api, sentinel, and cht-conf.

We are doing that, but I imagine that doesn't help with the basic auth issue? Could you elaborate on the best practice a little?

Do you mean, once the admin has been created?

I'm precomputing the hash.

Yes that would be a good workaround - create the admin with low iterations, then increase the iterations in couch and create all the other users. The downside is the most important account is the least secure, but at least the bulk of users have strong hashing.

I don't seem to have an option on the core account. At least there I can guarantee a strong passphrase.

[garethbowen]

I imagine that doesn't help with the basic auth issue?

You're right that it's not a workaround for the basic auth issue. If you had admin users that only accessed couchdb through webapps (CHT, app management, or fauxton) then you could increase their iterations because they're already using sessions once they're logged in. But most of the admin use cases are probably automation (cht-conf, api, sentinel) so this doesn't apply.

Could you elaborate on the best practice a little?

The best practice would be for every human or script to use a different admin login. In practice this may be difficult or impossible in some cases but should be done everywhere you can. The advantage is being able to revoke credentials which may have leaked without impacting other services, and also for better logging so you can see which script is doing what.

[fardarter]

The best practice would be for every human or script to use a different admin login.

This is what we're doing with humans and also some of our connecting services. Every script is a bit far for us rn.

[dianabarsan]

Discussing with @garethbowen , before increasing default iterations, we need to update authentication in some auxiliary services. Bumping this from the 4.5.0 until we've resolved clients using cookies instead of basic auth.

[garethbowen]

We're still working on rolling out cookie auth everywhere, so moving this to the next release.

[dianabarsan]

Two commits were pushed that added a new PouchDb plugin that changes the behavior of the PouchDb http adapter, making it use session authentication instead of basic authentication, and one that makes e2e test requests to use session authentication as well.

We still use request-promise-native to interact with CouchDb in several places in API and Sentinel, due to limitations of PouchDb APIs. This also needs updating to favor session authentication instead of basic auth.

I'm proposing to make a shared library that wraps request-promise-native to load requests with session cookies, similar to how we handle this in e2e tests. This potentially opens up the possibility of swapping request-promise-native to node-fetch later on.

[fardarter]

Hi @dianabarsan, I have a PR consolidating request-promise-native coming close to being merged (@garethbowen to confirm). That might suit your purposes?

[garethbowen]

Moved to 4.8.0 so as not to hold up the release.

[mrjones-plip]

Moved to 4.9.0 so as not to hold up the release.