[HN Gopher] Okta Bcrypt incident lessons for designing better APIs
___________________________________________________________________
Okta Bcrypt incident lessons for designing better APIs
Author : n0rdy
Score : 360 points
Date : 2025-02-05 21:10 UTC (1 days ago)
(HTM) web link (n0rdy.foo)
(TXT) w3m dump (n0rdy.foo)
| jedisct1 wrote:
| 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).
| n0rdy wrote:
| 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.
| masklinn wrote:
| 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.
| jszymborski wrote:
| 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
| jedisct1 wrote:
| `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.
| xmprt wrote:
| 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.
| masklinn wrote:
| 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.
| masklinn wrote:
| 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.
| IshKebab wrote:
| 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'.
| tptacek wrote:
| 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_.
| masklinn wrote:
| 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.
| dotancohen wrote:
| > 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!
| pclmulqdq wrote:
| 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).
| masklinn wrote:
| They clearly wanted something stronger than "a hash function"
| or they'd have reached for weaker cryptographic hashes.
| pclmulqdq wrote:
| 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.
| tptacek wrote:
| 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"?
| pclmulqdq wrote:
| 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.
| Vecr wrote:
| For 'unlimited' input size it should be SHA-3-512. Maybe
| too slow, but Bcrypt is slower, right? Less things to go
| wrong too.
| pclmulqdq wrote:
| 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.
| Vecr wrote:
| 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.
| pclmulqdq wrote:
| 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.
| tptacek wrote:
| Ew. Just HMAC. Don't use truncated SHA2.
| hn_go_brrrrr wrote:
| Why?
| jedisct1 wrote:
| hmac-bcrypt solves that problem very well, and should
| replace plain bcrypt: https://github.com/epixoip/hmac-
| bcrypt
| tptacek wrote:
| 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.
| petedoyle wrote:
| Maybe they wanted some cached data to get invalidated if
| users change their passwords?
| GoblinSlayer wrote:
| By cache they mean cached credentials.
|
| >The user previously authenticated creating a cache of
| the authentication
|
| Maybe, it's a password encrypted secret token.
| GoblinSlayer wrote:
| 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.
| earnestinger wrote:
| Could you give an example of real KDF?
| aaronmdjones wrote:
| I'm not the person you're replying to, but HKDF and
| PBKDF2
| pclmulqdq wrote:
| 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...
| tptacek wrote:
| 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.
| smw wrote:
| 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?
| tptacek wrote:
| I think it's good to want things.
| justin_oaks wrote:
| 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.
| re-thc wrote:
| > 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).
| GoblinSlayer wrote:
| Ship early, ship often, we can add security later.
| TeMPOraL wrote:
| 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.
| pclmulqdq wrote:
| Being too clever and being too casual are the same thing
| when it comes to matters of math.
| riffraff wrote:
| Perhaps the password is used as part of the cache Key so
| that a password update implicitly invalidates the cache?
| dspillett wrote:
| _> 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.
| jedisct1 wrote:
| bcrypt-pbkdf (used in OpenSSH) exists for that purpose.
| taurknaut wrote:
| > 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.
| bufferoverflow wrote:
| BCrypt should loudly fail if more than 72 bytes are sent to
| its input.
| taurknaut wrote:
| Maybe it should. Discarding the rest of the bytes works
| fine for _passwords_ , though. I guess that's just not
| sufficient.
| pclmulqdq wrote:
| 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.
| hinkley wrote:
| 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.
| mananaysiempre wrote:
| 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.
| benlivengood wrote:
| 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).
| tptacek wrote:
| 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).
| Zamicol wrote:
| 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.
| tptacek wrote:
| String KDFs are also slow. That's the basic strategy for
| making high-entropy keys out of low-entropy inputs.
| sandeepkd wrote:
| 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
| lelanthran wrote:
| > 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
| Zamicol wrote:
| 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).
| dataflow wrote:
| > 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?
| coolgoose wrote:
| 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?
| stavros wrote:
| Yes, the hashed payload contained a password, so presumably
| they didn't want to just SHA it.
| coolgoose wrote:
| But why not bcrypt the password, but sha the cache key on
| top?
| stavros wrote:
| I guess because they didn't anticipate this flaw.
| masklinn wrote:
| 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.
| coolgoose wrote:
| Thank you
| Dylan16807 wrote:
| It's astounding how bad the default API for Bcrypt is.
| tptacek wrote:
| Begs the question of why the payload contained a password,
| right?
| benmmurphy wrote:
| 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.
| tveita wrote:
| 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
| n0rdy wrote:
| 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.
| progmetaldev wrote:
| Yes, and you can increase the work factor to make it slower
| to generate, specifically to fight against brute-force.
| Zamicol wrote:
| 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.
| nabla9 wrote:
| > 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.
| edoceo wrote:
| Is the functions in libsodium enough? Provided they are used
| correctly?
| nabla9 wrote:
| Yes.
| ludwik wrote:
| 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.
| nabla9 wrote:
| The use case was KDF and they decided to do simple password
| hash signature hack instead by combining strings. They fucked
| it up.
| acdha wrote:
| 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.
| whalesalad wrote:
| damn that sounds like a rookie mistake for an organization who is
| literally in the business of secure auth
| Tostino wrote:
| 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?
| n0rdy wrote:
| > 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.
| CJefferson wrote:
| 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.
| Tostino wrote:
| 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.
| zorgmonkey wrote:
| 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.
| Dylan16807 wrote:
| 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.
| CJefferson wrote:
| 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.
| thayne wrote:
| > 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.
| unethical_ban wrote:
| 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.
| yusyusyus wrote:
| og crypt does the same shit but 8 characters. weird
| historical artifacts i suppose.
| eadmund wrote:
| > 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.
| sscarduzio wrote:
| 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.
| throwaway-9111 wrote:
| 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
| progmetaldev wrote:
| 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."
| bawolff wrote:
| 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]
| progmetaldev wrote:
| 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!
| ack_complete wrote:
| 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.
| progmetaldev wrote:
| 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).
| bux93 wrote:
| Ah, the tale I could tell you about "encrypted ZIP codes".
| jhhh wrote:
| 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.
| tialaramex wrote:
| 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.
| semicolon_storm wrote:
| 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.
| purist33 wrote:
| 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.
| TeMPOraL wrote:
| Isn't a _hash map_ supposed to already be internally hashing
| the string for you _and correctly handling collisions_?
| klysm wrote:
| This makes no sense - it's a hash map because it hashes
| things for you...
| masklinn wrote:
| A hash map still stores the entire key, which may be
| undesirable if the key datum can be large.
| pornel wrote:
| 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).
| rcleveng wrote:
| 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.
| bawolff wrote:
| > 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
| SebFender wrote:
| 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...
| ww520 wrote:
| 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?
| buzer wrote:
| > 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.
| ww520 wrote:
| > single long password could be used to authenticate as
| anyone.
|
| Only if everyone uses the same long prefix for password.
| buzer wrote:
| 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
| SebFender wrote:
| 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.
| renewiltord wrote:
| 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.
| benced wrote:
| Seriously, the number of "you weren't meant to fire that foot
| gun" defenses in this thread...
| sandeepkd wrote:
| 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
| AlfeG wrote:
| Ohhh, it's scrollable... I wondered why this small article
| gained so much attention...
| underdeserver wrote:
| 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)
| a-dub wrote:
| 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!
| kmarc wrote:
| 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...
| ratorx wrote:
| 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.
| llmthrow102 wrote:
| How does anyone take Okta seriously after this incident btw?
| philippta wrote:
| What's the reason behind bcrypt(userId + username + password)
| rather than just bcrypt(password) ?
| ReptileMan wrote:
| rainbow tables I guess
| Tade0 wrote:
| What if two different users have the same password?
| magicalhippo wrote:
| Bcrypt is salted[1], so that shouldn't matter?
|
| [1]: https://en.wikipedia.org/wiki/Bcrypt#Description
| Tade0 wrote:
| 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.
| magicalhippo wrote:
| 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.
| Tade0 wrote:
| 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.
| magicalhippo wrote:
| Yes, that's what I pointed out when you suggested there
| would be a problem with two different users having the
| same password.
| Tade0 wrote:
| 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.
| magicalhippo wrote:
| 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.
| Tade0 wrote:
| The the output of `bcrypt(password)` is:
| $2a$12$R9h/cIPz0gi.URNNX3kh2OPST9/PgBkqquzi.Ss7KIUgO2t0jW
| MUW \__/\/
| \____________________/\_____________________________/
| 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.
| magicalhippo wrote:
| 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
| johnisgood wrote:
| Why would that matter though?
| Lvl999Noob wrote:
| 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.
| pjc50 wrote:
| 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
| Lvl999Noob wrote:
| 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.
| pjc50 wrote:
| Further discussion upthread under
| https://news.ycombinator.com/item?id=42957300
| fabbari wrote:
| 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.
| johnisgood wrote:
| There are 2 comments to OP, and now the person can wonder
| which one is supposed to be slow or not.
| withinboredom wrote:
| I'm really surprised they didn't cover PHP since (almost?) every
| framework uses bcrypt in php these days.
| mariocesar wrote:
| 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.
| mariocesar wrote:
| 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.
| zero_k wrote:
| 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.
| nfriedly wrote:
| 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.
| deepsun wrote:
| If the input is 71 character, all the libraries happily accept
| it, but an attacker needs to guess only 1 character.
| arccy wrote:
| have separate salt / pepper / user id args
| tedunangst wrote:
| How is the library supposed to know you're doing that
| wrong?
| cmgriffing wrote:
| 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.
| thekemkid wrote:
| 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.
| jmuguy wrote:
| 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...
___________________________________________________________________
(page generated 2025-02-06 23:02 UTC)