GNU bug report logs - #57910
[PATCH] Add link to 'pre-inst-env' from 'installing from git' docs

Previous Next

To reply to this bug, email your comments to 57910 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Emma Turner <em.turner <at> tutanota.com>
To: Guix Patches <guix-patches <at> gnu.org>
Subject: [PATCH] Add link to 'pre-inst-env' from 'installing from git' docs
Date: Sun, 18 Sep 2022 14:41:55 +0200 (CEST)

[Message part 1 (text/plain, inline)]

[0001-doc-link-pre-inst-env-from-building-from-git-docs.patch (text/x-patch, attachment)]

Message #8 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Emma Turner <em.turner <at> tutanota.com>, control <at> debbugs.gnu.org,
 57909 <at> debbugs.gnu.org, 57910 <at> debbugs.gnu.org
Subject: Re: [bug#57909] Add link to 'pre-inst-env' from 'installing from git'
 docs
Date: Sun, 18 Sep 2022 19:26:00 +0200

[Message part 1 (text/plain, inline)]

merge 57909 57910
thanks

The given example "make authenticate" is insecure, it has a TOCTTOU 
problem as indicated at <https://issues.guix.gnu.org/22883#59>:

> Moreover, I don't think running 'make authenticate' after 'git pull'
> would really work -- after you pulled, git-authenticate could've been
> modified, so the verify-commit you did earlier doesn't apply anymore.

The solution that was proposed

> We can solve it by removing ./pre-inst-env from the command in ‘make
> authenticate’.

would be undone by the proposed patch.  Even then, it remains insecure, 
as an attacker could have modified the "make authenticate", as explained 
in more detail at <https://logs.guix.gnu.org/guix/2022-09-14.log#172610>.

As such, I think we really shouldn't recommend "make authenticate" (and 
even remove "make authenticate".  In fact, I think we should remove 
"make authenticate" and replace the instructions with a direct "guix git 
authenticate ...".

As such, I propose that:

  * you adjust the patch to note that authenticating the checkout is
    impossible if you don't already have Guix installed (instead of
    recommending the insecure "make authenticate")

  * I write a patch removing "make authenticate" and adjusting old uses
    of "make authenticate" to "guix git authenticate ...".

Greetings,
Maxime.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #13 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Emma Turner <em.turner <at> tutanota.com>, 57909 <at> debbugs.gnu.org,
 57910 <at> debbugs.gnu.org
Subject: Re: [bug#57909] Add link to 'pre-inst-env' from 'installing from git'
 docs
Date: Sun, 18 Sep 2022 20:53:23 +0200

[Message part 1 (text/plain, inline)]

On 18-09-2022 19:26, Maxime Devos wrote:
> [...]
> 
> As such, I propose that:
> 
>    * you adjust the patch to note that authenticating the checkout is
>      impossible if you don't already have Guix installed (instead of
>      recommending the insecure "make authenticate")
> 
>    * I write a patch removing "make authenticate" and adjusting old uses
>      of "make authenticate" to "guix git authenticate ...".


I have attached a patch for the latter.

Greetings,
Maxime.

[0001-WIP-Only-use-make-authenticate-for-etc-git-pre-push.patch (text/x-patch, attachment)]

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #16 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Emma Turner <em.turner <at> tutanota.com>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 57910 <at> debbugs.gnu.org, 57909 <at> debbugs.gnu.org
Subject: Re: [bug#57909] Add link to 'pre-inst-env' from 'installing from
 git' docs
Date: Mon, 19 Sep 2022 08:12:04 +0200 (CEST)

[Message part 1 (text/plain, inline)]

Hi Maxime,

Thanks for your replies.  I hadn't thought about the issue with make authenticate, but makes complete sense!  Thanks for explaining.

I've attached an updated patch, which I think does what you asked for (replacing make authenticate with guix git authenticate.

Many thanks,
Emma


Sep 18, 2022, 19:53 by maximedevos <at> telenet.be:

>
>
> On 18-09-2022 19:26, Maxime Devos wrote:
>
>> [...]
>>
>> As such, I propose that:
>>
>>    * you adjust the patch to note that authenticating the checkout is
>>      impossible if you don't already have Guix installed (instead of
>>      recommending the insecure "make authenticate")
>>
>>    * I write a patch removing "make authenticate" and adjusting old uses
>>      of "make authenticate" to "guix git authenticate ...".
>>
>
>
> I have attached a patch for the latter.
>
> Greetings,
> Maxime.
>

[0001-doc-contrib-recommend-guix-git-authenticate.patch (text/x-patch, attachment)]

Message #19 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Emma Turner <em.turner <at> tutanota.com>
To: 57910 <at> debbugs.gnu.org
Subject: [PATCH] Add link to 'pre-inst-env' from 'installing from git' docs
Date: Mon, 19 Sep 2022 11:37:05 +0200 (CEST)

Hi,

Sorry, I'd tried to close 57909 as soon as I realised I'd made duplicate issues, but then it's only just come through, and closed both issues since they were merged.

I'll hopefully remember going forward to give debbugs lots of time!

Emma

Message #24 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Emma Turner <em.turner <at> tutanota.com>
Cc: 57910 <at> debbugs.gnu.org, 57909 <at> debbugs.gnu.org
Subject: Re: [bug#57909] Add link to 'pre-inst-env' from 'installing from git'
 docs
Date: Mon, 19 Sep 2022 15:27:33 +0200

[Message part 1 (text/plain, inline)]

On 19-09-2022 08:12, Emma Turner wrote:
> Hi Maxime,
> 
> Thanks for your replies.  I hadn't thought about the issue with make authenticate, but makes complete sense!  Thanks for explaining.
> 
> I've attached an updated patch, which I think does what you asked for (replacing make authenticate with guix git authenticate.


Aside from the copyright line in doc/guix.texi, looks good to me, though 
given the security concerns and the impact on a _large_ number of users, 
it would be best if someone else verified things as well.

Greetings,
Maxime.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #27 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 57910 <at> debbugs.gnu.org, control <at> debbugs.gnu.org, 57909 <at> debbugs.gnu.org,
 Emma Turner <em.turner <at> tutanota.com>
Subject: Re: bug#57910: [PATCH] Add link to 'pre-inst-env' from 'installing
 from git' docs
Date: Sat, 24 Sep 2022 17:58:29 +0200

Hi,

Maxime Devos <maximedevos <at> telenet.be> skribis:

> As such, I think we really shouldn't recommend "make authenticate"
> (and even remove "make authenticate".  In fact, I think we should
> remove "make authenticate" and replace the instructions with a direct
> "guix git authenticate ...".

“make authenticate” runs ‘guix git authenticate’ with the right
parameters; importantly, it runs the already-installed ‘guix’, not the
one in the build tree, so it’s safe (prepending “./pre-inst-env”
wouldn’t be safe as you wrote).

So I’m not sure we really need changes; WDYT?

Ludo’.

Message #30 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 57910 <at> debbugs.gnu.org, control <at> debbugs.gnu.org, 57909 <at> debbugs.gnu.org,
 Emma Turner <em.turner <at> tutanota.com>
Subject: Re: bug#57910: [PATCH] Add link to 'pre-inst-env' from 'installing
 from git' docs
Date: Sat, 24 Sep 2022 18:23:10 +0200

[Message part 1 (text/plain, inline)]

On 24-09-2022 17:58, Ludovic Courtès wrote:
> Hi,
> 
> Maxime Devos<maximedevos <at> telenet.be>  skribis:
> 
>> As such, I think we really shouldn't recommend "make authenticate"
>> (and even remove "make authenticate".  In fact, I think we should
>> remove "make authenticate" and replace the instructions with a direct
>> "guix git authenticate ...".
> “make authenticate” runs ‘guix git authenticate’ with the right
> parameters; importantly, it runs the already-installed ‘guix’, not the
> one in the build tree, so it’s safe (prepending “./pre-inst-env”
> wouldn’t be safe as you wrote).
> 
> So I’m not sure we really need changes; WDYT?

While ordinarily, it is true that "make authenticate" runs "guix git 
authenticate" (and not ./pre-inst-env guix git authenticate), an 
attacker could have modified Makefile.am to _not_ call "guix git 
authenticate", as I've explained in the paragraph above the one you quoted:

> The solution that was proposed [...].  __Even then, it remains
> insecure, as an attacker could have modified the "make authenticate",
> as explained in more detail at
> <https://logs.guix.gnu.org/guix/2022-09-14.log#172610>. 

More concretely, I've worked out a method the hypothetical attacker 
could use the fact that "Makefile.am" is used before it is authenticated 
in the message pointed to by the link I quoted:

https://logs.guix.gnu.org/guix/2022-09-14.log#172610 :

<maximed>civodul: Currently, it's like verifying the authenticity of a 
gnupg tarball, by extracting the gnupg tarball, compiling it, and 
running the freshly compiled gnupg tarball.
<antipode>Translated to Guix:
<antipode>(1) You run "git pull" (2) an attacker has intercepted the 
network connection and modified Makefile.am's authenticate target to 
always 'succeed'. Additionally, the attacker inserts some malicious code 
somewhere (e.g. some code in Makefile.am to upload your GnuPG keys to 
evil.com). To add some stealth, the modified Makefile.am automatically 
reverts the malicious commit. (3) You run "make authenticate" as 
recommended by the manual, and now the attacker has your private keys.

Do you see a flaw in this explanation?

Greetings,
Maxime.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #33 received at 57910 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 57910 <at> debbugs.gnu.org, 57909 <at> debbugs.gnu.org,
 Emma Turner <em.turner <at> tutanota.com>
Subject: Re: bug#57910: [PATCH] Add link to 'pre-inst-env' from 'installing
 from git' docs
Date: Sun, 25 Sep 2022 22:05:59 +0200

Hi,

Maxime Devos <maximedevos <at> telenet.be> skribis:

> While ordinarily, it is true that "make authenticate" runs "guix git
> authenticate" (and not ./pre-inst-env guix git authenticate), an
> attacker could have modified Makefile.am to _not_ call "guix git
> authenticate", as I've explained in the paragraph above the one you
> quoted:

Oh you’re right; sorry for overlooking this.

So yes, that calls for recommending the full ‘guix git authenticate’
command for the initial checkout.

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.