Optionally change the cost of ActiveModel::SecurePassword encryption

[bcardarella]

Currently ActiveModel::SecurePassword defaults to BCrypt::Engine::DEFAULT_COST which is 10. It would be nice if we could override this. By overriding the instance method #password_digest_cost this can be done

[exviva]

How about making this an option on has_secure_password, i.e. has_secure_password digest_cost: 11

[bcardarella]

@exviva yeah, I like that much better. I'll update the pull request

[dhh]

What's the scenario where you actually need to raise the cost? What have you raised the cost to in the past? And what was the reason then?

[bcardarella]

@dhh it goes both ways, there are cases for increasing and decreasing the default cost. See the 'Cost' section here: https://github.com/codahale/bcrypt-ruby

I have not personally needed to increase the cost, but in a few years who knows?

[dhh]

Do you have some concrete examples of where you have increased or decreased the cost and the reasoning available for why?

[bcardarella]

Just forced a push that moves the cost as an option for has_secure_password

[bcardarella]

@dhh I have not had a need to increase the cost of the encryption. Then again, I've only used Bcrypt on one project. I'm sure there are plenty of cases where more security is necessary, maybe someone that has worked on a HIPAA app or something similar has had a need. But I cannot speak to that.

[cpjolicoeur]

Why the need to even give examples of needing go increase/decrease the cost? What is the downside to giving the user the option should the need arise in their use case? I see no downside to providing the option, only an upside

[bai]

@cpjolicoeur Convention over configuration? 10 is a quite sane default.

[bcardarella]

@bai convention over configuration doesn't apply in this case. Setting the cost is not required. But if you do need to change it the option exists. Otherwise one would need to overwrite the authenticate instance method to set a custom cost.

[dhh]

We don't pre-emptively add options to code until we have a good reason to and a strong use case. More code is more code. Also, this whole module is tiny. You could easily overwrite the 4-line #password= if you truly need to change something. So I'm closing this for now. Feel free to reopen if you come up with an incredibly compelling case that's backed by data showing how a different setting is necessary.

[cpjolicoeur]

I guess I still don't see how that argument makes sense.

The bcrypt-ruby library that is being used ITSELF provides the option for setting the cost, so why not pass that along to the user?

Its not like the rails codebase is adding code just for the sake of adding code, or even to extend functionality of an existing library. This patch is simply exposing a setting option to the user that already exists in the bcrypt-ruby implementation.

While I can't think of compelling reasons as the moment for increasing the cost, there are current reasons for decreasing the cost. Speed and response time being the biggest, especially for stateless systems doing authentication. I've lowered the cost factor on two projects doing auth for API usage with mobile devices where speed was a huge consideration.

In the future, as hardware capabilities increase, there will come a need to increase the cost/work factor as well.

• Craig

On Apr 17, 2011, at 4:27 AM, dhhreply@reply.github.com wrote:

We don't pre-emptively add options to code until we have a good reason to and a strong use case. More code is more code. Also, this whole module is tiny. You could easily overwrite the 4-line #password= if you truly need to change something. So I'm closing this for now. Feel free to reopen if you come up with an incredibly compelling case that's backed by data showing how a different setting is necessary.

Reply to this email directly or view it on GitHub:
#285 (comment)

[dhh]

cpjollcoeur, now you're introducing real arguments. Can you share the specifics of your mobile API usage that required lowering the cost? What cost did you arrive at? What where the before/after on time spent?

Every option we add increases the mental overhead of the system. We should battle every option until there's a strong, compelling reason to add it. And even then, we should consider if we can abstract the underlying tech such that everyone doesn't have to do their own math on which setting is good for what. Especially on something that's as simple as this.

[yfeldblum]

@dhh

The purpose of making the password hash algorithm slow is to make it extremely difficult for an attacker to perform an offline dictionary attack against one stolen password hash, let alone against a whole database of them. As the attackers' computers become faster or as the attacker is able to gain access to more of them, retaining the same level of protection requires that we raise the cost of the password hash algorithm. If the attacker's capacity to perform an offline dictionary attack doubles in two years, then websites should double the cost of the password hash algorithm every two years.

See the very existence of PBKDF2 (http://en.wikipedia.org/wiki/PBKDF2): "The added computational work makes password cracking much more difficult, and is known as key stretching. When the standard was written in 2000, the recommended minimum number of iterations was 1000, but the parameter is intended to be increased over time as CPU speeds increase." The same principle applies whether one uses bcrypt, scrypt, pbkdf2, or iterated hmac.

[bcardarella]

@dhh I'll try to gather some hard data, but @yfeldblum is pretty on the money. The current cost of '10' is adequate for today. But future-proofing the security is important too.

You can check out the Authlogic post on Bcrypt http://www.binarylogic.com/2008/11/22/storing-nuclear-launch-codes-in-your-app-enter-bcrypt-for-authlogic @dkubb advocates optional cost increase and says he rolls this into his own apps. I'll try pinging him to see if he can weigh in on this pull

[bcardarella]

@dhh my patch sets the default not to a hard 10 but to BCrypt::Engine::DEFAULT_COST. This value could change over time per the opinion of the maintainers of bcrypt-ruby

[dhh]

Those arguments are assuming that we wouldn't just choose a good default to stay current with whatever changes might happen over time. We're already using BCrypt::Engine::DEFAULT_COST, so if this value changes over time, we're automatically good. In addition, you can always redefine that constant if you must and are not willing to overwrite the 4-line #password= method.

In fact, if you're so well-versed in these matters that you do your own cost value analysis, stay up to date on it over time, and tweak this on a regular basis, you're not the target audience for this feature. This feature serves two purposes: 1) Tell Rails developers that BCrypt is a best practice, 2) Give them a little bit of sugar to persuade them to use it right now and they'll have AWESOME password security compared to the industry standard.

That Authlogic post only seems to reinforce this view of the world.

Thus, the only argument I've heard so far that's persuasive is that a default cost of 10 would not be suitable for a per-request authentication scheme, like an API or http auth. I'll await the presentation of data on that point, but unless that's very compelling, I've not heard other arguments that would change my mind on no setting here.

(Regardless, I do appreciate the debate. It's healthy. Especially on security matters.)

[dhh]

In my quick test on my laptop, BCrypt took 7ms on a single pass. Now that's great because it's slow compared to everything else, but I'd be very curious to see where 7ms proves to be a problem/bottleneck in a real Rails app. Even with per-request authentication. But I'll await your findings.

[bcardarella]

@dhh I just ran some benchmarks on a Slicehost VPS. Ruby 1.9.2:

irb(main):017:0> Benchmark.measure do
irb(main):018:1* 100.times { BCrypt::Password.create('somesecurepassword', :cost => 10) }
irb(main):019:1> end
=>   9.180000   0.000000   9.180000 (  9.081898)

With the default cost this means each request will take an additional 90ms. IMO pretty significant for APIs.

Because the cost is just a multiplier we can step it down to 9 and expect a 50% performance increase:

irb(main):023:0> Benchmark.measure do
irb(main):024:1* 100.times { BCrypt::Password.create('somesecurepassword', :cost => 9) }
irb(main):025:1> end
=>   4.560000   0.000000   4.560000 (  4.660387)

And that is just what we see here. Compared to SHA512:

irb(main):026:0> Benchmark.measure do
irb(main):027:1* 100.times { Digest::SHA512.hexdigest('somesecurepassword') }
irb(main):028:1> end
=>   0.000000   0.000000   0.000000 (  0.027072)

means we can step the BCrypt cost down some more for performance and still get better security than SHA512:

irb(main):029:0> Benchmark.measure do
irb(main):030:1* 100.times { BCrypt::Password.create('somesecurepassword', :cost => 2) }
irb(main):031:1> end
=>   0.150000   0.000000   0.150000 (  0.172343)

Each developer could choose the balance of speed/security. If they have some baddass production servers they might want to step it up or even keep the default. But not everybody has access to machines like this in production. Many people are deploying to VPS or EC2. (EC2 might be worse, you never really know what hardware you're on)

[dhh]

Yes, 90ms per request for API traffic that authenticates per-request is not going to fly. Was the app you were working on using both API and regular web access? Because this setting is per-model, not per-user method. So you simply accepted a lower cost, and thus security, across the entire app?

[bcardarella]

@dhh the highest traffic API I built was while I was at the DNC. We were not using BCrypt at the time (if I were to build it today I would use it) but performance was very important. An additional 90ms would have been a deal breaker. The app had both API and web authentication but we were consuming our own API so technically it was always pre-request authentication.

[bcardarella]

Well, now that I think about it that statement wasn't true. It was a year ago, fuzzy memory. :p We were consuming our own API but not for authentication.

If I recall, we had both API authentication and web authentication. The API authentication worked over HTTPS so that tacks on even more overhead to each request. I think we would have been fine with a reduced cost for the sake of keeping the same model for both authentication schemes. As I pointed out earlier, even a cost of 2 for BCrypt is still stronger than SHA512.

[yfeldblum]

@bcardarella Use token-based authentication for API clients. Token-based authentication does not rely on cryptography, let alone intentionally slow cryptography, when checking authentication (except to use the secure_compare method). The initial request to obtain a token could then take an additional 90ms to check the password; subsequent requests could take 1us to check the token.

[bcardarella]

@yfeldblum I believe we did that, I don't have access to the code any more as I'm not at the DNC so I cannot say for certain.

[dhh]

@yfeldblum, that's a good point. I don't think the right answer to 90ms is too slow to do per-request auth is to make less secure passwords. But rather to use a token based approach, ala OAuth as well.

So that shoots down the only reasonable argument I've been seeing for this option. Thus, we're back to no :)

[bcardarella]

Oh, looks like the DNC open sourced the API stuff. Dear god, we were using SHA1. https://github.com/dnclabs/lockbox/blob/master/app/models/partner.rb#L158-160

[cpjolicoeur]

@dhh, Our mobile apps were using authentication (not necessarily client login) to authorize requests against a webservice API. I dont still have our original test results lying around to give you, but each time you lower the work factor for BCrypt, the speed increases substantially.

We settled on a work factor of 8. Provides strong enough encryption and a long enough brute-force cracking time that we were comfortable for our use case, but again, that will vary per project and user taste.

As I said, I dont have my original benchmarks, but my results follow what brian posted earlier.

Also, regarding just using token based auth for API's, that is all fine and good but doesnt always fly. We, in fact, were using token auth, but due to the program specs, tokens expired after 24 hours which required re-auth to generate a new token. This is actually quite common, to have token expiry. (Think government contract apps with arcane, and sometimes pointless specs, that still need to be followed).

[yfeldblum]

@dhh The primary reason bcrypt is good is because it has an adjustable cost factor making it future-proof. It's a good idea to expose the very thing that makes bcrypt good and tell people that this is what they should be doing and this is why they should be doing it. Even if it makes the API slightly more complex because there's an extra optional parameter, it's worth it for developers who deal with persisting user passwords to know why bcrypt is a good choice. Having a :cost option will encourage developers to go out and learn about the offline dictionary attack and will therefore foster broader understanding about how the :cost in bcrypt guards against it. Leaving the :cost out of the ActiveModel API hides this important clue, discouraging people from learning about why the :cost is important. We should be educating developers about and pushing them to use MVC, REST, etc., - and security - by putting in sensible defaults that are easy to use but also putting in documented options to override the default when needed.

@bcardarella Putting the userid into the cookie-stored session (in the signed cookie jar, of course) is token-based auth. Everyone's already using it for Web sessions.

[dkubb]

I'm a little late to the discussion, but @bcardarella asked me on twitter to stop by so I figured I'd try to add something.

I agree with @dhh that we should hold off adding a :cost option until it's needed. That may seem like this conflicts with what I said in the Authlogic bcrypt comment, but I don't think it does.

For the common case people are better off using the default cost, assuming it's a good default, which I believe 10 is (for now).

For the person who wants control and knows enough about how bcrypt works, there is still the BCrypt::Engine::DEFAULT_COST constant. I have adjusted it in the past, but tbh I'm probably more likely to decrease it so the specs run fast. I would probably only increment the cost if the default in bcrypt-ruby doesn't keep pace. The work to hash the password doubles for every increment, so it's fine as long as the default cost increments every 2 years.

From an API design pov, I like the idea of putting a tiny barrier in place to setting the cost. It makes it more like that if someone does change the default they'll be doing it out of necessity and because they understand the trade-offs. Making it a bit "ugly" compared to other parts of the API underscores that it's something you should only set if you know what you're doing.

Since there is still discussion on this I think we're better off leaving the option out and waiting for some real-world cases to arise that forces the API to change. It's really easy to add options to an API, but really hard to remove them later.

[parndt]

We should just set the default cost to Time.now.strftime('%Y')[1..-1].to_i then it will scale as the years go by (starting with "011" [or, 11] this year) hitting the limit at the year 3000 when passwords will become immediately crackable!

;-) ;-)

[semaperepelitsa]

Lowering BCrypt cost might be useful in test environment where you don't care about password security but do care about speed. I already do this in test_helper:

require "bcrypt"
silence_warnings do
  BCrypt::Engine::DEFAULT_COST = BCrypt::Engine::MIN_COST
end

(Gives a free 4 second boost in my case.) But I think this could be done by default.

[henrik]

Lowering the cost for tests seems like a worthwhile use case. Trying that and also seeing speedups.

I agree with @dhh that low-level settings should only be exposed if necessary. In this case, I think a commented-out

config.active_model.secure_password.cost = :min_cost

in config/environments/test.rb might be a good idea. Or something like it. Maybe

config.active_model.secure_password.fast_instead_of_secure = true

to be very explicit.

Possibly it should just be always-on in tests and not configurable. Seems like something that could bite you if it was always on, though I can't think of a specific such case.

[lazyatom]

For what it's worth, decreasing the cost to 1 in the test environment reduced our test times by around 35% (from 0m35.390s to 0m22.965s). Regardless of whether or not the cost is completely configurable, I think it would be worth dropping the cost when Rails.env == 'test'.

[jeremy]

+1 to decreasing cost in test env. That can be a builtin Rails initializer rather than a config option. Anyone up for cutting a new pull request?

[steveklabnik]

Initializers don't tend to use the environment, though... wouldn't it be better to put them in the env file?

[trevorturk]

I worked with @jeremy on this pull request after reviewing the comments here: #8216

I'd 💝 some 👀 on it!