unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: zimoun <zimon.toutoune@gmail.com>
To: "Ludovic Courtès" <ludovic.courtes@inria.fr>, guix-devel@gnu.org
Subject: Re: Tricking peer review
Date: Tue, 19 Oct 2021 10:36:55 +0200	[thread overview]
Message-ID: <86ee8hfm1k.fsf@gmail.com> (raw)
In-Reply-To: <874k9if7am.fsf@inria.fr>

Hi,

On Fri, 15 Oct 2021 at 20:54, Ludovic Courtès <ludovic.courtes@inria.fr> wrote:

> --8<---------------cut here---------------start------------->8---
> $ guix build -f /tmp/content-addressed.scm  -S --check 
> La jena derivaĵo estos konstruata:
>    /gnu/store/nq2jdzbv3nh9b1mglan54dcpfz4l7bli-sed-4.8.tar.gz.drv
> building /gnu/store/nq2jdzbv3nh9b1mglan54dcpfz4l7bli-sed-4.8.tar.gz.drv...
>
> Starting download of /gnu/store/1mlpazwwa2mi35v7jab5552lm3ssvn6r-sed-4.8.tar.gz
> From https://ftpmirror.gnu.org/gnu/zed/sed-4.8.tar.gz...
> following redirection to `https://mirror.cyberbits.eu/gnu/zed/sed-4.8.tar.gz'...
> download failed "https://mirror.cyberbits.eu/gnu/zed/sed-4.8.tar.gz" 404 "Not Found"
>
> [...]
>
> Starting download of /gnu/store/1mlpazwwa2mi35v7jab5552lm3ssvn6r-sed-4.8.tar.gz
> From https://archive.softwareheritage.org/api/1/content/sha256:58e6751c41a7c25bfc6e9363a41786cff3ba5709cf11d5ad903cf7cce31cc3fb/raw/...
> downloading from https://archive.softwareheritage.org/api/1/content/sha256:58e6751c41a7c25bfc6e9363a41786cff3ba5709cf11d5ad903cf7cce31cc3fb/raw/ ...
>
> warning: rewriting hashes in `/gnu/store/mgais6lk92mm8n5kyx70knr11jbwgfhr-sed-4.8.tar.gz'; cross fingers
> successfully built /gnu/store/nq2jdzbv3nh9b1mglan54dcpfz4l7bli-sed-4.8.tar.gz.drv
> --8<---------------cut here---------------end--------------->8---

The message can be even more “spottable” than the URL
’archive.softwareheritage.org’ if the vault requires a cooking.  One
will see «SWH vault: requested bundle cooking, waiting for
completion...» or «SWH vault: retrying...».

Yeah, it is hidden with the option ’v0’ but it is the user’s
responsibility to check, IMHO.

> It’s nothing new, it’s what I do when I want to test the download
> fallbacks (see also ‘GUIX_DOWNLOAD_FALLBACK_TEST’ in commit
> c4a7aa82e25503133a1bd33148d17968c899a5f5).  Still, I wonder if it could
> somehow be abused to have malicious packages pass review.

Yes, it is nothing new.  Somehow, it is an issue with any
content-addressed system.  Here, we need to split the issue depending on
the origins:

 - url-fetch: the attacker has to introduce the tarballs into SWH.
   There is not so much means, from my understanding: SWH ingests
   tarballs via loaders, for instance gnu.org or sources.json or
   debian.org etc. Therefore the attacker has to introduce the malicious
   code to these platforms.

 - url-fetch without metadata (as your example), indeed, the reviewer
   could be abused; mitigated by the fact that “guix lint” spots the
   potential issue.

 - url-fetch with metadata: the attacker have to also corrupt
   Diasarchive-DB.   Otherwise, the tarball returned by SWH will not
   match the checksum.

 - svn-fetch, hg-fetch, cvs-fetch: no attack possible, yet.

 - git-fetch: it is the *real* issue.  Because it is easy for the
   attacker to introduce malicious code into SWH (create a repo on
   GitHub, click Save, done).  Then submit a package using it as you
   did.  It is the same case as url-fetch without metadata but easier
   for the attacker.  It is mitigated by “guix lint”.

That’s said, if I am an attacker and I would like to corrupt Guix, then
I would create a fake project mimicking a complex software.  For
instance, Gmsh is a complex C++ scientific software.  The correct URL is
<https://gmsh.info> and the source at
<https://gitlab.onelab.info/gmsh/gmsh>.  Then, as an attacker, I buy the
domain say gmsh.org and put a malicious code there.  Last, I send for
inclusion a package using this latter URL.  The reviewer would be
abused.

That’s why more eyes, less issues. :-)
   

> Also, just because a URL looks nice and is reachable doesn’t mean the
> source is trustworthy either.  An attacker could submit a package for an
> obscure piece of software that happens to be malware.  The difference
> here is that the trick above would allow targeting a high-impact
> package.

I agree.

> On the plus side, such an attack would be recorded forever in Git
> history.

I agree again. :-)

> All in all, it’s probably not as worrisome as it first seems.  However,
> it’s worth keeping in mind when reviewing a package.

I agree with a minor comment.  From my opinion, not enough patches are
going via guix-patches and are pushed directly.

For instance, the «Commit policy» section says «For patches that just
add a new package, and a simple one, it’s OK to commit, if you’re
confident (which means you successfully built it in a chroot setup, and
have done a reasonable copyright and license auditing).»

And from my point of view, new packages should *always* go via
guix-patches, wait 15 days, then push if no remark.  It lets the time
for the community to chime in.  And if not, it just slows down for 2
weeks.

Cheers,
simon


  parent reply	other threads:[~2021-10-19  8:46 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15 18:54 Tricking peer review Ludovic Courtès
2021-10-15 22:03 ` Liliana Marie Prikler
2021-10-15 22:28   ` Ryan Prior
2021-10-15 22:45     ` Liliana Marie Prikler
2021-10-15 22:59       ` Ryan Prior
2021-10-18  7:40     ` Ludovic Courtès
2021-10-18 19:56       ` Ryan Prior
2021-10-19  8:39       ` zimoun
2021-10-20 23:03         ` Leo Famulari
2021-10-21  8:14           ` zimoun
2021-10-15 23:13   ` Thiago Jung Bauermann
2021-10-18  7:47     ` Ludovic Courtès
2021-10-18  7:34   ` Ludovic Courtès
2021-10-19  8:36 ` zimoun [this message]
2021-10-19 12:56   ` Ludovic Courtès
2021-10-19 14:22     ` zimoun
2021-10-19 15:41       ` Incentives for review Ludovic Courtès
2021-10-19 16:56         ` zimoun
2021-10-19 19:14         ` Ricardo Wurmus
2021-10-19 19:34           ` Christine Lemmer-Webber
2021-10-19 19:50           ` Joshua Branson
2021-10-21 20:03           ` Ludovic Courtès
2021-10-20 21:37         ` Thiago Jung Bauermann
2021-10-21 13:38           ` Artem Chernyak
2021-10-22 20:03             ` Thiago Jung Bauermann
2021-10-23  1:43               ` Kyle Meyer
2021-10-23  3:42                 ` Thiago Jung Bauermann
2021-10-23  7:37                 ` zimoun
2021-10-23 16:18                   ` public-inbox/elfeed -> Maildir bridge (was: Incentives for review) Kyle Meyer
2021-10-24 12:18                   ` Jonathan McHugh
2021-10-21 16:06           ` Incentives for review Ricardo Wurmus
2021-10-21 16:32             ` zimoun
2021-10-22 20:06             ` Thiago Jung Bauermann
2021-10-21 15:07         ` Katherine Cox-Buday
2021-10-21 16:10           ` Ricardo Wurmus
2021-10-21 17:52             ` Katherine Cox-Buday
2021-10-21 18:21             ` Arun Isaac
2021-10-21 19:58               ` Ludovic Courtès
2021-10-21 21:42               ` Ricardo Wurmus
2021-10-22 10:48                 ` Arun Isaac
2021-10-22 11:21                   ` zimoun
2021-10-23  6:09                     ` Arun Isaac
2021-10-22 10:56                 ` Jonathan McHugh
2021-10-22  7:40               ` zimoun
2021-10-22 11:09                 ` Arun Isaac
2021-10-22  8:37               ` Jonathan McHugh
2021-10-22  9:15                 ` zimoun
2021-10-22 10:40                 ` Jonathan McHugh
2021-10-22 11:32                   ` zimoun
2021-10-21 21:18             ` Jonathan McHugh
2021-10-22 10:44               ` Arun Isaac
2021-10-22 11:06               ` Jonathan McHugh
2021-10-21 21:22           ` zimoun
2021-10-28 14:57             ` Katherine Cox-Buday
2021-10-21 17:51         ` Vagrant Cascadian
2021-10-24 11:47           ` Efraim Flashner
2021-10-20  8:22   ` Tricking peer review Giovanni Biscuolo
2021-10-20  9:10     ` zimoun
2021-10-20  8:29   ` patches for new packages proper workflow (Re: Tricking peer review) Giovanni Biscuolo
2021-10-20 23:09 ` Tricking peer review Leo Famulari
2021-10-21  7:12   ` Ludovic Courtès
2021-10-25 13:09 ` Christine Lemmer-Webber
2021-10-28  8:38   ` Ludovic Courtès

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

  List information: https://guix.gnu.org/

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

  git send-email \
    --in-reply-to=86ee8hfm1k.fsf@gmail.com \
    --to=zimon.toutoune@gmail.com \
    --cc=guix-devel@gnu.org \
    --cc=ludovic.courtes@inria.fr \
    /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.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

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).