git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Couder <christian.couder@gmail.com>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>,
	Patrick Steinhardt <ps@pks.im>, Elijah Newren <newren@gmail.com>,
	Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Christian Couder <christian.couder@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: [PATCH] fast-(import|export): improve on the signature algorithm name
Date: Thu, 24 Apr 2025 22:39:04 +0200	[thread overview]
Message-ID: <20250424203904.909777-1-christian.couder@gmail.com> (raw)

A recent commit, d9cb0e6ff8 (fast-export, fast-import: add support for
signed-commits, 2025-03-10), added support for signed commits.

However, when processing signatures `git fast-export` outputs "gpgsig
sha1" not just when it encounters an OpenPGP SHA-1 signature, but also
when it encounters an SSH or X.509 signature. This is not very
informative to say the least, and this might prevent tools that process
the output from easily and properly handling signatures.

Let's improve on that by reusing the existing code from
"gpg-interface.{c,h}" to detect the signature algorithm, and then put
the signature algorithm name (like "openpgp", "x509" or "ssh") instead
of "sha1" in the output. If we can't detect the signature algorithm we
will use "unknown". It might be a signature added by an external tool
and we should likely keep it.

Similarly on the `git fast-import` side, let's use the existing code
from "gpg-interface.{c,h}" to check if a signature algorithm name is
valid. In case of an "unknown" signature algorithm name, we will warn
but still keep it. Future work might implement several options to let
users deal with it in different ways, and might implement checking
known signatures too.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---

This is a follow up from cc/signed-fast-export-import that was merged
by 01d17c0530 (Merge branch 'cc/signed-fast-export-import', 2025-03-29)
and introduced the support for signed commits.

The format that this series implemented was lacking a bit, so the goal
with this patch is to improve it and handle signed commits a bit more
consistently in the code base. It also shows in the tests and in our
documentation that SSH and X.509 signatures are supported.

 Documentation/git-fast-export.adoc |  5 +++
 Documentation/git-fast-import.adoc | 15 +++++++-
 builtin/fast-export.c              |  8 ++--
 builtin/fast-import.c              | 14 ++++---
 gpg-interface.c                    | 11 ++++++
 gpg-interface.h                    | 10 +++++
 t/t9350-fast-export.sh             | 60 +++++++++++++++++++++++++++++-
 7 files changed, 112 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-fast-export.adoc b/Documentation/git-fast-export.adoc
index 413a527496..d03aeca781 100644
--- a/Documentation/git-fast-export.adoc
+++ b/Documentation/git-fast-export.adoc
@@ -54,6 +54,11 @@ of tools that call 'git fast-export' but do not yet support
 '--signed-commits', you may set the environment variable
 'FAST_EXPORT_SIGNED_COMMITS_NOABORT=1' in order to change the default
 from 'abort' to 'warn-strip'.
++
+When exported, signature starts with "gpgsig <alg>" where <alg> is the
+signature algorithm name as identified by Git (e.g. "openpgp", "x509",
+"ssh", or "sha256" for SHA-256 OpenPGP signatures), or "unknown" for
+signatures that can't be identified.
 
 --tag-of-filtered-object=(abort|drop|rewrite)::
 	Specify how to handle tags whose tagged object is filtered out.
diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc
index 7b107f5e8e..50b6d2cc1d 100644
--- a/Documentation/git-fast-import.adoc
+++ b/Documentation/git-fast-import.adoc
@@ -521,7 +521,20 @@ The optional `gpgsig` command is used to include a PGP/GPG signature
 that signs the commit data.
 
 Here <alg> specifies which hashing algorithm is used for this
-signature, either `sha1` or `sha256`.
+signature. Current valid values are:
+
+* "openpgp" for SHA-1 OpenPGP signatures,
+
+* "sha256" for SHA-256 OpenPGP signatures,
+
+* "x509" for X.509 (GPGSM) signatures,
+
+* "ssh", for SSH signatures,
+
+* "unknown" for signatures that can't be identified (a warning is
+  emitted).
+
+Signatures are not yet checked in the current implementation though.
 
 `encoding`
 ^^^^^^^^^^
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 170126d41a..d00f02dc74 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -29,6 +29,7 @@
 #include "quote.h"
 #include "remote.h"
 #include "blob.h"
+#include "gpg-interface.h"
 
 static const char *fast_export_usage[] = {
 	N_("git fast-export [<rev-list-opts>]"),
@@ -700,9 +701,10 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 	}
 
 	if (*commit_buffer_cursor == '\n') {
-		if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor)))
-			signature_alg = "sha1";
-		else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
+		if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig", &commit_buffer_cursor))) {
+			const char *name = get_signature_name(signature);
+			signature_alg = name ? name : "unknown";
+		} else if ((signature = find_commit_multiline_header(commit_buffer_cursor + 1, "gpgsig-sha256", &commit_buffer_cursor)))
 			signature_alg = "sha256";
 	}
 
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 63880b595c..59e991a03c 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -29,6 +29,7 @@
 #include "commit-reach.h"
 #include "khash.h"
 #include "date.h"
+#include "gpg-interface.h"
 
 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
@@ -2830,12 +2831,15 @@ static void parse_new_commit(const char *arg)
 			"encoding %s\n",
 			encoding);
 	if (sig_alg) {
-		if (!strcmp(sig_alg, "sha1"))
-			strbuf_addstr(&new_data, "gpgsig ");
-		else if (!strcmp(sig_alg, "sha256"))
+		if (!strcmp(sig_alg, "sha256"))
 			strbuf_addstr(&new_data, "gpgsig-sha256 ");
-		else
-			die("Expected gpgsig algorithm sha1 or sha256, got %s", sig_alg);
+		else if (valid_signature_name(sig_alg))
+			strbuf_addstr(&new_data, "gpgsig ");
+		else if (!strcmp(sig_alg, "unknown")) {
+			warning("Unknown gpgsig algorithm name!");
+			strbuf_addstr(&new_data, "gpgsig ");
+		} else
+			die("Invalid gpgsig algorithm name, got '%s'", sig_alg);
 		string_list_split_in_place(&siglines, sig.buf, "\n", -1);
 		strbuf_add_separated_string_list(&new_data, "\n ", &siglines);
 		strbuf_addch(&new_data, '\n');
diff --git a/gpg-interface.c b/gpg-interface.c
index 0896458de5..dc6ea904d0 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -144,6 +144,17 @@ static struct gpg_format *get_format_by_sig(const char *sig)
 	return NULL;
 }
 
+const char *get_signature_name(const char *buf)
+{
+	struct gpg_format *format = get_format_by_sig(buf);
+	return format ? format->name : NULL;
+}
+
+int valid_signature_name(const char *name)
+{
+	return (get_format_by_name(name) != NULL);
+}
+
 void signature_check_clear(struct signature_check *sigc)
 {
 	FREE_AND_NULL(sigc->payload);
diff --git a/gpg-interface.h b/gpg-interface.h
index e09f12e8d0..332707facc 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -47,6 +47,16 @@ struct signature_check {
 
 void signature_check_clear(struct signature_check *sigc);
 
+/*
+ * Return the name of the signature (like "openpgp", "x509" or "ssh").
+ */
+const char *get_signature_name(const char *buf);
+
+/*
+ * Is the signature name valid (like "openpgp", "x509" or "ssh").
+ */
+int valid_signature_name(const char *name);
+
 /*
  * Look at a GPG signed tag object.  If such a signature exists, store it in
  * signature and the signed content in payload.  Return 1 if a signature was
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index dda9e7c3e7..2e2c83d153 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -326,7 +326,7 @@ test_expect_success GPG 'signed-commits=abort' '
 test_expect_success GPG 'signed-commits=verbatim' '
 
 	git fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&
-	grep "^gpgsig sha" output &&
+	grep "^gpgsig openpgp" output &&
 	grep "encoding ISO-8859-1" output &&
 	(
 		cd new &&
@@ -340,7 +340,7 @@ test_expect_success GPG 'signed-commits=verbatim' '
 test_expect_success GPG 'signed-commits=warn-verbatim' '
 
 	git fast-export --signed-commits=warn-verbatim --reencode=no commit-signing >output 2>err &&
-	grep "^gpgsig sha" output &&
+	grep "^gpgsig openpgp" output &&
 	grep "encoding ISO-8859-1" output &&
 	test -s err &&
 	(
@@ -381,6 +381,62 @@ test_expect_success GPG 'signed-commits=warn-strip' '
 
 '
 
+test_expect_success GPGSM 'setup x509 signed commit' '
+
+	git checkout -b x509-signing main &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo "x509 content" >file_for_x509 &&
+	git add file_for_x509 &&
+	git commit -S -m "X.509 signed commit" &&
+	X509_COMMIT=$(git rev-parse --verify HEAD) &&
+	git checkout main
+
+'
+
+test_expect_success GPGSM 'x509 signature identified' '
+
+	git fast-export --signed-commits=verbatim --reencode=no x509-signing >output 2>err &&
+	grep "^gpgsig x509" output &&
+	test ! -s err &&
+	(
+		cd new &&
+		git fast-import &&
+		STRIPPED=$(git rev-parse --verify refs/heads/x509-signing) &&
+		test $X509_COMMIT = $STRIPPED
+	) <output &&
+	test_might_fail git update-ref -d refs/heads/x509-signing
+
+'
+
+test_expect_success GPGSSH 'setup ssh signed commit' '
+
+	git checkout -b ssh-signing main &&
+	test_config gpg.format ssh &&
+	test_config user.signingkey "${GPGSSH_KEY_PRIMARY}" &&
+	echo "ssh content" >file_for_ssh &&
+	git add file_for_ssh &&
+	git commit -S -m "SSH signed commit" &&
+	SSH_COMMIT=$(git rev-parse --verify HEAD) &&
+	git checkout main
+
+'
+
+test_expect_success GPGSSH 'ssh signature identified' '
+
+	git fast-export --signed-commits=verbatim --reencode=no ssh-signing >output 2>err &&
+	grep "^gpgsig ssh" output &&
+	test ! -s err &&
+	(
+		cd new &&
+		git fast-import &&
+		STRIPPED=$(git rev-parse --verify refs/heads/ssh-signing) &&
+		test "$SSH_COMMIT" = "$STRIPPED"
+	) <output &&
+	test_might_fail git update-ref -d refs/heads/ssh-signing
+
+'
+
 test_expect_success 'setup submodule' '
 
 	test_config_global protocol.file.allow always &&
-- 
2.49.0.392.g2fa1c74b07


             reply	other threads:[~2025-04-24 20:39 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-24 20:39 Christian Couder [this message]
2025-04-24 21:19 ` [PATCH] fast-(import|export): improve on the signature algorithm name Junio C Hamano
2025-04-24 21:59   ` Elijah Newren
2025-04-24 22:58     ` Junio C Hamano
2025-05-26 10:35       ` Christian Couder
2025-05-27 15:18         ` Junio C Hamano
2025-05-28 17:29           ` Junio C Hamano
2025-05-28 20:06             ` Elijah Newren
2025-05-28 21:59               ` Junio C Hamano
2025-05-28 23:15                 ` Elijah Newren
2025-05-29  3:14                   ` Junio C Hamano
2025-06-02 15:56                     ` Christian Couder
2025-06-02 15:56             ` Christian Couder
2025-06-02 16:20               ` Junio C Hamano
2025-05-26 10:34   ` Christian Couder
2025-04-24 21:41 ` Elijah Newren
2025-05-26 10:34   ` Christian Couder
2025-04-24 22:05 ` brian m. carlson
2025-05-26 10:35   ` Christian Couder
2025-04-24 23:25 ` Junio C Hamano
2025-05-26 10:33 ` [PATCH v2 0/6] extract algo information from signatures Christian Couder
2025-05-26 10:33   ` [PATCH v2 1/6] gpg-interface: simplify ssh fingerprint parsing Christian Couder
2025-05-26 10:33   ` [PATCH v2 2/6] gpg-interface: use left shift to define GPG_VERIFY_* Christian Couder
2025-05-26 10:33   ` [PATCH v2 3/6] doc/verify-commit: update and improve the whole doc Christian Couder
2025-05-26 10:33   ` [PATCH v2 4/6] gpg-interface: extract hash algorithm from signature status output Christian Couder
2025-05-26 10:33   ` [PATCH v2 5/6] gpg-interface: extract SSH key type " Christian Couder
2025-05-26 10:33   ` [PATCH v2 6/6] verify-commit: add a --summary flag Christian Couder
2025-05-26 16:03   ` [PATCH v2 0/6] extract algo information from signatures Elijah Newren
2025-06-19 13:38     ` Christian Couder
2025-06-02 22:17   ` brian m. carlson
2025-06-19 13:37     ` Christian Couder
2025-06-18 15:18   ` [PATCH v3] fast-(import|export): improve on commit signature output format Christian Couder
2025-06-19 13:36     ` [PATCH v4] " Christian Couder
2025-06-19 14:55       ` Junio C Hamano
2025-07-08  9:16         ` Christian Couder
2025-06-19 21:44       ` Elijah Newren
2025-06-20 16:12         ` Christian Couder
2025-06-20 19:20           ` Junio C Hamano
2025-07-08  9:16             ` Christian Couder
2025-06-26 19:11           ` Elijah Newren
2025-07-08  9:16             ` Christian Couder
2025-07-07 22:58       ` Junio C Hamano
2025-07-08  3:35         ` Christian Couder
2025-07-08  5:03           ` Junio C Hamano
2025-07-08  6:38             ` Patrick Steinhardt
2025-07-08 11:08               ` Christian Couder
2025-07-08 16:38                 ` Junio C Hamano
2025-07-09  0:19                   ` Christian Couder
2025-07-09 15:35                     ` Junio C Hamano
2025-07-10  8:25                     ` Patrick Steinhardt
2025-07-10 15:29                       ` Christian Couder
2025-07-10 15:33                       ` Junio C Hamano
2025-07-08 10:17             ` Christian Couder
2025-07-08  9:17       ` [PATCH v5] " Christian Couder
2025-07-08 21:58         ` Junio C Hamano
2025-07-08 23:08         ` Elijah Newren
2025-07-09  0:03           ` Junio C Hamano
2025-07-09  0:10             ` Elijah Newren
2025-07-09 10:18             ` Christian Couder
2025-07-09 10:15           ` Christian Couder
2025-07-09 14:12         ` [PATCH v6] " Christian Couder
2025-07-09 23:14           ` Junio C Hamano
2025-07-14 21:07           ` Elijah Newren
2025-07-14 21:23             ` Junio C Hamano
2025-07-25 16:11               ` Christian Couder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250424203904.909777-1-christian.couder@gmail.com \
    --to=christian.couder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).