From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd36.google.com (mail-io1-xd36.google.com [IPv6:2607:f8b0:4864:20::d36]) by sourceware.org (Postfix) with ESMTPS id A7F1B3858409 for ; Mon, 1 Apr 2024 11:44:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A7F1B3858409 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A7F1B3858409 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::d36 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711971883; cv=none; b=b0yeNohkmJ8ofFr7m1kXVEfD+M6+6PlPYFvpo1cx+kfiZAiNx6Tfte7TZWTWWrsPFzJqA6M2mi9TQ5LkwRitre+tZSEi2SBRxnAVsx8cUBLxF82c9//5eGfM4zHEzStKamZYYXoIV30KEY1JjjdY5vWesHQzfce8cxT6uFtDC+g= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711971883; c=relaxed/simple; bh=U5Gnne1PnVgraVTQdIJLPmLAdVkFjR9Xu4rnsa5y/Hk=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=Sl5QYmnztgAL0+FUilsSRivo372ry77PutEEp+vWh1FEpl1YnXn7FPhGSJO0MtngndjlVtxkuUwQduvNqSQ6pYHRWxcFFbRNBy6qumdcRI/LguMiebhDFTlALm3/0xIZszdOIjbVjvqrDyrBijX661YcN2b1yIJb+Tz74tUQ5Bw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-io1-xd36.google.com with SMTP id ca18e2360f4ac-7cc0b35b75bso132285139f.3 for ; Mon, 01 Apr 2024 04:44:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1711971880; x=1712576680; darn=sourceware.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=U5Gnne1PnVgraVTQdIJLPmLAdVkFjR9Xu4rnsa5y/Hk=; b=AVbed6R0n7ZqWLo0VG9MgW6gfTxt7MC+XfjJdiR4G+sQs22B+/WkbAgvPZprRBZaWu W4vYRrjiiXnNBj5U9Xwep61uxZFYKUSrjU5YReQhER3Bf0YhFwQUb039HtDHYJl8QmG9 t6fuWB7g4NAUH42M9xqIBLDuynT6RbLUs8nN0sCHU68nTw30pjle3974kGOLPnS9HTcu kRTEg305TzZ5rEyv3hvlB0rFe1Z3SUJzzIwRqSqSULNcNBrbJp9GAkrcqeE2p683bf5d 0RztRY+b7QFMyOsgdVg+2vfzdQZyQVlyb0pMuPnGTqPOfchX7WbVro7mo0r8bxOi6rpP /CNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711971880; x=1712576680; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=U5Gnne1PnVgraVTQdIJLPmLAdVkFjR9Xu4rnsa5y/Hk=; b=kNK8Uyj/k40m+dnvK+AVAtLSuOpRb2RPcOEZd1SRUt0heS4QSAXOygbBvTpOsf9KiS keM6Ibvy6Dm/24B3D6VSleQLEYqUkWwlYDdH6k0qPl2X5szbxIKtMaH1xMJRkdKwZeVu ccGi4nSz5utYsC/xxT45O5m+l+WqntRwCZ0fVGSFZs9zkCYSulxwCkszM55h21mfZzXZ m4lFNueCzX3xJ/rCzXkXe1ZLGI/aTl1LRiJAjlRL/DWUX9XxXj+RD2xyyOYba27DnFq0 /9bwKOXwShz9S+CMZVrHBNdHoJI/91Yfya5FDaSlaJcm3iC3qbMmDWt6KLt6+ItPSXpu lvUw== X-Forwarded-Encrypted: i=1; AJvYcCX8wcK84RNIZys9JztfdTIMJF9bifju410ZiS+tXAqFSZrv3/SFL2AUaA3Ao9jbmr7hfO5C3jy1UeIO7xCfbaG8R6MoDAywJg== X-Gm-Message-State: AOJu0Yzbr3sRzBTi49M7Y8yLsOyX8rP+y0xKI92YsWmK543dksKLtL5x iU73lq8NL+vMUAFp2WkE5bytKVyshkaBCmZw2yWz18QoFAlVXkL054hMSFTJgNHZIMbsQRDyKwj S/GCQ0zxGBguMxtktRuGH116S1+TJxjtMn5o= X-Google-Smtp-Source: AGHT+IEmO64O2JMaYqiqfigVQS3ujHNKd+a1Y3dM1uZcyy23NqvbgrKzGDXT+WGQ4amiWvJF5yHT68vYrvkXnML2sz8= X-Received: by 2002:a05:6602:807:b0:7d0:7ee8:5154 with SMTP id z7-20020a056602080700b007d07ee85154mr9483399iow.18.1711971879761; Mon, 01 Apr 2024 04:44:39 -0700 (PDT) MIME-Version: 1.0 References: <877chhe80o.fsf@gentoo.org> In-Reply-To: From: Rui Ueyama Date: Mon, 1 Apr 2024 20:44:28 +0900 Message-ID: Subject: Re: Remove dependency on libjansson To: Luca Boccassi Cc: Sam James , Fangrui Song , Binutils Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Apr 1, 2024 at 7:38=E2=80=AFPM Luca Boccassi wro= te: > > On Mon, 1 Apr 2024 at 10:50, Sam James wrote: > > > > Fangrui Song writes: > > > > > On Sun, Mar 31, 2024 at 8:31=E2=80=AFPM Rui Ueyama = 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 Ma= y > > >> 2022 to embed a JSON string into a .note section for package > > >> management for Fedora and other Linux distributions. At the same tim= e, > > >> 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 exist= s. > > >> > > >> 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 librar= y > > >> is maintained properly. However, the stakes are high; if someone tak= es > > >> control of the library and introduces malicious code, they could > > >> execute a Ken Thompson-style supply chain attack. Since GNU ld is us= ed > > >> to build essentially everything, the attacker could in theory gain t= he > > >> power to not just contaminate a specific program such as openssh, bu= t > > >> every executable in an official Linux distribution image. I think th= e > > >> 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 depend= ency. > > > > > > 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. I'd like to say that that's not an instinctive reaction to the xz incident, as I expressed a concern when the dependency was being introduced. I also want to say that I'm not trying to drop dependencies left and right without thinking. I understand that the error check would be sometimes useful, but I'd like to propose reevaluating its cost and benefit. IIUC, the --package-metadata option is, as its name suggests, used by packaging tools such as dpkg-dev or rpmbuild. My understanding is that this option is not intended to be used directly by developers at large, but rather, the JSON string is generated by a very limited set of packaging tools from package description files and passed to the linker. If that's the case, the string format check itself is an extra measure to catch rare string construction bugs, given that JSON strings are machine-generated and not manually crafted. While we may still do extra checks for catching possible tooling bugs, I wonder how difficult it would be to assign this additional verification to jq or a similar tool instead of the linker. IIRC, dpkg comes with a tool to scan all executables in a directory to find packaging errors. Isn't it a better place to run the extra format check? Would it be too complex to output the .note section contents after linking and pipe them to jq to confirm the embedded string is a valid JSON string?