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

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

[brian m. carlson]

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

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

[brian m. carlson]

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(-)

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

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

[brian m. carlson]

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

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

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

[brian m. carlson]

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(-)

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

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

[Junio C Hamano]

"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.

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 935 bytes --]

On Sun, Jun 14, 2015 at 02:23:47PM -0700, Junio C Hamano wrote:
> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

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

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 14.06.2015 23:23:
> "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