Live data from Hacker News

Xz: Can you spot the single character that disabled Linux landlock?

git.tukaani.org

231–240 of 322 posts

Re: Xz: Can you spot the single character that disabled Linux landlock?

#231
post #195
post #75

Earlier quoted context omitted.

That’s what makes this so clever: these systems were born in the era where you couldn’t trust anything - compilers sometimes emitted buggy code, operating systems would claim to be Unix but had weird inconsistencies on everything from system calls to command line tool arguments, etc. - so they just try to compile a test script and if it fails assume that the feature isn’t supported. This is extremely easy to miss sin…

You could just run tests for the feature detection on a known system or a dozen(VMs are cheap). The big problem is that most code is not tested at all or test errors are flat out ignored.

Nothing “just” about it. VMs weren’t cheap when the C world started using autoconf - that was a feature mostly known on IBM mainframes – and in any case that couldn’t do what you need. The goal is not to figure out what’s there on systems _you_ control, it’s to figure out what is actually available on the systems other users are building on. Think about just this library, for example, your dozen VMs wouldn’t even be enough to cover modern Linux much less other platforms but even if it was (drop most architectures and only support the latest stable versions for the sake of argument), you’d still need to build some reliable prober for a particular bug or odd configuration.

The problem here wasn’t the concept of feature detection but that it was sabotaged by someone trusted and easily missed in the noise of other work. What was needed was someone very carefully reviewing that commit or build output to notice that the lockdown feature was being disabled on systems where it was fully enabled, and we already know that maintainer time was in short supply. Any other approach would likely have failed because the attacker would have used the same plausible sounding language to explain why it needed to be a complicated dynamic check.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#232

Earlier quoted context omitted.

I see your point, but suggesting adding an additional library dependency while we're discussing a supply chain attack is quite ironic.

Should've said function call not library call. My bad. Basically if you already have the linux/landlock.h, that should provide everything you need to do without explicit references to SYS...

Now we are running in circles. As you see in the git commit, the compile check was added because the existance of linux/landlock.h alone was not enough to check that the feature can be used.

This header defines the data types for the Linux kernel interface, but not how the syscall landlock_create_ruleset(2) will be issued. That is provided by libc either as a separate wrapper function (does not exist in glibc) or the implementation needs to use syscall(SYS_landlock_create_ruleset, ...), with the constant also being provided by libc. That is how it works for all syscalls and you won't be able to change this.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#233

Earlier quoted context omitted.

Some might say RMS was right all along.

I would actually say that he is completely wrong in this case. Open source created this problem.

And you think proprietary code doesn't have this problem?

Can you prove it? Where's the evidence? ;)

Re: Xz: Can you spot the single character that disabled Linux landlock?

#235

Earlier quoted context omitted.

you can have ten comments about the name of a variable, but no one bats an eye at a new npm package introduced. Also, devs that wrote code that Google depends on can't pass the leetcode gate check to get a job there. Our industry is a laughingstock.

The last sentence is an overreach to me, but I have experienced much of the same bike-shedding during code reviews. 95% of them are useless. Read that twice; I am not joking, sadly. I am not against code reviews, but in my experience(!), the reviewers are not incentivized to do a thorough job. Seriously, if you deliver a new feature vs do a deep, difficult code reviews, which one benefits you more? To repeat: I don't…

One 1 hour doing code review is not really stolen from doing feature work really is it? For the vast majority its stolen from playing video games or some other non-work.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#236

Earlier quoted context omitted.

I don't think it fails the build. It's part of a test, trying to compile a bit of code. If it compiles the test is true and a certain feature is enabled. If the compilation fails the the feature is disabled. This is quite common in build systems. I had such a test produce incorrect results in the kernel build system recently. The problem is that the tests should really look carefully for an expected error message. If…

Problem is that every compiler/compiler version will have different messages for the same error. And xz is the kind of project that gets built on really really random homegrown compilers. Maybe we need an option to make the compilers output a standard JSON or whatever...

> Maybe we need an option to make the compilers output a standard JSON or whatever...

While I like the idea, I fear this would go the same route as UserAgent strings, with everybody giving carefully crafted lies. The main issue is that bugs are only known in retrospect.

As an example, suppose Compiler A and Compiler B both implement Feature X. Therefore, they both produce JSON output indicating that they support Feature X. A year later, developers for Compiler A find and fix a bug in its implementation of Feature X. Now, how do we handle that case?

* Update the version for Feature X as by Compiler A? Now Compiler B is publishing the same version as the buggy Compiler A did, and would get marked as not having a usable implementation.

* Update the spec, forcing a version update for Feature X in both Compiler A and Compiler B? Now a bugfix in one compiler requires code updates in others.

* In the build system, override the JSON output from Compiler A, such that Feature X is marked as unavailable for the buggy versions? Now the JSON is no longer trustable, as it requires maintaining a list of exceptions.

* Try to trigger the bug that Compiler A just fixed, so that the build system can recognize the buggy versions, regardless of the JSON output? Now we're back to the starting point, with features determined based on attempted compilation.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#237

Earlier quoted context omitted.

A misplaced punctuation has some plausible deniability. Like the author could say he was distracted and fat fingered nonsense, honest mistake. A utf character from a language that has zero chance of being mapped to your programmer's keyboard in the middle of a code line, that would be obvious intentional tampering or at the very least raise some eyebrows.

I mean, I agree that the punctuation mishap is a better cover story, but why would any particular language have “zero chance” of being mapped to the programmer’s keyboard?

> why would any particular language have “zero chance” of being mapped to the programmer’s keyboard?

You left out a very important qualifier: in the middle of a code line

The chances of someone accidentally fat-fingering a keyboard layout switch, typing one character, then switching back again without noticing while typing a line of code are very slight indeed.

Plus the fact that you'd need to speak a language written in that alphabet to have a plausible reason for having the keyboard layout enabled in the first place.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#238

Earlier quoted context omitted.

Not everything is detectable at runtime, such as syscall numbers (which is what's being tested for here).

Runtime syscall number detection is very common in practice, since the kernel returns ENOSYS to enable that exact ability for glibc and other shim libraries

That's syscall for _availability_ detection, not its number. There's no way to ask the kernel "what's the number of the syscall commonly known as read()".

Re: Xz: Can you spot the single character that disabled Linux landlock?

#239
post #7

Earlier quoted context omitted.

So that function checked if the following C code compiled, and only in that situation enabled the landlock? Except that lone period, hard to recognize because of its small size and proximity to the left edge of the diff, caused the C code to become always invalid, hence keeping the landlock always disabled? That's both vilely impressive and impressively vile. I didn't even spot it on my first read-through.

Even more evil would have been to replace this line (void)SYS_landlock_create_ruleset; with this: (void)SYS_landloсk_create_ruleset;

With the . the author can claim it was unintentional. It's impossible (or at least very hard) to claim the Cyrillic c is unintentional.

To me, that makes the . more evil.

Re: Xz: Can you spot the single character that disabled Linux landlock?

#240
post #92

Earlier quoted context omitted.

the period right there on the left edge? if I saw that in a patch I'd be through the roof, that looks completely intentional

I love this type of hindsight to 20-10 comment. "If I saw...". That is a BIG if. Plenty of smart people on that mailing also missed it. I missed it myself when I opened the HN lead link. Very subtle.

I dont use mailing lists for patches, I use gerrit that would put a great big highlight over that character
Post reply on HN