The ‘concise’ output style setting is bad with Opus 5.5, as it will often spit out a cryptic (is this really English?!), allergic-to-commas concise response, which takes more time to parse.
< mundane detail>
< If you don't run this command your computer will explode>
< exposition >
< mundane reminder to follow up something else >
Then 3 back and forths later, your computer explodes, and claude has the cheek to point out when questioned that it did warn you.In the past I could get an explanation, where I was able to track the line of thought. Now when I read through the explanation it is like someone has taken two or three explanations and merged them into each other, like there is a combining of iterations without clarity.
example:
The tree is blue.
The tall tree has the color of blue.
the tree, while tall, is blue.
That finally gets squished into: the blue tall tree has height and is evident of blue.
Is this double speak or injected double effort of words?
Except it turns out that at least one of those things is because I didn't enable JS, and the static text is wrong!
(Turning on JS did not actually help, because then the page is filled with horrible, cheesy animations and I can't actually read any of it.)
The surface-level hints are gone, but the attempt to make every sentence pithy, the decisions on what information to present and what to assume the audience knows, and the over-emotionalization are all still there.
And that just ends up indicting the C language itself. Can't send errors down a separate channel; folks might not inspect errno. Can't return NULL on error, folks might compare it blindly to another result. No tagged unions, so can't return one. Can't crash on error because there's no scoped error handling. Can't return a struct for checking .error/.result separately, because alignment and equality (and tearing and accidental clobbering and more) make that brittle as hell for widely-used APIs.
C is not a good language for library APIs. Like, really not. It has its uses/strengths, sure, but without even getting into the typical gripes people have about C, libraries--one of the things that it is used for the most--are one of the things for which it is the least well-suited.
"But it has a stable ABI!" So does a land mine. Until you touch it.
/// Hash the phrase with the given setting.
fn crypt(phrase: &[u8], setting: &[u8]) -> [u8; CRYPT_OUTPUT_SIZE];
Then, in a later version, `crypt` needs to become fallible, so they'd need to signal errors somehow. But because they really want to preserve backward compatibility, they can't replace the output with a Result type. And so they end up doing the same thing that was done in libcrypt - they output some error value and hackily make sure it's not equal to the `setting` string, under the mistaken assumption that the output of `crypt` would only ever be compared to the `setting` string. And then the same problem happens.Maybe I'm wrong, but it looks to me like the exact same incentives and ways to screw things up exist in C and in Rust for this particular bug.
The problem is not that you can write bad stuff in rust, the problem is that this particular usage pattern can't be sanely coded in C. This is a public API that should inform users and enforce certain properties. A Results type does exactly that, you would have to go way out of your way to force invalid stuff through that type alone.
int crypt2(const char *phrase, const char *setting, char *out);
You can call such a signature "insane" if you want, and that's fine, but it would have averted the bug that is the topic of discussion.For example, in a Rust web app, you can choose to use String for encoded URLs, unencoded URL fragments, escaped html, unescaped HTML, usernames, etc. Or you can choose to use newtypes and carefully design all APIs to only take and return the correct newtypes.
But if you have a stronger and more expressive type system, you are more inclined to use it, instead of looking for silly workaround like in C.
But in your version, you can't preserve backwards compatibility, making an infallible function fallible is a breaking change. For something comparable to libc you might want to (eg Rust std still has mem::uninitialized), but even with security issues, I feel like people would be more inclined to deprecate for a new function (or panic, which does have its own problems).
A good type system also means things are written with better forwards compatibility, and can be safely changed later. The Rust equivalent to `crypt` likely wouldn't exist: a low-level library would just provide hash functions, leaving callers to mitigate DOS etc; a broader library might return some opaque handle with equality/serialization, which you couldn't try to compare to a passwordless user; a high-level library would parse the account store itself. None of these really need to change in incompatible ways, they have all the information to add features and fixes at their own level of abstraction.
"Make invalid states unrepresentable" is such a useful adage when designing even basic systems, much less the gates to control your server.
It is the opposite that is unsafe, that is, when the returned value can't be NULL, but it stills returns a pointer. C++ has references in an attempt to fix this problem.
Because C tends to prioritize performance over safety, a NULL pointer dereference is considered undefined behavior and the compiler can invoke nasal demons if it makes the code go faster, but with the right options, you can have it checked and invoke what in other languages would call a panic handler, but on UNIX is the SIGSEGV signal. You can then attach a debugger or read the core file and see what went wrong.
Note: I am not talking about "char *" for strings, that one is unsafe, but NULL is not. It is just a special case of an option type, or a result type, or an error union. Modern languages just extended this idea to objects other than pointers, and with more than one type of error.
You don't have to think C is sound or good to distinguish a disfavored API style from mathematical unsoundness. There are infinitely many type safe APIs and languages that are, despite their soundness, ugly and hard to use.
I think almost all (tech) arguments stem from simple disagreements and/or different assumptions made on the definition of a (subjective) term.
That's a bit weird? In Rust, if you have an thing that returns Result<String, String> that's still type safe, even though success and error types are the same.
I think I know what you want to express..
You might say it's in-band vs out-of-band error signalling?
These types are not the same:
Ok<String>
Err<String>Nit: the type constructor is `Result`: given two types `A` and `B` (not instances of those types, but actual types) it gives you a new type `Result<A, B>`.
`Ok` and `Err` are called "variants" and can be used to construct a new _instance_ of `Result<A, B>`.
Of course that breaks down, when the success type is also an error message, eg when you are looking up something in a catalogue of error messages.
Rust has no trouble with this, for exactly the reasons you line out.
I had never heard that phrase, and was delighted at the answer when I googled it its origin, thank you:
http://www.catb.org/jargon/html/N/nasal-demons.html
> “When the compiler encounters [a given undefined construct] it is legal for it to make demons fly out of your nose” (the implication is that the compiler may choose any arbitrarily bizarre way to interpret the code without violating the ANSI C standard)
I've heard nasal demons invoked as a literal possibility. No they are not a literal possibility.
Which is zero, to be clear. There's not even a standard USB device class for a nasal demon releaser
A null check elision is not a demon up my nose - it's a null check elision.
That's... a bizarre choice, considering how religious C is about not making any assumptions about what a machine can and cannot do, even going to the trouble of working with non-IEEE floats and int types that are not a multiple of 8 bits.
There’s no requirement that the in-memory representation of a null pointer be all-zeroes, only that 0 in source code represents whatever it is.
Regardless, I agree that it’s a bizarre choice.
Without the ability to express a non-nullable type, C cannot be unsafe in this way!
Would this mean all pointers in C are type safe?
These are not the same, and your second sentence suggests why. Option has but a single error type, but result types and error unions allow arbitrary types of error.
That depends on how you interpret the pointer type, but in any case there are multiple interpretation and just the fact that these exist will create confusion and type unsafety.
If NASA were to use the equivalent of `pub struct Newtons(f32); pub struct PoundsForce(f32);`, where the types Newton and PoundsForce were not interchangeable, this would never have happened[1].
[1] I'm aware that that story is a bit more complicated than that, that CSV files were involved (which are, again, not type safe), and that, in that particular scenario, more would have to be done to preserve type safety across formats, vendors and codebases, but my point still stands.
https://lexi-lambda.github.io/blog/2020/11/01/names-are-not-...
> The newtype is opaque, so the only way to observe it is to convert it back to a [f32].
You don't even need a fancy result type. A simple Option<String> would've forced downstream users to consider the failure case, avoiding this situation in the first place.
Now what?
Once again, no, because these unknown programs would just crash on the `.unwrap()`; not get in a silently corrupted state. Same causes, different effects.
Whereas if the library returns something that is unambiguously an option type, consumers unambiguously call unwrap at their own risk.
Of course you do have a point (assuming I'm picking up what you're suggesting correctly) that all pointers in C are option types. But I think there's a cognitive ergonomics aspect to this where an option type more clearly relays the library authors' intent.
However a better API (to me) that I would have written was:
crypt_error_t crypt(...args, char *result, size_t resultLength)
That way btw the function doesn't need to allocate memory inside, which is (being a mostly embedded programmer) something I don't like at all. Plus returning an error enum makes it clear instead of having to look at errno.But POSIX API is not something well designed, it dates back to the '80 so... probably today would be written completely differently
Another example that C programmers love: using 0 to mean infinity. Eg when you have a timeout setting, and 10 means ten seconds, but 0 means infinity seconds instead of zero seconds.
What Rust calls an enum, but an enum with payloads. In C terms it would be a discriminated union.
In Haskell, the closest thing to adding 5 to an optional value would be something like:
fmap (5+) my_optional_value
This one leaves 'Nothing' alone, but adds 5 in case the value is present.However OpenSSH passes a different hash to musl’s crypt() than what it later uses to compare, so musl doesn’t switch to the alternate return output because from its perspective, it’s already guaranteed not to accidentally compare the same as the input hash.
wtaf ?
musl switched to returning `*` so folks stopped accidentally using NULL results, which was causing crashes and risked false-positives comparing NULL to itself.
That was a misfeature and should never have been implemented. But I bet it was an easy band-aid change to make!
Why * specifically? Why not some random guid generated before committing, if it's so important to not let buggy software crash?
But buggy software might not crash; it might compare NULL == NULL and misbehave, instead.
So there's really no good option here, which is why C is a rancid choice of language for publicly-usable APIs. But we're stuck with it. So it goes.
But issues caused by folks forgetting to check errno, miscomparing NULL to NULL when calling multiple APIs, dereferencing external-API-returned NULL due to missed error checking, mishandling inherited errno after successful action, or--the most common--mishandling negative integer return sentinels meant to indicate errors? Those are incredibly common.
Standard C api calls return an integer, which is 0 on success or nonzero on error. Ignoring it is an intentional choice. `errno` is the hack that shouldn't really exist because it's trivially easy to forget to check that and also has issues with threading (patched by making it a thread local) and more recently, coroutines (which admit no general purpose patch because they're a userspace construct).
Another reason why this is terrible is that it's unclear what the returned pointer's lifetime is. Is it until the next call to crypt? If so, is it thread-local like errno? If it's passed on to the user, is it safe to call free() on it, or is there a crypt_free function that should be called instead?
But all of this, and I mean all of it pales in comparison to returning '*' on error.
100% agree. This is an egregious failure. But C's error-handling story would be bad even without this, and the badness of C's error handling is what motivated the (bad) decision to return *.
It'd be great if we wrote systems software in a language that supported advanced features like...
checks notes
...returning multiple values.
Hell, even in VBA you had to at least spend the keystrokes to type "on error resume next".
If the password field contains some string that is not a valid result of crypt(3), for instance ! or *, the user will not
be able to use a unix password to log in (but the user may log in the system by other means).So they made the (very bad, but motivated by C's broken design/conventions) decision to return a magic string instead.
You can argue that C has broken design/conventions, but the musl decision was really really awkward.
0. There are tons of libc functions that return NULL on failure, it is conventional way in C to indicate failure.
1. There's no much to decide, just fail to login if crypt() fails with NULL.
2. errno is standard way in C to check error code of a failing function, but in case of login you hardly need this, just fail!
I totally agree that fixing this issue in musl was a pretty dubious solution. Don't fix what's not broken, fix broken application instead. Publish a CVE if you cannot fix it.
I looked through FreeBSD source code for how they use crypt(), it's checked for NULL everywhere!
Dude, they documented their rationale. It was dumb and wrong but totally relatable. Irresponsible users were getting crashes, and I’m sorry to tell you that for a lot of library maintainers
> Returning NULL on error is actually an excellent choice because most programs will crash if they try to use it without checking.
…is not something they want to write in responses to bug reports, even though it is quite true. It’s human nature to want to prevent your stuff causing problems for others, even if the problems are fully their fault. Most people cannot and will not be Steve “so don’t hold it like that” Jobs, for fear that their decision will be just as short-sighted as his was.
In this case, that was obviously the dead wrong call. But I’ve been in that seat, and “closed: user error” for the 100th time feels bad to write. Add to that a touch of misplaced cleverness, and you get “hey…what if we just returned an impossible string instead of an error?”
Evil requires motive or malice, and I’m having trouble coming up with those in this case. These patches weren’t concealed or miscategorized a la xz, the behavior was discussed in the open. They were just, in hindsight, really poor decisions made for ordinary reasons. Hanlon’s Razor and all that.
That's the job of a OSS library author.
Agreed, but I can’t think of any other widespread ABIs that are better. Rust’s is not stable both C++ and Rust have concepts that don’t map into other languages well, so using them wholesale is a no go. So it would probably need to be a subset of those. Honestly just tagged unions would probably be enough. Maybe the solution is to get native tagged unions into C…
There is the concept of "defense in depth": having each layer of code and configuration be sane and secure. Even if you program in Rust, you should be vigilant in that sense. This kind of code amounts to removing one or two layers.
Of all the things to mitigate...
1) crashing should be an option - it noisily indicates "there's a problem" and if it faults and dumps core, you've got a guided tour of your problem.
2) Is testing NULL as a passable option likely? I'd guess a more likely idiom is:
if(NULL==(str=somefunc(arg1, arg2))){
return err;
}> The behavior of crypt on errors isn't well standardized. Some implementations simply can't fail (except by crashing the program), others return a null pointer or a fixed string. Most implementations don't set errno, but some do. POSIX specifies returning a null pointer and setting errno, but it defines only one possible error, ENOSYS, in the case where crypt is not supported at all.
In my opinion the problem is that the behavior of crypt is underspecified - it isn't even specified how it signals failure, which is shocking.
You can't check errno to see if there was a failure because, as the man page says, the value of errno is unspecified when crypt succeeds.
> You can't check errno to see if there was a failure because, as the man page says, the value of errno is unspecified when crypt succeeds.
You can set errno to zero before the call. If the value is non-zero after the call, that is a good heuristic for failure.Wise words. I think early in my career I tended towards coding as defensively as possible in a very specific way of "don't crash", as I definitely was not working in codebases I felt I had a full picture of. "I don't know who calls this method, and I don't want to get a call in the middle of the night that I broke prod" so I would say, return an empty string when asked to format a datetime except I'm passed a string or an int. (Yes, I was working in dynamic, untyped languages, too).
Later on, even in untyped languages, I came around more to "Throw loud and specific exceptions when asked to do things that are nonsensical" - it requires more overall cojones though because it may cause "your code" to crash. But the realization I came to was that it's better to crash (a MaliciouslyLongPasswordError was uncaught) because "AuthManager asked PasswordService to check a password that was 2MB long" than to paper that over until something like this comes out. Or, the more common situation, you're passed garbage, you return something that won't choke the caller at the time, but what it saves to the database in that scenario ends up being corrupt data. 6 months later someone notices, and has to sort out a data mess that may or may not be recoverable.
I dig it.
That seems orchestrated.
https://lists.llvm.org/pipermail/llvm-dev/2019-June/133308.h...
> 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_.
Fedora asked libxcrypt to give them back glibc's behavior, returning NULL, and they obliged by adding ./configure --disable-failure-tokens. But it's an option that Debian never enabled.
https://github.com/besser82/libxcrypt/issues/25
https://salsa.debian.org/md/libxcrypt/-/blob/master/debian/r...
Of course given the chance that one might do a direct string comparison between crypt's return and the passwd/shadow file's hash field it was still a not well thought out choice.
So the knock-on effect is that libraries that do have errors other than ENOSYS to accommodate, have to return magic 'impossible' values instead of setting errno and returning NULL, because of the weight of existing applications that expect nothing but the standard, traditional AT&T Unix (and indeed old BSD), behaviour.
Check the GNU libc crypt() doco. It defines a whole family of 'impossible' values that signal errors, by dint of listing a load of magic characters that should never be in the encrypted string. Including asterisk.
Also check FreeBSD's OpenPAM for a case in point. Even today, it does not check for a NULL return from crypt() and leaps straight to strcmp().
* https://cgit.freebsd.org/src/tree/lib/libpam/modules/pam_uni...
If you want to see real history here, check out the 1992 comment by Eric Andrew Young, that is still in the source for OpenBSD today (in des/des_fcrypt.c), on how SSLeay's crypt() had to be adjusted so that any single character password field, including "*", did not end up successfully matching itself.
* https://cvsweb.openbsd.org/annotate/src/lib/libcrypto/des/At...
There is simply no universe in which a security-conscientious reviewer looks at the code, spots * being a valid return value, remembers that * is the default value in /etc/shadow for "no password login please" and then is all like:
This is fine, you check for * and return + instead.
There are more 32-long character sequences than atoms in the universe. Pick one, any of them instead.
Damn, that's a bug! Please file bug report.
* https://cgit.freebsd.org/src/tree/lib/libcrypt
As I said, the callers of crypt() have for a long time been using the knowledge that crypt() on their platform simply never returns NULL. Unix has never been returning NULL from crypt() as far back as 7th Edition, when it simply always returned a pointer to a single internal static buffer.
* https://github.com/dspinellis/unix-history-repo/blob/Researc...
The reality is that all C library people have to work with a corpus of Unix applications that do not check for crypt() returning NULL that goes back almost 50 years, and an error return mechanism that barely exists outwith the pages of the SUS.
That includes callers in single-user mode in process 1 in some configurations.
* https://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/init/init.c?an...
* https://cgit.freebsd.org/src/tree/secure/lib/libcrypt/crypt-...
* https://man.freebsd.org/cgi/man.cgi?query=crypt&sektion=3&fo...
NetBSD's manual is more detailed on the subject, and explains the bind that library implementors have been in for decades, thanks to almost 50 years of applications knowing what crypt() 'really' does.
* https://man.netbsd.org/crypt.3#RETURN%20VALUES
And this despite the SUS trying to shoehorn an error return mechanism into crypt(3) in order to accommodate the problems that operating systems had exporting encryption code, two decades afterwards. (The 1997 GoSolo book explains that this is why there's an ENOSYS error.)
No. https://man7.org/linux/man-pages/man3/crypt.3.html
My guess is that you are reading the libxcrypt crypt(3) manual: https://www.mankier.com/3/crypt
That's not GNU libc, but frequently deployed on Linux systems.
* https://manpages.debian.org/trixie/libcrypt-dev/crypt.3.en.h...
Although that's a fine distinction for the point being made, as it's still the manual page for the crypt() function that one gets on the operating system, which that other manual page is not.
* https://sourceware.org/git/?p=glibc.git;a=commit;h=e6e3c6668...
Notice that Debian didn't list your manual page, which doesn't accompany a crypt() function at all, since it isn't actually the GNU libc manual but a separate independent manual for Linux. The GNU libc manual nowadays acknowledges Michael Glad's work but no longer documents crypt(). It used to, and it used to explain some of the 'impossible' encrypted string values:
* https://sourceware.org/glibc/manual/2.38/html_node/Passphras...
You noticed that I'd not got the GNU libc manual. Neither had you. (-:
Not to programmatically parse 'ls' output or some /proc/some/file.
Eg. git started as Perl scripts then moved to C to realy have data straight from C calls from system.
So for example all Internet things calling "openssl -some -options ...." are pure trash :>>
I am also in the camp that CLI tools should return objects that can be parsed (by the shell or otherwise).
OpenSSL??
> $ret = crypt($pw, $hash); $ret == $hash
musl is actually safe against this use. OpenSSL takes a bizarre code path, detects that $hash is too short, calls crypt($pw, $other_string), forgets that it detected that the hash was too short, and checks $hash == return value.
See my other comment.
A typo: `$ret = crypt($pw, $salt)`
The goal from an attacker’s perspective is to get this to be true: `$ret = crypt($pw, $hash); $ret == $hash` when called by OpenSSH’s sshd. This is how sshd validates that the user logging in knew the account password.
The `$hash` is the “settings” to the crypt call and will normally look like “$6$--gibberish” for users with passwords set, but could look like “*” or “!” for users not intended to be able to login.
The bug is from the intersection of all of these things:
1. musl can return “*” for errors, rather than leave the return value NULL. Other libc have adopted something like this too, but seem to use a different result “*0”.
2. musl can also return “+” for errors, but only does so if $hash starts with “*”. This is intended to eliminate the possibility of an error causing `$ret == $hash` to be true, however it assumes that applications calling `crypt()` will set $pw and $hash to values corresponding to the `$ret == $hash` comparison to be made immediately afterwards.
3. OpenSSH can be convinced to pass the hash of a different user if the hash is “*” or “!” to avoid timing attacks that can enumerate the user list, so it will set $pw and $hash to correspond to different users. $hash will be a real “$6$--gibberish” style hash in this case, and OpenSSH expects `crypt()` to either return NULL or return a hash string. But now there’s two hashes OpenSSH uses. It does: `$ret = crypt($pw, $hash2); $ret == $hash1`;
4. This should normally not be a problem because OpenSSH will pass in valid $pw and $hash2 that should not lead to errors in hashing, so the $ret that musl generates should be a real hash that cannot match the “*” or “!” $hash1 for the login-disabled user. However as a DDoS mitigation, musl will return error if the password is too long when using SHA-2-based hashes (the default, it seems).
5. This error will result in a non-NULL result of “*” or “+”. Since OpenSSH passed in a real hash ($hash2), musl will return “*”, which compares equal to the actual user hash $hash1 for the disabled-login user.
The OpenSSL adjustment ensures a minimum hash length for the user before logging them in, even if crypt() succeeds somehow.