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

[Maxime Devos <maximedevos@telenet.be>]

[-- Attachment #1.1.1: Type: text/plain, Size: 513 bytes --]



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.

[-- Attachment #1.1.2: 0001-WIP-Only-use-make-authenticate-for-etc-git-pre-push.patch --]
[-- Type: text/x-patch, Size: 4859 bytes --]

From a00ac3d016131f05c977e727f8ac15ea437aec7e Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Sun, 18 Sep 2022 19:52:16 +0200
Subject: [PATCH] WIP Only use "make authenticate" for etc/git/pre-push.

As mentioned in <https://issues.guix.gnu.org/57909>, "make authenticate"
cannot be used for authentication, as it makes use of a Makefile.am
(and configure.ac) that might be provided by the attacker.

As such, only use this is the etc/git/pre-push hook, where it can be
reasonably assumed the current commit is 'safe' and it only needs
to check that the safety is properly conveyed to other people (by
having the commits be signed correctly).

To aid with the transition from "make authenticate" to "guix git
authenticate", print an error message from "make authenticate",
directing the user to use the safe "guix git authenticate" instead.

TODO missing: other uses of "make authenticate" in the documentation.

* Makefile.am (authenticate): Rename to ...
(authenticate-self-check): ... this, and add a new target
(authenticate): that depends on authenticate-self-check and additionally
prints the error message.
* doc/contributing.texi (Commit Access): Adjust for target rename.
* etc/git/pre-push: Adjust for target rename.
---
 Makefile.am           | 20 ++++++++++++++------
 doc/contributing.texi |  2 +-
 etc/git/pre-push      |  2 +-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 22dcc43f99..bfabf0bf2e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,6 +16,7 @@
 # Copyright © 2019 Efraim Flashner <efraim@flashner.co.il>
 # Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
 # Copyright © 2021 Andrew Tropin <andrew@trop.in>
+# Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
 #
 # This file is part of GNU Guix.
 #
@@ -804,12 +805,19 @@ channel_intro_signer = BBB0 2DDF 2CEA F6A8 0D1D  E643 A2A0 6DF2 A33A 54FA
 
 # Authenticate the current Git checkout by checking signatures on every commit.
 GUIX_GIT_KEYRING = origin/keyring
-authenticate:
+authentication_command = guix git authenticate '--keyring=$(GUIX_GIT_KEYRING)' --cache-key=channels/guix --stats '$(channel_intro_commit)' '$(channel_intro_signer)'
+authenticate-self-check:
 	$(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)"
+	$(authentication_command)
+authenticate: authenticate-self-check
+	$(AM_V_at)echo "\"make authenticate\" is insecure, you need to run"
+	$(AM_V_at)echo "$(authentication_command)"
+	$(AM_V_at)echo "instead.  Do **not** do that inside a ./pre-inst-env,"
+	$(AM_V_at)echo "that would be insecure because of a TOCTTOU problem."
+	$(AM_V_at)echo "Because of the TOCTTOU problem, you likely cannot trust"
+	$(AM_V_at)echo "these instructions unless you have already"
+	$(AM_V_at)echo "authenticated the repository by other means."
+	$(AM_V_at)exit 1
 
 # Assuming Guix is already installed and the daemon is up and running, this
 # rule builds from $(srcdir), creating and building derivations.
@@ -1076,7 +1084,7 @@ cuirass-jobs: $(GOBJECTS)
 .PHONY: gen-ChangeLog gen-AUTHORS gen-tarball-version
 .PHONY: assert-no-store-file-names assert-binaries-available
 .PHONY: assert-final-inputs-self-contained check-channel-news
-.PHONY: clean-go make-go as-derivation authenticate
+.PHONY: clean-go make-go as-derivation authenticate authenticate-self-check
 .PHONY: update-guix-package update-NEWS cuirass-jobs release
 
 # Downloading up-to-date PO files.
diff --git a/doc/contributing.texi b/doc/contributing.texi
index de1d34cc03..353cb71caf 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -1740,7 +1740,7 @@ git config user.signingkey CABBA6EA1DC0FF33
 To check that commits are signed with correct key, use:
 
 @example
-make authenticate
+make authenticate-self-check
 @end example
 
 You can prevent yourself from accidentally pushing unsigned or signed
diff --git a/etc/git/pre-push b/etc/git/pre-push
index 59671b0d58..7fdc533d09 100755
--- a/etc/git/pre-push
+++ b/etc/git/pre-push
@@ -32,7 +32,7 @@ do
 		# Only use the hook when pushing to Savannah.
 		case "$2" in
 		    *.gnu.org*)
-			exec make authenticate check-channel-news
+			exec make authenticate-self-check check-channel-news
 			exit 127
 			;;
 		    *)

base-commit: 31a56967e2869c916b7a5e8ee570e8e10f0210a5
prerequisite-patch-id: 2712efb97bf33985fd0658e4dd8e936dc08be5fe
prerequisite-patch-id: 9d2409b480a8bff0fef029b4b095922d4957e06f
prerequisite-patch-id: 51a32abca3efec1ba67ead59b8694c5ea3129ad3
prerequisite-patch-id: 9092927761a340c07a99f5f3ed314a6add04cdee
prerequisite-patch-id: d0af09fbd5ee0ef60bdee53b87d729e46c1db2ca
prerequisite-patch-id: 4fee177b2d8c9478c6a7b8ce1ca9072942f39863
-- 
2.37.3


[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]