Merge branch 'sg/gpg-sig'

Teach "merge/pull" to optionally verify and reject commits that are
not signed properly.

* sg/gpg-sig:
  pretty printing: extend %G? to include 'N' and 'U'
  merge/pull Check for untrusted good GPG signatures
  merge/pull: verify GPG signatures of commits being merged
  commit.c/GPG signature verification: Also look at the first GPG status line
  Move commit GPG signature verification to commit.c

Documentation/merge-options.txt

@@ -84,6 +84,11 @@ option can be used to override --squash.
 	Pass merge strategy specific option through to the merge
 	strategy.
 
+--verify-signatures::
+--no-verify-signatures::
+	Verify that the commits being merged have good and trusted GPG signatures
+	and abort the merge in case they do not.
+
 --summary::
 --no-summary::
 	Synonyms to --stat and --no-stat; these are deprecated and will be

Documentation/pretty-formats.txt

@@ -131,7 +131,8 @@ The placeholders are:
 - '%B': raw body (unwrapped subject and body)
 - '%N': commit notes
 - '%GG': raw verification message from GPG for a signed commit
-- '%G?': show either "G" for Good or "B" for Bad for a signed commit
+- '%G?': show "G" for a Good signature, "B" for a Bad signature, "U" for a good,
+  untrusted signature and "N" for no signature
 - '%GS': show the name of the signer for a signed commit
 - '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}`

builtin/merge.c

@@ -49,7 +49,7 @@ static const char * const builtin_merge_usage[] = {
 static int show_diffstat = 1, shortlog_len = -1, squash;
 static int option_commit = 1, allow_fast_forward = 1;
 static int fast_forward_only, option_edit = -1;
-static int allow_trivial = 1, have_message;
+static int allow_trivial = 1, have_message, verify_signatures;
 static int overwrite_ignore = 1;
 static struct strbuf merge_msg = STRBUF_INIT;
 static struct strategy **use_strategies;
@@ -199,6 +199,8 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
 		N_("abort if fast-forward is not possible")),
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
+	OPT_BOOL(0, "verify-signatures", &verify_signatures,
+		N_("Verify that the named commit has a valid GPG signature")),
 	OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
 		N_("merge strategy to use"), option_parse_strategy),
 	OPT_CALLBACK('X', "strategy-option", &xopts, N_("option=value"),
@@ -1246,6 +1248,39 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
 
+	if (verify_signatures) {
+		for (p = remoteheads; p; p = p->next) {
+			struct commit *commit = p->item;
+			char hex[41];
+			struct signature_check signature_check;
+			memset(&signature_check, 0, sizeof(signature_check));
+
+			check_commit_signature(commit, &signature_check);
+
+			strcpy(hex, find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));
+			switch (signature_check.result) {
+			case 'G':
+				break;
+			case 'U':
+				die(_("Commit %s has an untrusted GPG signature, "
+				      "allegedly by %s."), hex, signature_check.signer);
+			case 'B':
+				die(_("Commit %s has a bad GPG signature "
+				      "allegedly by %s."), hex, signature_check.signer);
+			default: /* 'N' */
+				die(_("Commit %s does not have a GPG signature."), hex);
+			}
+			if (verbosity >= 0 && signature_check.result == 'G')
+				printf(_("Commit %s has a good GPG signature by %s\n"),
+				       hex, signature_check.signer);
+
+			free(signature_check.gpg_output);
+			free(signature_check.gpg_status);
+			free(signature_check.signer);
+			free(signature_check.key);
+		}
+	}
+
 	strbuf_addstr(&buf, "merge");
 	for (p = remoteheads; p; p = p->next)
 		strbuf_addf(&buf, " %s", merge_remote_util(p->item)->name);

commit.c

@@ -1041,6 +1041,76 @@ free_return:
 	free(buf);
 }
 
+static struct {
+	char result;
+	const char *check;
+} sigcheck_gpg_status[] = {
+	{ 'G', "\n[GNUPG:] GOODSIG " },
+	{ 'B', "\n[GNUPG:] BADSIG " },
+	{ 'U', "\n[GNUPG:] TRUST_NEVER" },
+	{ 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
+};
+
+static void parse_gpg_output(struct signature_check *sigc)
+{
+	const char *buf = sigc->gpg_status;
+	int i;
+
+	/* Iterate over all search strings */
+	for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
+		const char *found, *next;
+
+		if (!prefixcmp(buf, sigcheck_gpg_status[i].check + 1)) {
+			/* At the very beginning of the buffer */
+			found = buf + strlen(sigcheck_gpg_status[i].check + 1);
+		} else {
+			found = strstr(buf, sigcheck_gpg_status[i].check);
+			if (!found)
+				continue;
+			found += strlen(sigcheck_gpg_status[i].check);
+		}
+		sigc->result = sigcheck_gpg_status[i].result;
+		/* The trust messages are not followed by key/signer information */
+		if (sigc->result != 'U') {
+			sigc->key = xmemdupz(found, 16);
+			found += 17;
+			next = strchrnul(found, '\n');
+			sigc->signer = xmemdupz(found, next - found);
+		}
+	}
+}
+
+void 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;
+
+	sigc->result = 'N';
+
+	if (parse_signed_commit(commit->object.sha1,
+				&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->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);
+	strbuf_release(&payload);
+	strbuf_release(&signature);
+}
+
+
+
 void append_merge_tag_headers(struct commit_list *parents,
 			      struct commit_extra_header ***tail)
 {

commit.h

@@ -5,6 +5,7 @@
 #include "tree.h"
 #include "strbuf.h"
 #include "decorate.h"
+#include "gpg-interface.h"
 
 struct commit_list {
 	struct commit *item;
@@ -232,4 +233,13 @@ extern void print_commit_list(struct commit_list *list,
 			      const char *format_cur,
 			      const char *format_last);
 
+/*
+ * Check the signature of the given commit. The result of the check is stored
+ * in sig->check_result, 'G' for a good signature, 'U' for a good signature
+ * from an untrusted signer, 'B' for a bad signature and 'N' for no signature
+ * 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);
+
 #endif /* COMMIT_H */

git-pull.sh

@@ -39,7 +39,7 @@ test -z "$(git ls-files -u)" || die_conflict
 test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
-log_arg= verbosity= progress= recurse_submodules=
+log_arg= verbosity= progress= recurse_submodules= verify_signatures=
 merge_args= edit=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
@@ -125,6 +125,12 @@ do
 	--no-recurse-submodules)
 		recurse_submodules=--no-recurse-submodules
 		;;
+	--verify-signatures)
+		verify_signatures=--verify-signatures
+		;;
+	--no-verify-signatures)
+		verify_signatures=--no-verify-signatures
+		;;
 	--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
 		dry_run=--dry-run
 		;;
@@ -283,7 +289,7 @@ true)
 	eval="$eval --onto $merge_head ${oldremoteref:-$merge_head}"
 	;;
 *)
-	eval="git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only"
+	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval  $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval \"\$merge_name\" HEAD $merge_head"
 	;;

gpg-interface.h

@@ -1,6 +1,18 @@
 #ifndef GPG_INTERFACE_H
 #define GPG_INTERFACE_H
 
+struct signature_check {
+	char *gpg_output;
+	char *gpg_status;
+	char result; /* 0 (not checked),
+		      * N (checked but no further result),
+		      * U (untrusted good),
+		      * G (good)
+		      * B (bad) */
+	char *signer;
+	char *key;
+};
+
 extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);
 extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status);
 extern int git_gpg_config(const char *, const char *, void *);

pretty.c

@@ -766,14 +766,7 @@ struct format_commit_context {
 	const struct pretty_print_context *pretty_ctx;
 	unsigned commit_header_parsed:1;
 	unsigned commit_message_parsed:1;
-	unsigned commit_signature_parsed:1;
+	struct signature_check signature_check;
-	struct {
-		char *gpg_output;
-		char *gpg_status;
-		char good_bad;
-		char *signer;
-		char *key;
-	} signature;
 	char *message;
 	size_t width, indent1, indent2;
 
@@ -956,64 +949,6 @@ static void rewrap_message_tail(struct strbuf *sb,
 	c->indent2 = new_indent2;
 }
 
-static struct {
-	char result;
-	const char *check;
-} signature_check[] = {
-	{ 'G', "\n[GNUPG:] GOODSIG " },
-	{ 'B', "\n[GNUPG:] BADSIG " },
-};
-
-static void parse_signature_lines(struct format_commit_context *ctx)
-{
-	const char *buf = ctx->signature.gpg_status;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(signature_check); i++) {
-		const char *found = strstr(buf, signature_check[i].check);
-		const char *next;
-		if (!found)
-			continue;
-		ctx->signature.good_bad = signature_check[i].result;
-		found += strlen(signature_check[i].check);
-		ctx->signature.key = xmemdupz(found, 16);
-		found += 17;
-		next = strchrnul(found, '\n');
-		ctx->signature.signer = xmemdupz(found, next - found);
-		break;
-	}
-}
-
-static void parse_commit_signature(struct format_commit_context *ctx)
-{
-	struct strbuf payload = STRBUF_INIT;
-	struct strbuf signature = STRBUF_INIT;
-	struct strbuf gpg_output = STRBUF_INIT;
-	struct strbuf gpg_status = STRBUF_INIT;
-	int status;
-
-	ctx->commit_signature_parsed = 1;
-
-	if (parse_signed_commit(ctx->commit->object.sha1,
-				&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;
-	ctx->signature.gpg_output = strbuf_detach(&gpg_output, NULL);
-	ctx->signature.gpg_status = strbuf_detach(&gpg_status, NULL);
-	parse_signature_lines(ctx);
-
- out:
-	strbuf_release(&gpg_status);
-	strbuf_release(&gpg_output);
-	strbuf_release(&payload);
-	strbuf_release(&signature);
-}
-
-
 static int format_reflog_person(struct strbuf *sb,
 				char part,
 				struct reflog_walk_info *log,
@@ -1199,27 +1134,29 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 	}
 
 	if (placeholder[0] == 'G') {
-		if (!c->commit_signature_parsed)
+		if (!c->signature_check.result)
-			parse_commit_signature(c);
+			check_commit_signature(c->commit, &(c->signature_check));
 		switch (placeholder[1]) {
 		case 'G':
-			if (c->signature.gpg_output)
+			if (c->signature_check.gpg_output)
-				strbuf_addstr(sb, c->signature.gpg_output);
+				strbuf_addstr(sb, c->signature_check.gpg_output);
 			break;
 		case '?':
-			switch (c->signature.good_bad) {
+			switch (c->signature_check.result) {
 			case 'G':
 			case 'B':
-				strbuf_addch(sb, c->signature.good_bad);
+			case 'U':
+			case 'N':
+				strbuf_addch(sb, c->signature_check.result);
 			}
 			break;
 		case 'S':
-			if (c->signature.signer)
+			if (c->signature_check.signer)
-				strbuf_addstr(sb, c->signature.signer);
+				strbuf_addstr(sb, c->signature_check.signer);
 			break;
 		case 'K':
-			if (c->signature.key)
+			if (c->signature_check.key)
-				strbuf_addstr(sb, c->signature.key);
+				strbuf_addstr(sb, c->signature_check.key);
 			break;
 		}
 		return 2;
@@ -1357,8 +1294,8 @@ void format_commit_message(const struct commit *commit,
 	rewrap_message_tail(sb, &context, 0, 0, 0);
 
 	logmsg_free(context.message, commit);
-	free(context.signature.gpg_output);
+	free(context.signature_check.gpg_output);
-	free(context.signature.signer);
+	free(context.signature_check.signer);
 }
 
 static void pp_header(const struct pretty_print_context *pp,

t/t7612-merge-verify-signatures.sh

@@ -0,0 +1,61 @@
+#!/bin/sh
+
+test_description='merge signature verification tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success GPG 'create signed commits' '
+	echo 1 >file && git add file &&
+	test_tick && git commit -m initial &&
+	git tag initial &&
+
+	git checkout -b side-signed &&
+	echo 3 >elif && git add elif &&
+	test_tick && git commit -S -m "signed on side" &&
+	git checkout initial &&
+
+	git checkout -b side-unsigned &&
+	echo 3 >foo && git add foo &&
+	test_tick && git commit -m "unsigned on side" &&
+	git checkout initial &&
+
+	git checkout -b side-bad &&
+	echo 3 >bar && git add bar &&
+	test_tick && git commit -S -m "bad on side" &&
+	git cat-file commit side-bad >raw &&
+	sed -e "s/bad/forged bad/" raw >forged &&
+	git hash-object -w -t commit forged >forged.commit &&
+	git checkout initial &&
+
+	git checkout -b side-untrusted &&
+	echo 3 >baz && git add baz &&
+	test_tick && git commit -SB7227189 -m "untrusted on side"
+
+	git checkout master
+'
+
+test_expect_success GPG 'merge unsigned commit with verification' '
+	test_must_fail git merge --ff-only --verify-signatures side-unsigned 2>mergeerror &&
+	test_i18ngrep "does not have a GPG signature" mergeerror
+'
+
+test_expect_success GPG 'merge commit with bad signature with verification' '
+	test_must_fail git merge --ff-only --verify-signatures $(cat forged.commit) 2>mergeerror &&
+	test_i18ngrep "has a bad GPG signature" mergeerror
+'
+
+test_expect_success GPG 'merge commit with untrusted signature with verification' '
+	test_must_fail git merge --ff-only --verify-signatures side-untrusted 2>mergeerror &&
+	test_i18ngrep "has an untrusted GPG signature" mergeerror
+'
+
+test_expect_success GPG 'merge signed commit with verification' '
+	git merge --verbose --ff-only --verify-signatures side-signed >mergeoutput &&
+	test_i18ngrep "has a good GPG signature" mergeoutput
+'
+
+test_expect_success GPG 'merge commit with bad signature without verification' '
+	git merge $(cat forged.commit)
+'
+
+test_done