Security lessons from liblzma
You might have seen reports today about a very complex exploit added to
recent versions of liblzma. Fortunately, it was only enabled two months
ago and has not been pushed to most stable operating systems like Debian
and Ubuntu. The original detection report is:
https://www.openwall.com/lists/oss-security/2024/03/29/4
And this ycombinator discussion has details:
https://news.ycombinator.com/item?id=39865810
It looks like an earlier commit with a binary blob "test data"
contained the bulk of the backdoor, then the configure script
enabled it, and then later commits patched up valgrind errors
caused by the backdoor. See the commit links in the "Compromised
Repository" section.
and I think the configure came in through the autoconf output file
'configure', not configure.ac:
This is my main take-away from this. We must stop using upstream
configure and other "binary" scripts. Delete them all and run
"autoreconf -fi" to recreate them. (Debian already does something
like this I think.)
Now, we don't take pull requests, and all our committers are known
individuals, but this might have cautionary lessons for us.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Sat, Mar 30, 2024 at 11:37 AM Bruce Momjian <bruce@momjian.us> wrote:
You might have seen reports today about a very complex exploit added to
recent versions of liblzma. Fortunately, it was only enabled two months
ago and has not been pushed to most stable operating systems like Debian
and Ubuntu. The original detection report is:
Incredible work from Andres. The attackers made a serious strategic
mistake: they made PostgreSQL slightly slower.
Hi,
On 2024-03-29 18:37:24 -0400, Bruce Momjian wrote:
You might have seen reports today about a very complex exploit added to
recent versions of liblzma. Fortunately, it was only enabled two months
ago and has not been pushed to most stable operating systems like Debian
and Ubuntu. The original detection report is:https://www.openwall.com/lists/oss-security/2024/03/29/4
And this ycombinator discussion has details:
https://news.ycombinator.com/item?id=39865810
It looks like an earlier commit with a binary blob "test data"
contained the bulk of the backdoor, then the configure script
enabled it, and then later commits patched up valgrind errors
caused by the backdoor. See the commit links in the "Compromised
Repository" section.and I think the configure came in through the autoconf output file
'configure', not configure.ac:This is my main take-away from this. We must stop using upstream
configure and other "binary" scripts. Delete them all and run
"autoreconf -fi" to recreate them. (Debian already does something
like this I think.)
I don't think that's a useful lesson here, actually. In this case, if you ran
autoreconf -fi in a released tarball, it'd just recreate precisely what
the tarball already contained, including the exploit.
I think the issue in this case rather was that the tarball contains files that
are not in the release - a lot of them. The attackers injected the
'activating' part of the backdoor into the release tarball, while it was not
present in the git tree. They did that because they knew that distributions
often build from release tarballs.
If the release pre-backdoor release tarball had been identical to the git
repository, this would likely have been noticed by packagers - but it was
normal for there to be a lot of new files.
We traditionally had also a lot of generated files in the tarball that weren't
in our git tree - but we removed a lot of that a few months ago, when we
stopped including bison/flex/other generated code.
We still include generated docs, but that's much harder to exploit at
scale. However, they still make it harder to verify that the release is
exactly the same as the git tree.
Now, we don't take pull requests, and all our committers are known
individuals, but this might have cautionary lessons for us.
I am doubtful that every committer would find something sneaky hidden in
e.g. one of the test changes in a large commit. It's not too hard to hide
something sneaky. I comparison to that hiding something in configure.ac seems
less likely to succeed IMO, that imo tends to be more scrutinized. And hiding
just in configure directly wouldn't get you far, it'd just get removed when
the committer or some other committer at a later time, regenerates configure.
Greetings,
Andres Freund
On 29 Mar 2024, at 23:59, Andres Freund <andres@anarazel.de> wrote:
On 2024-03-29 18:37:24 -0400, Bruce Momjian wrote:
Now, we don't take pull requests, and all our committers are known
individuals, but this might have cautionary lessons for us.I am doubtful that every committer would find something sneaky hidden in
e.g. one of the test changes in a large commit. It's not too hard to hide
something sneaky.
One take-away for me is how important it is to ship recipes for regenerating
any testdata which is included in generated/compiled/binary format. Kind of
how we in our tree ship the config for test TLS certificates and keys which can
be manually inspected, and used to rebuild the testdata (although the risk for
injections in this particular case seems low). Bad things can still be
injected, but formats which allow manual review at least goes some way towards
lowering risk.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
One take-away for me is how important it is to ship recipes for regenerating
any testdata which is included in generated/compiled/binary format.
IMO that's a hard, no-exceptions requirement just for
maintainability's sake, never mind security risks.
regards, tom lane
On Fri, Mar 29, 2024 at 7:00 PM Andres Freund <andres@anarazel.de> wrote:
I am doubtful that every committer would find something sneaky hidden in
e.g. one of the test changes in a large commit. It's not too hard to hide
something sneaky. I comparison to that hiding something in configure.ac seems
less likely to succeed IMO, that imo tends to be more scrutinized. And hiding
just in configure directly wouldn't get you far, it'd just get removed when
the committer or some other committer at a later time, regenerates configure.
I agree with this. If I were trying to get away with a malicious
commit, I'd look for files that other people would be unlikely to
examine closely, or would have difficulty examining closely. Test data
or test scripts seem like great possibilities. And I also would like
it to be part of some relatively large commit that is annoying to read
through visually. We don't have a lot of binary format files in the
tree, which is good, but there's probably some things like Unicode
tables and ECPG expected output files that very, very few people ever
actually examine. If we had a file in the tree that looked based on
the name like an expected output file for a test, but there was no
corresponding test, how many of us would notice that? How many of us
would scrutinize it? Imagine hiding something bad in the middle of
that file somewhere.
Maybe we need some kind of tool that scores files for risk. Longer
files are riskier. Binary files are riskier, as are text files that
are something other than plain English/C code/SGML. Files that haven't
changed in a long time are not risky, but files with few recent
changes are riskier than files with many recent changes, especially if
only 1 or 2 committers made all of those changes, and especially if
those commits also touched a lot of other files. Of course, if we had
a tool like this that were public, I suppose anyone targeting PG would
look at the tool and try to find ways around its heuristics. But maybe
we should have something and not disclose the whole algorithm
publicly, or even if we do disclose it all, having something is
probably better than having nothing. It might force a hypothetical bad
actor to do things that would be more likely to be noticed by the
humans.
We might also want to move toward signing commits and tags. One of the
meson maintainers was recommending that on-list not long ago.
We should think about weaknesses that might occur during the packaging
process, too. If someone who alleges that their packaging PG is really
packaging PG w/badstuff123.patch, how would we catch that?
An awful lot of what we do operates on the principle that we know the
people who are involved and trust them, and I'm glad we do trust them,
but the world is full of people who trusted somebody too much and
regretted it afterwards. The fact that we have many committers rather
than a single maintainer probably reduces risk at least as far as the
source repository is concerned, because there are more people paying
attention to potentially notice something that isn't as it should be.
But it's also more potential points of compromise, and a lot of things
outside of that repository are not easy to audit. I can't for example
verify what the infrastructure team is doing, or what Tom does when he
builds the release tarballs. It seems like a stretch to imagine
someone taking over Tom's online identity while simultaneously
rendering him incommunicado ... but at the same time, the people
behind this attack obviously put a lot of work into it and had a lot
of resources available to craft the attack. We shouldn't make the
mistake of assuming that bad things can't happen to us.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2024-03-30 16:50:26 -0400, Robert Haas wrote:
We might also want to move toward signing commits and tags. One of the
meson maintainers was recommending that on-list not long ago.
I don't know how valuable the commit signing really is, but I strongly agree
that we should sign both tags and tarballs.
We should think about weaknesses that might occur during the packaging
process, too. If someone who alleges that their packaging PG is really
packaging PG w/badstuff123.patch, how would we catch that?
I don't think we realistically can. The environment, configure and compiler
options will influence things too much to do any sort of automatic
verification.
But that shouldn't stop us from ensuring that at least the packages
distributed via *.postgresql.org are reproducibly built.
Another good avenue for introducing an attack would be to propose some distro
specific changes to the packaging teams - there's a lot fewer eyes there. I
think it might be worth working with some of our packagers to integrate more
of their changes into our tree.
I can't for example verify what the infrastructure team is doing, or what
Tom does when he builds the release tarballs.
This one however, I think we could improve upon. Making sure the tarball
generation is completely binary reproducible and providing means of checking
that would surely help. This will be a lot easier if we, as dicussed
elsewhere, I believe, split out the generated docs into a separately
downloadable archive. We already stopped including other generated files
recently.
We shouldn't make the mistake of assuming that bad things can't happen to
us.
+1
Greetings,
Andres Freund
On Sat, Mar 30, 2024 at 04:50:26PM -0400, Robert Haas wrote:
On Fri, Mar 29, 2024 at 7:00 PM Andres Freund <andres@anarazel.de> wrote:
I am doubtful that every committer would find something sneaky hidden in
e.g. one of the test changes in a large commit. It's not too hard to hide
something sneaky. I comparison to that hiding something in configure.ac seems
less likely to succeed IMO, that imo tends to be more scrutinized. And hiding
just in configure directly wouldn't get you far, it'd just get removed when
the committer or some other committer at a later time, regenerates configure.I agree with this. If I were trying to get away with a malicious
commit, I'd look for files that other people would be unlikely to
examine closely, or would have difficulty examining closely. Test data
or test scripts seem like great possibilities. And I also would like
it to be part of some relatively large commit that is annoying to read
through visually. We don't have a lot of binary format files in the
tree, which is good, but there's probably some things like Unicode
tables and ECPG expected output files that very, very few people ever
actually examine. If we had a file in the tree that looked based on
the name like an expected output file for a test, but there was no
corresponding test, how many of us would notice that? How many of us
would scrutinize it? Imagine hiding something bad in the middle of
that file somewhere.
So, in this case, the hooks were in 'configure', but not configure.ac,
and the exploit was in a test file which was in the tarball but _not_ in
the git tree. So, they used the obfuscation of 'configure's syntax, and
the lack of git oversight by not putting the test files in the git tree.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On 3/30/24 17:12, Andres Freund wrote:
Hi,
On 2024-03-30 16:50:26 -0400, Robert Haas wrote:
We might also want to move toward signing commits and tags. One of the
meson maintainers was recommending that on-list not long ago.I don't know how valuable the commit signing really is, but I strongly agree
that we should sign both tags and tarballs.
+1
We should think about weaknesses that might occur during the packaging
process, too. If someone who alleges that their packaging PG is really
packaging PG w/badstuff123.patch, how would we catch that?I don't think we realistically can. The environment, configure and compiler
options will influence things too much to do any sort of automatic
verification.But that shouldn't stop us from ensuring that at least the packages
distributed via *.postgresql.org are reproducibly built.Another good avenue for introducing an attack would be to propose some distro
specific changes to the packaging teams - there's a lot fewer eyes there. I
think it might be worth working with some of our packagers to integrate more
of their changes into our tree.
Huge +1 to that. I have thought many times, and even said to Devrim, a
huge number of people trust him to not be evil.
Virtually every RPM source, including ours, contains out of tree patches
that get applied on top of the release tarball. At least for the PGDG
packages, it would be nice to integrate them into our git repo as build
options or whatever so that the packages could be built without any
patches applied to it. Add a tarball that is signed and traceable back
to the git tag, and we would be in a much better place than we are now.
I can't for example verify what the infrastructure team is doing,
Not sure what you feel like you should be able to follow -- anything
specific?
or what Tom does when he builds the release tarballs.
Tom follows this, at least last time I checked:
https://wiki.postgresql.org/wiki/Release_process
This one however, I think we could improve upon. Making sure the tarball
generation is completely binary reproducible and providing means of checking
that would surely help. This will be a lot easier if we, as dicussed
elsewhere, I believe, split out the generated docs into a separately
downloadable archive. We already stopped including other generated files
recently.
again, big +1
We shouldn't make the mistake of assuming that bad things can't happen to
us.+1
and again with the +1 ;-)
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On 3/30/24 19:54, Joe Conway wrote:
On 2024-03-30 16:50:26 -0400, Robert Haas wrote:
or what Tom does when he builds the release tarballs.
Tom follows this, at least last time I checked:
Reading through that, I wonder if this part is true anymore:
In principle this could be done anywhere, but again there's a concern
about reproducibility, since the results may vary depending on
installed bison, flex, docbook, etc versions. Current practice is to
always do this as pgsql on borka.postgresql.org, so it can only be
done by people who have a login there. In detail:
Maybe if we split out the docs from the release tarball, we could also
add the script (mk-release) to our git repo?
Some other aspects of that wiki page look out of date too. Perhaps it
needs an overall update? Maybe Tom and/or Magnus could weigh in here.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Joe Conway <mail@joeconway.com> writes:
On 3/30/24 19:54, Joe Conway wrote:
Tom follows this, at least last time I checked:
https://wiki.postgresql.org/wiki/Release_process
Reading through that, I wonder if this part is true anymore:
In principle this could be done anywhere, but again there's a concern
about reproducibility, since the results may vary depending on
installed bison, flex, docbook, etc versions. Current practice is to
always do this as pgsql on borka.postgresql.org, so it can only be
done by people who have a login there. In detail:
The reproducibility argument would still apply to the docs (in
whatever form we're packaging them), but hopefully not to the
basic source tarball.
Maybe if we split out the docs from the release tarball, we could also
add the script (mk-release) to our git repo?
If memory serves, the critical steps are already in our source tree,
as "make dist" (but I'm not sure how that's going to work if we want
to get away from using autoconf/make). It's not clear to me how much
of the rest of mk-release is relevant to people who might be trying to
generate things elsewhere. I'd like mk-release to continue to work
for older branches, too, so it's going to be some sort of hybrid mess
for a few years here.
regards, tom lane
On Sat, Mar 30, 2024 at 07:54:00PM -0400, Joe Conway wrote:
Virtually every RPM source, including ours, contains out of tree patches
that get applied on top of the release tarball. At least for the PGDG
packages, it would be nice to integrate them into our git repo as build
options or whatever so that the packages could be built without any patches
applied to it. Add a tarball that is signed and traceable back to the git
tag, and we would be in a much better place than we are now.
How would someone access the out-of-tree patches? I think Debian
includes the patches in its source tarball.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On Sat, Mar 30, 2024 at 09:52:47PM -0400, Bruce Momjian wrote:
On Sat, Mar 30, 2024 at 07:54:00PM -0400, Joe Conway wrote:
Virtually every RPM source, including ours, contains out of tree patches
that get applied on top of the release tarball. At least for the PGDG
packages, it would be nice to integrate them into our git repo as build
options or whatever so that the packages could be built without any patches
applied to it. Add a tarball that is signed and traceable back to the git
tag, and we would be in a much better place than we are now.How would someone access the out-of-tree patches? I think Debian
includes the patches in its source tarball.
If you ask where they are maintained, the answer is here:
https://salsa.debian.org/postgresql/postgresql/-/tree/17/debian/patches?ref_type=heads
the other major versions have their own branch.
Michael
On 3/30/24 21:52, Bruce Momjian wrote:
On Sat, Mar 30, 2024 at 07:54:00PM -0400, Joe Conway wrote:
Virtually every RPM source, including ours, contains out of tree patches
that get applied on top of the release tarball. At least for the PGDG
packages, it would be nice to integrate them into our git repo as build
options or whatever so that the packages could be built without any patches
applied to it. Add a tarball that is signed and traceable back to the git
tag, and we would be in a much better place than we are now.How would someone access the out-of-tree patches? I think Debian
includes the patches in its source tarball.
I am saying maybe those patches should be eliminated in favor of our
tree including build options that would produce the same result.
For example, these patches are applied to our release tarball files when
the RPM is being built for pg16 on RHEL 9:
Nothing too crazy, but wouldn't it be better if no patches were required
at all?
Ideally we should have reproducible builds so that starting with our
tarball (which is traceable back to the git release tag) one can easily
obtain the same binary as what the RPMs/DEBs deliver.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi,
On Sun, 2024-03-31 at 08:15 -0400, Joe Conway wrote:
I am saying maybe those patches should be eliminated in favor of our
tree including build options that would produce the same result.
Works for me, as a long as I can commit them and upcoming potential
patches to PostgreSQL git repo.
OTOH, we also carry non-patches like README files, systemd unit files,
pam files, setup script, etc., which are very RPM specific.
Regards,
--
Devrim Gündüz
Open Source Solution Architect, PostgreSQL Major Contributor
Twitter: @DevrimGunduz , @DevrimGunduzTR
Joe Conway <mail@joeconway.com> writes:
I am saying maybe those patches should be eliminated in favor of our
tree including build options that would produce the same result.
I don't really see how that can be expected to work sanely.
It turns the responsibility for platform-specific build issues
on its head, and it doesn't work at all for issues discovered
after we make a release. The normal understanding of how you
can vet a distro's package is that you look at the package
contents (the SRPM in Red Hat world and whatever the equivalent
concept is elsewhere), check that the contained tarball
matches upstream and that the patches and build instructions
look sane, and then build it locally and check for a match to
the distro's binary package. Even if we could overcome the
obstacles to putting the patch files into the upstream tarball,
we're surely not going to include the build instructions, so
we'd not have moved the needle very far in terms of whether the
packager could do something malicious.
regards, tom lane
Hi,
On Sat, 2024-03-30 at 21:52 -0400, Bruce Momjian wrote:
How would someone access the out-of-tree patches?
Here are the v17 patches:
You can replace -17 with -16 (and etc) for the other major releases.
Please note that both Debian folks and me build about 300 other packages
to support the ecosystem. Just saying.
Regards,
--
Devrim Gündüz
Open Source Solution Architect, PostgreSQL Major Contributor
Twitter: @DevrimGunduz , @DevrimGunduzTR
On 3/31/24 11:49, Tom Lane wrote:
Joe Conway <mail@joeconway.com> writes:
I am saying maybe those patches should be eliminated in favor of our
tree including build options that would produce the same result.I don't really see how that can be expected to work sanely.
It turns the responsibility for platform-specific build issues
on its head, and it doesn't work at all for issues discovered
after we make a release. The normal understanding of how you
can vet a distro's package is that you look at the package
contents (the SRPM in Red Hat world and whatever the equivalent
concept is elsewhere), check that the contained tarball
matches upstream and that the patches and build instructions
look sane, and then build it locally and check for a match to
the distro's binary package. Even if we could overcome the
obstacles to putting the patch files into the upstream tarball,
we're surely not going to include the build instructions, so
we'd not have moved the needle very far in terms of whether the
packager could do something malicious.
True enough I guess.
But it has always bothered me how many patches get applied to the
upstream tarballs by the OS package builders. Some of them, e.g. glibc
on RHEL 7, include more than 1000 patches that you would have to
manually vet if you cared enough and had the skills. Last time I looked
at the openssl package sources it was similar in volume and complexity.
They might as well be called forks if everyone were being honest about it...
I know our PGDG packages are no big deal compared to that, fortunately.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Hi,
On Sun, Mar 31, 2024 at 01:05:40PM -0400, Joe Conway wrote:
But it has always bothered me how many patches get applied to the upstream
tarballs by the OS package builders. Some of them, e.g. glibc on RHEL 7,
include more than 1000 patches that you would have to manually vet if you
cared enough and had the skills. Last time I looked at the openssl package
sources it was similar in volume and complexity. They might as well be
called forks if everyone were being honest about it...
I think this more an artifact of how RHEL development works, i.e. trying
to ship the same major version of glibc for 10 years, but still fix lots
of bugs and possibly some performance improvements your larger customers
ask for. So I guess a lot of those 1000 patches are just cherry-picks /
backports of upstream commits from newer releases.
I guess it would be useful to maybe have another look at the patches
that are being applied for apt/yum.postgresql.org for the 18 release
cycle, but I do not think those are a security problem. Not sure about
RPM builds, but at least in theory the APT builds should be
reproducible.
What would be a significant gain in security/trust was an easy
service/recipe on how to verify the reproducibility (i) by independently
building packages (and maybe the more popular extensions) and comparing
them to the {apt,yum}.postgresql.org repository packages (ii) by being
able to build the release tarballs reproducibly.
Michael
Michael Banck <mbanck@gmx.net> writes:
On Sun, Mar 31, 2024 at 01:05:40PM -0400, Joe Conway wrote:
But it has always bothered me how many patches get applied to the upstream
tarballs by the OS package builders.
I think this more an artifact of how RHEL development works, i.e. trying
to ship the same major version of glibc for 10 years, but still fix lots
of bugs and possibly some performance improvements your larger customers
ask for. So I guess a lot of those 1000 patches are just cherry-picks /
backports of upstream commits from newer releases.
Yeah. Also, precisely because they keep supporting versions that are
out-of-support according to upstream, the idea that all the patches
can be moved upstream isn't going to work for them, and they're
unlikely to be excited about partial solutions.
The bigger problem though is: if we do this, are we going to take
patches that we fundamentally don't agree with? For instance,
if a packager chooses to rip out the don't-run-server-as-root check.
(Pretty sure I've heard of people doing that.) That would look like
blessing things we don't think are good ideas, and it would inevitably
lead to long arguments with packagers about why-dont-you-do-this-some-
other-way. I'm not excited about that prospect.
regards, tom lane
