public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Luca Boccassi <bluca@debian.org>
To: Sam James <sam@gentoo.org>
Cc: Fangrui Song <i@maskray.me>, Rui Ueyama <rui314@gmail.com>,
	Binutils <binutils@sourceware.org>
Subject: Re: Remove dependency on libjansson
Date: Mon, 1 Apr 2024 10:38:25 +0000	[thread overview]
Message-ID: <CAMw=ZnQGFq86W-L-Uq1m7TRGDvpemoTNeBccXmZtFzB5z+ajjQ@mail.gmail.com> (raw)
In-Reply-To: <877chhe80o.fsf@gentoo.org>

On Mon, 1 Apr 2024 at 10:50, Sam James <sam@gentoo.org> wrote:
>
> Fangrui Song <i@maskray.me> writes:
>
> > On Sun, Mar 31, 2024 at 8:31 PM Rui Ueyama <rui314@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> The recent xz incident demonstrated that supply chain attacks are a
> >> real threat, and dependence on third-party libraries can have
> >> significant consequences.
> >>
> >> In the wake of the incident, I propose we remove the dependency on
> >> libjansson from GNU ld.
> >>
> >> First of all, why does GNU ld depend on libjansson which is a JSON
> >> parsing library? GNU ld gained the `--package-metadata` option in May
> >> 2022 to embed a JSON string into a .note section for package
> >> management for Fedora and other Linux distributions. At the same time,
> >> the dependency on libjansson, a library for parsing JSON-format
> >> strings, was introduced to validate an argument for that option. If an
> >> argument is not a valid JSON string, ld reports an error. If the
> >> library is unavailable, or if `--disable-jansson` was passed to the
> >> configure script, the library will not be linked and the error check
> >> will be disabled. By default, the library will be linked if it exists.
> >>
> >> I opposed adding an extra dependency to GNU ld just for string
> >> verification purposes because it didn't seem worth adding extra
> >> dependency to the linker. LLVM lld and the mold linker also support
> >> the option, but they do not verify if the argument is a valid JSON
> >> string -- they simply treat it as an opaque string. If libjansson is
> >> unavailable, even GNU ld doesn't verify arguments. Therefore, the
> >> verification is not trustworthy, and the reader must be prepared for a
> >> malformed JSON string when reading a .note section. Moreover,
> >> verifying a string is straightforward without the feature; you can
> >> simply `echo` the string to pipe it to `jq` for verification before
> >> passing it to GNU ld.
> >>
> >> I just checked /usr/bin/ld on Ubuntu 24.04, which is set to be
> >> released this month, and the dependency on libjansson was indeed
> >> present.
> >>
> >> How much risk does it pose? Probably not much, as long as the library
> >> is maintained properly. However, the stakes are high; if someone takes
> >> control of the library and introduces malicious code, they could
> >> execute a Ken Thompson-style supply chain attack. Since GNU ld is used
> >> to build essentially everything, the attacker could in theory gain the
> >> power to not just contaminate a specific program such as openssh, but
> >> every executable in an official Linux distribution image. I think the
> >> risk is not worth taking. I believe we just should remove the string
> >> verification code and the dependency on the library from GNU ld.
> >>
> >> Rui Ueyama
> >
> > Thanks for bringing this up again. I support removing the json dependency.
> >
> > I lightly expressed my concern
> > https://sourceware.org/pipermail/binutils/2022-May/120846.html and
> > there might be others unsure about the dependency as well.
>
> I'd like to hear bluca's take before making up my mind. Note that it's
> also automagic right now IIRC (enabled if installed, not opt-in).
>
> But my take on it so far is that it doesn't sound worth it.

$ apt-cache rdepends libjansson4 | wc -l
310

Sorry, but this looks to me like a knee-jerk reaction that misses the
wood for the trees. The xz issue is that an extremely complex and
sophisticated social engineering attack was planned and executed over
multiple years. If it hadn't been on xz, it would have been on another
project. Dropping dependencies left and right without thinking will
not stop the next attack, as there is no shortage of overworked
maintainers to try and abuse. In fact, avoiding common libraries and
everyone reimplementing the wheel _increases_ the attack
opportunities.

The reason for this integration is that finding issues at build time
is cheaper than finding them at runtime or release time. We rely on
this in Fedora, Debian and Ubuntu to ensure valid data is written (in
all packages in the former, in an increasing set of opt-ins in the
latters) - in fact, validating input _before_ shipping binaries
_improves_ security, rather than decreasing it, given the payload is
parsed by a privileged component.

So if it was removed upstream, we'd just have to patch it back in
downstream in each interested distro - which if you recall, was
exactly how sshd was exploited, due to the lack of an upstream common
implementation. This is opt-in, so distributions that are not
interested can just avoid enabling it, with no extra effort required.
So please, leave it as-is. Thank you.

  reply	other threads:[~2024-04-01 10:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01  3:31 Rui Ueyama
2024-04-01  7:28 ` Fangrui Song
2024-04-01  9:39   ` Sam James
2024-04-01 10:38     ` Luca Boccassi [this message]
2024-04-01 10:44       ` Sam James
2024-04-01 11:44       ` Rui Ueyama
2024-04-01 12:16         ` Luca Boccassi
2024-04-01 13:23           ` Rui Ueyama
2024-04-02  0:54             ` Luca Boccassi
2024-04-02  9:40               ` Rui Ueyama
2024-04-02 13:10                 ` Orlando Arias
2024-04-03 14:58                   ` Michael Matz
2024-04-03 15:37                     ` Orlando Arias

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMw=ZnQGFq86W-L-Uq1m7TRGDvpemoTNeBccXmZtFzB5z+ajjQ@mail.gmail.com' \
    --to=bluca@debian.org \
    --cc=binutils@sourceware.org \
    --cc=i@maskray.me \
    --cc=rui314@gmail.com \
    --cc=sam@gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).