Hacker News

points by NewJazz 3 days ago | 0 comments

musl’s crypt() returns the string * as an error indicator when the key is over 256 bytes long. * is also exactly what the stored field contains for every account with no password, so the “hash” matches and the login succeeds.

Does anyone know why musl does this? Are they the only C library that behaves this way?

For a C library that famously tries to adhere to POSIX as much as possible, this seems like a crazy choice. From POSIX standard:

Upon successful completion, crypt() shall return a pointer to the encoded string. The first two bytes of the returned value shall be those of the salt argument. Otherwise, it shall return a null pointer and set errno to indicate the error.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/c...

[–] hnfong 3 days ago

There's a very detailed trace of how the bug was developed over decades at sshhole.com and the relevant discussion in https://news.ycombinator.com/item?id=50378228

[–] NewJazz 3 days ago

Thanks yeah just woke up, finished making my way through the single-article website and still unclear if other libcs do the same thing (the article says libxcrypt picked it up, but only after musl did it).

[–] NewJazz 2 days ago

Some more clarity ... Multiple implementations have these sentinel values (not OpenBSD). But unfortunately only musl had a sentinel value that was also used in shadow files...

https://hachyderm.io/@ncopa@fosstodon.org/117564809789053485

[–] worksonmine 3 days ago

Apparently because some devs didn't check for NULL which resulted in crashes. Weird choice to fix other peoples' bad programming by deviating from the spec.

[–] cyphar 3 days ago

To be fair (as the sshole site mentions) it is also necessary to fake a password hash check to avoid timing side channels revealing to remote clients whether users exist, so returning a sentinel value is not a horrifically unreasonable solution to make sure even naive code does approximately the right thing.

Though I am surprised that all of the code examples do not use constant-time strcmp(). Actually, I wonder if even constant-time strncmp() would still leak information about whether the account is disabled because in that case both arguments are much shorter? Food for thought.

But yeah, the crypt(3) API is very 80s C, the man page even features a lengthy description of a long-deprecated and utterly broken hashing algorithm using DES that is even implied to be broken in the text itself (and the warning message is so old it refers to "parallel computers" as though they were a new concept). Given that the default mode is to use a broken hashing algorithm, I'm shocked anyone actually uses the libc crypt(3) -- surely it's trivial to just write your own that isn't awful...?

[–] mik1998 3 days ago

Constant time strcmp doesn't fix it. The problem is that the user doesn't have a password at all (the hash is *). But which hashing algorithm is used is encoded in the password. So they pick a different user's hash to not leak the information that there is no password. Alternate solution would be to hardcode a specific algorithm, but that could also leak info in many cases, I guess.

[–] cyphar 2 days ago

No, I get that -- my first point was meant to allude to that but maybe it wasn't phrased particularly well. The bit about constant-time strcmp() was about comparing the hashes afterwards, which was a classic attack to extract the password hash for offline cracking (though salts probably make it untenable?).

In any case, the fact you need access to an existing hash to not select something stupid like DES is all due to crypt(3) being a wacky API and while callers could work around it by selecting more reasonable defaults it's a pretty big foot gun.

[–] sysguest 2 days ago

hmm why not make some fake-users just for this stuff?

[–] forthac 2 days ago

you mean like daemon?

[–] api 3 days ago

TBH that sounds like a straight up bug in that it deviates dangerously from the spec.

[–] shakna 2 days ago

The spec only defines a single error state - ENOSYS, that is basically only for systems that don't implement crypt at all.

Error handling is underspecified. The spec doesn't give you enough well-defined behaviour, meaning everyone lists pages of error-handling extensions they have added.

[–] NewJazz 3 days ago

Which is notable because musl is more known for adhering to the spec to the point where they break programs that assume nonstandard behavior.

[–] silisili 3 days ago

> adhering to the spec

Pedantic, but more "author's understanding of the spec" which isn't always correct.

> assume nonstandard behavior

I go back and forth on this one (spec vs standard behavior). It feels like at some point the most widely used impl -is- the spec, as we've seen in other places (web standards).

[–] mort96 3 days ago

The spec is pretty unambiguous here: https://pubs.opengroup.org/onlinepubs/9799919799/functions/c...

> Upon successful completion, crypt() shall return a pointer to the hashed password [...]. Otherwise, it shall return a null pointer and set errno to indicate the error.

[–] shakna 2 days ago

You need to add that the spec _also_ only defines a singular error state, that isn't applicable here.

> The crypt() function shall fail if:

> [ENOSYS]

> The functionality is not supported on this implementation.

[–] mort96 2 days ago

I don't think those error sections are generally meant to be exhaustive?

[–] pdw 2 days ago

To the best of my knowledge, only OpenBSD and Fedora have a crypt(3) that returns NULL. Everybody else returns a string starting with "*" on error.

For example on Debian the manpage says:

> Upon error, crypt_r, crypt_rn, and crypt_ra write an _invalid_ hash to the output field of their _data_ argument, and crypt writes an invalid hash to its static storage area. This string will be shorter than 13 characters, will begin with a ‘*’, and will not compare equal to _setting_.

[–] mannyv 2 days ago

This is either a bad engineering choice or an intentional bug. An empty string would have been better. Why '*' instead of, say, a tilde, @ symbol, or something that generally isn't used?

[–] NewJazz 2 days ago

I'm not entirely sure an empty string, tilde, or @ symbol would really have been better, except in retrospect.