Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Okta Bcrypt incident lessons for designing better APIs

n0rdy.foo

Post reply on HN
Print view View original

141–150 of 169 posts

Re: Okta Bcrypt incident lessons for designing better APIs

#141

Post by deepsun »

nfriedly wrote: post #139

I strongly agree with the conclusion that the libraries should reject input they can't correctly handle instead of silently truncating it. I co-maintain a rate-limiting library that had some similar rough edges, where it wouldn't always be obvious that you were doing it wrong. (For example: limiting the IP of your reverse proxy rather than the end user, or the inverse: blindly accepting any X-Forwarded-For header, in…

If the input is 71 character, all the libraries happily accept it, but an attacker needs to guess only 1 character.

Re: Okta Bcrypt incident lessons for designing better APIs

#142

Post by Tade0 »

magicalhippo wrote: post #137
Tade0 wrote: post #130

Earlier quoted context omitted.

I'm getting the feeling that there's some kind of miscommunication here. If only the password is used to generate the hash then that password, when used to match against a previously stored hash(cache key here), will also match it, thus producing the exact same vulnerability, but worse because it's enough to have the same password as someone else. Salting does not help here at all.

The whole point of salting is to avoid exactly that scenario, and, as I linked to, bcrypt requires salt. So when you read "bcrypt(password)", that just means the salt is implicit, not that it isn't salted.

The the output of `bcrypt(password)` is:

  $2a$12$R9h/cIPz0gi.URNNX3kh2OPST9/PgBkqquzi.Ss7KIUgO2t0jWMUW
  \__/\/ \____________________/\_____________________________/
  Alg Cost      Salt                        Hash
The Salt part is randomly generated. When you call `bcrypt.compare(output, password)` it uses the salt that's contained in `output`. Two calls of `bcrypt(password)` will generate different outputs(so different salts and thus different hashes), but still if you run `bcrypt.compare(output1, password)` and `bcrypt.compare(output2, password)` they will both match as long `password` was used to generate both.

In short: you can't use just the password as that's going to match a cache key that was generated by whoever typed in this exact password. The salt is only there to prevent offline attacks.

Re: Okta Bcrypt incident lessons for designing better APIs

#143

Post by arccy »

deepsun wrote: post #141
nfriedly wrote: post #139

I strongly agree with the conclusion that the libraries should reject input they can't correctly handle instead of silently truncating it. I co-maintain a rate-limiting library that had some similar rough edges, where it wouldn't always be obvious that you were doing it wrong. (For example: limiting the IP of your reverse proxy rather than the end user, or the inverse: blindly accepting any X-Forwarded-For header, in…

If the input is 71 character, all the libraries happily accept it, but an attacker needs to guess only 1 character.

have separate salt / pepper / user id args

Re: Okta Bcrypt incident lessons for designing better APIs

#144

Post by dataflow »

tptacek wrote: post #3

Bcrypt is a password hash, not a KDF, which is the way it was used in this API. It's super unclear to me why they wanted a string-based KDF here at all; does anyone have more context? I've in the past been annoying about saying I think we should just call all password hashes "KDFs", but here's a really good illustration of why I was definitely wrong about that. A KDF is a generally-useful bit of cryptography joinery;…

> Bcrypt is a password hash, not a KDF

I feel gross calling a function that just blatantly ignores part of its input a hash, much less a password hash. It's like calling a rock a fish, because they're both in water, despite the lack of swimming. In any case, a hash that ignores some of its input is certainly not a cryptographically secure hash, so why is it being used for crypto?

Re: Okta Bcrypt incident lessons for designing better APIs

#145

Post by magicalhippo »

Tade0 wrote: post #142
magicalhippo wrote: post #137

Earlier quoted context omitted.

The whole point of salting is to avoid exactly that scenario, and, as I linked to, bcrypt requires salt. So when you read "bcrypt(password)", that just means the salt is implicit, not that it isn't salted.

The the output of `bcrypt(password)` is: $2a$12$R9h/cIPz0gi.URNNX3kh2OPST9/PgBkqquzi.Ss7KIUgO2t0jWMUW \__/\/ \____________________/\_____________________________/ Alg Cost Salt Hash The Salt part is randomly generated. When you call `bcrypt.compare(output, password)` it uses the salt that's contained in `output`. Two calls of `bcrypt(password)` will generate different outputs(so different salts and thus different has…

But if you're using bcrypt to compute a dictionary lookup key, you're not going to use bcrypt.compare as it would require a linear scan and be slow as a snail.

Rather you use the bcrypt output itself as the lookup key. And if you do that then the salt will indeed do what it's designed to do.

The function you used is a convenience function which generates random salt, but you can specify your own as is done in this[1] illustration of the Okta incident.

What bcrypt.compare does is essentially to extract the salt from the provided previous output, compute new hash using that and the provided password, and check that the old and new hashes matches.

As such it's equivalent to comparing the outputs of two different "runs" where the same salt is used (modulo timing attacks).

So if you need to recompute the lookup key then you need to use the same salt value.

[1]: https://kondukto.io/blog/okta-vulnerability-bcrypt-auth

Re: Okta Bcrypt incident lessons for designing better APIs

#146

Post by rcleveng »

jhhh wrote: post #47

I can see the incident was a jumping off point to talk about bad APIs (bcrypt probably should error >72) but it sounds like the actual bug was they weren't checking the value in the cache matched the data they used in the hash for the key. The authentication cache check should survive any arbitrarily bad hashing algorithm because all of them are going to have collisions (pigeonhole principal). Even an arbitrarily 'st…

I've experienced that Most incidents, whether it's security, performance, or availability, etc. rarely have one single thing that goes wrong. Theres's a chain of events that happen.

Poor API design can make it easier for other contributing factors (checking cache here, but could also be not running load tests, not fuzzing, human error, etc.) to cause incidents.

I'm glad to see this come out, plus which libraries handle out of bounds conditions with errors vs. fix-up the input to cause silent failures.

Re: Okta Bcrypt incident lessons for designing better APIs

#147

Post by GoblinSlayer »

GoblinSlayer wrote: post #97
tptacek wrote: post #35

Earlier quoted context omitted.

I don't think it's a good idea for people to adopt new bcrypt constructions so that they can use it to generate cache keys (or, worse, other keys). (I need that "man standing up in the town hall meeting" meme for this.) Just use a real KDF, if that's really what you want. I'm still confused what password-derived material is doing in a Redis key.

By cache they mean cached credentials. >The user previously authenticated creating a cache of the authentication Maybe, it's a password encrypted secret token.

On the other hand Redis on the server side makes more sense as an LDAP backup, but then mapping should be user id -> bcrypt hash.

Re: Okta Bcrypt incident lessons for designing better APIs

#148

Post by jmuguy »

I was curious how bcrypt-ruby would handle this. It does not throw an error for input length larger than 72. However the actual API for the gem makes it pretty clear its for hashing a password, and not just hashing in general - as you can see from the code.

https://gist.github.com/neontuna/dffd0452d09a0861106c0a46669...

Re: Okta Bcrypt incident lessons for designing better APIs

#149

Post by tedunangst »

arccy wrote: post #143
deepsun wrote: post #141

Earlier quoted context omitted.

If the input is 71 character, all the libraries happily accept it, but an attacker needs to guess only 1 character.

have separate salt / pepper / user id args

How is the library supposed to know you're doing that wrong?

Re: Okta Bcrypt incident lessons for designing better APIs

#150

Post by pornel »

purist33 wrote: post #67
jhhh wrote: post #47

I can see the incident was a jumping off point to talk about bad APIs (bcrypt probably should error >72) but it sounds like the actual bug was they weren't checking the value in the cache matched the data they used in the hash for the key. The authentication cache check should survive any arbitrarily bad hashing algorithm because all of them are going to have collisions (pigeonhole principal). Even an arbitrarily 'st…

Something similar happens in my company too. In a particular place, we use the hash of a string as the key in a hashmap instead of the string itself, because the hash is smaller and is easier to compare after the initial map has been made. It is a 64bit hash too. I have been crying about this everytime it comes up, and the response is, it will never happen. My problem is that we will never know if it ever happens too…

Yikes.

It's valid to assume "it will never happen" for 128 bits or more (if the hash function isn't broken) since chance of a random collision is astronomically small, but a collision in 64 bits is within realm of possibility (50% chance of hitting a dupe among 2^32 items).

Post reply on HN