Live data from Hacker News

Okta Bcrypt incident lessons for designing better APIs

n0rdy.foo

101–110 of 169 posts

Re: Okta Bcrypt incident lessons for designing better APIs

#101
post #3

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

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 drawi…

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.

Re: Okta Bcrypt incident lessons for designing better APIs

#102

Earlier quoted context omitted.

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.

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.

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

Re: Okta Bcrypt incident lessons for designing better APIs

#104
post #49
post #6

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

Re: Okta Bcrypt incident lessons for designing better APIs

#105
post #94

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)

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.

Re: Okta Bcrypt incident lessons for designing better APIs

#106
post #51
post #6

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

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.

Re: Okta Bcrypt incident lessons for designing better APIs

#107

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?

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.

Re: Okta Bcrypt incident lessons for designing better APIs

#109
post #37

Earlier quoted context omitted.

`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 functio…

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.

Post reply on HN