Hacker News
points by BoppreH 2 days ago | 0 comments
In-band signaling[1] strikes again. * should not have been used as special value, neither for "no password" in /etc/passwd nor "error while hashing the password" in crypt. I'm afraid we'll find many more of these abuses, specially in string manipulations. The null was ok, an exception (or equivalent) better, and Result<T> (or equivalent) best.

[1] https://en.wikipedia.org/wiki/In-band_signaling#Computer_dat...

[–] hedora 2 days ago
Exceptions are almost never the right choice. They create extremely complicated control flow extremely quickly, and then you have to test all the different control flows. Java software is an example of how this goes sideways fast. There's no way to create a class that prevents callers from reading the object before the constructor runs (in multi-threaded software, new first hands a reference to the caller, which can put it on the heap, then it calls the constructor), and objects are default initialized, so all fields (even static ones that are assigned in the constructor) can be null.

So, any field de-reference in Java can produce a null pointer exception. Try testing all paths in something like:

try { foo.sideEffect1().sideEffect2()...sideEffectN(); } catch { bar.sideEffect1().sideEffect2()...sideEffectM(); } finally { quuz.sideEffect1().sideEffect2()...sideEffectP(); }

There are M * N * P paths. The only way to write testable code with exceptions is to panic on uncaught exception, and to never catch unexpected exceptions. Even then, you're relying on nothing unexpectedly throwing the exception type you caught.

I'd argue Result<T> >> NULL >> sentries >> exceptions for this reason. Sentries are terrible, but at least they're not computationally hard to use correctly.

[–] BoppreH 2 days ago
I'd argue that's the fault of having default nullability, not the exceptions. You would have the same problem if every attribute access returned a Result<T> too.

And I'm not a fan of breaking control flow, but I still find it preferable to nulls. A null may travel far from the place where it was generated before blowing up, making troubleshooting harder, or it might come from a function you called for its side effect but forgot to check the return value. I gotta say I'm also a careful enthusiast of checked exceptions, it's great knowing which ways a function can fail.

But I think the consensus is growing that Result<T> (plus warnings for unused return values) is simply the superior choice.

[–] hedora 2 days ago
C has what you are asking for, mostly:

   int foo f() __attribute__((warn_unused_result))
As long as an error condition on the return value causes a crash on use, then it's mostly the same as result. (The main remaining difference being that you do an implicit `.unwrap()` in C, and in things like rust that's an explicit code smell.)
[–] gf000 2 days ago
> There's no way to create a class that prevents callers from reading the object before the constructor runs

That can only happen if you leak the reference (given we are talking about java language level semantics and not byte code), which I give you is unfortunately not that hard to do.

As for exceptions, I think they would pair beautifully with software transactional memory, which would work well with effects. In that case (and no side effects) it would not matter what code path we took, we could just go back to a known safe point.

Because ergonomy-wise I still think exceptions win (even though I do like Maybe types, and Rust's ? macro is plenty good): - they let one focus on the actual happy path code - no error is ever swallowed, unless explicitly written so (looking at you go with useless if errs that fail to properly handle the error), they are always handled somewhere - they automatically "unwrap" the Ok result, and automatically bubble up the Err side. - bubbling up is especially useful because most errors can't be handled at their source (e.g. IO error is meaningless, how you handle it depends on what you want to do in the first place, which is done several layers higher) - they automatically attach a stackframe/source, while with result types you may get a list of Ok Ok Ok Err and have no idea how that Err got there.

[–] hedora 1 day ago
As the author of the class, I cannot prevent my caller from leaking the reference, so my public API allows for pre-constructor invocations of all my methods.

Software transactional memory doesn't really work with exceptions, since it precludes the use of catch + finally. It looks more like a top-level panic handler (where the throwing code cannot permute the heap, which is cool). To get exceptions, you'd need nested software transactional memory, which gets into obscure transaction processing theory. There's also the problem that your transaction could side-effect the outside world, then throw and be aborted, so you need to rework how I/O works if you walk down that path.

Exceptions are basically write-only, so I agree that their ergonomics for that use case are pretty good. Panic is better though, since it's really no error handling. Panic is easier to test, and easier to read / understand.

[–] Sohcahtoa82 2 days ago
> There's no way to create a class that prevents callers from reading the object before the constructor runs (in multi-threaded software, new first hands a reference to the caller, which can put it on the heap, then it calls the constructor)

Which is why the pattern of double-checked locking doesn't work.

https://en.wikipedia.org/wiki/Double-checked_locking

[–] cowlevel 2 days ago
It's wrong. The caller doesn't get the reference until the constructor runs. Although it looks like two operations in bytecode, the only thing you can legally do with an uninitialised object reference is call its constructor.
[–] hedora 1 day ago
In java, the caller gets the reference before the constructor runs. It's in the language spec. It's terrible. You can prove it yourself with something like:

   this.dangerous = new Lol();
and then have Lol()'s constructor sleep for 10 sec then set done = true at the end of the contstructor. In another thread:

   while(!this.dangerous) { }
   assert(this.done) // will throw.
I guess you could use double-checked locking as a class author by having every public method on your class do a if (!this.done) { throw; }, and make sure there are no public fields.

That's pretty lame though.

[–] cowlevel 1 day ago
That's a data race. The caller gets the reference after the constructor runs, but writes to non-final fields from the constructor might not be visible to other threads.
[–] quotemstr 2 days ago
Exceptions work fine and enable a clean separation of business logic and error handling. Visibility into partially-initialized objects is a language design issue independent of exceptions. In a non-exceptional language, the corresponding access to uninitialized fields would just be bugs, not errors you can see.

Nothing in your post actually supports the idea that exceptions are "computationally hard". The CFG error edges exist however the errors are spelled

[–] hedora 1 day ago
It is difficult to get to M * N * P control flow edges with O(1) result unwrapping code.
[–] quotemstr 1 day ago
You have exactly as many edges if every fallible routine can throw and every infallible one is marked as nothrow.
[–] hamilyon2 2 days ago
> never catch unexpected exceptions

Yes, broad exception catch clauses are simply incorrect code.

Panic is maybe an overreaction, "scratch everything and orderly abort" is sufficient.

[–] Ey7NFZ3P0nzAe 2 days ago
I'm inclined to say a Panic is absolutely preferable to root auth.
[–] z3t4 2 days ago
You should never ignore exceptions! The least you can do is to crash the program. Then you fix the bugs via battle hardening until it no longer crashes. Making the program not crash by ignoring the errors will just mess with the state of the program until you have no idea what caused it. It's better to let it crash, load the dump into your debugger tool to see what went wrong, then fix the bug, and repeat until you have a stable program. Rather then just live by the motto "ignorance is a bliss".
[–] za3faran 1 day ago
> There's no way to create a class that prevents callers from reading the object before the constructor runs

Doesn't this address the issue? https://openjdk.org/jeps/539

[–] jedbrooke 2 days ago
Unfortunately we’re moving to llm based systems which rely entirely on in-band signaling.
[–] __MatrixMan__ 2 days ago
PSA: you can typically declare different types of string so that your type system objects to mixing data of different types even though deep down we know they're both strings.
[–] BoppreH 2 days ago
If you mean type aliases, that's a good idea within a single codebase, but the distinction between different types of strings doesn't survive serialization so it wouldn't have prevented this error. What they need is a way to signal errors out-of-band, like Result<T> or exceptions.
[–] __MatrixMan__ 2 days ago
In python, aliases are separate from typing.NewType, but you're right: it's only enforced within a codebase and then only if the dev cares to run the typechecker.

So yeah, I guess it either needs to happen at runtime, or we just do signaling and user data on separate file descriptors.

[–] Normal_gaussian 2 days ago
This is presuming that the type system is nomitive rather than structural and there is not serialisation/deserialisation or type refinement occurring. A structural difference is much more preferable.
[–] throw10920 2 days ago
This is a side-effect of Unix-y "it's all plain text" design that leans extremely heavily upon in-band signalling (among other design decisions).

If /etc/shadow was a JSON (or XML, even, although that would make me feel gross), this bug would have been much easier to see, because a field that reads '"hash": ""' sticks out a lot* more than a custom microformat that you don't understand.

With data structures, there's never "plain text" - it's all structured. The question is whether you want to use existing formats and tools, or create a new one for every tool and literally make footguns like this.

[–] __MatrixMan__ 2 days ago
A few years back I un-bashed my life as much as possible in favor of nushell (though it's not the only shell to encourage structure). I feel like it has really helped me notice a lot of this-needs-more-structure type problems outside of my shell that I otherwise wouldn't have.

The elegance of the unix-y approach is actually that it's all bits. You can just join things together and shove the bits through, that's a wild simplification compared with having vendor-specific kinds of plugs. But we could have that kind of compatibility while also benefiting from structure.

So I encourage everybody to give a shell with structured output a try, let it rewire your brain, and then ask yourself: how is the OS holding this back from being everything it could be? Because if we build that OS, it won't encourage these problems.

When I imagine it, it would probe either side of a pipe to see if they support negotiation. If not, send the bits through in the same old unixy way. If so, use the newest schema for structured data that is supported by both sides.

[–] jonhohle 2 days ago
At Amazon we had RecordStream which implemented most of the standard Unix tools on structured streams. It was pretty easy to write filters, aggregators, and formatted output in something not much more complicated than sed/awk/cut/join/sort pipelines. There was a convenient structured log format for services that was often used as input.

https://github.com/benbernard/RecordStream

[–] hedora 2 days ago
That looks really cool. I've been heading the opposite direction: posix shell for everything.

I have a bunch of management scripts, etc, that need to run on everything from a potato to a T-1000, and that's the only language that's always available. (OpenBSD sh, FreeBSD sh, busybox, bash, dash, zsh all can cope with it).

I'd love something that's at least JSON-typed (or equivalent), but that can be bootstrapped up on least-common-denominator posix.

Has anyone built something similar? (Yes, I'm being picky -- bun / ts is already a pretty low bar for cross platform support, but these scripts do stuff like install node, so I'm not totally crazy for asking.)

[–] cowlevel 2 days ago
Python?
[–] hedora 1 day ago
Not installed universally, and it also doesn't reliably run python programs. (Minor + major semver changes break backwards compatibility, and pip mutates the global system state.)
[–] yearolinuxdsktp 2 days ago
Returning “*” instead of NULL to signal failure is the real footgun. Don’t blame the format.

For historical reasons, we could’ve ended up JSON as “hash”: “*” just the same. The /etc/shadow format lets you skip values completely, like “::”, so your argument doesn’t hold.

Terminating an app because of a NULL dereference is the other footgun. Sadly, we still have “panic” in modern languages like Go and Rust.

[–] hedora 2 days ago
"panic" is a good thing. There's always some error condition that your code cannot handle. At some point, the least bad thing to do is crash.

For instance, what if your error handling code throws?

One spectacular example of panic saving the day was the Apollo 11 1202 error. It was basically a watchdog timer that fired if the landing computer was starved of CPU time. A misconfiguration caused the first moon landing to hit CPU overload. When that happened, the computer displayed 1202 and rebooted, then ran things in priority order. Apollo 11 landed in the middle of a tight 'panic, reboot, adjust thrusters' loop. Thanks to core memory, most of the heap survived the reboot, so the reboot step was fast enough for it to mostly land without human interaction. The pilot had to make ~1 adjustment to get it to actually hit the landing site, but the panic-ridden computer handled the rest of the descent.

https://dpma.de/english/our_office/publications/ingeniouswom...

[–] zbentley 2 days ago
sshole-type APIs < panicking APIs < required error handling.

Panic is fine, but fundamentally a whole-program crash is undesirable in a really wide range of use-cases. I don't want other SSH connections to crash (or the server to stop accepting valid logins until someone restarts sshd) if one person tries to log in with a credential that triggers a panicking bug in crypt(). Same for crashes in request handlers and so on.

The language/runtime itself doesn't (and probably cannot, at least if it's a general purpose language) distinguish between local and global error conditions, so aborts end up being either unnecessarily damaging, or languages like Go end up normalizing panic "localization"/handling in dangerous ways. Isolation using threads isn't robust enough. Isolation using processes is far too fiddly, slow, and opaque.

The better solution is required error handling. If user code must engage with the possibility of an error state in order to access a result, that's a huge step forward. They can do the error handling wrong, but if you make most people have to do it at all most of the problems will go away.

There are other good solutions too, like Erlang's process boundaries, but those tend to have drawbacks that make them inapplicable to a lot of programming areas. Tagged unions, on the other hand, are only inapplicable in C library code because of language defects and terrible community conventions around ABIs.

"I forgot to check for an error" should not be implicated in bugs in 2026; we have had a large menu of possible alternatives (checked exceptions, unchecked exceptions, tagged unions, etc.) for decades. But we're still over here writing C (and Go), languages pathologically incapable of providing this ability.

[–] hedora 2 days ago
SSH specifically has separate processes per connection the last time I checked, so a panic does the right thing: The attacker's connection drops uncleanly.

This is also why you can upgrade SSH over SSH without having the connection drop mid-stream.

Again: You cannot avoid "I forgot to check for an error". What happens if you get a sigpipe on stdout when printing the "I crashed" line? If Linux overcommits memory? If you turn off overcommit, port all software to check for + cleanly handle NULL from malloc (a substantial effort!), but then get a machine check exception from a double bit ECC error during an infallible read?

[–] zbentley 2 days ago
Sort of. It does some of the validation/auth code in the sshd parent daemon.

It has to: if it did 100% of the post-accept(2) work in a subprocess it would be pretty easy to remotely forkbomb/PID-exhaust a system by spamming invalid logins.

> You cannot avoid "I forgot to check for an error"

Yes you can. Checked exceptions and tagged unions both force calling code to be written with error-awareness.

In SSH's case, "I forgot to check" might lead to crashes (which might knock over sshd so new connections are impossible), or it might lead to false-positives via comparing NULL == NULL.

I'd much rather that boneheaded API users have to write code like

    resultstruct x = authenticate_user(user)
    if x.haserror {
        rv = { .user = user, .result = "successfully authenticated" } 
    } else {
        rv = { .user = user, .result = x.result }
    }
    return rv
At least that way some keystrokes are required to do the stupid thing. People still will (https://www.schneier.com/blog/archives/2008/05/random_number...), but the harder we can make it, the better.
[–] hedora 1 day ago
How do you handle out of stack space on 'authenticate_user'? What about the machine check exception loading the DRAM page that contains "successfully authenticated"?
[–] throw10920 2 days ago
> is the real footgun

This contains a false snuck premise - that one and only one thing was the footgun. This is categorically not true. An obtuse, poorly-understood, purpose-built text-based DSL is absolutely a footgun and if it was replaced with a JSON and inactive accounts had been indicated by `"active": false` instead of a freaking asterisk then this would absolutely would not have happened.

> For historical reasons, we could’ve ended up JSON as “hash”: “” just the same.

That's dishonest, because it intentionally conceals the fact that that's a vanishingly* unlikely scenario.

People have brains that, factually, are far more likely to see a single context-less character "" and not realize the implications of it than to see `"hash": ""` and think "oh, that's normal".

This is just straight-up psychologically true.

> The /etc/shadow format lets you skip values completely, like “::”

Totally irrelevant.

> so your argument doesn’t hold.

False `::` is both (1) still harder to understand than `"active": false` and (2) even though the syntax existed it wasn't used here because everyone thought they understood the system.

Don't make excuses for this insane text syntax. It is, factually, harder to understand than a decently-written JSON format.

[–] dns_snek 2 days ago
The issue isn't the config, but the fact that crypt() doesn't have any way to signal failure. In better designed languages you would have exceptions, result types, or something. Even in C you have "return integer indicating success or error" which would've prevented the confusion.

Now the public API is hard to change but you could still avoid the issue by returning a longer token like "==DANGEROUS_CRYPT_ERROR_length==" or "==DANGEROUS_CRYPT_ERROR_unsupported_alg==" which would never match the password field unless someone was really trying to induce the issue.

[–] knome 2 days ago
>but the fact that crypt() doesn't have any way to signal failure

https://pubs.opengroup.org/onlinepubs/009604599/functions/cr...

>Upon successful completion, crypt() shall return a pointer to the encoded string. The first two characters 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.

returning a sentinel value of NULL and setting errno is pretty standard C, and shouldn't cause an error.

OP's sshhole link specifically notes that it _should_ return NULL, but some MUSL devs decided they wanted it to return trash instead, and the trash they returned instead of NULL caused this.

All because some devs were too lazy to check the result and another dev decided to be clever and "fix" NULL errors from crypt by sending back 'unmatchable' trash banking on devs not checking NULL.

edit: apparently crypt implementations have been trash ignoring the NULL return for a long time. this isn't only on MUSL, but it's still because people ignored and tried to work around the existing out-of-band error mechanism.

[–] dns_snek 2 days ago
> returning a sentinel value of NULL and setting errno is pretty standard C, and shouldn't cause an error.

> OP's sshhole link specifically notes that it _should_ return NULL

I meant there's no mechanism besides returning NULL that would make it hard to ignore the issue (exceptions need to be caught, results need to be unwrapped and their failure handled, an integer return value makes it clearer that there's something there that you need to check). I also missed errno codes when glancing at the manpage earlier which should illustrate the problem.

And yes, whoever first had the brilliant idea to start returning garbage never should've done that, I'm not disputing that.

[–] zbentley 2 days ago
This is true, and "trash" is putting it mildly.

But all those sentinels were added for the reason that, when your code is a public API intended to be widely used by who-knows-who, it needs to be really difficult to misuse it. NULL-plus-errno is easy to misuse. Yes, it's a dangerous mistake and your own fault if you do it. But people have been making that mistake for years and don't show any signs of stopping.

The root cause here is that C is fundamentally a rotten choice for public APIs of any kind. I griped more here: https://news.ycombinator.com/item?id=50381800

[–] n_plus_1_acc 2 days ago
The spec implies a string of at least two characters in the success case, am I reading that right?
[–] mafuy 2 days ago
If this is true, then the entire thread branch here on HN is entirely wrong. What is going on??
[–] fc417fc802 2 days ago
It's subtly wrong, just like most of the rest of the commentary in this comment section. Yes, crypt is specified to return NULL on failure but if you read the rest of that page it is only permitted to fail with ENOSYS. The spec does not allow for failure for any other reason.

So if you're strictly adhering to the spec in this case then unfortunately NULL is not an acceptable way to signal a generic error condition. Hence all the seemingly bizarre special values. (In a strange turn of events all those experts doing things that looked weird turned out to know what they were doing. Go figure.)

[–] hedora 2 days ago
People don't RTFM, either on HN or when implementing "standards compliant" stuff like MUSL.

No error handling scheme can fix that problem. For instance, if a Rust API says I should return a Result<String, ENOSYS>, I can always write code like `if(ENOSYS) { return Ok("*") } instead of returning Err(ENOSYS).

Then my code would be compatible with MUSL.

[–] zbentley 2 days ago
Sure, but if you didn't do that, and returned the tagged union (in Rust or whatever language), your users would necessarily be forced to handle it in order to process the result.

The only reason this library did the insane thing of returning "*" is because it was extremely easy to cause crashes or false-positives (NULL == NULL) by returning NULL+errno. The reason that was easy is because C lacks a means of making error handling mandatory (exceptions, tuple returns, tagged unions, whatever).

If you return a tagged union, users might mis-handle it, it's true (https://blog.cloudflare.com/18-november-2025-outage/). But they have to handle it, which is a requirement that C APIs are incapable of providing.

[–] hedora 2 days ago
You have to handle NULL in C, or your program crashes (though I suppose a misguided optimization of strcmp could do "a == b || really_strcmp(a, b)", which gets us back to your bug).

Also, in things like rust you can just do result.unwrap() if you want. In general, language constructs don't really save you from bad programmers.

[–] dns_snek 2 days ago
> I can always write code like `if(ENOSYS) { return Ok("*") } instead of returning Err(ENOSYS)

You can do that but it's going to feel bad to write that and it's going to stand out like a sore thumb to anyone who reads it. But "errno" is just ambient state that's completely invisible unless you happen to have the manpage fresh in your memory.

[–] hedora 2 days ago
They had to write

   if(ENOSYS) {
     return "*";
   }
after seeing that other implementations use different sentries like "*0" and that the spec says they MUST return NULL. That must have felt bad too.
[–] slashdave 2 days ago
A lazy attitude like "if we just used <n> language, all our problems would be fixed".
[–] eqvinox 2 days ago
Yup, that's the way to explain it in order to blame musl.

Here's how to explain it in order to blame openssh:

openssh processes passwords and doesn't want to reveal through timing whether a user has a valid password (or exists at all). So far so good.

What they do if they hit a user with no valid password (i.e. "*") is that they look for another user with a valid password, to use the seed from. A bit weird already, but OK from a perspective of avoiding implementation specifics on seeds and hash algorithms. They know at this point that the result must be to deny access even if the hashed value matches, because that hash is for another user.

So they put the password received over ssh into crypt() with this other user's hashed value to get a seed. And then accept it if it matches, apparently?

Homework: what happens if the password actually matches another user? Can I log in as root if openssh randomly picks my user for the fake salt, and I type my own password correctly?

[–] FLeXMurphy 2 days ago
Except it's not trash. It's a backdoor funded by our glorious leader Kim Jong Un.
[–] panzi 2 days ago
Personally I think there was nothing wrong with returning NULL. "But if people don't check for it it crashes their programs!" Good! Let it crash loudly! Then they notice and fix the issue.

Also I think it could have returned ":". Given the file format of /etc/shadow and /etc/passwd I don't think it's possible to have a : as a value for anything.

[–] hnfong 2 days ago
> Also I think it could have returned ":". Given the file format of /etc/shadow and /etc/passwd I don't think it's possible to have a : as a value for anything.

I wonder if somebody else would have thought the exact same thing in a different (non-/etc/*) context, and then you get the same effect in a more obscure manner.

[–] panzi 2 days ago
Indeed. Let's keep NULL.
[–] throw10920 2 days ago
> The issue isn't the config

That's a meaningless statement. There isn't a singular "the" issue like you said. Any one of several different technical decisions would have completely prevented the issue. Including, to my point, /etc/shadow being a JSON file where an account being locked is an `"active": false` field instead of an asterisk with no intrinsic meaning.

[–] BoppreH 2 days ago
No, that's just more in-band signaling!

Some badly designed password hashing algorithms (cough bcrypt cough) have maximum password lengths, ignoring everything after X characters. So it could happen that I set my password to a 100-character string, it gets saved as "==DANGEROUS_CRYPT_ERROR_length==", and then anybody can log in to my account by entering an unrelated long string that returns the same "error".

If a function promises a string result, it should only provide a string if the function succeeded. The null was much better than magic strings.

[–] dns_snek 2 days ago
I know, it's bad. I agree that it should've stayed NULL. But if you really really really think you have an acceptable reason to do something like this (and you probably don't), the least you can do is use a non-trivial token.
[–] BoppreH 2 days ago
If I was really desperate and could only return a valid string, I would probably use a message and add a random component to avoid ever matching two errors. Like SQL, where null != null.
[–] BoppreH 2 days ago
Yes, absolutely. Take for example this cURL command:

  NO_PROXY=1 curl -qs --data @./my\ message.txt -- \
  "https://user:password@example.com/user/${userid}/my%20message.txt"
Random parts of it will be interpreted by the shell, others by cURL, or the filesystem, or the remote server. They each have their own encoding rules, nested. I count ten distinct separators and nine ways of escaping/tagging. It's one command. No, just no.

We have seriously overused strings and we're now paying the price.

[–] dolmen 2 days ago
Also: free text is the input format of LLMs...
[–] UqWBcuFx6NV4r 2 days ago
At this point I’m resigning myself to think that we’ve just gotta wait for some people to die. It feels very dramatic and glib (no pun intended lol) and whatever to say, but at this point it feels like there’s a certain hostility in some parts to changing anything from the platonic ideals of however computing happened to work in 2004. “Oh we don’t need to mess with it!”
[–] xorcist 2 days ago
How did you arrive at that conclusion? If anything, this vulnerability happened because someone decided to change the behaviour of crypt() without fully understanding the consequences. This specific vulnerability did not exist in 2004.
[–] timhh 2 days ago
I agree with him. I only managed to get asterisks enabled by default in sudo because sudo-rs isn't owned by people who have decided that the 70s was the perfect decade and nothing should ever change. And look at the insane pushback when it did!

I can't even get an obvious UX improvement through PAM because they like it broken.

https://github.com/linux-pam/linux-pam/issues/778

[–] gspr 2 days ago
I think this is a bit unfair. The way I see it, a lot of people are saying that changing breaks lots of old stuff, and risks complete fragmentation. Those are real costs to change. It might still be worth it, but let's not pretend that everyone is resisting change just because it's change.
[–] abirch 2 days ago
The costs to change are dropping significantly with AI.
[–] gspr 2 days ago
Citation needed.
[–] Philpax 2 days ago
[–] gspr 2 days ago
All I see is sausage makers advocating for a sausage-based diet.
[–] raverbashing 2 days ago
> "it's all plain text" design that leans extremely heavily upon in-band signalling

Honestly I've been shaking my fist at these since 0 becomes True/Success in Bash and the opposite everywhere else

[–] xorcist 2 days ago
Error code 0 means success in a lot more places than just bash.
[–] cwillu 2 days ago
That's not specifically bash though, that's process exit codes
[–] raverbashing 2 days ago
Yes. But it's still contradictory
[–] OnlineTux 2 days ago
The question you are asking when querying a process exit code is "did this program exit with an error?". The default assumption is "no, it completed successfully", so 0 (false) is the logical expected default return.

A function is expected to produce a result - so the question is "what was the result?". !0 (something) is the logical expected default return.

I've never found it to be contradictory.

[–] xigoi 2 days ago
But in shell, 0 is true, not false. The program called true returns 0.
[–] Dylan16807 1 day ago
"True has 0 errors" is the straightforward part.

"False pretends to have an error" is where it gets weird. It's not that they got booleans backwards, it's that they're stretching a tenuous analogy a bit too far.

[–] killerstorm 2 days ago
I'd say another UNIX-y thing - "C is good enough" - is part of the problem. C makes programming painful which results in all sorts of hacks.

E.g. in Pascal you can can define a record and then defined file of record to have typed I/O. I.e. it comes with built-in serialization format supported by compiler. Not a good one - but perhaps a step in a good direction.

OTOH C people starving for serialization often believe that `struct` byte layout made by a specific version of a compiler is a serialization format - a view not endorsed by the standard.

[–] adwn 2 days ago
Pascal's record typed files have the exact same problems regarding endianness and word size as directly writing structs to files in C, so I don't see how they're any better. They're also a complete deadend and unusable for pretty much any file format from the last 40 years+, since you can't even handle a header or magic bytes at the start.
[–] sepeth 2 days ago
It also looked to me a bit like a semipredicate problem:

https://en.wikipedia.org/wiki/Semipredicate_problem

[–] mkl 2 days ago
Even using "no_password" and "error_while_hashing" would have prevented it. The problem was picking a generic, meaningless signal string.
[–] mort96 2 days ago
That would've happened to fix it, because "error_while_hashing" and "no_password" happen to not be valid password hashes. The right way would've been to have different syntax for hashes and non-hashes; you could've made it so that a hash starts with a '#' and if it doesn't start with a '#', the rest of the string should be ignored.

Or, for backwards compatibility, start non-hash strings with a character which can't appear in a hash, such as '@'. Your examples would then become '@no_password' and '@error_while_hashing'.

[–] SoftTalker 2 days ago
Well neither * nor ! (the two characters I've seen to indicate "no password") will appear in a valid hash. The problem was using a character that could also be returned as an error indicator, IIUC.
[–] mort96 2 days ago
Yeah but it's not forwards compatible with future hashes where '*' is suddenly a legal hash, which is what happened here. You need a grammar which lets you tell the difference at the parsing stage between "this is a password hash" and "this account has no password". That's not the same as letting the value '*' flow through the system as normal and hope it doesn't happen to match the hash.
[–] SoftTalker 2 days ago
True, but if they would have picked ?, or #, or any character other than * with its designated special meaning in the /etc/shadow file, then we would not be in this pickle.
[–] cowlevel 2 days ago
hashes can't start with * and there's no reason to think they ever will
[–] mort96 1 day ago
Well, turns out that they sometimes do
[–] f33d5173 1 day ago
There were different syntaxes for hashes and non hashes. The problem was that both a non hash "this password isn't set" and a non hash "this call to crypt produced an error" used the same non password value.
[–] deniska 2 days ago
Technically they did use a character which can't appear in a hash, '*'.
[–] mort96 2 days ago
Right but not in a "rigorous" way. They kept the format of the file the same: the string between the colon characters is a hash. They exploited the fact that an asterisk happens to never appear in a hash, and just let the system believe the asterisk was supposed to be a hash to be matched again.

In a sense, they should've changed the parsing logic to return a struct with a field 'password_hash: Option<String>', and defined the grammar of the field to be: "a hash string, or '*' for no login". The bug is, in part, caused by the haphazard way they don't change any formal grammar or contract and just relied on the fact that '' happens to* never be a valid hash.

It's the difference between "happens to work" and "defined to work by construction". The former approach breaks once a hash function is invented where '*' is a legal hash (such as this new musl crypt() hash function where all strings longer than 256 bytes hash to '*'). The latter keeps working regardless because the asterisk is treated as "no password" already at the parser stage.

[–] hobo123 1 day ago
Should have set crypt_errno.