unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: wolf <wolf@wolfsden.cz>
To: Wojtek Kosior <koszko@koszko.org>
Cc: "Josselin Poiret" <dev@jpoiret.xyz>,
	"Simon Tournier" <zimon.toutoune@gmail.com>,
	"Nicolas Débonnaire" <n.debonnaire@gmail.com>,
	guix-devel@gnu.org
Subject: Re: Building from git
Date: Fri, 8 Sep 2023 13:11:48 +0200	[thread overview]
Message-ID: <ZPsBdMVbbkN9tFUh@ws> (raw)
In-Reply-To: <20230908114756.61b28cf2.koszko@koszko.org>

[-- Attachment #1: Type: text/plain, Size: 5437 bytes --]

On 2023-09-08 11:47:56 +0200, Wojtek Kosior wrote:
> Hello Josselin
> 
> > wolf <wolf@wolfsden.cz> writes:
> > 
> > > Hmm, but the recipe for the authenticate rule comes from the (possibly)
> > > compromised source, no?  So the attacker can just modify the recipe instead of
> > > the command going the authentication.  Am I missing something?  
> > 
> > You can use a previously trusted guix to do the authentication.  `make
> > authenticate` is here for committers to check that their commits are all
> > properly signed before pushing (it's used as a pre-push hook).
> 
> From my understanding of the documentation, `make authenticate` is not
> just for committers but for all people who do a `git pull` in Guix tree
> and want to verify that the newly pulled commits do come from the
> committers. It it is not the case, then the documentation should
> probably be modified to make it clear.
> 
> The recipe is not from an untrusted source mecause the Makefile is not
> tracked by git. Rather, it gets generated when first building Guix. And
> — as the documentation instructs — the initial checkout gets
> authenticated with `guix git authenticate` rather than with `make
> authenticate` so it can't get compromised that easily.
> 
> Had someone managed to serve us a commit that adds another Makefile
> with a backdoor, git would report a conflict upon pulling. I believe
> this is what the implementors had in mind. Please clarify if this is
> wrong.

Yes, I believe this reasoning is wrong.  Even ignoring the fact that people
might run git clean or use worktrees, you can just attack the Makefile.am.  I
created a new commit in my checkout:

    commit b3b378ad8f725f16be0602113e7f2d2afd89a920 (HEAD -> master)
    Author: x <y@z>
    Date:   Fri Sep 8 11:04:44 2023 +0000
    
        this commit is so not signed and valid
    
    diff --git a/Makefile.am b/Makefile.am
    index 922913355c..e5f7c37491 100644
    --- a/Makefile.am
    +++ b/Makefile.am
    @@ -883,10 +883,7 @@ channel_intro_signer = BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA
     GUIX_GIT_KEYRING = origin/keyring
     authenticate:
            $(AM_V_at)echo "Authenticating Git checkout..." ;       \
    -       guix git authenticate                                   \
    -           --keyring=$(GUIX_GIT_KEYRING)                       \
    -           --cache-key=channels/guix --stats                   \
    -           "$(channel_intro_commit)" "$(channel_intro_signer)"
    +       echo "Don't worry, your checkout is just fine... :)"
     
     # Assuming Guix is already installed and the daemon is up and running, this
     # rule builds from $(srcdir), creating and building derivations.

guix git authenticate fails, as expected:

    Authenticating commits 9edb3f6 to b3b378a (1 new commits)...
    [##############################################################################]guix git: error: commit b3b378ad8f725f16be0602113e7f2d2afd89a920 lacks a signature

The missing new line after ] is somewhat meh, but it correctly fails.  However
make authenticate does pass:

    $ guix shell -D guix guix --pure -- make authenticate
     cd . && /bin/sh /home/wolf/src/guix/build-aux/missing automake-1.16 --gnu Makefile
    Makefile.am:896: warning: AM_GNU_GETTEXT used but 'po' not in SUBDIRS
     cd . && /bin/sh ./config.status Makefile depfiles
    config.status: creating Makefile
    config.status: executing depfiles commands
    Authenticating Git checkout...
    Don't worry, your checkout is just fine... :)

I mean, if make authenticate is just for the convenience of the committers, then
this is completely fine.  But the documentation does not currently read that
way.

> 
> I do see 1 loophole here, though. One could serve a compromised
> makefile under the name "GNUmakefile" and `make authenticate` would
> happily choose it over the non-compromised "Makefile". I was planning
> to start a new thread about it for some time... but this one seems like
> a just as appropriate place to mention the issue.
> 
> It shouldn't be hard to fix. It boils down to having ./configure create
> a GNUmakefile as well. Perhaps as a symlink to the original Makefile?
> 
> Best,
> Wojtek
> 
> -- (sig_start)
> website: https://koszko.org/koszko.html
> fingerprint: E972 7060 E3C5 637C 8A4F  4B42 4BC5 221C 5A79 FD1A
> follow me on Fediverse: https://friendica.me/profile/koszko/profile
> 
> ♥ R29kIGlzIHRoZXJlIGFuZCBsb3ZlcyBtZQ== | ÷ c2luIHNlcGFyYXRlZCBtZSBmcm9tIEhpbQ==
> ✝ YnV0IEplc3VzIGRpZWQgdG8gc2F2ZSBtZQ== | ? U2hhbGwgSSBiZWNvbWUgSGlzIGZyaWVuZD8=
> -- (sig_end)
> 
> 
> On Fri, 08 Sep 2023 11:10:37 +0200 Josselin Poiret <dev@jpoiret.xyz> wrote:
> 
> > Hi,
> > 
> > wolf <wolf@wolfsden.cz> writes:
> > 
> > > Hmm, but the recipe for the authenticate rule comes from the (possibly)
> > > compromised source, no?  So the attacker can just modify the recipe instead of
> > > the command going the authentication.  Am I missing something?  
> > 
> > You can use a previously trusted guix to do the authentication.  `make
> > authenticate` is here for committers to check that their commits are all
> > properly signed before pushing (it's used as a pre-push hook).
> > 
> > Best,

W.

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-09-08 11:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-02  9:03 Building from git Nicolas Débonnaire
2023-09-05 14:18 ` Wojtek Kosior via Development of GNU Guix and the GNU System distribution.
2023-09-07 12:06 ` Simon Tournier
2023-09-07 16:37   ` Bruno Victal
2023-09-07 17:45   ` wolf
2023-09-07 18:59     ` Simon Tournier
2023-10-23 17:16       ` Nicolas Débonnaire
2023-09-08  9:10     ` Josselin Poiret
2023-09-08  9:47       ` Wojtek Kosior via Development of GNU Guix and the GNU System distribution.
2023-09-08 11:11         ` wolf [this message]
2023-09-09  8:32           ` Josselin Poiret

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=ZPsBdMVbbkN9tFUh@ws \
    --to=wolf@wolfsden.cz \
    --cc=dev@jpoiret.xyz \
    --cc=guix-devel@gnu.org \
    --cc=koszko@koszko.org \
    --cc=n.debonnaire@gmail.com \
    --cc=zimon.toutoune@gmail.com \
    /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).