Merge branch 'bc/gpg-verify-raw'

"git verify-tag" and "git verify-commit" have been taught to share
more code, and then learned to optionally show the verification
message from the underlying GPG implementation.

* bc/gpg-verify-raw:
  verify-tag: add option to print raw gpg status information
  verify-commit: add option to print raw gpg status information
  gpg: centralize printing signature buffers
  gpg: centralize signature check
  verify-commit: add test for exit status on untrusted signature
  verify-tag: share code with verify-commit
  verify-tag: add tests

10 files changed, 246 insertions, 36 deletions

diff --git a/Documentation/git-verify-commit.txt b/Documentation/git-verify-commit.txt
index 9413e2802..ecf4da16c 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/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt
index f88ba96f0..d590edceb 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-commit.c b/builtin/verify-commit.c
index ec0c4e3d8..38bedf8f9 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -18,25 +18,21 @@ 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, unsigned flags)
 {
 	struct signature_check signature_check;
+	int ret;
 
 	memset(&signature_check, 0, sizeof(signature_check));
 
-	check_commit_signature(lookup_commit(sha1), &signature_check);
-
-	if (verbose && signature_check.payload)
-		fputs(signature_check.payload, stdout);
-
-	if (signature_check.gpg_output)
-		fputs(signature_check.gpg_output, stderr);
+	ret = check_commit_signature(lookup_commit(sha1), &signature_check);
+	print_signature_buffer(&signature_check, flags);
 
 	signature_check_clear(&signature_check);
-	return signature_check.result != 'G';
+	return ret;
 }
 
-static int verify_commit(const char *name, int verbose)
+static int verify_commit(const char *name, unsigned flags)
 {
 	enum object_type type;
 	unsigned char sha1[20];
@@ -54,7 +50,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, flags);
 
 	free(buf);
 	return ret;
@@ -71,8 +67,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;
+	unsigned flags = 0;
 	const struct option verify_commit_options[] = {
 		OPT__VERBOSE(&verbose, N_("print commit contents")),
+		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
 		OPT_END()
 	};
 
@@ -83,11 +81,14 @@ int cmd_verify_commit(int argc, const char **argv, const char *prefix)
 	if (argc <= i)
 		usage_with_options(verify_commit_usage, verify_commit_options);
 
+	if (verbose)
+		flags |= GPG_VERIFY_VERBOSE;
+
 	/* sometimes the program was terminated because this signal
 	 * 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++], flags))
 			had_error = 1;
 	return had_error;
 }
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 53c68fce3..00663f6a3 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -18,21 +18,30 @@ 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, unsigned flags)
 {
+	struct signature_check sigc;
 	int len;
+	int ret;
+
+	memset(&sigc, 0, sizeof(sigc));
 
 	len = parse_signature(buf, size);
-	if (verbose)
-		write_in_full(1, buf, len);
 
-	if (size == len)
+	if (size == len) {
+		if (flags & GPG_VERIFY_VERBOSE)
+			write_in_full(1, buf, len);
 		return error("no signature found");
+	}
+
+	ret = check_signature(buf, len, buf + len, size - len, &sigc);
+	print_signature_buffer(&sigc, flags);
 
-	return verify_signed_buffer(buf, len, buf + len, size - len, NULL, NULL);
+	signature_check_clear(&sigc);
+	return ret;
 }
 
-static int verify_tag(const char *name, int verbose)
+static int verify_tag(const char *name, unsigned flags)
 {
 	enum object_type type;
 	unsigned char sha1[20];
@@ -52,7 +61,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, flags);
 
 	free(buf);
 	return ret;
@@ -69,8 +78,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;
+	unsigned flags = 0;
 	const struct option verify_tag_options[] = {
 		OPT__VERBOSE(&verbose, N_("print tag contents")),
+		OPT_BIT(0, "raw", &flags, N_("print raw gpg status output"), GPG_VERIFY_RAW),
 		OPT_END()
 	};
 
@@ -81,11 +92,14 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 	if (argc <= i)
 		usage_with_options(verify_tag_usage, verify_tag_options);
 
+	if (verbose)
+		flags |= GPG_VERIFY_VERBOSE;
+
 	/* sometimes the program was terminated because this signal
 	 * 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++], flags))
 			had_error = 1;
 	return had_error;
 }
diff --git a/commit.c b/commit.c
index 6e2103cef..62223a33e 100644
--- a/commit.c
+++ b/commit.c
@@ -1232,33 +1232,24 @@ free_return:
 	free(buf);
 }
 
-void check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
-	struct strbuf gpg_output = STRBUF_INIT;
-	struct strbuf gpg_status = STRBUF_INIT;
-	int status;
+	int ret = 1;
 
 	sigc->result = 'N';
 
 	if (parse_signed_commit(commit, &payload, &signature) <= 0)
 		goto out;
-	status = verify_signed_buffer(payload.buf, payload.len,
-				      signature.buf, signature.len,
-				      &gpg_output, &gpg_status);
-	if (status && !gpg_output.len)
-		goto out;
-	sigc->payload = strbuf_detach(&payload, NULL);
-	sigc->gpg_output = strbuf_detach(&gpg_output, NULL);
-	sigc->gpg_status = strbuf_detach(&gpg_status, NULL);
-	parse_gpg_output(sigc);
+	ret = check_signature(payload.buf, payload.len, signature.buf,
+		signature.len, sigc);
 
  out:
-	strbuf_release(&gpg_status);
-	strbuf_release(&gpg_output);
 	strbuf_release(&payload);
 	strbuf_release(&signature);
+
+	return ret;
 }
 
 
diff --git a/commit.h b/commit.h
index 9a1fa961d..4983bdd3d 100644
--- a/commit.h
+++ b/commit.h
@@ -379,7 +379,7 @@ extern void print_commit_list(struct commit_list *list,
  * at all.  This may allocate memory for sig->gpg_output, sig->gpg_status,
  * sig->signer and sig->key.
  */
-extern void check_commit_signature(const struct commit *commit, struct signature_check *sigc);
+extern int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
 
 int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 68b0c814f..3dc2fe397 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -60,6 +60,43 @@ void parse_gpg_output(struct signature_check *sigc)
 	}
 }
 
+int check_signature(const char *payload, size_t plen, const char *signature,
+	size_t slen, struct signature_check *sigc)
+{
+	struct strbuf gpg_output = STRBUF_INIT;
+	struct strbuf gpg_status = STRBUF_INIT;
+	int status;
+
+	sigc->result = 'N';
+
+	status = verify_signed_buffer(payload, plen, signature, slen,
+				      &gpg_output, &gpg_status);
+	if (status && !gpg_output.len)
+		goto out;
+	sigc->payload = xmemdupz(payload, plen);
+	sigc->gpg_output = strbuf_detach(&gpg_output, NULL);
+	sigc->gpg_status = strbuf_detach(&gpg_status, NULL);
+	parse_gpg_output(sigc);
+
+ out:
+	strbuf_release(&gpg_status);
+	strbuf_release(&gpg_output);
+
+	return sigc->result != 'G' && sigc->result != 'U';
+}
+
+void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
+{
+	const char *output = flags & GPG_VERIFY_RAW ?
+		sigc->gpg_status : sigc->gpg_output;
+
+	if (flags & GPG_VERIFY_VERBOSE && sigc->payload)
+		fputs(sigc->payload, stdout);
+
+	if (output)
+		fputs(output, stderr);
+}
+
 /*
  * Look at GPG signed content (e.g. a signed tag object), whose
  * payload is followed by a detached signature on it.  Return the
diff --git a/gpg-interface.h b/gpg-interface.h
index 87a4f2e3f..ea68885ad 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -1,6 +1,9 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
+#define GPG_VERIFY_VERBOSE	1
+#define GPG_VERIFY_RAW		2
+
 struct signature_check {
 	char *payload;
 	char *gpg_output;
@@ -27,5 +30,8 @@ extern int verify_signed_buffer(const char *payload, size_t payload_size, const
 extern int git_gpg_config(const char *, const char *, void *);
 extern void set_signing_key(const char *);
 extern const char *get_signing_key(void);
+extern int check_signature(const char *payload, size_t plen,
+	const char *signature, size_t slen, struct signature_check *sigc);
+void print_signature_buffer(const struct signature_check *sigc, unsigned flags);
 
 #endif
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
new file mode 100755
index 000000000..4608e7134
--- /dev/null
+++ b/t/t7030-verify-tag.sh
@@ -0,0 +1,115 @@
+#!/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 2 >file && test_tick && git commit -a -m second &&
+	git tag -s -m second second &&
+
+	git checkout side &&
+	echo 3 >elif && git add elif &&
+	test_tick && git commit -m "third on side" &&
+
+	git checkout master &&
+	test_tick && git merge -S side &&
+	git tag -s -m merge merge &&
+
+	echo 4 >file && test_tick && git commit -a -S -m "fourth unsigned" &&
+	git tag -a -m fourth-unsigned fourth-unsigned &&
+
+	test_tick && git commit --amend -S -m "fourth signed" &&
+	git tag -s -m fourth fourth-signed &&
+
+	echo 5 >file && test_tick && git commit -a -m "fifth" &&
+	git tag fifth-unsigned &&
+
+	git config commit.gpgsign true &&
+	echo 6 >file && test_tick && git commit -a -m "sixth" &&
+	git tag -a -m sixth sixth-unsigned &&
+
+	test_tick && git rebase -f HEAD^^ && git tag -s -m 6th sixth-signed HEAD^ &&
+	git tag -m seventh -s seventh-signed &&
+
+	echo 8 >file && test_tick && git commit -a -m eighth &&
+	git tag -uB7227189 -m eighth eighth-signed-alt
+'
+
+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_expect_success GPG 'detect fudged signature' '
+	git cat-file tag seventh-signed >raw &&
+	sed -e "s/seventh/7th forged/" raw >forged1 &&
+	git hash-object -w -t tag forged1 >forged1.tag &&
+	test_must_fail git verify-tag $(cat forged1.tag) 2>actual1 &&
+	grep "BAD signature from" actual1 &&
+	! grep "Good signature from" actual1
+'
+
+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
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 13331e533..18e5cf066 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -81,6 +81,44 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPG 'verify-commit exits success on untrusted signature' '
+	git verify-commit eighth-signed-alt 2>actual &&
+	grep "Good signature from" actual &&
+	! grep "BAD signature from" actual &&
+	grep "not certified" actual
+'
+
+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
+			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 &&