threads / patch / 39629

patch, 3 partsRaw gpg output support for verify-commit and verify-tag

Subject: [PATCH 0/3] Raw gpg output support for verify-commit and verify-tag

## tl;dr

7 messages between Jun 14, 2015 and Jun 15, 2015. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

brian m. carlson· Jun 14, 2015, 18:51 UTC · lore

Currently, verify-commit and verify-tag produce human-readable output. This is great for humans, and awful for machines. It also lacks a lot of the information that GnuPG's --status-fd output provides.

For example, if you wanted to know
* the hash algorithm;
* whether the signature was made with a subkey; or
* the OpenPGP signature version
none of that information is available in the human-readable output.

We've had people in the past come to the list who require signed commits in their corporate environment. It's not unreasonable to expect that they might want to programmatically verify signatures, including aspects of the signatures we don't currently expose. It's also much nicer to parse the machine-readable output we already collect than hoping GnuPG doesn't change its output.

This series introduces a --raw option for verify-commit and verify-tag. If it's used, they provide the gpg --status-fd output on standard error instead of the human-readable output. The series also adds tests for verify-tag, since there were none; these are based off the ones for verify-commit.

In writing this series, I noticed an incompatibility between verify-commit and verify-tag. If a valid signature is made with an untrusted key, verify-commit will exit 1, but verify-tag will exit 0. I'm unclear on what we can do about this now, short of adding another option. This is because the two commands share little common code.

brian m. carlson (3):
  verify-commit: add option to print raw gpg status information
  verify-tag: add tests
  verify-tag: add option to print raw gpg status information
 Documentation/git-verify-commit.txt |   4 ++
 Documentation/git-verify-tag.txt    |   4 ++
 builtin/verify-commit.c             |  13 ++--
 builtin/verify-tag.c                |  21 +++++--
 t/t7030-verify-tag.sh               | 116 ++++++++++++++++++++++++++++++++++++
 t/t7510-signed-commit.sh            |  32 ++++++++++
 6 files changed, 178 insertions(+), 12 deletions(-)
 create mode 100755 t/t7030-verify-tag.sh
-- 
2.4.0
brian m. carlson· Jun 14, 2015, 18:51 UTC · re: brian m. carlson · lore

[PATCH 1/3] verify-commit: add option to print raw gpg status information

verify-commit by default displays human-readable output on standard error. However, it can also be useful to get access to the raw gpg status information, which is machine-readable, allowing automated implementation of signing policy. Add a --raw option to make verify-commit produce the gpg status information on standard error instead of the human-readable format.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-verify-commit.txt |  4 ++++
 builtin/verify-commit.c             | 13 +++++++------
 t/t7510-signed-commit.sh            | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 6 deletions(-)
Show changes to 3 files +43 −6

Documentation/git-verify-commit.txt, builtin/verify-commit.c, t/t7510-signed-commit.sh

diff --git a/Documentation/git-verify-commit.txt b/Documentation/git-verify-commit.txt
index 9413e28..ecf4da1 100644
--- a/Documentation/git-verify-commit.txt
+++ b/Documentation/git-verify-commit.txt
@@ -16,6 +16,10 @@ Validates the gpg signature created by 'git commit -S'.
 
 OPTIONS
 -------
+--raw::
+	Print the raw gpg status output to standard error instead of the normal
+	human-readable output.
+
 -v::
 --verbose::
 	Print the contents of the commit object before validating it.
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index ec0c4e3..04646fc 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -18,7 +18,7 @@ static const char * const verify_commit_usage[] = {
 		NULL
 };
 
-static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned long size, int verbose)
+static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned long size, int verbose, int raw)
 {
 	struct signature_check signature_check;
 
@@ -30,13 +30,13 @@ static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned l
 		fputs(signature_check.payload, stdout);
 
 	if (signature_check.gpg_output)
-		fputs(signature_check.gpg_output, stderr);
+		fputs(raw ? signature_check.gpg_status : signature_check.gpg_output, stderr);
 
 	signature_check_clear(&signature_check);
 	return signature_check.result != 'G';
 }
 
-static int verify_commit(const char *name, int verbose)
+static int verify_commit(const char *name, int verbose, int raw)
 {
 	enum object_type type;
 	unsigned char sha1[20];
@@ -54,7 +54,7 @@ static int verify_commit(const char *name, int verbose)
 		return error("%s: cannot verify a non-commit object of type %s.",
 				name, typename(type));
 
-	ret = run_gpg_verify(sha1, buf, size, verbose);
+	ret = run_gpg_verify(sha1, buf, size, verbose, raw);
 
 	free(buf);
 	return ret;
@@ -70,9 +70,10 @@ static int git_verify_commit_config(const char *var, const char *value, void *cb
 
 int cmd_verify_commit(int argc, const char **argv, const char *prefix)
 {
-	int i = 1, verbose = 0, had_error = 0;
+	int i = 1, verbose = 0, had_error = 0, raw = 0;
 	const struct option verify_commit_options[] = {
 		OPT__VERBOSE(&verbose, N_("print commit contents")),
+		OPT_BOOL(0, "raw", &raw, N_("print raw gpg status output")),
 		OPT_END()
 	};
 
@@ -87,7 +88,7 @@ int cmd_verify_commit(int argc, const char **argv, const char *prefix)
 	 * was received in the process of writing the gpg input: */
 	signal(SIGPIPE, SIG_IGN);
 	while (i < argc)
-		if (verify_commit(argv[i++], verbose))
+		if (verify_commit(argv[i++], verbose, raw))
 			had_error = 1;
 	return had_error;
 }
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 13331e5..ef316f2 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -81,6 +81,38 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPG 'verify signatures with --raw' '
+	(
+		for commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed
+		do
+			git verify-commit --raw $commit 2>actual &&
+			grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			echo $commit OK || exit 1
+		done
+	) &&
+	(
+		for commit in merge^2 fourth-unsigned sixth-unsigned seventh-unsigned
+		do
+			test_must_fail git verify-commit --raw $commit 2>actual &&
+			! grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			echo $commit OK || exit 1
+		done
+	) &&
+	(
+		for commit in eighth-signed-alt
+		do
+			# Undefined trust causes verify-commit to exit abnormally.
+			test_must_fail git verify-commit --raw $commit 2>actual &&
+			grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			grep "TRUST_UNDEFINED" actual &&
+			echo $commit OK || exit 1
+		done
+	)
+'
+
 test_expect_success GPG 'show signed commit with signature' '
 	git show -s initial >commit &&
 	git show -s --show-signature initial >show &&
-- 
2.4.0
brian m. carlson· Jun 14, 2015, 18:51 UTC · re: brian m. carlson · lore

[PATCH 2/3] verify-tag: add tests

verify-tag was lacking tests. Add some, mirroring those used for verify-commit.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t7030-verify-tag.sh | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100755 t/t7030-verify-tag.sh
Show changes to t/t7030-verify-tag.sh +85 −0
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
new file mode 100755
index 0000000..9ca0d29
--- /dev/null
+++ b/t/t7030-verify-tag.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='signed tag tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success GPG 'create signed tags' '
+	echo 1 >file && git add file &&
+	test_tick && git commit -m initial &&
+	git tag -s -m initial initial &&
+	git branch side &&
+	echo a &&
+
+	echo 2 >file && test_tick && git commit -a -m second &&
+	git tag -s -m second second &&
+	echo b &&
+
+	git checkout side &&
+	echo 3 >elif && git add elif &&
+	test_tick && git commit -m "third on side" &&
+	echo c &&
+
+	git checkout master &&
+	test_tick && git merge -S side &&
+	git tag -s -m merge merge &&
+	echo d &&
+
+	echo 4 >file && test_tick && git commit -a -S -m "fourth unsigned" &&
+	git tag -a -m fourth-unsigned fourth-unsigned &&
+	echo e &&
+
+	test_tick && git commit --amend -S -m "fourth signed" &&
+	git tag -s -m fourth fourth-signed &&
+	echo f &&
+
+	echo 5 >file && test_tick && git commit -a -m "fifth" &&
+	git tag fifth-unsigned &&
+	echo g &&
+
+	git config commit.gpgsign true &&
+	echo 6 >file && test_tick && git commit -a -m "sixth" &&
+	git tag -a -m sixth sixth-unsigned &&
+	echo h &&
+
+	test_tick && git rebase -f HEAD^^ && git tag -s -m 6th sixth-signed HEAD^ &&
+	git tag -m seventh -s seventh-signed &&
+	echo i &&
+
+	echo 8 >file && test_tick && git commit -a -m eighth &&
+	git tag -uB7227189 -m eighth eighth-signed-alt &&
+	echo j
+'
+
+test_expect_success GPG 'verify and show signatures' '
+	(
+		for tag in initial second merge fourth-signed sixth-signed seventh-signed
+		do
+			git verify-tag $tag 2>actual &&
+			grep "Good signature from" actual &&
+			! grep "BAD signature from" actual &&
+			echo $tag OK || exit 1
+		done
+	) &&
+	(
+		for tag in fourth-unsigned fifth-unsigned sixth-unsigned
+		do
+			test_must_fail git verify-tag $tag 2>actual &&
+			! grep "Good signature from" actual &&
+			! grep "BAD signature from" actual &&
+			echo $tag OK || exit 1
+		done
+	) &&
+	(
+		for tag in eighth-signed-alt
+		do
+			git verify-tag $tag 2>actual &&
+			grep "Good signature from" actual &&
+			! grep "BAD signature from" actual &&
+			grep "not certified" actual &&
+			echo $tag OK || exit 1
+		done
+	)
+'
+
+test_done
-- 
2.4.0
brian m. carlson· Jun 14, 2015, 18:51 UTC · re: brian m. carlson · lore

[PATCH 3/3] verify-tag: add option to print raw gpg status information

verify-tag by default displays human-readable output on standard error. However, it can also be useful to get access to the raw gpg status information, which is machine-readable, allowing automated implementation of signing policy. Add a --raw option to make verify-tag produce the gpg status information on standard error instead of the human-readable format.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-verify-tag.txt |  4 ++++
 builtin/verify-tag.c             | 21 +++++++++++++++------
 t/t7030-verify-tag.sh            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 6 deletions(-)
Show changes to 3 files +50 −6

Documentation/git-verify-tag.txt, builtin/verify-tag.c, t/t7030-verify-tag.sh

diff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index f88ba96..d590edc 100644
--- a/Documentation/git-verify-tag.txt
+++ b/Documentation/git-verify-tag.txt
@@ -16,6 +16,10 @@ Validates the gpg signature created by 'git tag'.
 
 OPTIONS
 -------
+--raw::
+	Print the raw gpg status output to standard error instead of the normal
+	human-readable output.
+
 -v::
 --verbose::
 	Print the contents of the tag object before validating it.
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 53c68fc..745c013 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,9 +18,12 @@ static const char * const verify_tag_usage[] = {
 		NULL
 };
 
-static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
+static int run_gpg_verify(const char *buf, unsigned long size, int verbose, int raw)
 {
+	struct strbuf status = STRBUF_INIT, human_readable = STRBUF_INIT;
+	const struct strbuf *output = raw ? &status : &human_readable;
 	int len;
+	int ret;
 
 	len = parse_signature(buf, size);
 	if (verbose)
@@ -29,10 +32,15 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 	if (size == len)
 		return error("no signature found");
 
-	return verify_signed_buffer(buf, len, buf + len, size - len, NULL, NULL);
+	ret = verify_signed_buffer(buf, len, buf + len, size - len, &human_readable,
+			&status);
+
+	fputs(output->buf, stderr);
+
+	return ret;
 }
 
-static int verify_tag(const char *name, int verbose)
+static int verify_tag(const char *name, int verbose, int raw)
 {
 	enum object_type type;
 	unsigned char sha1[20];
@@ -52,7 +60,7 @@ static int verify_tag(const char *name, int verbose)
 	if (!buf)
 		return error("%s: unable to read file.", name);
 
-	ret = run_gpg_verify(buf, size, verbose);
+	ret = run_gpg_verify(buf, size, verbose, raw);
 
 	free(buf);
 	return ret;
@@ -68,9 +76,10 @@ static int git_verify_tag_config(const char *var, const char *value, void *cb)
 
 int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 {
-	int i = 1, verbose = 0, had_error = 0;
+	int i = 1, verbose = 0, had_error = 0, raw = 0;
 	const struct option verify_tag_options[] = {
 		OPT__VERBOSE(&verbose, N_("print tag contents")),
+		OPT_BOOL(0, "raw", &raw, N_("print raw gpg status output")),
 		OPT_END()
 	};
 
@@ -85,7 +94,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	 * was received in the process of writing the gpg input: */
 	signal(SIGPIPE, SIG_IGN);
 	while (i < argc)
-		if (verify_tag(argv[i++], verbose))
+		if (verify_tag(argv[i++], verbose, raw))
 			had_error = 1;
 	return had_error;
 }
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 9ca0d29..0ac15e6 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -82,4 +82,35 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPG 'verify signatures with --raw' '
+	(
+		for tag in initial second merge fourth-signed sixth-signed seventh-signed
+		do
+			git verify-tag --raw $tag 2>actual &&
+			grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			echo $tag OK || exit 1
+		done
+	) &&
+	(
+		for tag in fourth-unsigned fifth-unsigned sixth-unsigned
+		do
+			test_must_fail git verify-tag --raw $tag 2>actual &&
+			! grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			echo $tag OK || exit 1
+		done
+	) &&
+	(
+		for tag in eighth-signed-alt
+		do
+			git verify-tag --raw $tag 2>actual &&
+			grep "GOODSIG" actual &&
+			! grep "BADSIG" actual &&
+			grep "TRUST_UNDEFINED" actual &&
+			echo $tag OK || exit 1
+		done
+	)
+'
+
 test_done
-- 
2.4.0
Junio C Hamano· Jun 14, 2015, 21:23 UTC · re: brian m. carlson · lore

Re: [PATCH 0/3] Raw gpg output support for verify-commit and verify-tag

"brian m. carlson" <sandals@crustytoothpaste.net> writes:
Show 9 quoted lines
> Currently, verify-commit and verify-tag produce human-readable output.
> This is great for humans, and awful for machines.  It also lacks a lot
> of the information that GnuPG's --status-fd output provides.
>
> For example, if you wanted to know
> * the hash algorithm;
> * whether the signature was made with a subkey; or
> * the OpenPGP signature version
> none of that information is available in the human-readable output.

What this series wants to achieve makes a lot of sense to me. One comment I have after skimming the patfches is that I want the new "raw" bit added not as a yet another parameter after "verbose", but by turning the existing "verbose" into an "unsigned flag" flag word (with "#define GPG_VERIFY_VERBOSE 01") and then defining use of a new bit in the same flag word "#define GPG_VERIFY_RAW 01" or something. That way, over time other people add differnt things to the interface, we would not have to end up with 47 different parameters each of which signals a single bit.

Thanks.
brian m. carlson· Jun 14, 2015, 22:14 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] Raw gpg output support for verify-commit and verify-tag

On Sun, Jun 14, 2015 at 02:23:47PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> What this series wants to achieve makes a lot of sense to me.  One
> comment I have after skimming the patfches is that I want the new
> "raw" bit added not as a yet another parameter after "verbose", but
> by turning the existing "verbose" into an "unsigned flag" flag word
> (with "#define GPG_VERIFY_VERBOSE 01") and then defining use of a
> new bit in the same flag word "#define GPG_VERIFY_RAW 01" or
> something.  That way, over time other people add differnt things to
> the interface, we would not have to end up with 47 different
> parameters each of which signals a single bit.
I'll wait for other responses (if any) and then reroll.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Michael J Gruber· Jun 15, 2015, 08:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] Raw gpg output support for verify-commit and verify-tag

Junio C Hamano venit, vidit, dixit 14.06.2015 23:23:
Show 23 quoted lines
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
>> Currently, verify-commit and verify-tag produce human-readable output.
>> This is great for humans, and awful for machines.  It also lacks a lot
>> of the information that GnuPG's --status-fd output provides.
>>
>> For example, if you wanted to know
>> * the hash algorithm;
>> * whether the signature was made with a subkey; or
>> * the OpenPGP signature version
>> none of that information is available in the human-readable output.
> 
> What this series wants to achieve makes a lot of sense to me.  One
> comment I have after skimming the patfches is that I want the new
> "raw" bit added not as a yet another parameter after "verbose", but
> by turning the existing "verbose" into an "unsigned flag" flag word
> (with "#define GPG_VERIFY_VERBOSE 01") and then defining use of a
> new bit in the same flag word "#define GPG_VERIFY_RAW 01" or
> something.  That way, over time other people add differnt things to
> the interface, we would not have to end up with 47 different
> parameters each of which signals a single bit.
> 
> Thanks.

Back then the idea was to have verify-commit available *yesterday* (before any refactoring) since we needed a way to verify the new commit signatures programmatically.

Maybe now would be a good time to do the refactoring between verify-tag and verify-commit that is still missing, and to add new features then (unless they are pressing matters or fixes).

BTW: verify-tag and verify-commit should treat untrusted good signatures
in exactly the same way. Anything else is an error that needs to be fixed.
Michael

← back to recent threads