From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id gMSbGsnlgWB5eAEAgWs5BA (envelope-from ) for ; Thu, 22 Apr 2021 23:08:25 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id bv8RFsnlgWCfYgAAbx9fmQ (envelope-from ) for ; Thu, 22 Apr 2021 21:08:25 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 16F361A479 for ; Thu, 22 Apr 2021 23:08:25 +0200 (CEST) Received: from localhost ([::1]:58496 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZgYi-0002RF-A7 for larch@yhetil.org; Thu, 22 Apr 2021 17:08:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48446) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lZgYZ-0002R9-RJ for guix-devel@gnu.org; Thu, 22 Apr 2021 17:08:15 -0400 Received: from mira.cbaines.net ([2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27]:57017) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lZgYX-0007Hu-Jf for guix-devel@gnu.org; Thu, 22 Apr 2021 17:08:15 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id ED7B127BC7C; Thu, 22 Apr 2021 22:08:11 +0100 (BST) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id d448da50; Thu, 22 Apr 2021 21:08:11 +0000 (UTC) References: <87tunz11mf.fsf@netris.org> <87r1j30xmo.fsf@netris.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: =?utf-8?Q?L=C3=A9o?= Le Bouter Subject: Re: A "cosmetic changes" commit that removes security fixes In-reply-to: Date: Thu, 22 Apr 2021 22:08:09 +0100 Message-ID: <8735vihvt2.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=2a01:7e00:e000:2f8:fd4d:b5c7:13fb:3d27; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Raghav Gururajan , Leo Prikler , guix-devel@gnu.org Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1619125705; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=8+LfsOZJGfRB53UHCWMrckiN1js7j7L+l76kXc2lKCE=; b=s63wmPCpdVlD0JLx0Dv0hrmYynTkvHsyUnPbmVwSWLfardVuhxf/OmIBd+P0Og5avLfzvc t0Os8eHDR3bcUBUqckaM2Q9BKRFAJaHcrWMcFCi5NJrVtZLtBZtywhnJd907c5VjVQ4IvF NodVbH1j+WQFxVey3JgL6T6FO9Z/zAUc+2EdvSaEhFqs8VDAEWeLyBefO/SYKgmED2tJi3 aONAufgkn1aaqdCdRiMGrdL4/ptYEOuUOb1i6Cu1Kc1bDxP/54KhsYhAwAtObzh5+6ATll iw4L/XJePEvpyizNzLZG89Dg2N1nAXyO8y6Yj49/zcMdb3LBr8V8Va4y5C/taQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1619125705; a=rsa-sha256; cv=none; b=VGbpfsjTQnJrloDXoW/qRYDh1BHalt8OUeo1fgsb8fevJdIqC1U+s0A1LqqOR0k4N876Nz 8X5n7dCasplkDU4A9l1RdEp78wYEml3k4++p2NWGTArXnikG7NhlooRwB64jq6byZDkD/m 0cTfOQ7SWEFB4UE28YdZfAU3ROxCkMvo0N5iBMp1SNTELhIVYx8GJ1yXQ56d+Tab0/S273 c6kzbGfFquecQUQARP2Xp9NU3HXX0vxmDIUXW6ed5P7TrJ6oe8OipdK58AWquOmPV8rKdF 4q8tSIe/Nwr2CQucHH0HlhvN3QtIeYL6yiOOTR0HCgPiDcJPIi3WUFU7WbkkCg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -4.55 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 16F361A479 X-Spam-Score: -4.55 X-Migadu-Scanner: scn0.migadu.com X-TUID: n1eOtMfX1yq2 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable L=C3=A9o Le Bouter writes: > On Thu, 2021-04-22 at 00:08 -0400, Mark H Weaver wrote: >> Hi Raghav, >>=20 >> Raghav Gururajan writes: >>=20 >> > > Those commits on 'core-updates' were digitally signed by L=C3=A9o Le >> > > Bouter >> > > and have the same problems: they remove >> > > security >> > > fixes, and yet the summary lines indicate that only "cosmetic >> > > changes" >> > > were made. >> >=20 >> > Yeah, the commit title didn't mention the change but the commit >> > message did. >>=20 >> I'm sorry, but that won't do. There are at least three things wrong >> with these commits: >>=20 >> (1) The summary lines were misleading, because they implied that no >> functional changes were made. >>=20 >> (2) The commit messages were misleading, because they failed to >> mention >> that security holes which had previously been fixed were now >> being >> re-introduced. That wasn't at all obvious. >>=20 >> Commits like these, which remove patches that had fixed security >> flaws, are fairly common: someone casually looking over the >> commit >> log might assume that the patches could be safely removed because >> a >> version update was done at the same time, rendering those patches >> obsolete. >>=20 >> (3) Although your 'glib' commit was immediately followed by a 'glib' >> update, rendering it harmless, your misleading 'cairo' commit >> left >> 'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our >> 'core-updates' and 'wip-gnome' branches. Those will need to be >> fixed now. >>=20 >> L=C3=A9o Le Bouter is also culpable here, because = he >> digitally signed the misleading 'cairo' commit that's on our >> 'core-updates' branch, which re-introduced CVE-2018-19876 and >> CVE-2020-35492. >>=20 >> --8<---------------cut here---------------start------------->8--- >> commit f94cdc86f644984ca83164d40b17e7eed6e22091 >> gpg: Signature made Fri 26 Mar 2021 05:13:57 PM EDT >> gpg: using RSA key >> 148BCB8BD80BFB16B1DE0E9145A8B1E86BCD10A6 >> gpg: Good signature from "L=C3=A9o Le Bouter " >> [unknown] >> gpg: WARNING: This key is not certified with a trusted signature! >> gpg: There is no indication that the signature belongs to >> the owner. >> Primary key fingerprint: 148B CB8B D80B FB16 B1DE 0E91 45A8 B1E8 >> 6BCD 10A6 >> Author: Raghav Gururajan >> Date: Fri Dec 4 00:48:43 2020 -0500 >>=20 >> gnu: cairo: Make some cosmetic changes. >>=20=20=20=20=20 >> * gnu/packages/patches/cairo-CVE-2018-19876.patch, >> gnu/packages/patches/cairo-CVE-2020-35492.patch: Remove patches. >> * gnu/local.mk (dist_patch_DATA): Unregister them. >> * gnu/packages/gtk.scm (cairo): Make some cosmetic changes. >> [replacement]: Remove. >> (cairo/fixed): Remove. >>=20=20=20=20=20 >> Signed-off-by: L=C3=A9o Le Bouter >> --8<---------------cut here---------------end--------------->8--- >>=20 >> https://git.sv.gnu.org/cgit/guix.git/commit/?h=3Dcore-updates&id=3Df94cd= c86f644984ca83164d40b17e7eed6e22091 >>=20 >> Even the most superficial skimming of this commit should have >> immediately raised red flags, because the summary line is clearly >> inaccurate. It shows a lack of careful review, to put it mildly. >>=20 >> Mark > > Hello Mark, > > I don't share your analysis, the security fixes werent stripped because > glib/cairo was also updated to latest version in subsequent commits > which were pushed all at once. > > Careful review was done, and that's why I signed-off and GPG-signed the > commits. Nobody was put at risk by these commits and no security fixes > were stripped. I think the guidance is that commits should include one set of related changes, so if the patches/replacement can be removed because the package is being updated, those related changes should be in the same commit. If there are other unrelated changes, they can go in other commits. Especially if the commits are being pushed at the same time, it's worth making sure this happens, so that it's easier to review and look at the changes in a sensible way. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmCB5blfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XcnVw/6A5EQIxsrqQxOz9+94DrtuSUv6tUIr6V3 5H/CdY6qxkaxfjbfURWV9yTFSIvn1cWR3ZroxQl4JpKy31gkYyfWoyhDBgv5SFXl OX3VUYtDAQn5CP44HIQkSgyhlJPkM2hLhe4rnqmBnVwaVntOJ/nofENv6FmnN6DR 44JTBqpD/cqGfDdpjtxpY5CWgEGQz/tRj+CoeGGWiRgE0CU61bNb9tD13/L4jP3n 84jP41ADCaGwgfjUaBc/4un/GHnYLdZFfCHeNSUUgMAufTypzrwVcSXtao1Pg9lg 8DPb+SzTJzaMUsFAR9SLGetOElpZthUni9SvqRgvbf2Gaz3hc3QTqbSfFq2vIZvk typc89XevGhu/4CCu7XjFpq3KX//1PMvwoC6xKGcWF56gOer/LJwg4t2h4Z+zqah eCASvgAbefoCfMBjx8J1dEvtplRpREZjTzSIzukomjfMQYY+I48ea9iN04cFX0wj J5cRVlFRSIwNSoHIzvFPmzBhMk6JPUTTDLn4PJJrVro4LuezBRKBY6TZsAAyFvDA 9tZMGduZ6BPZVqiJNwCehKcZsL6AsMhuU7043/ebJkM4RMAo+8Wc+ODzqPIHzaVJ Jt+zgYSfUP8ZNz4lDuQR+Y6v7ldiFnWlKsVkjMV6Nwj4Tpg77JbxnLQ0RExO9ubP stoHW8X7mKc= =lcr2 -----END PGP SIGNATURE----- --=-=-=--