Okta Bcrypt incident lessons for designing better APIs | Hacker News Reader

Okta Bcrypt incident lessons for designing better APIs

n0rdy.foo

377 pointsn0rdy166 comments

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; a password hash has exactly one job.

The value is the combination of userid, username, and password, so in threads on other platforms people have hypothesised that the developer tried to play it safe and use a password hash because of the password's presence.

Also I'm not sure the average developer understands the distinction.

  > Also I'm not sure the average developer understands the distinction.

I'm an average developer. I'm not sure that I understand exactly. What should I be reading, or what can you tell me?

Thank you!

They didn't want a KDF, as far as I know, but they wanted a hash function with unlimited input size.

Including the username in the hash input gives you guaranteed domain separation between users that you don't get from salts/nonces. Its a generally good idea if you have a hash function with unlimited input size (all modern cryptographic hash functions except bcrypt have unlimited input size).

They clearly wanted something stronger than "a hash function" or they'd have reached for weaker cryptographic hashes.

They wanted a hard-to-compute cryptographic hash function. Today, that means bcrypt or something with a KDF construction. However, they needed one with unlimited input size, which rules out bcrypt.

Or just a hash of the bcrypt hash, for the password!

I don't like using thought-stopping cliches any more than anybody else does, but this design feels a little cargo-culted. All this stuff follows the more fundamental question of "why is the password mixed into a cache key"?

Yeah, I think both of the following would have worked if they wanted the password involved in a cache key and they wanted bcrypt to be used:

* bcrypt(SHA-512(PW || stuff))

* SHA(stuff || bcrypt(PW))

Disclaimer: Not cryptography advice.

It's still unclear to me why the password is in there.

For 'unlimited' input size it should be SHA-3-512. Maybe too slow, but Bcrypt is slower, right? Less things to go wrong too.

All of the SHA functions allow unlimited input size. And yes, bcrypt computation time dwarfs that of SHA-3.

The SHA-3 family has "extendable-output functions," which can ostensibly be used to generate unlimited numbers of bits (albeit with only a given security level). These are new to SHA-3.

SHA-3 has more internal state, it really is plausibly better at handling very large data. If 'unlimited' is really less than a gigabyte, there's no problem. It's mostly the preimage series of attacks and length extension at that point. SHA-3 is better on those. SHA-512 has zero length extension attack resistance.

Internal state length may be a bit of a red herring (note that SHA-3 makes up for that longer internal state by ingesting more data per round), but SHA-3 probably has a higher security margin than the SHA-2 construction mostly because we have had sponge constructions for less time than we have had Merkle-Damgard constructions. NIST basically forced a higher security margin on SHA-3. You are correct about the length extension attacks (although these are mitigated by using SHA-2-512/256 for example), but I don't think that matters here.

Ew. Just HMAC. Don't use truncated SHA2.

Why?

hmac-bcrypt solves that problem very well, and should replace plain bcrypt: https://github.com/epixoip/hmac-bcrypt

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.

Maybe they wanted some cached data to get invalidated if users change their passwords?

Then use some other data which can act as a proxy for that, like the date of the last credential change. Using the password itself is a terrible security smell.

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.

Could you give an example of real KDF?

I'm not the person you're replying to, but HKDF and PBKDF2

HMAC-bcrypt is a more complicated version of the first construction I proposed, and it would need a rigorous cryptanalysis if someone wanted to actually use it in production. It sounds like Okta actually wanted PBKDF2(stuff) here.

An authentication company should have known this...

I feel like the "authentication company should have known" thing is unuseful; most developers at "security" companies are just ordinary generalist developers. Ironically, I think they boned themselves by trying to be too clever here, not too casual.

You don't think a company whose entire reason for being is providing security services for other companies should have designs related to authentication reviewed by security experts?

I think it's good to want things.

I was an employee of a company providing security services. I can attest to that company being filled with generalist programmers.

None were trained in security principles. None were security experts. And there was no security review.

> You don't think a company whose entire reason for being...

That's the assumption everyone makes and it's dangerous.

The fact that someone or some entity does something and only special doesn't make them the best at it (or even close). It's just what they do to survive (and earn).

Ship early, ship often, we can add security later.

Cybersecurity != security in the technical/mathematical sense. It's related, but not the same.

For businesses, cybersecurity is (like everything else, ultimately) about minimizing costs related to digital threats. That is - any threat scenario can be modelled as T*D, where T is "how likely it's going to happen (per year)", and D is "how much it'll cost us when it does (per incident)"; the result is the expected yearly loss, denominated in dollars. The less of it you have (integrated over all scenarios you can think of), the better, but prevention and mitigation also cost money, so what you're actually minimizing is the (expected loss + mitigation costs); i.e. makes no sense to spend more on improving something than it'll save you.

The reason for this exposition dump is: actual security at the technical level is one of many ways of improving TxD, and usually is neither cheap nor the most interesting one. It's also mostly focused on the "T side" (minimizing risk of an incident), which is harder to move than the "D side" - reducing impact.

The service an authentication company is selling is not "cryptographically unbreakable authentication". What they're selling is roughly: "low-T auth sytem cheaper than you could build&operate yourself + if it breaks it's our fault". That is, more than lowering "T side", they're offering to let you shift part of the liability to them, which significantly lowers "D side".

Internally, how they do it is up to them. But there's only so much need for technical security experts - you obviously can't sell a broken system (everyone has to at least pay a lip service to real security, otherwise people get angry, politicians get interested, and costs start to multiply rapidly), but eventually, it's cheaper to focus on your ability to take on liability from your customers and discharge it somewhere else, which involves improving operations, customer service, etc. - all the stuff you need regular, non-security-expert programmers for.

Note the bit about discharging liability. After working in cybersec and GRC for a bit, I realized security is best understood in terms of managing liability (which corresponds to minimizing the D part of TxD from earlier). That's the primary product of most security service companies, as well as security frameworks and associated compliance audits. They do improve the technical side somewhat too, but that's not why those things are bought. They're bought so, when something happens (something eventually always happen), you could point at the SOC.2 audit results and a string of security contracts and say, "we've followed all the best practices, there was nothing more we could do; crime happens, not our fault" - and have the liability flow down the contractual agreements to other companies, which do the same, until, like spring rain flowing down the mountains, into rivers, into sea, it all gets turned into insurance payouts in the end :).

Might sound cynical, but it's probably the right and reasonable thing to be happening 90% of the time. Shit happens, criminals be criminals, opportunity costs are real, etc.

Being too clever and being too casual are the same thing when it comes to matters of math.

Perhaps the password is used as part of the cache Key so that a password update implicitly invalidates the cache?

> It's still unclear to me why the password is in there.

Perhaps they did not want to apply cache invalidation purely by the passage of time, or want that passage of time to be long, but wanted to treat a credentials update as a cache invalidating event. A safer way to implement that would perhaps be to have a concept of a version of an account, incremented when authentication options or other significant properties change, and including that in the cache key.

I'm not sure why it would matter though: even if a credentials change does invalidate the cache from the PoV of the user looking up information, the information is potentially still in the cache so could still be referred to by someone else who has gained knowledge of the old credentials.

bcrypt-pbkdf (used in OpenSSH) exists for that purpose.

Would scrypt solve this problem? Or is it same as bcrypt? Or is scrypt depending on the hardware?

> but they wanted a hash function with unlimited input size

I'm kind of baffled how they came to use bcrypt for this. Bcrypt is not exactly subtle about only supporting 72 bytes of input. And this is at a company who provides auth as a service; I've got to imagine they had multiple engineers who knew this (I guess not working on that code). Hell, I know this and I've only used bcrypt twice and I'm nowhere near a security/crypto guy.

BCrypt should loudly fail if more than 72 bytes are sent to its input.

Maybe it should. Discarding the rest of the bytes works fine for passwords, though. I guess that's just not sufficient.

In my book, discarding entropy is a generally dumb thing to do. Passwords are usually under 72 chars, but a lot of people use concatenations of usernames and passwords in their hash to get guaranteed domain separation between users.

I believe there have been earlier protocols where the user’s secrets were used as a KDF to generate credentials in such a way that the server never sees the user’s password.

I’m wondering if okta was inspired by those.

So let me take on the burden of stupid here: how are a password hash and a string-based KDF different? (I mean, the oldest well-known example of the former literally calls itself a PBKDF.) I understand this particular function from strings to large fixed numbers was limited in the length of the string it would accept, and I agree that’s a problem, but it feels like a problem orthogonal to the distinction you’re drawing.

A potential distinction is entropy preservation. For password hashes you usually want to preserve as much entropy as possible although one could argue that beyond 256 bits of output it may not matter (only one-time pads would suffer from smaller output). KDFs on the other hand must output a correctly-sized key for a particular cipher and so have further constraints on output choices (and potentially avoiding weak keys, e.g. for elliptic curve point generation).

There is a huge amount of overlap, in that most modern password hashes can be used and are sort of fundamentally based on the idea of a string KDF. The big differences are, as you can see here, that a string or password KDF will take an arbitrarily long string, and that a KDF produces some specific raw random output, and a password hash produces a verifier string (the raw random hash, plus usually some metadata encoded some way; for bcrypt, that's the algorithm and cost and salt, for instance).

Password hash functions are designed to be slow, are designed to be use with salts, and may have low entropy inputs. Being slow is a waste for (true) KDFs, salts aren't relevant (although nonces may be), and are designed for high entropy inputs.

The naming overlap between the two is bad, so the industry has tried to move towards naming the two differently. Password hashing functions are not ideal KDFs, even though a particular primitive may be secure for use as a KDF. That's a root of some of the confusion.

String KDFs are also slow. That's the basic strategy for making high-entropy keys out of low-entropy inputs.

Password hash functions are intentionally designed to be extremely slow (at an exponential scale). While this makes perfect sense for password hashing, it is nonsensical to have such an intentional, configurable slowdown mechanism in KDFs. KDFs have computational cost, but having the kind of extreme slowdown that password hash functions have makes no sense for purpose designed KDFs, especially when needing to derive many keys.

Hypothetically here is one way it might have played out

Product - we need to provide service availability even if the AD is down

Engineer - Ok, may be we can store the ~credentials in cache

Security - oh, in that case make sure everything in cache is hashed properly with the recommended Bcrypt algorithm

Engineer - We got the approval from the security, we are in much safer zone, lets deliver and get a win

> I've in the past been annoying about saying I think we should just call all password hashes "KDFs"

I remember :-): https://news.ycombinator.com/item?id=42899432

Unfortunately the industry defined Bcrypt as a KDF for some time, even though it is better named as a "password hashing function". Cryptography has a history of being bad at picking good names for new work.

In addition to (true) KDFs, people often want a HKDF (HMAC-based Key Derivation Function) or hierarchical deterministic key derivation function (HDK).

> 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?

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 'strong' hash function with no input truncation, as long as it has a fixed width result, will have this property. Thus, any arguing in the comments here about different hash functions with different truncation properties is moot.

The analogy is something like creating a hash map whose insert function computes the slot for the key and unconditionally puts the value there instead of checking if the keys are the same during a collision. No amount of tinkering with the hash function fixes this problem. The algorithm is wrong. A hashmap should survive and be correct even giving it a hash function that always returns 4.

I would guess that they felt comfortable that the bcrypt output (192 bits) is enough that collisions are very unlikely. If these were already partitioned by customer, rather than being a single cache for the entire Okta userbase that seems fine. You're going to have weird cosmic ray bugs more often than a natural collision.

Now, the data structure they're using for a cache will use some sort of hash table, likely in memory, so maybe they've got the 192-bit bcrypt "key" and then that's hashed again, perhaps well or perhaps badly [e.g. C++ really likes using the identity function so hash(12345) = 12345] but then a modulo function is applied to find the key in an index and then we go groping about to look for the Key + Value pair. That part the API probably took care of, so even if the hash has size 6-bits, the full 192-bit key was checked. But the original data (userid:username:password) is not compared, only that 192-bit cache key.

Not sure about that. A hash function suitable for security sensitive work, used properly, should make a collision so unlikely that you can basically forget it that it's even possible.

Think about it, that's what hashing passwords relies on. We don't store a plaintext password for a final check if the password hash matches, we count on a collision being basically impossible.

A hashmap is different, because it's using a much weaker hash function with far fewer security guarantees.

Plus, you're assuming the original values are even kept around for comparison. The cache key likely just mapped to something simple like a boolean or status flag.

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.

Isn't a hash map supposed to already be internally hashing the string for you and correctly handling collisions?

This makes no sense - it’s a hash map because it hashes things for you…

A hash map still stores the entire key, which may be undesirable if the key datum can be large.

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).

> valid, 128 bits

The birthday paradox is a thing. If you have 128 bits of entropy, you expect the 50% mark to be proportional to 64-bit keys, not 128 bits. 64 bits is a lot, but in my current $WORK project if I only had 128 bits of entropy the chance of failure any given year would be 0.16%. That's not a lot, but it's not a negligible amount either.

Bigger companies care more. Google has a paper floating around about how "64 bits isn't as big as it used to be" or something to that effect, complaining about how they're running out of 64-bit keys and can't blindly use 128-bit random keys to prevent duplication.

> bits of entropy

Consumer-grade hash functions are often the wrong place to look for best-case collision chances. Take, e.g., the default Python hash function which hashes each integer to itself (mod 2^64). The collision chance for truly random data is sufficiently low, but every big dictionary I've seen in a real-world Python project has had a few collisions. Other languages usually make similar tradeoffs (almost nobody uses crytographic hashes by default since they're too slow). I wouldn't, by default, trust a generic 1-million-bit hash to not have collisions in a program of any size and complexity. 128 bits, even with low enough execution counts to otherwise make sense, is also unlikely to pan out in the real world.

Hashtables have collisions because they don't use all bits of hash, they calculate index=hash%capacity. It doesn't matter, how you calculate the hash, if you have only a few places to insert an item, they will collide.

Right, but the problem they were describing was storing the "hash" in a hash table, not storing the item using a hash. For that, it absolutely matters, and the fact that it was a 128-bit hash IMO isn't good enough because the hash function itself likely sucks.

I agree that 128 bits is on the lower end of "never", but you still need to store trillions of hashes to have a one-in-a-trillion chance to see a collision (and that's already the overall probability, you don't multiply it by the number of inserts to get 1:1 chance :) I don't think anybody in the world has ever seen a collision of a cryptographically strong 128-bit hash that wasn't a bug or attack.

Birthday paradox applies when you store the items together (it's a chance of collision against any existing item in the set), so overall annual hashing churn isn't affected (more hashes against a smaller set doesn't increase your collision probability as quickly).

Based on currently available public estimates, Google stores around 2^75 bytes, most of that backed by a small number of very general-purpose object stores. A lot of that is from larger files, but you're still approaching birthday-paradox numbers for in-the-wild 128-bit hash collisions.

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.

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, including those potentially set by a malicious user.) A couple years back, I spent some time adding in runtime checks that detect those kinds of issues and log a warning. Since then, we've had a significant reduction in the amount of not-a-bug reports and, I assume, significantly fewer users with incorrect configurations.

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?

If these tools had a runtime check, then the cache key creation would have failed out.

72 is the max length of id, username, and password combined. If that combination is over 72, then failure and the cache key would not have been created. So, no, the attacker would not need to guess only one character of a password.

I enjoyed the article and the detailed analysis for different languages. The conclusion is probably the part where most of the disagreement lies. API design is is not really at fault here if we consider the purpose of the API and the intended output.

The API was designed to generate a hash for a password (knowledge factor) and for performance and practical reasons a limit has been picked up (72). The chances that some one knows your first 72 characters of password implies that the probably is a lot higher for the abuser to have remaining characters too.

While smaller mistake here in my opinion was not knowing the full implementation details of a library, the bigger mistake was trying to use the library to generate hash of publicly available/visible information

Ohhh, it's scrollable... I wondered why this small article gained so much attention...

Yeah, the fact that I can't have my mouse in the normal position and scroll the actual article was a problem 10 times or more while trying to read the thing...

> limit has been picked up (72)

There's nothing wrong with a limit. The problem is that the library silently does the wrong thing when the limit is breached, rather than failing loudly.

This is a completely unreasonable API. It reminds me of the `mysql_real_escape_string` vs. `mysql_escape_string`. The default API must be the strict one. You should be able to configure it to be broken but silent truncation is an insane piece of functionality. There is no universe in which this is logical. One might as well just have everything return void* and then put in the documentation what type to cast to. The invariant is clearly a historical accident.

As a mistake, it's fine. Everyone writes up things like that. But defending it as an affirmatively good decision is wild.

Seriously, the number of “you weren’t meant to fire that foot gun” defenses in this thread…

[u/forgot-CLHS](https://www.reddit.com/r/lisp/comments/1ikrz1g/shout_out_to_...) notes that Common Lisp's defact standard cryptography library Ironclad's [implementation](https://github.com/sharplispers/ironclad/blob/master/src/kdf...) avoids such problems!

``` (defmethod derive-key ((kdf bcrypt) passphrase salt iteration-count key-length) (declare (type (simple-array (unsigned-byte 8) (*)) passphrase salt)) (unless (<= (length passphrase) 72) (error 'ironclad-error :format-control "PASSPHRASE must be at most 72 bytes long."))...) ```

Have they did a bcrypt(password + userId + username), it won't be so bad. Order of entropy is important.

Also I'm not sure what functionality the authentication cache provides, but their use of bcrypt(userId + username + password) implies the password is kept around somewhere, which is not the best practice.

OT. Has Argon2 basically overtaken Bcrypt in password hashing in recent years?

> Have they did a bcrypt(password + userId + username), it won't be so bad. Order of entropy is important.

That depends on how exactly it was used. If it was simply used to check if previous authentication was successful (without the value containing information who it was successful for) then single long password could be used to authenticate as anyone.

> single long password could be used to authenticate as anyone.

Only if everyone uses the same long prefix for password.

No. If the value of the cache key is simply true/false then someone would first login to their own account using the long password. This would result in storing:

bcrypt(longpassword + 123456 + me@foobar.com) = bcrypt(longpassword) = hash1 -> true

If they then try login as you@bar.com using same password there would be a cache lookup:

bcrypt(longpassword + 1111111 + you@bar.com) = bcrypt(longpassword) = hash1 -> true

The bcrypt implementation in the Zig standard library has both the bcrypt() function (where truncation is explicitly documented) and the bcryptWithoutTruncation() function (which is recommended and automatically pre-hashes long passwords).

Author here: thanks for reading the post.

It's great to hear that Zig covered both cases. However, I'd still prefer the opposite behavior: a safe (without truncation) default `bcrypt()` and the unsafe function with the explicit name `bcryptWithTruncation()`.

My opinion is based on the assumption that the majority of the users will go with the `bcrypt()` option. Having AI "helpers" might make this statistic even worse.

Do you happen to know Zig team's reasoning behind this design choice? I'm really curious.

Note that the "safe" version makes very bespoke choices: it prehashes only overlong password, and does so with hmac-sha512 (which it b64-encodes). So it would very much be incompatible with other bcrypt implementations when outside of the "correct space".

These choices are documented in the function's docstring, but not obvious, nor do they seem encoded in a custom version.

Sounds like it would then make sense to hide crypto primitives and their footguns under a "hazmat" or "danger" namespace, sorta like webcrypto or libsodium.

So something like crypto.danger.bcrypt and crypto.bcryptWithTruncation

`bcrypt()` is bcrypt as implemented everywhere else, and is required for interoperability with other implementations. If you don't truncate, this is not `bcrypt` any more.

`bcryptWithTruncation()` is great for applications entirely written in Zig, but can create hashes that would not verify with other implementations.

The documentation of these functions is very explicit about the difference.

The verification function includes a `silently_truncate_password` option that is also pretty explicit.

To be fair, I can't think of a single context where I would want to truncate a password before hashing so interoperability with other systems isn't worth letting by a dangerous edge case in my opinion. I'd rather have the system break for the handful of users with a 72+ char password than overlook a potential critical security issue.

You might need it if you’re porting / reimplementing a system and have to be compatible with an existing base of hashed truncated passwords.

I would agree that it should not just be called “bcrypt” though, likely no function of this module should be, they should either explain their risks or clarify their safety.

Or possibly only a version which fails if passed more than 72 bytes or any nul.

Seems odd to keep the broken one as the default[0], make the "recommended" one so much longer, and provide none which simply errors on overlong passwords.

Also neither seems to warn about the NUL issue.

[0] I assume for compatibility purpose, but it still seems very dubious.

Disappointing that Zig would get the API design wrong too. I would have expected better.

If you don't see the mistake, Google 'yaml.safe_load'.

I am curious why bcrypt was used for hashing in the first place and not something like sha-512

Is there a reason I might be missing?

Yes, the hashed payload contained a password, so presumably they didn't want to just SHA it.

But why not bcrypt the password, but sha the cache key on top?

I guess because they didn't anticipate this flaw.

Also prehashing opens you up to an other bcrypt flaw you need to be aware of: it stops at the first NUL byte, so you need to use some sort of binary-to-text encoding on top of the hash to ensure you don't have any of those in the data you ultimately hand off to bcrypt.

Thank you

It's astounding how bad the default API for Bcrypt is.

Begs the question of why the payload contained a password, right?

They wanted the cache entry to be invalidated when the password changed. Just using username as the key and storing the bcrypt password inside the cache entry and checking the password on load seems like a better solution if it was possible.

Storing the bcrypt password in the entry would make a dump of the cache almost as good as a dump of the password database. At least this way a dump of the cache makes the key opaque and requires you to guess both the username/id and password together, assuming they're not repeated in the cache value.

According to the security advisory this cache was for AD/LDAP delegated authentication, so they don't have their own password database with a version field or similar for sensible invalidation.

I guess the requirements could be something like:

  - different username/password combinations must have separately cached results

  - mitigate a potential data leak by putting all the entropy we have available together with the password material and using a slow password hashing function

There is a discussion about that on the security stackexchange (https://security.stackexchange.com/questions/133239/what-is-...). The TLDR:

> SHA-2 family of hashes was designed to be fast. BCrypt was designed to be slow.

Slow == harder to brute-force == more secure.

Yes, and you can increase the work factor to make it slower to generate, specifically to fight against brute-force.

Password hash functions are designed to be slow, are designed to be use with salts, and may have low entropy inputs.

Hash functions themselves are general purpose and don't protect against low entropy inputs (low entropy passwords). They also don't protect against rainbow tables (pre-calculated digests for common or popular passwords). For password hashing you want something slow and something with unique entropy for each user's password to prevent rainbow attacks.

It doesn't solve the problem of weak passwords, but it's the best that can be done with weak passwords. The only improvement is to enforce strong passwords.

In Node, you would commonly reach for the builtin core "node:crypto" module to run cryptographic functionality like this. I wondered why that wasn't used here, but bcryptjs was. After digging into it a little, node doesn't ship with core support for bcrypt, because it's not supported by OpenSSL.

The node crypto module is essentially an API that offloads crypto work to OpenSSL. If we dig into OpenSSL, they won't support bcrypt. Bcrypt won't be supported by OpenSSL because of reasons to do with standardisation. https://github.com/openssl/openssl/issues/5323

Since bcrypt is not a "standardised" algorithm, it makes me wonder why Okta used it, at all?

I remember in uni studying cryptography for application development and even then, back in 2013, it was used and recommended, but not standardised. it says a lot that 12 years on it still hasn't been.

damn that sounds like a rookie mistake for an organization who is literally in the business of secure auth

Reminds me of when I saw a junior developer calling SHA-1 on an incrementing integer ID, with no salt. We had a long talk about it, he thought it was too "scrambled" to allow anyone to recognize what was being done. He shouldn't have been so junior, he was 4 or 5 years into his career. I had to be the bad guy and override his decision without further discussing why it was a bad idea, and I really tried for a good 45 minutes to explain things. He got it a week later when I showed him rainbow tables, and I felt bad having to tell him to just do what I said for the solution, but sometimes you just have to make the decision to say "do what I said, I'm sorry you don't understand, I tried to explain."

Rainbow tables is not the (only) reason you dont want to hash something low entropy like an incrementing int, and adding a salt wouldn't make this secure.

[Im assuming the usual definition of salt where it is known by the attacker... a pepper would be fine]

I agree, and I guess I did use salt differently than how most people see it, rather than how it is most effective. I never stored the salt in the database alongside the password. I would use something from the user that wouldn't change without a password change, as well as some type of semi-long data that also got hashed and put into the "pepper". Even if it's a file on disk that contains data that is read into memory and hashed with something that doesn't change (or at least can't change without the user also re-entering or creating a new password). Also, thank you for teaching me the term "pepper", because I feel like that is so relatable, but also different enough to correlate the two, but show how "pepper" is more powerful and useful!

I've seen this before, a belief that just because the output looks random that it is secure. It's like storing license plates -- just hashing them without additional seasoning is of little use, because the number of possible license plates is so low that they can easily be brute forced.

Similarly, a developer I worked with once claimed that CRC32 was sufficient verification because CRC32s changed so drastically depending on the data that they were difficult to forge. He was surprised to find out not only is it trivial to update a CRC32, but also to determine the CRC polynomial itself from very few samples.

I guess it feels good to let someone know that what they're doing is not cryptographically secure, but at the same time, you have to tell them that seemingly random numbers and/or letters doesn't mean they've come up with something useful.

I've tried to stay positive and explain that they will fool nearly everyone, the technology they used is usually recognizable to the type of people that would want to bypass it for their own gain (or knowledge, assuming white hat types poking around). Usually putting a spin on how they came up with something that looks secure was a great idea, but the type of people that will exploit something like what they built, will recognize patterns easily (and now that AI is around, you could even make them feel better by stating how there is software built to recognize these patterns).

Ah, the tale I could tell you about "encrypted ZIP codes".

That is such a rookie mistake. It's not some hidden information that bcrypt has a 72 char limit. Pretty widely documented in multiple implementations and languages.

How does a company whose only job is security screw that up so badly?

> How does a company whose only job is security screw that up so badly?

While I don't have any answers to this, I've realized that it's an ideal showcase of why fuzzy testing is useful.

On the other hand, why not have implementations assert if they are given a string longer than 72 chars? It feels to me like no-one would ever do that on purpose, so it's a massive issue which is easy to accidentally make with a really important function.

Don't disagree there. I asked my self the same question the first few times I had to use it.

Silently truncating the data is about the worst way to deal with it from a security standpoint. No idea why that decision was made back in the day.

It is almost never I good idea to assert in a library, unless the error is truly unrecoverable. I think returning an error code\throwing an exception would be very reasonable and a much better API than failing silently though.

An exception is fine if the language has them. I don't think "assert" was meant super literally and exactly the way C does it.

An error code is risky.

Honestly, I would assert. Returning an error code just gives users another thing to ignore, and incorrectly use the return value (if implemented like C, where you usually get a single internet back, treating the error code as a hash would be even worse!)

Apps crashing with assets is awfully, but at least it screams at your when you failed to read the docs, target than incorrectly storing users data for the rest of time.

> almost never I good idea to assert in a library, unless the error is truly unrecoverable

Like getting an input that is too long? :)

I think a library asserting that the preconditions of its arguments are true is fine.

On one hand, I have never heard of a password hashing algorithm that truncated to 72 characters. I assumed all one-way hashing functions were arbitrary length input. "arbitrary input -> fixed output" has always been part of the definition of hash, to me.

On the other hand, I'm not a security developer at Okta.

og crypt does the same shit but 8 characters. weird historical artifacts i suppose.

> It's not some hidden information that bcrypt has a 72 char limit. Pretty widely documented in multiple implementations and languages.

One of the points of the article is that documentation isn’t enough: one cannot allow callers to misuse one’s API.

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...

Hold on, in the Rust example, how does `err_on_truncation` get set? TFA completely ignored that there's a setting somewhere (probably incorrectly defaulting to false)

the rust library exposes a handful of "non_truncating_*" functions that enable error handling. i would expect this to be for drop-in compatibility with old code.

amusingly, the python "library" is just a thin wrapper around the same rust library.

protip: a lot of cryptography primitives actually aren't that complicated in terms of the code itself (and often can be quite elegant, compact and pleasing to the eye). if it's important, probably worth just reading it.

it's what people wrap them with or the systems they build that get messy!

In the bcrypt crate there is an explicit method for it:

    bcrypt::non_truncating_hash()

https://docs.rs/bcrypt/latest/bcrypt/

Funnily, TFA later also suggests that such function should exist...

Being pedantic, TFA suggests something slightly different. The non_truncating_hash should be the default (and called something that reflects it, eg. just hash), and a separate truncating_hash function may exist. The difference (from an API design perspective) is pretty massive.

I'm confused, it seems that the OP wants to use Bcrypt as an encoding/decoding utility.

About solutions, Django hashes by default the password only with a salt. I'm not sure why it would be valuable to combine user_id+username+password. I've always assumed that using salt+password was the best practice.

Regarding the API design, I agree now with OP after reading other comments on HN. The API would be improved if it clearly indicates to the user when truncation is done, even if this understanding is implied by principle.

what I would have naturally done without anticipating any flaw (and probably be just OK):

   cache_key = sha(sha(id + username) + bcrypt(pass))

with sha256 or something.

Why not a simple sha(id + username + bcrypt(pass))

Is there any security issues with that? I'm a "newb" in this area, so I'm genuinely curious about the flaws with the naive approach

I've seen this multiple times - even better I don't know how many ways we found a simple workaround or bypass of the complete process in so many apps... In essence this has nothing to do with the API itself but the way in which is another ballgame altogether. Great post though.

> On the other hand, such long usernames are not very usual, which I agree with

Weird take. Usernames are often chosen by the user. Less so in corporate world but definitely not unheard of

Many of my usernames at my company are based on my email and it's pretty long - by the time you add the domain it's a good 47 characters...

I was at a Python conference once and met someone whose email ended '@boehringer-ingelheim.com'. That's 25 letters right there.

https://www.boehringer-ingelheim.com/media-stories/press-rel... has a camilla.krogh_lauritzen@boehringer-ingelheim.com at 48 characters, for example.

Can someone explain, in clear layman terms, what the difference is between a password hash and a KDF? I have went through this whole thread and tried to look around online but I still don't understand.

Password hash is designed for matching: take the salt, add it to the password, run it through the hash, compare it to the stored hash. The important properties are:

- MUST be non-reversible, including against tricks like "rainbow tables"

- should be somewhat expensive to discourage just trying all possible passwords against a (leaked) hash

KDF is a key derivation function. The value will be used as a key in, say, AES. The important properties are:

- should distribute entropy as well as possible, across the required width of output bits

- reversibility less important as the derived key shouldn't be stored anywhere

- may or may not want artificially inflated cost to discourage cracking

I still have no idea now haha. Your answer and Fabbari's are total opposites. If I am understanding right, you are saying that Password Hash is how a password should be stored while a KDF is not meant for storing passwords. Fabbari is saying the opposite of this, that KDF should be used for storing passwords while password hashes should not.

Further discussion upthread under https://news.ycombinator.com/item?id=42957300

A password hash is a simple hash of a password. Hash algorithms are made to be fast. KDF - key deriving functions - are slow by design and are made to derive a key from a given string. They are designed to be slow to make password searching slower. This is a 2c tour of the topic.

There are 2 comments to OP, and now the person can wonder which one is supposed to be slow or not.

Another incident at Okta? Oh no! Its security has _always_ been a mess. It's a dumpster fire and no client of their cares because their identity systems are so messed up, that it's better to have the mess of Okta, than the mess they are sitting on. It's kinda crazy they get away with such incredibly bad security practices. Like... this bcrypt issue has been know for a LONG while. We used to test for it 8-10 years ago.

There's either (1) nobody competent enough there to know (which is likely not true, I had a pentester friend recently join, and she is very good), or, more likely (2) management doesn't care and/or doesn't give enough authority to IT security personnel.

As long as clients don't have any better options, Okta will stay this way.

> no client of their cares because their identity systems are so messed up, that it's better to have the mess of Okta, than the mess they are sitting on

Yes, this is very true.

Also some companies realize that they can screw up royally because they do no have the proper knowledge, and authentication is not a core business of theirs.

I can understand them. I also use mail systems I am not that happy with, but I have this comforting idea that if they have a problem, 3B people are waiting together with me for it to be solved, and that's the kind of pressure that helps.

I'm really surprised they didn't cover PHP since (almost?) every framework uses bcrypt in php these days.

PHP's password_* functions make it difficult to misuse in this particular way. There's no function in that API which hashes a password with a controllable salt and returns the result; there's only password_hash(), which always uses a random salt, and password_verify(), which rehashes a password internally and returns a bool for whether it matched.

(It's still got the truncates-at-72 problem with PASSWORD_BCRYPT, though.)

What's the reason behind bcrypt(userId + username + password) rather than just bcrypt(password) ?

rainbow tables I guess

What if two different users have the same password?

Bcrypt is salted[1], so that shouldn't matter?

[1]: https://en.wikipedia.org/wiki/Bcrypt#Description

Are you sure?

bcrypt stores the salt and retrieves it for comparison - otherwise you wouldn't be able to generate a matching hash.

Consider the case where a user has a very long username and sets their password to their userId + username + password thus recreating the scenario which lead to the incident.

That was not my point. My point was there wouldn't be a hash collision just by two users with the same password due to the salting.

There's no hash collision here, just two different hashes, each with its own salt, matching the same original phrase.

If you use only the password to generate the cache key, then this password will match regardless of salt, so users with the same password will generate a cache key matching that password.

Yes, that's what I pointed out when you suggested there would be a problem with two different users having the same password.

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.

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

Why would that matter though?

How does anyone take Okta seriously after this incident btw?

> was used to generate the cache key where we hash a combined string of userId + username + password.

Don't conceive your own cryptographic hacks. Use existing KDF designed by professionals.

Is the functions in libsodium enough? Provided they are used correctly?

Yes.

Simply hashing your data (using an established hashing algorithm/library combo) to later compare two hashes in order to check whether the data has changed doesn’t usually feel like rolling your own crypto.

The use case was KDF and they decided to do simple password hash signature hack instead by combining strings. They fucked it up.

Of course they fucked it up, as evidenced by their bad security incident. The only question is whether you can really chalk this particular one up to a problem with "rolling your own crypto." That mantra exists for a reason, but it doesn’t feel like it really applies this time. It seems more like they used established crypto—just not the right one for this particular use case.

Concatenating strings before giving it to the hash function instead of using KFD is rolling your own.

I would bet that if you surveyed working programmers 9 out of 10 would say that they thought bcrypt() was a “KDF designed by professionals”. The treacherous API is not as well known as it should be.

Reply on news.ycombinator.com