[PATCH] maint: Suggest ‘guix git authenticate’ for initial authentication.

  • Open
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • pelzflorian (Florian Pelz)
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
normal
L
L
Ludovic Courtès wrote on 7 May 16:13 +0200
[PATCH] maint: Suggest ‘guix git authen ticate’ for initial authentication.
(address . guix-patches@gnu.org)
35c1e4eead584b9e24d3efc3638d6da4fd24cf8d.1715091056.git.ludo@gnu.org
The previous recommendation, running ‘make authenticate’, was insecure
because it led users to run code from the very repository they want to
authenticate:


* Makefile.am (commit_v1_0_0, channel_intro_commit)
(channel_intro_signer, GUIX_GIT_KEYRING, authenticate): Remove.
* Makefile.am (.git/hooks/%): New target, generalization of previous
‘.git/hooks/pre-push’ target.
(nodist_noinst_DATA): Add ‘.git/hooks/post-merge’.
* doc/contributing.texi (Building from Git): Suggest ‘guix git
authenticate’ instead of ‘make authenticate’.
* etc/git/post-merge: New file.
* etc/git/pre-push: Run ‘guix git authenticate’ instead of ‘make
authenticate’.

Reported-by: Skyler Ferris <skyvine@protonmail.com>
Change-Id: Ia415aa8375013d0dd095e891116f6ce841d93efd
---
Makefile.am | 30 +++++++++---------------------
doc/contributing.texi | 29 ++++++++++++++++++++++-------
etc/git/post-merge | 3 +++
etc/git/pre-push | 4 +++-
4 files changed, 37 insertions(+), 29 deletions(-)
create mode 100755 etc/git/post-merge

Hello there!

This addresses the security issue Skyler reported regarding
‘make authenticate’, basically removing the makefile target
and adjusting documentation accordingly. It also adds a
‘post-merge’ hook like ‘guix git authenticate’ now does.

This assumes users have a (very) recent ‘guix git authenticate’
command, but I think that’s acceptable because this targets
an audience of developers.

Thoughts?

Ludo’.


Toggle diff (144 lines)
diff --git a/Makefile.am b/Makefile.am
index 77c05ff63b7..d1d953b8923 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,5 +1,5 @@
# GNU Guix --- Functional package management for GNU
-# Copyright © 2012-2023 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2012-2024 Ludovic Courtès <ludo@gnu.org>
# Copyright © 2013 Andreas Enge <andreas@enge.fr>
# Copyright © 2015, 2017 Alex Kost <alezost@gmail.com>
# Copyright © 2016, 2018 Mathieu Lirzin <mthl@gnu.org>
@@ -895,22 +895,6 @@ $(guix_install_go_files): install-nobase_dist_guilemoduleDATA
install-data-hook:
touch "$(DESTDIR)$(guileobjectdir)/guix/config.go"
-# Commit corresponding to the 'v1.0.0' tag.
-commit_v1_0_0 = 6298c3ffd9654d3231a6f25390b056483e8f407c
-
-# Introduction of the 'guix' channel. Keep in sync with (guix channels)!
-channel_intro_commit = 9edb3f66fd807b096b48283debdcddccfea34bad
-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:
- $(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)"
-
# Assuming Guix is already installed and the daemon is up and running, this
# rule builds from $(srcdir), creating and building derivations.
as-derivation:
@@ -1227,13 +1211,13 @@ 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
.PHONY: update-guix-package update-NEWS cuirass-jobs release
# Git auto-configuration.
-.git/hooks/pre-push: etc/git/pre-push
+.git/hooks/%: etc/git/%
$(AM_V_at)if test -d .git; then \
- cp etc/git/pre-push .git/hooks/pre-push; \
+ cp "$<" "$@"; \
fi
.git/config: etc/git/gitconfig
@@ -1256,7 +1240,11 @@ COMMIT_MSG_MAGIC = VGhpcyBpcyB0aGUgY29tbWl0LW1zZyBob29rIG9mIEd1aXg=
# from a tarball. Do not add dependencies on these to *_DATA when building
# from a tarball, as that breaks the build.
if in_git_p
-nodist_noinst_DATA = .git/hooks/pre-push .git/config .git/hooks/commit-msg
+nodist_noinst_DATA = \
+ .git/hooks/pre-push \
+ .git/hooks/post-merge \
+ .git/config \
+ .git/hooks/commit-msg
endif
# Downloading up-to-date PO files.
diff --git a/doc/contributing.texi b/doc/contributing.texi
index 66f4e86d0a9..0005c846dc1 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -276,25 +276,40 @@ Building from Git
checkout by running:
@example
-make authenticate
+guix git authenticate \
+ 9edb3f66fd807b096b48283debdcddccfea34bad \
+ "BBB0 2DDF 2CEA F6A8 0D1D E643 A2A0 6DF2 A33A 54FA"
@end example
The first run takes a couple of minutes, but subsequent runs are faster.
+On subsequent runs, you can run the command without any arguments since
+the @dfn{introduction} (the commit ID and OpenPGP fingerprints above)
+will have been recorded@footnote{This requires a recent version of Guix,
+from May 2024 or more recent.}:
-Or, when your configuration for your local Git repository doesn't match
+@example
+guix git authenticate
+@end example
+
+When your configuration for your local Git repository doesn't match
the default one, you can provide the reference for the @code{keyring}
-branch through the variable @code{GUIX_GIT_KEYRING}. The following
+branch @i{via} the @option{-k} option. The following
example assumes that you have a Git remote called @samp{myremote}
pointing to the official repository:
@example
-make authenticate GUIX_GIT_KEYRING=myremote/keyring
+guix git authenticate \
+ -k myremote/keyring \
+ 9edb3f66fd807b096b48283debdcddccfea34bad \
+ "BBB0 2DDF 2CEA F6A8 0D1D E643 A2A0 6DF2 A33A 54FA"
@end example
+@xref{Invoking guix git authenticate}, for more information on this
+command.
+
@quotation Note
-You are advised to run @command{make authenticate} after every
-@command{git pull} invocation. This ensures you keep receiving valid
-changes to the repository.
+By default, hooks installed such that @command{guix git authenticate} is
+invoked anytime you run @command{git pull} or @command{git push}.
@end quotation
After updating the repository, @command{make} might fail with an error
diff --git a/etc/git/post-merge b/etc/git/post-merge
new file mode 100755
index 00000000000..f2ad37d35c4
--- /dev/null
+++ b/etc/git/post-merge
@@ -0,0 +1,3 @@
+#!/bin/sh
+# Authenticate the repo upon 'git pull' and similar.
+exec guix git authenticate
diff --git a/etc/git/pre-push b/etc/git/pre-push
index 59671b0d583..325b23854bb 100755
--- a/etc/git/pre-push
+++ b/etc/git/pre-push
@@ -32,7 +32,9 @@ do
# Only use the hook when pushing to Savannah.
case "$2" in
*.gnu.org*)
- exec make authenticate check-channel-news
+ set -e
+ make check-channel-news
+ exec guix git authenticate
exit 127
;;
*)

base-commit: 014875b29e68da6357a5323e6dd1eaa74a05b753
--
2.41.0
P
P
pelzflorian (Florian Pelz) wrote 5 days ago
Re: [bug#70818] [PATCH] maint: Suggest ‘guix git authenticate’ for initial authentication.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87cyposo2d.fsf@pelzflorian.de
Thank you Ludo for this careful implementation. I find One typo:

Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (16 lines)
> diff --git a/doc/contributing.texi b/doc/contributing.texi
> index 66f4e86d0a9..0005c846dc1 100644
> --- a/doc/contributing.texi
> +++ b/doc/contributing.texi
> @@ -276,25 +276,40 @@ Building from Git
> […]
> @quotation Note
> -You are advised to run @command{make authenticate} after every
> -@command{git pull} invocation. This ensures you keep receiving valid
> -changes to the repository.
> +By default, hooks installed such that @command{guix git authenticate} is
> +invoked anytime you run @command{git pull} or @command{git push}.
> @end quotation
>
> After updating the repository, @command{make} might fail with an error

By default, hooks *are*

Regards,
Florian
?