There is nothing inherently C-caused in this vulnerability.
Nothing is inherently caused by anything, in the sense that you can achieve any desired outcome with any given tool, if you put enough effort into it. Yet, we still say that there are good and bad tools, and similarly, there are languages that make it easier or harder to follow best practices.
In-band signaling with sentinel values is an astonishingly bad practice which is made astonishingly pervasive by C's bad type system.
In this case the problem is that the specification for crypt() doesn’t allow for error cases except for the complete absence of crypt() itself. In practice most code is written as if crypt(&str, &str) -> String (or perhaps more accurately with the moral equivalent of .unwrap()) which in the past was reasonable when the code only works on implementations that have crypt().
So the question is then, how should crypt() evolve to support error cases that were not previously considered, such as malformed salt, salt that specifies an unsupported algorithm, unreasonably long passwords, etc. And there are two levels of answer to this question: at a small scale, what can or should the crypt() API look like, and at a larger scale, what are the standards and compatibility considerations for the various options.
At the small scale, musl decided to return an in-band value that is poorly chosen because it conflicts with existing assumptions about what strings crypt() can return. (A better choice would have been “:” since that cannot appear inside a field in the password file, so it can never match an account that doesn’t allow password logins.) Other implemenentations have chosen to expand the vestigial NULL return so that it can be used for dynamic errors that did not previously exist.
At the larger scale, the better choice depends on how the libc maintainers view their relationship with the programs that use libc and with POSIX. If you view the ecosystem as essentially fixed, too difficult to change, then musl’s in-band error is reasonable (except for the extremely dangerous choice of error value). On the other hand, if you think that the amount of effort needed to fix callers is small enough then it’s reasonable to improve the API.
Then there’s the question of how to fix POSIX so that it doesn’t lead to poor choices like musl’s. And that depends on whether changes to POSIX should occur first in the spec or first in implementations (or perhaps in parallel). I tend to think that specs should not be treated as if they were handed down by god, and when (as in this case) there’s an obvious way to fix a bad API, implementations shouldn’t be scared to lead the effort to fix it by writing code that deviates from the spec, especially if that is likely to encourage the spec to be changed to match.
musl decided to return an in-band value that is poorly chosen because it conflicts with existing assumptions about what strings crypt() can return. (A better choice would have been “:” since that cannot appear inside a field in the password file, so it can never match an account that doesn’t allow password logins.)
To be clear, musl doesn't return "*" for accounts that don't allow password logins, it returns "x" in those scenarios (source). The reason why "*" was returned in OpenSSH was because as you noted in your summary, OpenSSH after determining that the account doesn't support password authentication and the request should fail, decides to call crypt with a different users salt to burn time. But then decides to act on the result on the call even though it already had determined that the login attempt should fail.
I can see how might one argue that musl's choice of leaving a boobytrap for people that are misusing the crypt API is anti-social behaviour. But the lions share of the vulnerability lies in OpenSSH.
Then there’s the question of how to fix POSIX so that it doesn’t lead to poor choices like musl’s. And that depends on whether changes to POSIX should occur first in the spec or first in implementations
OpenSSH's upstream, OpenBSD stopped using crypt in 2007 (auth-password.c). Replacing crypt with auth-usercheck. That's plenty of time for the spec to catch up to implementations.
OpenSSH after determining that the account doesn't support password authentication and the request should fail, decides to call crypt with a different users salt to burn time. But then decides to act on the result on the call even though it already had determined that the login attempt should fail.
I'm guessing that it was acting on the result to prevent an optimizing compiler from discarding the entire call to crypt() — compiler optimizations are known for breaking constant–time cryptography. In-band invalid result seems like a reasonable way to make sure that both paths have identical timings – they execute exactly the same code. As long as the invalid result is actually invalid, that is.
In this case the problem is that the specification for crypt() doesn’t allow for error cases except for the complete absence of crypt() itself.
This is a fundamental misunderstanding of how POSIX works. Functions specified by POSIX can define errnos that either must or may be set in certain circumstances, but unless the function's specification explicitly states otherwise, then implementations may return any other appropriate errno as well.
Implementations shall not generate a different error number from one required by this volume of POSIX.1-2024 for an error condition described in this volume of POSIX.1-2024, but may generate additional errors unless explicitly disallowed for a particular function.
That’s a good point, but you also need to consider the preceding sentence:
Implementations may generate error numbers listed here under circumstances other than those described, if and only if all those error conditions can always be treated identically to the error conditions as described in this volume of POSIX.1-2024.
In the case of crypt() the difficulty is introducing errors that depend on the password and/or salt, because those kinds of errors need to be handled differently from the only error defined in POSIX (which will always fail or always succeed). It’s a practical difficulty too, because (as I wrote) there is old software that doesn’t check for errors from crypt() because historically it had no errors.
In the case of crypt() the difficulty is introducing errors that depend on the password and/or salt, because those kinds of errors need to be handled differently from the only error defined in POSIX (which will always fail or always succeed).
Not at all, musl could just as well choose to return a single errno value for all cases in which crypt was called with invalid arguments, which would be infinitely more expressive than what it currently does.
It’s a practical difficulty too, because (as I wrote) there is old software that doesn’t check for errors from crypt() because historically it had no errors.
That's not really true, it always had errors across various implementations, people failing to care doesn't mean that it could never happen.
Musl used to report errors from crypt but decided to stop doing so because C programmers don't know to check for NULL pointers. In the process they made buggy programs not crash but also made correct programs misbehave. That seems like a clear bug to me.
And by that rationale we should stop signaling errors everywhere that returns NULL because people skip NULL checks everywhere, including when calling malloc.
musl could just as well choose to return a single errno
Are you sure? POSIX doesn't normally allow routines to just invent new errors to be returned, so if you have surveyed a large number of POSIX-compatible applications that use crypt() and can report to them that programs will tolerate it, then you can just write to them and then yes, musl could use another errno.
I suspect that many applications don't check crypt() since it presently only returns ENOSYS and so fall-back on the assumption that the database is plain-text on every error which might be just as bad.
I think it would have been better to simply to put crypt() in the same bin we put gets()
POSIX doesn't normally allow routines to just invent new errors to be returned
[citation needed].
So sayeth POSIX:
Implementations may support additional errors not included in this list, may generate errors included in this list under circumstances other than those described here, or may contain extensions or limitations that prevent some errors from occurring.
[…] Implementations shall not generate a different error number from one required by this volume of POSIX.1-2024 for an error condition described in this volume of POSIX.1-2024, but may generate additional errors unless explicitly disallowed for a particular function.
Anyway, musl doesn't need to invent a new errno here, this is covered by EINVAL, which can be returned by any function defined by POSIX unless the definition explicitly states that it may not return other errors.
(And this is exactly what OpenBSD does on invalid salts, btw. See crypt.c and related files.)
The same norms the article points to would apply just as well here, IMO.
Evolving an unsound API would come with a lot of careful consideration and review, and in-band signaling wouldn't even enter into the discussion. The impact would be measurable with Crater, and automated tooling might even be developed if feasible to fix callers.
I think the point of the article is that Rust-the-engineering-culture is what matters. Rust-the-language is one expression of that culture.
If you view the ecosystem as essentially fixed, too difficult to change, then musl’s in-band error is reasonable (except for the extremely dangerous choice of error value). On the other hand, if you think that the amount of effort needed to fix callers is small enough then it’s reasonable to improve the API.
This is one place where static linking has a somewhat surprising security advantage: when your dependents must relink/recompile, there's less need to "fix the ecosystem despite itself" with hacks like these that come back later to bite you. Static linking makes the ecosystem more coherent.
People also miss an important detail. While "" is a weird return, and I agree ":" seems like a strictly better option in all cases, musl does check if the first character of the "salt" is "" before returning "*" and instead returns "+" in that case.
The confounding factor here was that OpenSSH chose to pass one user's "salt" to crypt and then compare the result with a different user's "salt". I suspect that maybe it was to avoid accidentally introducing another timing side-channel, but I wonder why this was done instead of comparing against the same "salt" and then discarding the result.
In-band signaling with sentinel values is an astonishingly bad practice which is made astonishingly pervasive by C's bad type system.
Those are a lot of opinions!
I don't agree that "in-band signalling with sentinel values" are "inherent" to C, but perhaps poor choices young programmers make, and my opinions is that software risk, whether that is a "security vulnerability" or just annoyingly buggy, largely comes from the dogmatic teaching/learning process most young programmers experience with software.
I know a lot of people have wasted a lot of time looking for tools to make them better, but that's just not how tools work.
Sometimes. Sometimes it's a great way to organize memory. For instance, a pool of interned null-terminated strings can be referenced using only 4 bytes each, as opposed to 16 bytes each for a pointer and length. If you have a lot of references to short strings, the savings are quite meaningful.
This has nothing to do with C or any programming language; this is a fundamental property of information theory.
Furthermore, it is possible to model sentinel-terminated pointers in the type system.
For instance, a pool of interned null-terminated strings can be referenced using only 4 bytes each, as opposed to 16 bytes each for a pointer and length.
If you can address terminated string using 4 byte indexes then you can do that with length-specified ones as well, and obviously if the entire buffer is accessible using 4 byte indexes you don't need more than that for the length. And if you have "a lot of references to short strings" then you would probably get away with 2 or even 1-byte lengths. And now you have an overhead of 5 bytes per, which is exactly what you have with terminated strings.
And as a bonus the scheme now doesn't break when the strings are e.g. hashes which can contain nulls.
I think you're using "sentinel value" different than I (and probably the person you're replying to) do. I use it to mean "value that occupies the same type as the non-error return values". As you correctly say, it is possible to have a sentinel-like representation in memory, which can be very efficient, while still having the type system differentiate between sentinels and non-error values at the language level.
It doesn't have a type system able to express "this is a failure that the caller must handle". Error handling in C is enforced by shaming programmers, which demonstrably is less effective than compile-time errors.
It doesn't have a type system able to express "this is a failure that the caller must handle".
C23 has [[nodiscard]] functions, and there has been an equivalent feature for decades in GNU C.
But in the case of crypt() the caller needs to use the return value to check the password, and if errors are represented by returning NULL in the idiomatic manner, then a caller will crash if it fails to handle the error.
So this vulnerability would not have occurred if crypt() was written in idiomatic C.
The vulnerability is caused in part because musl doesn’t return an error in the idiomatic way, but instead signals an error with a value that can match a disabled account (which should fail) in the same way that a successful return matches an account with the right password (which should succeed). (And in part because OpenSSH uses a dummy salt for checking disabled accounts.)
It doesn't have a type system able to express "this is a failure that the caller must handle".
To be fair, most programming languages don't. But at least languages like Rust nudge you very strongly, and most importantly, makes handling the potential error super ergonomic and ignoring it awkward.
Your opinion. I think it was the programmer, but I tend to not blame tools.
It doesn't have a type system able to express "this is a failure that the caller must handle"
That's your lack of imagination.
In my own APIs, I use negative pointers for special values I need to handle specially- not just errors, but also ciphertext. That means if someone gets it wrong they get SIGSEGV and will fix it quick. Such a thing is impossible in most languages but trivial in C.
I think it was the programmer, but I tend to not blame tools.
We really, really need to rid ourselves of this harmful, outdated safety culture. Every other engineering discipline has learned decades ago that "operator error" is not acceptable as a root cause analysis. If the same tool or system repeatedly causes humans to make certain mistakes, it needs to be changed.
That's like saying wearing reflective safety vests is useless because the photons that are hitting the retina of the driver do not deccelerate the car that moves towards you. Which is technically true but also a total non-sequitur.
I know, and I knew this would come up, but I didn't get around to correcting the phrasing. The fact that we are having this discussion shows how suitable the metaphor is ;-)
It is considered massless and having zero rest mass, but it has momentum. Just not the Newtonian kind of momentum which is proportional to mass and velocity (p=mv). Instead Einstein came up with a relationship between the rest mass, momentum and energy of a particle: E²=(pc)²+(mc²)².
One may work backwards from there and invent a quantity known as the "relativistic mass" which would be non-zero for a photon, but it's fallen out of fashion among physicists as far as I'm aware. The more famous formulation of Einstein's equation uses relativistic mass (or assumes no motion).
(I don't know the exact history of the above, perhaps they came up with relativistic mass first, but this is how I like to think of it).
A photon's energy is inversely proportional to its wavelength (E=hc/λ) and the momentum may be derived from there to be p=h/λ. h is Planck's constant.
The post specifies a reason why the author think that C programming leads to this behavior.
You’re free to argue this reason is wrong. But you cannot responsibly just ignore the reason and state that this is not an issue with C. You might as well just say “No.”
Yes there is, in any language with proper union types you would be fired for even suggesting such a thing as representing NULL by "*". This is truely just a case of C having an insufficiently powerfull typesystem to express things correctly.
The idea behind crypt is fantastic: having pluggable and configurable hashing mechanisms which (in theory at least) can be transparently upgraded when a user logs in allows for longevity of a single API in a constantly changing reality, while staying backwards compatible.
The actual API leaves a lot to be desired of course, and can be traced back to wanting to stay backwards compatible for many decades.
What if the API was initially designed such that there was initially no concept of a user without a password, and you needed to compatibly shoehorn something in after the fact without breaking millions of lines of existing, ISO-standardized APIs?
Do you design every Haskell API with the idea that some day someone may add an error case, even though none currently exist?
That would be an issue, but in this case crypt was defined to be fallible from the start, and that's one of the key players here. In the C version, crypt returned a string or null, but the type system doesn't differentiate between those two, so it was easier for callers to incorrectly handle the return value. But in Haskell, crypt would have returned Either (I think?) with the string or an explicit error. And at that point, the caller cannot implicitly ignore anything, they have to deal with both cases explicitly, and the problem would never have shown up in the first place.
As I understand it from fanf's summary, the error case has always been present, but originally it was only used if crypt wasn't available on a particular system. Based on current Rust API norms, I imagine that would probably have been written as Result<String, CryptError>, (or possibly a broader error type), with that error being non-exhaustive to allow for future possible error codes.
All that said, I think this is a bit of a "if my grandmother had wheels, she'd be a bicycle" sort of discussion. A lot of Rust's design philosophies, and the tendencies in the Rust community, come from direct experience of the issues of C API design. There is no world where libc would have been designed then according to modern design sensibilities, and a lot of the constraints that were considered important then are no longer seen as important, while other issues (like designing APIs that can be evolved) were not as high a priority.
I think the interesting observation here is how we can develop future APIs - and how important things like ADTs are for designing those APIs in such a way that the constraints and behaviours of the function are clear to callers of that function.
What if there isn’t an error variant today? Then a breaking change would add an error variant, and a RustSec advisory would probably be published as well.
I suspect the same would apply to Haskell. (Or maybe not, but I didn't bring up Haskell.)
but you wouldn't get the same logic in Haskell or whatever, because languages with better constructs for exceptional results use them.
C APIs don't even agree whether returning 0 means false or not-an-error-code. This is absolutely a C problem: primitivism of the language lead to a widespread problem of inconsistent ad-hoc error handling. Inability to enforce proper error handling by the compiler leads to the vicious cycle of sloppy library users summoning nasal demons, and library authors mitigating that with the crappy sentinels.
Expect for the part where there is no reason to have the same logic written in Haskell in the first place, because the issue that was fixed by this logic (callers not handling the error case) is not possible: a hypothetical haskell crypt would have returned Maybe String at the very least and the caller would have not been able to ignore the None.
And then there is a second layer of having no reason to have the same logic written in Haskell: doing in-band signalling in Haskell is abnormal.
So as the essay explains this is a C issue, an issue of the culture and norms of C programming, as well as (much like memory safety even if it’s not that) the weakness of the C language and its type system.
IOW, you can write C in any language. But the point is that this bad error handling, which is idiomatic in C, would be a big red flag in Haskell, Rust, and many other languages.
The fact that AWS offers official images that use a libc reimplementation that's an endless source of bugs is a disservice to their customers.
It is very good and important that musl exists as an alternative to glibc, and I question whether characterizing it as an "endless source of bugs" is fair.
I love C, I pretty much did C for the past twenty years of my life.
Yet I acknowledge that is a C problem (but Python, JavaScript, ... also have the same problem). We must move to an non legacy language when possible (I'm not saying Python or JavaScript are legacy).
We have enough computer power and modern languages to have good type systems and rule out most of the bugs. Yet we don't have enough "human power" to move all the legacy code to modern alternative.
It's not a flame war. Just saying that doing any new modern technology in C is absurd nowadays.
JS and Python don't have this problem - such APIs can (and do) throw exceptions instead of relying on callers checking for a special sentinel value. They can also just return undefined/None without worrying about callers causing UB.
The article implies that Rust is magically more secure. uutils audit discovered 44 CVEs for things that are well known by experienced C programmers (many TOC/TOU) and not covered by Rust type system. These CVEs are the reason moving away from C for existing things is not as simple.
One of the first professional projects of my career was solving numerous TOCTTOUs in Meta's Watchman file system monitor, written in C. Required tons of man page reading and lots of careful reasoning and empirical analysis. There was a lot of random C bullshit I had to deal with as well; I'd much rather be solving TOCTTOU issues in Rust.
I do think it was a bit suboptimal for uutils to be thrust into the limelight like this.
Not to be too obtuse but C doesn't help out with TOC/TOU either right?
Like in theory it feels like C programmers who understand these issues well would be able to use Rust effectively, and provide APIs to end users that are safer as a result of being burned so much in the past, right?
Or is there something I'm missing here. Is C giving us something here? Or is this more of an inertia argument + "rewrites intro these risks" thing?
No, C does not help at all, but I think this is a well-known issue for C programmers (because it was a source of vulnerabilities a long time ago), while this may not be the case recently (as "demonstrated" by the uutils vulnerabilities). I was mostly answering to the argument that Rust is also safer because of the culture around it, not only because its intrinsics qualities.
The article implied no such thing. It pointed out that Rust gives you tools to create safer API's than C does and that the presence of those tools creates a culture where you are more likely to interact with and produce safer APIs than C.
Rust is not the only language that does so but it does stand out by having both those tools and also the performance of C.
Is there any technical constraint preventing C from acquiring a result type and/or multi-value return? I get there are social constraints and ecosystem issues that could arise but are those the only reasons?
Technically it seems doable to extend enum and switch to have at least some minimal tagged unions and basic pattern matching.
I think the problem is political and social:
Microsoft thinks C is dead, so they won't add any such syntax to MSVC unless C++ also adopts it, but C++ won't adopt it because they have std::variant at home. Microsoft's veto has chilling effect on C standardization and adoption of features. Projects worrying about portability and back compat would end up wrapping it in macros, creating ugly syntax nobody likes.
Remaining C users like it exactly the way it is, and will resist addition of a new design pattern.
Even without these obstacles it will take few years for the C working group to consider it, then a few more years before it lands in a C spec, then few more years before compilers ship it, then a few more years before distros switch to those compilers, then a few more years before notable C projects consider the syntax old enough to be acceptable to migrate to it. And even then, stdlib and POSIX probably won't ever change.
Pretty sure you can already sort of pull off multi-value returns just by returning a struct by value. Result types can be easy or hard depending on how strict you want compile-time checks to be (basic discriminated unions are easy; statically ruling out misuse is harder, especially given C's relative paucity of information hiding options).
Also I feel that once you drop social and ecosystem constraints there probably isn't much left to prevent any language from changing in arbitrary ways?
Technically, C programs not checking the return value for NULL is equivalent to a Rust program just calling unwrap, and the library then changing to never return an error.
I also think that would be frowned upon in both languages. However the difference is that Rust doesn’t implement a completely ossified API where callers expect to continue to work forever even if they misuse the API.
In fact we have seen at least a Cloudflare outage being caused by overuse of unwrap, so I’m not sure there is a strong claim that this problem cannot happen in rust.
The big difference is that having your program safely crash with a panic is better than not crashing and accidentally authenticating any user as root. I'll take a cloudflare outage caused by crashlooping over an unsafe auth check by accident any day.
The engineering norms of C developers allow for "*" to be an acceptable sentinel value indicating an error; the engineering norms of Rust developers simply do not.
I call BS. This is empty Us vs. Them rhetoric masquerading as language advocacy.
The icing on the turd is writing "*sshole" instead of, say, "ss*hole". Who does that?
Rust has Option, Result and panic!() for such cases, and there's a decade of real-world experience and hundreds of thousands of libraries demonstrating that using them is indeed the norm.
OTOH C has been using sentinel values forever. EOF is in stdlib. NULL is normal and the standard C doesn't even have non-nullable pointers. In-band signalling without compiler enforcement is the norm in C.
The name is sshhole, but the page title reads as *sshole, the 'h' is only shadowed, and the first character is an asterisk.
This is confusing, and it's all just for a juvenile pun.
lorddimwit | 17 hours ago
There is nothing inherently C-caused in this vulnerability.
intelfx | 17 hours ago
Nothing is inherently caused by anything, in the sense that you can achieve any desired outcome with any given tool, if you put enough effort into it. Yet, we still say that there are good and bad tools, and similarly, there are languages that make it easier or harder to follow best practices.
In-band signaling with sentinel values is an astonishingly bad practice which is made astonishingly pervasive by C's bad type system.
hailey | 17 hours ago
In-band signalling in this context is also known as the semipredicate problem and like you say, it is pervasive in C
fanf | 17 hours ago
I wrote a summary of the things that led to this bug and one of the crucial issues that this article misses is standards conformance, and how standards can or don’t evolve to deal with new circumstances.
In this case the problem is that the specification for crypt() doesn’t allow for error cases except for the complete absence of crypt() itself. In practice most code is written as if crypt(&str, &str) -> String (or perhaps more accurately with the moral equivalent of .unwrap()) which in the past was reasonable when the code only works on implementations that have crypt().
So the question is then, how should crypt() evolve to support error cases that were not previously considered, such as malformed salt, salt that specifies an unsupported algorithm, unreasonably long passwords, etc. And there are two levels of answer to this question: at a small scale, what can or should the crypt() API look like, and at a larger scale, what are the standards and compatibility considerations for the various options.
At the small scale, musl decided to return an in-band value that is poorly chosen because it conflicts with existing assumptions about what strings crypt() can return. (A better choice would have been “:” since that cannot appear inside a field in the password file, so it can never match an account that doesn’t allow password logins.) Other implemenentations have chosen to expand the vestigial NULL return so that it can be used for dynamic errors that did not previously exist.
At the larger scale, the better choice depends on how the libc maintainers view their relationship with the programs that use libc and with POSIX. If you view the ecosystem as essentially fixed, too difficult to change, then musl’s in-band error is reasonable (except for the extremely dangerous choice of error value). On the other hand, if you think that the amount of effort needed to fix callers is small enough then it’s reasonable to improve the API.
Then there’s the question of how to fix POSIX so that it doesn’t lead to poor choices like musl’s. And that depends on whether changes to POSIX should occur first in the spec or first in implementations (or perhaps in parallel). I tend to think that specs should not be treated as if they were handed down by god, and when (as in this case) there’s an obvious way to fix a bad API, implementations shouldn’t be scared to lead the effort to fix it by writing code that deviates from the spec, especially if that is likely to encourage the spec to be changed to match.
PuercoPop | 9 hours ago
To be clear, musl doesn't return
"*"for accounts that don't allow password logins, it returns "x" in those scenarios (source). The reason why"*"was returned in OpenSSH was because as you noted in your summary, OpenSSH after determining that the account doesn't support password authentication and the request should fail, decides to call crypt with a different users salt to burn time. But then decides to act on the result on the call even though it already had determined that the login attempt should fail.I can see how might one argue that musl's choice of leaving a boobytrap for people that are misusing the crypt API is anti-social behaviour. But the lions share of the vulnerability lies in OpenSSH.
OpenSSH's upstream, OpenBSD stopped using crypt in 2007 (auth-password.c). Replacing crypt with auth-usercheck. That's plenty of time for the spec to catch up to implementations.
Psentee | 5 hours ago
I'm guessing that it was acting on the result to prevent an optimizing compiler from discarding the entire call to
crypt()— compiler optimizations are known for breaking constant–time cryptography. In-band invalid result seems like a reasonable way to make sure that both paths have identical timings – they execute exactly the same code. As long as the invalid result is actually invalid, that is.remmyfoo | 14 hours ago
This is a fundamental misunderstanding of how POSIX works. Functions specified by POSIX can define errnos that either must or may be set in certain circumstances, but unless the function's specification explicitly states otherwise, then implementations may return any other appropriate errno as well.
— https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_03
fanf | 14 hours ago
That’s a good point, but you also need to consider the preceding sentence:
In the case of crypt() the difficulty is introducing errors that depend on the password and/or salt, because those kinds of errors need to be handled differently from the only error defined in POSIX (which will always fail or always succeed). It’s a practical difficulty too, because (as I wrote) there is old software that doesn’t check for errors from crypt() because historically it had no errors.
remmyfoo | 13 hours ago
Not at all, musl could just as well choose to return a single errno value for all cases in which
cryptwas called with invalid arguments, which would be infinitely more expressive than what it currently does.That's not really true, it always had errors across various implementations, people failing to care doesn't mean that it could never happen.
Musl used to report errors from
cryptbut decided to stop doing so because C programmers don't know to check for NULL pointers. In the process they made buggy programs not crash but also made correct programs misbehave. That seems like a clear bug to me.And by that rationale we should stop signaling errors everywhere that returns NULL because people skip NULL checks everywhere, including when calling
malloc.geocar | 10 hours ago
Are you sure? POSIX doesn't normally allow routines to just invent new errors to be returned, so if you have surveyed a large number of POSIX-compatible applications that use crypt() and can report to them that programs will tolerate it, then you can just write to them and then yes, musl could use another errno.
I suspect that many applications don't check crypt() since it presently only returns
ENOSYSand so fall-back on the assumption that the database is plain-text on every error which might be just as bad.I think it would have been better to simply to put crypt() in the same bin we put gets()
remmyfoo | 5 hours ago
[citation needed].So sayeth POSIX:
— https://pubs.opengroup.org/onlinepubs/9799919799/functions/V2_chap02.html#tag_16_03
Anyway, musl doesn't need to invent a new errno here, this is covered by
EINVAL, which can be returned by any function defined by POSIX unless the definition explicitly states that it may not return other errors.(And this is exactly what OpenBSD does on invalid salts, btw. See
crypt.cand related files.)stringy | 10 hours ago
The same norms the article points to would apply just as well here, IMO.
Evolving an unsound API would come with a lot of careful consideration and review, and in-band signaling wouldn't even enter into the discussion. The impact would be measurable with Crater, and automated tooling might even be developed if feasible to fix callers.
I think the point of the article is that Rust-the-engineering-culture is what matters. Rust-the-language is one expression of that culture.
moltonel | 6 hours ago
This is one place where static linking has a somewhat surprising security advantage: when your dependents must relink/recompile, there's less need to "fix the ecosystem despite itself" with hacks like these that come back later to bite you. Static linking makes the ecosystem more coherent.
atk | 2 hours ago
People also miss an important detail. While "" is a weird return, and I agree ":" seems like a strictly better option in all cases, musl does check if the first character of the "salt" is "" before returning "*" and instead returns "+" in that case.
The confounding factor here was that OpenSSH chose to pass one user's "salt" to
cryptand then compare the result with a different user's "salt". I suspect that maybe it was to avoid accidentally introducing another timing side-channel, but I wonder why this was done instead of comparing against the same "salt" and then discarding the result.geocar | 10 hours ago
Those are a lot of opinions!
I don't agree that "in-band signalling with sentinel values" are "inherent" to C, but perhaps poor choices young programmers make, and my opinions is that software risk, whether that is a "security vulnerability" or just annoyingly buggy, largely comes from the dogmatic teaching/learning process most young programmers experience with software.
I know a lot of people have wasted a lot of time looking for tools to make them better, but that's just not how tools work.
andrewrk | 9 hours ago
Sometimes. Sometimes it's a great way to organize memory. For instance, a pool of interned null-terminated strings can be referenced using only 4 bytes each, as opposed to 16 bytes each for a pointer and length. If you have a lot of references to short strings, the savings are quite meaningful.
This has nothing to do with C or any programming language; this is a fundamental property of information theory.
Furthermore, it is possible to model sentinel-terminated pointers in the type system.
masklinn | 7 hours ago
If you can address terminated string using 4 byte indexes then you can do that with length-specified ones as well, and obviously if the entire buffer is accessible using 4 byte indexes you don't need more than that for the length. And if you have "a lot of references to short strings" then you would probably get away with 2 or even 1-byte lengths. And now you have an overhead of 5 bytes per, which is exactly what you have with terminated strings.
And as a bonus the scheme now doesn't break when the strings are e.g. hashes which can contain nulls.
muvlon | 7 hours ago
I think you're using "sentinel value" different than I (and probably the person you're replying to) do. I use it to mean "value that occupies the same type as the non-error return values". As you correctly say, it is possible to have a sentinel-like representation in memory, which can be very efficient, while still having the type system differentiate between sentinels and non-error values at the language level.
kornel | 14 hours ago
C failed to prevent such preventable problem.
It doesn't have a type system able to express "this is a failure that the caller must handle". Error handling in C is enforced by shaming programmers, which demonstrably is less effective than compile-time errors.
fanf | 13 hours ago
C23 has [[nodiscard]] functions, and there has been an equivalent feature for decades in GNU C.
But in the case of crypt() the caller needs to use the return value to check the password, and if errors are represented by returning NULL in the idiomatic manner, then a caller will crash if it fails to handle the error.
So this vulnerability would not have occurred if crypt() was written in idiomatic C.
The vulnerability is caused in part because musl doesn’t return an error in the idiomatic way, but instead signals an error with a value that can match a disabled account (which should fail) in the same way that a successful return matches an account with the right password (which should succeed). (And in part because OpenSSH uses a dummy salt for checking disabled accounts.)
gspr | 6 hours ago
To be fair, most programming languages don't. But at least languages like Rust nudge you very strongly, and most importantly, makes handling the potential error super ergonomic and ignoring it awkward.
geocar | 10 hours ago
Your opinion. I think it was the programmer, but I tend to not blame tools.
That's your lack of imagination.
In my own APIs, I use negative pointers for special values I need to handle specially- not just errors, but also ciphertext. That means if someone gets it wrong they get SIGSEGV and will fix it quick. Such a thing is impossible in most languages but trivial in C.
muvlon | 8 hours ago
We really, really need to rid ourselves of this harmful, outdated safety culture. Every other engineering discipline has learned decades ago that "operator error" is not acceptable as a root cause analysis. If the same tool or system repeatedly causes humans to make certain mistakes, it needs to be changed.
BenjaminRi | 17 hours ago
That's like saying wearing reflective safety vests is useless because the photons that are hitting the retina of the driver do not deccelerate the car that moves towards you. Which is technically true but also a total non-sequitur.
sammko | 15 hours ago
Technically the photons do decelerate the car. Just not by much.
BenjaminRi | 15 hours ago
I know, and I knew this would come up, but I didn't get around to correcting the phrasing. The fact that we are having this discussion shows how suitable the metaphor is ;-)
dzwdz | 55 minutes ago
Why? I thought they have 0 mass. (I'm clueless about physics.)
sammko | 8 minutes ago
It is considered massless and having zero rest mass, but it has momentum. Just not the Newtonian kind of momentum which is proportional to mass and velocity (p=mv). Instead Einstein came up with a relationship between the rest mass, momentum and energy of a particle: E²=(pc)²+(mc²)².
One may work backwards from there and invent a quantity known as the "relativistic mass" which would be non-zero for a photon, but it's fallen out of fashion among physicists as far as I'm aware. The more famous formulation of Einstein's equation uses relativistic mass (or assumes no motion).
(I don't know the exact history of the above, perhaps they came up with relativistic mass first, but this is how I like to think of it).
A photon's energy is inversely proportional to its wavelength (E=hc/λ) and the momentum may be derived from there to be p=h/λ. h is Planck's constant.
hyperpape | 14 hours ago
The post specifies a reason why the author think that C programming leads to this behavior.
You’re free to argue this reason is wrong. But you cannot responsibly just ignore the reason and state that this is not an issue with C. You might as well just say “No.”
timthelion | 16 hours ago
Yes there is, in any language with proper union types you would be fired for even suggesting such a thing as representing NULL by "*". This is truely just a case of C having an insufficiently powerfull typesystem to express things correctly.
Zash | 17 hours ago
Having recently dug into this topic a bit, I think the better question is
So many subtly different input formats for the algorithm and salt...
sjamaan | 5 hours ago
The idea behind crypt is fantastic: having pluggable and configurable hashing mechanisms which (in theory at least) can be transparently upgraded when a user logs in allows for longevity of a single API in a constantly changing reality, while staying backwards compatible.
The actual API leaves a lot to be desired of course, and can be traced back to wanting to stay backwards compatible for many decades.
ianloic | 17 hours ago
This is a musl bug not a C issue. If you had the same logic written in Haskell or whatever your language of choice was you'd get the same issue.
The fact that AWS offers official images that use a libc reimplementation that's an endless source of bugs is a disservice to their customers.
timthelion | 16 hours ago
Yes, but if you were to do this (store NULL as "*") rather than use union types in Haskell you would be doing something wrong.
orib | 14 hours ago
What if the API was initially designed such that there was initially no concept of a user without a password, and you needed to compatibly shoehorn something in after the fact without breaking millions of lines of existing, ISO-standardized APIs?
Do you design every Haskell API with the idea that some day someone may add an error case, even though none currently exist?
Johz | 14 hours ago
That would be an issue, but in this case
cryptwas defined to be fallible from the start, and that's one of the key players here. In the C version,cryptreturned a string or null, but the type system doesn't differentiate between those two, so it was easier for callers to incorrectly handle the return value. But in Haskell,cryptwould have returnedEither(I think?) with the string or an explicit error. And at that point, the caller cannot implicitly ignore anything, they have to deal with both cases explicitly, and the problem would never have shown up in the first place.orib | 14 hours ago
It wasn't initially failible -- failability was added quite a long time ago, though, when migrating to encrypted passwords in /etc/shadow.
Johz | 3 hours ago
As I understand it from fanf's summary, the error case has always been present, but originally it was only used if
cryptwasn't available on a particular system. Based on current Rust API norms, I imagine that would probably have been written asResult<String, CryptError>, (or possibly a broader error type), with that error being non-exhaustive to allow for future possible error codes.All that said, I think this is a bit of a "if my grandmother had wheels, she'd be a bicycle" sort of discussion. A lot of Rust's design philosophies, and the tendencies in the Rust community, come from direct experience of the issues of C API design. There is no world where libc would have been designed then according to modern design sensibilities, and a lot of the constraints that were considered important then are no longer seen as important, while other issues (like designing APIs that can be evolved) were not as high a priority.
I think the interesting observation here is how we can develop future APIs - and how important things like ADTs are for designing those APIs in such a way that the constraints and behaviours of the function are clear to callers of that function.
sunshowers | 12 hours ago
From the post:
I suspect the same would apply to Haskell. (Or maybe not, but I didn't bring up Haskell.)
masklinn | 7 hours ago
I wouldn't be surprised by that either, but as an alternative Haskell also has exceptions.
kornel | 14 hours ago
but you wouldn't get the same logic in Haskell or whatever, because languages with better constructs for exceptional results use them.
C APIs don't even agree whether returning 0 means
falseor not-an-error-code. This is absolutely a C problem: primitivism of the language lead to a widespread problem of inconsistent ad-hoc error handling. Inability to enforce proper error handling by the compiler leads to the vicious cycle of sloppy library users summoning nasal demons, and library authors mitigating that with the crappy sentinels.masklinn | 8 hours ago
Expect for the part where there is no reason to have the same logic written in Haskell in the first place, because the issue that was fixed by this logic (callers not handling the error case) is not possible: a hypothetical haskell crypt would have returned
Maybe Stringat the very least and the caller would have not been able to ignore theNone.And then there is a second layer of having no reason to have the same logic written in Haskell: doing in-band signalling in Haskell is abnormal.
So as the essay explains this is a C issue, an issue of the culture and norms of C programming, as well as (much like memory safety even if it’s not that) the weakness of the C language and its type system.
moltonel | 6 hours ago
IOW, you can write C in any language. But the point is that this bad error handling, which is idiomatic in C, would be a big red flag in Haskell, Rust, and many other languages.
alexwennerberg | 5 hours ago
It is very good and important that musl exists as an alternative to glibc, and I question whether characterizing it as an "endless source of bugs" is fair.
singpolyma | 14 hours ago
musl is libc... If you're not in C you don't need it at all
ianloic | 14 hours ago
Other than golang, all popular languages rely on libc.
aw1621107 | 14 hours ago
Even Go has to rely on libc (or equivalent) when the lower-level interface is either unstable (macOS) or restricted (OpenBSD).
ache | 15 hours ago
I love C, I pretty much did C for the past twenty years of my life.
Yet I acknowledge that is a C problem (but Python, JavaScript, ... also have the same problem). We must move to an non legacy language when possible (I'm not saying Python or JavaScript are legacy).
We have enough computer power and modern languages to have good type systems and rule out most of the bugs. Yet we don't have enough "human power" to move all the legacy code to modern alternative.
It's not a flame war. Just saying that doing any new modern technology in C is absurd nowadays.
kornel | 14 hours ago
JS and Python don't have this problem - such APIs can (and do) throw exceptions instead of relying on callers checking for a special sentinel value. They can also just return
undefined/Nonewithout worrying about callers causing UB.ache | 2 hours ago
Considering this C don't have this problem. It could have return NULL. That is not my point, that is not the point of the article.
vbernat | 10 hours ago
The article implies that Rust is magically more secure. uutils audit discovered 44 CVEs for things that are well known by experienced C programmers (many TOC/TOU) and not covered by Rust type system. These CVEs are the reason moving away from C for existing things is not as simple.
sunshowers | 8 hours ago
One of the first professional projects of my career was solving numerous TOCTTOUs in Meta's Watchman file system monitor, written in C. Required tons of man page reading and lots of careful reasoning and empirical analysis. There was a lot of random C bullshit I had to deal with as well; I'd much rather be solving TOCTTOU issues in Rust.
I do think it was a bit suboptimal for uutils to be thrust into the limelight like this.
rtpg | 8 hours ago
Not to be too obtuse but C doesn't help out with TOC/TOU either right?
Like in theory it feels like C programmers who understand these issues well would be able to use Rust effectively, and provide APIs to end users that are safer as a result of being burned so much in the past, right?
Or is there something I'm missing here. Is C giving us something here? Or is this more of an inertia argument + "rewrites intro these risks" thing?
vbernat | 3 hours ago
No, C does not help at all, but I think this is a well-known issue for C programmers (because it was a source of vulnerabilities a long time ago), while this may not be the case recently (as "demonstrated" by the uutils vulnerabilities). I was mostly answering to the argument that Rust is also safer because of the culture around it, not only because its intrinsics qualities.
zaphar | 2 hours ago
The article implied no such thing. It pointed out that Rust gives you tools to create safer API's than C does and that the presence of those tools creates a culture where you are more likely to interact with and produce safer APIs than C.
Rust is not the only language that does so but it does stand out by having both those tools and also the performance of C.
dvogel | 16 hours ago
Is there any technical constraint preventing C from acquiring a result type and/or multi-value return? I get there are social constraints and ecosystem issues that could arise but are those the only reasons?
kornel | 14 hours ago
Technically it seems doable to extend
enumandswitchto have at least some minimal tagged unions and basic pattern matching.I think the problem is political and social:
std::variantat home. Microsoft's veto has chilling effect on C standardization and adoption of features. Projects worrying about portability and back compat would end up wrapping it in macros, creating ugly syntax nobody likes.Even without these obstacles it will take few years for the C working group to consider it, then a few more years before it lands in a C spec, then few more years before compilers ship it, then a few more years before distros switch to those compilers, then a few more years before notable C projects consider the syntax old enough to be acceptable to migrate to it. And even then, stdlib and POSIX probably won't ever change.
aw1621107 | 15 hours ago
Pretty sure you can already sort of pull off multi-value returns just by returning a struct by value. Result types can be easy or hard depending on how strict you want compile-time checks to be (basic discriminated unions are easy; statically ruling out misuse is harder, especially given C's relative paucity of information hiding options).
Also I feel that once you drop social and ecosystem constraints there probably isn't much left to prevent any language from changing in arbitrary ways?
riking | 18 hours ago
The powerful thing about community norms in the LLM age is that it's what the LLM will replicate.
If you make secure and memory-safe code the average, then the LLM will output secureish and memory safeish code.
mxey | 4 hours ago
Technically, C programs not checking the return value for NULL is equivalent to a Rust program just calling unwrap, and the library then changing to never return an error.
I also think that would be frowned upon in both languages. However the difference is that Rust doesn’t implement a completely ossified API where callers expect to continue to work forever even if they misuse the API.
In fact we have seen at least a Cloudflare outage being caused by overuse of unwrap, so I’m not sure there is a strong claim that this problem cannot happen in rust.
zaphar | 2 hours ago
The big difference is that having your program safely crash with a panic is better than not crashing and accidentally authenticating any user as root. I'll take a cloudflare outage caused by crashlooping over an unsafe auth check by accident any day.
jjuran | 16 hours ago
I call BS. This is empty Us vs. Them rhetoric masquerading as language advocacy.
The icing on the turd is writing "*sshole" instead of, say, "ss*hole". Who does that?
ekuber | 15 hours ago
The people who published the vulnerability: https://sshhole.com/
kornel | 14 hours ago
Rust has
Option,Resultandpanic!()for such cases, and there's a decade of real-world experience and hundreds of thousands of libraries demonstrating that using them is indeed the norm.OTOH C has been using sentinel values forever.
EOFis in stdlib.NULLis normal and the standard C doesn't even have non-nullable pointers. In-band signalling without compiler enforcement is the norm in C.masklinn | 7 hours ago
Also positive integer for successes negative for errors, pids v pgids (and more), NUL bytes for string termination, ...
ache | 15 hours ago
The original website does that.
The name is sshhole, but the page title reads as *sshole, the 'h' is only shadowed, and the first character is an asterisk. This is confusing, and it's all just for a juvenile pun.
Ref: https://sshhole.com/
jjuran | 8 hours ago
Well, crap. I obviously didn't visit the site, so it looks like I'm the *sshole. :-/
sigmonsez | 15 hours ago
we dont need more write in rust preach
MatejKafka | 6 hours ago
We don't, we just need to stop writing new software in C if we want dumb vulnerabilities like this to stop happening.
carlomonte | 18 hours ago
whom exactly does the author address? do we need another flame war?