From a00ac3d016131f05c977e727f8ac15ea437aec7e Mon Sep 17 00:00:00 2001 From: Maxime Devos Date: Sun, 18 Sep 2022 19:52:16 +0200 Subject: [PATCH] WIP Only use "make authenticate" for etc/git/pre-push. As mentioned in , "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 # Copyright © 2021 Chris Marusich # Copyright © 2021 Andrew Tropin +# Copyright © 2022 Maxime Devos # # 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