Conclusions from CVE-2024-3094 (libxz disaster)

[Daniel Golle]

On Mon, Apr 01, 2024 at 02:49:46PM +0200, Petr Štetiar wrote:
> Daniel Golle <daniel at makrotopia.org> [2024-03-30 15:30:49]:
> 
> Hi,
> 
> > In many ways, we are already better
> 
> I would probably avoid such bold statements and would be more humble, since
> you never know why OpenWrt wasn't directly targeted.

We are not "better" but "better off". You cut my sentence in a quite
significant place here. Mind that little word "off" here which makes
all the difference. I assume you misread my statement, no offence,
of course.

It did not at all intend to be a bold statement regarding OpenWrt's
security practises what-so-ever.

Again:
"In many ways, we are already better off than most Linux distros out
there -- not because of deliberate decisions with security in mind,
but because of our tendency to minimalism and avoiding bloat due to
resource limitations on the target devices, and having a reduced
attack surface is just an indirect consequence of that."

Maybe I should have added ", at best." at the end of the sentence.

> 
> > I believe that the current tendency to use tarballs rather than
> > (reproducible!) git checkouts is also problematic to begin with.
> 
> Git checkouts are currently problematic as well, IIRC the build is going to
> happily use whatever Git is happy with. I mean, if the hash of the downloaded
> tarball source code doesn't match, then the tarball is removed and Git clone
> is performed, new source code tarball is produced, but the tarball hash is not
> going to be checked again.
> 
> Perhaps this package source code integrity checks should be mandatory, not
> optional?

I agree, especially also as PKG_SOURCE_VERSION isn't necessarily a
hash, but can also be a reference to a git tag -- and that can be
replaced by a malicious maintainer or git hoster (though it would be
kinda stupid, as everyone with a downstream copy of the repo would
most likely notice that).

Never the less: A git tag does not really replace an integrity check,
especially as we also don't very signed git tags at all.

> 
> > So why not **always** use that instead of potentially shady and hard to
> > verify tarballs?
> 
> In this case, they were targeting specific audience and this attack vector was
> cheapest/fastest, so the source code origin doesn't really matter.

I tend to slightly disagree, because an attacker will always chose
what ever means are necessary or sufficient. Using tarballs instead of
checkouts increases the attack surface in the sense that validating
the tarball content is extra work for package maintainers.

> 
> > Why do we need to rely one proprietary hacks such as Gibhub codeload
> > just to safe a few megabytes of traffic and a few seconds of build
> > time?
> 
> Ok, I don't like GH either, but I find this irrelevant, origin of the source
> code is not a problem, the content is the problem.

... and that crazy m4 script had to be noticed. As a diff one would ask:
Why was that change necessary?

Maybe we can learn a bit from Android's build system here:
They unpack the release tarballs into a git repo, so that makes the diff
more visible and obvious again for maintainers. That would get the best
of both worlds (at a significant resource pricetag, though...)

> 
> > There are even too many problems to reproduce even those supposedly
> > automated Github-generated tarballs. Nobody actually checks that.
> 
> FYI we do on the CI https://github.com/openwrt/actions-shared-workflows/blob/main/.github/workflows/reusable_build.yml#L224

Nice one. Wasn't aware of that.

> 
> > 9bd7d8b, c7c2257, 77368ec, 86994e1, 954142f, 4c5d910, 21f713d, ...
> > Probably all of those have trivial causes and there isn't anything
> > malicious going on there.
> 
> I agree, I guess, that in some cases it might point to a subtle bug somewhere
> in the source code tarball packaging path (host kernel, tools, container?),
> maybe another backdoor in the works/testing? :P

0_o

> 
> Anyway, we should perhaps consider treating this situations in supply chain
> more seriously, so perhaps in this cases of package hash failures, we need to
> document it better, with more details in the commit message and maybe even
> better, gather always more evidence in a separate GH issue, so its possible to
> reconstruct the complete picture if we really find out 2 years later, that it
> was something malicious going on somewhere? Whatever it might be.

+1

> 
> > Always using git checkouts instead of tarballs would also makes it
> > much easier for maintainers to at least have a quick look at the
> > changes made in an upstream project between versions (a quick scroll
> > over  'git diff oldtag..newtag' or even just 'git log --stat
> > oldtag..newtag' doesn't take much more time than manually validating a
> 
> Although I mostly (always?) doing that Git diff/log, during bumps/reviews, I'm
> sorry, but I'm not able to spot such masked backdoors :-)

Honestly, I believe in this case it would have at least rised on of my
eyebrows. Not the binaries for the test-cases (where the actual
exploit code is hidden), I would have totally ignored those, I admit.
But the m4 hackery which is excessive and seemingly unnecessary.

> 
> > release tarball GPG signature in most cases, if there even is any...).
> 
> And speaking about the signatures:
> 
>  openwrt.git $ ~/bin/git-signed-commits-stats.sh "1 year ago"
> 
>    Total commits: 3267 (since 1 year ago)
>    Good signatures: 94 (2.00%)
>    Unsigned commits: 2456 (75.00%)
>    Bad signatures: 0 (0%)
>    Unknown signature status: 717 (21.00%)
> 
>  (script source https://gist.github.com/ynezz/e6f4219b7be20fa0f44b0f3832c26d59)

Nice script! And yes, should we encourage signing commits?
Another bullet for the to-be-written maintainer guidelines, I suppose.

> 
> > Hiding a malicious change in a commit is infinitely harder than hiding it in
> > a tarball.
> 
> Harder? Yes. Impossible? No.

That's the whole point, ain't it?

I mean, sure, "impossible" would be beautiful, but all of Linux and
the ecosystem around it in general is far from being constructed in a
way that would allow for such assesments. Throw it all away and start
from scratch, forget about C and UNIX and all that then. Sadly.


Cheers


Daniel