Featured image of post Every link was a fix

Every link was a fix

*sshole let people log into accounts that can't be logged into, and when you pull it apart, each piece of the chain was someone fixing something. So what am I trusting when I patch?

I’m aware I’m in danger of becoming a bit of a stuck record. Since May I’ve written about AI and curl’s bug bounty, about waving through anything under an 8, and only three weeks ago about PHP2Shell and who picks up the bill when AI digs up a bug that’s been sat in plain sight for twenty years. So when someone sent me sshhole.com last week (an amusing write-up of the newest one, and a surprisingly good plain-English fix guide too), I had the facepalm everyone else had… and then a second, quieter groan, at the thought of writing the same post again.

So I’m not going to. Well, not mostly. AI did find this one, and it gets its paragraph further down. But what’s stuck with me since is less who found it, and more that when you pull the bug apart, every link in the chain that caused it was someone fixing something.

A password that opens a locked door

Right, a quick bit of how it works first, since the rest hangs off it.

On Linux, an account’s password is stored as a hash, and an account that isn’t meant to be logged into with a password gets a marker instead of a hash, usually a * or a !. A hash doesn’t come out as a lone asterisk, so no typed password can match it. To check a password, sshd hands it to a C library function called crypt(), along with a “setting” (the prefix that says which algorithm and which salt to use), and compares what comes back with what’s stored.

On musl, the small C library that Alpine Linux is built on, a crypt() that fails doesn’t return an error. It returns a string, and the string is *. A SHA-style password longer than 256 bytes makes it fail. So you send a 257-byte password to an account marked *, crypt() gives up and returns *, sshd compares that with the stored *… and they match. You’re in, on an account that was never meant to have a password at all.

It needs a specific set of conditions: OpenSSH built against musl, PAM switched off, password logins switched on, an account marked *, and at least one other account with a real hash (I’ll come back to why). That’s a narrow window, and musl’s maintainer Rich Felker confirms that Dropbear isn’t affected, and Dropbear is what a lot of home routers run by default (OpenWrt’s, for one). Nothing I run is in that window either (my images are Debian or Wolfi, both glibc). But Alpine marks root as * out of the box, so on a box where all five line up, the account you get into is root!

Pulling it apart, I couldn’t find a careless piece anywhere in it. Each one was a reasonable person solving a real problem for the people downstream of them.

It starts with the asterisk itself, which is older than I am (by nearly five months, I checked). Here’s the daemon account from 3BSD’s password file, committed in January 1980 (Research Unix V7 had an x there a year earlier, so this is as far back as I could trace the star):

daemon:*:1:1:The devil himself:/:

The devil himself! I couldn’t make that up if I tried. The * there is a convention: no password can ever produce it, so no one logs in with it. It has never really been a contract, it’s just what everyone has done for forty-six years.

Then in 2012 musl made crypt() return * on failure rather than a null pointer, and the commit message says why: “a large portion of programs which call crypt are not prepared for its failure”, so handing back an unmatchable hash was “reportedly a better behavior than failing on error”. That’s a fix. Callers were crashing, and a crash in a login path is its own kind of bad. And musl even guarded against the obvious hole, returning x instead of * if the setting it was given started with a *. Belt and braces.

A year later Thorsten Glaser, on a Debian bug about glibc’s crypt(), suggested the glibc world copy musl’s idea, credited it to musl’s maintainer, and called it “the best compromise/solution”. As it turned out, the glibc distros got there by a different road. libxcrypt, which took over crypt() on most of them from 2018, had been returning a failure token since at least 2009, a habit it shares with Openwall’s crypt_blowfish, and its token “will begin with a *, and will not compare equal to setting” (crypt.3). The token was *0. One character longer, and as far as I can tell that one character is most of why the glibc world isn’t reading about this today.

In 2016 OpenSSH fixed CVE-2016-6210, where an attacker could tell real usernames from made-up ones by timing how long a very long password took to be rejected. Real accounts got the slow hash, made-up ones didn’t, and the difference showed. The fix was to hash the password for all accounts, including the ones with no usable hash, and to do that sshd borrows a salt from an account that does have one. That’s the “another account with a real hash” condition from earlier.

And in 2019 Alpine fixed CVE-2019-5021, a critical one where its Docker images shipped root with an empty password. Root got a !, which turned out to block key logins too when PAM is off (aports#10806), so a couple of years later it became a *, with the note “this still forbids password login but allows logins via ssh keys with pam disabled”.

Five links, five fixes. There’s a kinda grim symmetry in there too, if you squint: a medium-severity bug driven by a very long password got fixed in a way that, ten years on, opened a root login driven by a very long password.

The guard was watching the wrong string

So where did it go wrong? Both libraries had a guard, and both guards make one promise: whatever comes back on failure won’t match the setting.

Old sshd didn’t hand a stored * to crypt() at all. When the stored field was too short to hold a salt, it used xx instead, an old DES salt, and DES copes with an over-long password by quietly ignoring everything past the eighth character. Nothing failed, so nothing came back as *. Then the 2016 fix swapped xx for no salt at all, and a missing salt makes sshd borrow one from another account. Now the setting was a real $6$... prefix, SHA-512 gives up past 256 bytes, and the stored field was still *. musl’s guard went on faithfully protecting the setting, which wasn’t the string being compared, for about ten years.

I don’t think that makes the guard worthless, and I’d push back on anyone who said so. It was of its time and entirely acceptable when it was written. Picture an engineer in 2016 with five moving pieces spread across three projects and four decades… how were they meant to see where those pieces would meet? It took agents running about 17,600 deliberately odd logins across differently built servers, looking for the places where the builds disagreed, to find it. That’s my paragraph on AI, and I’ll leave it there.

Then look at what the owners said afterwards. Felker’s position is that it’s “not a bug in musl just an erroneous use by the caller”, and OpenSSH’s fix commit says “we do not consider the root cause to be within OpenSSH but we are well equipped to avoid it”. Both point elsewhere, and both patched anyway. OpenSSH’s fix makes sshd skip the comparison when the stored field is too short to be a real hash. It stopped relying on someone else’s guard and checked for itself, right where the answer gets used.

Guard it where you use it

That last bit landed close to home. Only a few days ago I had my own version of this argument on go-tool-base, about whether the framework should police the values a consumer’s config might hand it. I came down on the side of no. A filter up front has to predict all the ways a value could be misused, and you end up patching edge cases in perpetuity. A check at the point of use only has to understand one use. So the consumer guards where it uses the value, and that’s that.

sshd’s fix is that principle, applied a decade late. But I want to be careful not to turn it into “trust no one”. That isn’t what I think, and it isn’t what happened either. The 2016 sshd team trusted crypt() to behave, and crypt() had a guard that, at the time, did what they needed. Trusting it was reasonable. It’s just that the contract between them was never written down anywhere, so when one side moved, the other had no way of knowing.

Trust is reputation

That brings me to the uncomfortable bit. I’d have installed that 2016 fix almost as soon as it came out. Renovate runs nightly across everything I own, and I try to patch CVEs of all severities as fast as I can. I’d have been an early adopter of the very change that opened this hole.

And I’d do it again, because the alternative isn’t realistic. It has been nigh on impossible, for as long as I’ve been doing this, to test and validate all the patches for all the software I consume. Imagine asking everyday, non-engineering people to build and test their own Windows or Office patches before installing them. What we actually run on is trust that upstream developers are doing the best they possibly can in the interests of the people downstream. The 2016 patch was just that.

Historically, trust is reputation. The two are inextricably linked, and you don’t really get one without the other. Alpine’s reputation took a dent last week, and I still trust them. The fix was committed in Alpine’s tree a few minutes after the announcement went out, musl says the mechanism is “something we have been wanting to change for a while now” (musl, 11-06), and there’s already a proposal to have musl on Alpine return a proper error instead of * (aports#18553). That would undo the 2012 fix, which is its own small full circle. It looks a lot like people learning from it and getting on with the job.

But the world is moving quickly, and I suspect we need to move with it. Do we start separating trust from reputation? I don’t know, if I’m honest. What I do know is that I’ve always tried to do some due diligence before adopting a third-party component, and sometimes that diligence has ended with me building my own. The estate has its own errors package because my trust in the previous one had waned and I couldn’t find anything better, and it had to clear the bar I hold myself to before building anything myself.

AI makes that kind of build much easier than it used to be. Is it a good idea? Maybe, maybe not. I’ve created and still look after a lot of things, and the maintenance is not cheap. Plenty of developers won’t want that weight, and the ones who take on enough of it can end up feeling like the Titan Atlas with the globe on his shoulders (the heavens, strictly speaking, but the globe is how everyone pictures him). So I’m inclined to say each engineer and organisation will draw their trust boundary in a different place… the important part is that they actually draw it, and know where it is.

Day nought or day three

There’s one question this has left me chewing on, and I don’t have an answer yet.

My Renovate presets make ordinary updates sit for three days before they land, because, in the words of my own config, “a soak exists to let OTHER people find problems we cannot”. That’s my trust-in-the-community argument, written into a JSON file. But a fix that arrives flagged as a security advisory skips the soak entirely and merges the same day. So the class of patch that opened this hole is the one my setup trusts most, and fastest. (For the record, Renovate wouldn’t even have seen this particular fix, since it arrived as a package revision inside an image. My CI base image gets rebuilt daily on a schedule for exactly that reason, since Renovate can’t see apk packages at all, but that’s a post for another day.)

Is day nought right for a security fix? My gut says a known hole beats a speculative one. But that’s exactly what the 2016 team would have said, and they were right too.

I’m going to have to ponder that one. And when I’ve made my mind up, I promise the post won’t be about AI finding a bug. Probably.

Built with Hugo · Theme Stack designed by Jimmy