diff mbox series

[bug#57909] Add link to 'pre-inst-env' from 'installing from git' docs

Message ID NCFQ9xf--3-2@tutanota.com
State New
Headers
Series [bug#57909] Add link to 'pre-inst-env' from 'installing from git' docs |

Commit Message

Emma Turner Sept. 18, 2022, 11:47 a.m. UTC

Comments

Emma Turner Sept. 18, 2022, 2:59 p.m. UTC | #1 
This is a duplicate of https://issues.guix.gnu.org/57910
Maxime Devos Sept. 18, 2022, 5:26 p.m. UTC | #2 
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.
Maxime Devos Sept. 19, 2022, 1:01 p.m. UTC | #3 
reopen 57909
thanks

Looks like the closing was accidental, reopening.

See <https://debbugs.gnu.org/server-control.html> if you're interested 
in how to use these debbugs commands (anyone can send those, somehow the 
wide permissions don't cause problems).

Greetings,
Maxime.
Ludovic Courtès Sept. 24, 2022, 3:58 p.m. UTC | #4 
Hi,

Maxime Devos <maximedevos@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’.
Maxime Devos Sept. 24, 2022, 4:23 p.m. UTC | #5 
On 24-09-2022 17:58, Ludovic Courtès wrote:
> Hi,
> 
> Maxime Devos<maximedevos@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.
Ludovic Courtès Sept. 25, 2022, 8:05 p.m. UTC | #6 
Hi,

Maxime Devos <maximedevos@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’.
diff mbox series

Patch

From 4849857fcf13f8de572d030cd15defd1f2b84768 Mon Sep 17 00:00:00 2001
From: Emma Turner <em.turner@tutanota.com>
Date: Sun, 18 Sep 2022 12:40:17 +0100
Subject: [PATCH] doc: link pre-inst-env from building from git docs

---
 doc/contributing.texi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/contributing.texi b/doc/contributing.texi
index 17a54f94cc..d4cd57141d 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -159,6 +159,14 @@  checkout by running:
 make authenticate
 @end example
 
+If you get an error of @code{guix: command not found}, then you can refer
+to the new instance you built above, by running the following
+(see @pxref{Running Guix Before It Is Installed}):
+
+@example
+./pre-inst-env make authenticate
+@end example
+
 The first run takes a couple of minutes, but subsequent runs are faster.
 
 Or, when your configuration for your local Git repository doesn't match
-- 
2.36.1