[PATCH 1/3] merge: add config option for verifySignatures

Subsystems: documentation, the rest

STALE3162d

9 messages, 2 authors, 2017-12-10 · open the first message on its own page

[PATCH 1/3] merge: add config option for verifySignatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-09 09:06:21 

Verify the signature of the tip commit when `merge.verifySignatures` is
true.  This can be overridden with `--no-verify-signatures`.

Signed-off-by: Hans Jerry Illikainen <redacted>
---
 Documentation/merge-config.txt     |  7 +++++++
 builtin/merge.c                    |  2 ++
 t/t7612-merge-verify-signatures.sh | 43 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index df3ea3779..76571cd3b 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -26,6 +26,13 @@ merge.ff::
 	allowed (equivalent to giving the `--ff-only` option from the
 	command line).
 
+merge.verifySignatures::
+	Verify that the tip commit of the side branch being merged is
+	signed with a valid key, i.e. a key that has a valid uid: in the
+	default trust model, this means the signing key has been signed
+	by a trusted key. If the tip commit of the side branch is not
+	signed with a valid key, the merge is aborted.
+
 include::fmt-merge-msg-config.txt[]
 
 merge.renameLimit::
diff --git a/builtin/merge.c b/builtin/merge.c
index 612dd7bfb..30264cfd7 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
 		show_diffstat = git_config_bool(k, v);
+	else if (!strcmp(k, "merge.verifysignatures"))
+		verify_signatures = git_config_bool(k, v);
 	else if (!strcmp(k, "pull.twohead"))
 		return git_config_string(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh
index 8ae69a61c..f1a74a683 100755
--- a/t/t7612-merge-verify-signatures.sh
+++ b/t/t7612-merge-verify-signatures.sh
@@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with verification' '
 	test_i18ngrep "does not have a GPG signature" mergeerror
 '
 
+test_expect_success GPG 'merge unsigned commit with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only 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 bad signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only $(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 commit with untrusted signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only 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_i18ngrep "has a good GPG signature" mergeoutput &&
+	git checkout initial
+'
+
+test_expect_success GPG 'merge signed commit with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	git merge --verbose --ff-only side-signed >mergeoutput &&
+	test_i18ngrep "has a good GPG signature" mergeoutput &&
+	git checkout initial
 '
 
 test_expect_success GPG 'merge commit with bad signature without verification' '
-	git merge $(cat forged.commit)
+	git merge $(cat forged.commit) &&
+	git checkout initial
+'
+
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=false' '
+	test_config merge.verifySignatures false &&
+	git merge $(cat forged.commit) &&
+	git checkout initial
+'
+
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true and --no-verify-signatures' '
+	test_config merge.verifySignatures true &&
+	git merge --no-verify-signatures $(cat forged.commit) &&
+	git checkout initial
 '
 
 test_done
-- 
2.11.0

[PATCH 2/3] t: add tests for pull --verify-signatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-09 09:06:09 

Add tests for `pull --verify-signatures` with untrusted, bad and no
signatures.  Previously the only test for `--verify-signatures` was to
make sure that `pull --rebase --verify-signatures` result in a warning
(t5520-pull.sh).

Signed-off-by: Hans Jerry Illikainen <redacted>
---
 t/t5573-pull-verify-signatures.sh | 78 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100755 t/t5573-pull-verify-signatures.sh

diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh
new file mode 100755
index 000000000..700247910
--- /dev/null
+++ b/t/t5573-pull-verify-signatures.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='pull signature verification tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success GPG 'create repositories with signed commits' '
+	echo 1 >a && git add a &&
+	test_tick && git commit -m initial &&
+	git tag initial &&
+
+	git clone . signed &&
+	(
+		cd signed &&
+		echo 2 >b && git add b &&
+		test_tick && git commit -S -m "signed"
+	) &&
+
+	git clone . unsigned &&
+	(
+		cd unsigned &&
+		echo 3 >c && git add c &&
+		test_tick && git commit -m "unsigned"
+	) &&
+
+	git clone . bad &&
+	(
+		cd bad &&
+		echo 4 >d && git add d &&
+		test_tick && git commit -S -m "bad" &&
+		git cat-file commit HEAD >raw &&
+		sed -e "s/bad/forged bad/" raw >forged &&
+		git hash-object -w -t commit forged >forged.commit &&
+		git checkout $(cat forged.commit)
+	) &&
+
+	git clone . untrusted &&
+	(
+		cd untrusted &&
+		echo 5 >e && git add e &&
+		test_tick && git commit -SB7227189 -m "untrusted"
+	)
+'
+
+test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures unsigned 2>pullerror &&
+	test_i18ngrep "does not have a GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with bad signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
+	test_i18ngrep "has a bad GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull signed commit with --verify-signatures' '
+	git pull --verify-signatures signed >pulloutput &&
+	test_i18ngrep "has a good GPG signature" pulloutput &&
+	git checkout initial
+'
+
+test_expect_success GPG 'pull commit with bad signature without verification' '
+	git pull --ff-only bad 2>pullerror &&
+	git checkout initial
+'
+
+test_expect_success GPG 'pull commit with bad signature with --no-verify-signatures' '
+	test_config merge.verifySignatures true &&
+	test_config pull.verifySignatures true &&
+	git pull --ff-only --no-verify-signatures bad 2>pullerror &&
+	git checkout initial
+'
+
+test_done
-- 
2.11.0

[PATCH 3/3] pull: add config option for verifySignatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-09 09:06:16 

Verify the signature of the tip commit when `pull.verifySignatures` is
true.  This option overrides `merge.verifySignatures` on pull, and can
be disabled with the option `--no-verify-signatures`.

Signed-off-by: Hans Jerry Illikainen <redacted>
---
 Documentation/config.txt          |  8 ++++++++
 builtin/pull.c                    | 25 +++++++++++++++++++++++++
 t/t5520-pull.sh                   | 18 ++++++++++++++++++
 t/t5573-pull-verify-signatures.sh | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1598ee70..0cd2bc597 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2596,6 +2596,14 @@ pull.ff::
 	allowed (equivalent to giving the `--ff-only` option from the
 	command line). This setting overrides `merge.ff` when pulling.
 
+pull.verifySignatures::
+	Verify that the tip commit of the side branch being merged is
+	signed with a valid key, i.e. a key that has a valid uid: in the
+	default trust model, this means the signing key has been signed
+	by a trusted key. If the tip commit of the side branch is not
+	signed with a valid key, the merge is aborted. This setting
+	overrides `merge.verifySignatures` when pulling.
+
 pull.rebase::
 	When true, rebase branches on top of the fetched branch, instead
 	of merging the default branch from the default remote when "git
diff --git a/builtin/pull.c b/builtin/pull.c
index 166b777ed..791365915 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -300,6 +300,28 @@ static const char *config_get_ff(void)
 }
 
 /**
+ * If pull.verifySignatures is unset, returns NULL. If pull.verifySignatures is
+ * "true", returns "--verify-signatures". If pull.verifySignatures is "false",
+ * returns "--no-verify-signatures". Otherwise, die with an error.
+ */
+static const char *config_get_verify_signatures(void)
+{
+	const char *value;
+
+	if (git_config_get_value("pull.verifysignatures", &value))
+		return NULL;
+
+	switch (git_parse_maybe_bool(value)) {
+	case 0:
+		return "--no-verify-signatures";
+	case 1:
+		return "--verify-signatures";
+	default:
+		die(_("Invalid value for pull.verifysignatures: %s"), value);
+	}
+}
+
+/**
  * Returns the default configured value for --rebase. It first looks for the
  * value of "branch.$curr_branch.rebase", where $curr_branch is the current
  * branch, and if HEAD is detached or the configuration key does not exist,
@@ -849,6 +871,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (!opt_ff)
 		opt_ff = xstrdup_or_null(config_get_ff());
 
+	if (!opt_verify_signatures)
+		opt_verify_signatures = xstrdup_or_null(config_get_verify_signatures());
+
 	if (opt_rebase < 0)
 		opt_rebase = config_get_rebase();
 
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59c4b778d..cdf1fd213 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -416,6 +416,15 @@ test_expect_success "pull --rebase warns on --verify-signatures" '
 	test_i18ngrep "ignoring --verify-signatures for rebase" err
 '
 
+test_expect_success "pull --rebase warns on pull.verifySignatures=true" '
+	test_config pull.verifySignatures true &&
+	git reset --hard before-rebase &&
+	git pull --rebase . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep "ignoring --verify-signatures for rebase" err
+'
+
 test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
 	git reset --hard before-rebase &&
 	git pull --rebase --no-verify-signatures . copy 2>err &&
@@ -424,6 +433,15 @@ test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
 	test_i18ngrep ! "verify-signatures" err
 '
 
+test_expect_success "pull --rebase does not warn on pull.verifySignatures=false" '
+	test_config pull.verifySignatures false &&
+	git reset --hard before-rebase &&
+	git pull --rebase . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep ! "verify-signatures" err
+'
+
 # add a feature branch, keep-merge, that is merged into master, so the
 # test can try preserving the merge commit (or not) with various
 # --rebase flags/pull.rebase settings.
diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh
index 700247910..d1e8263d9 100755
--- a/t/t5573-pull-verify-signatures.sh
+++ b/t/t5573-pull-verify-signatures.sh
@@ -47,22 +47,54 @@ test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
 	test_i18ngrep "does not have a GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull unsigned commit with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only unsigned 2>pullerror &&
+	test_i18ngrep "does not have a GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull commit with bad signature with --verify-signatures' '
 	test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
 	test_i18ngrep "has a bad GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull commit with bad signature with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only bad 2>pullerror &&
+	test_i18ngrep "has a bad GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' '
 	test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror &&
 	test_i18ngrep "has an untrusted GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull commit with untrusted signature with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with untrusted signature with pull.verifySignatures=true and merge.verifySignatures=false' '
+	test_config merge.verifySignatures false &&
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull signed commit with --verify-signatures' '
 	git pull --verify-signatures signed >pulloutput &&
 	test_i18ngrep "has a good GPG signature" pulloutput &&
 	git checkout initial
 '
 
+test_expect_success GPG 'pull signed commit with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	git pull signed >pulloutput &&
+	test_i18ngrep "has a good GPG signature" pulloutput &&
+	git checkout initial
+'
+
 test_expect_success GPG 'pull commit with bad signature without verification' '
 	git pull --ff-only bad 2>pullerror &&
 	git checkout initial
-- 
2.11.0

Re: [PATCH 1/3] merge: add config option for verifySignatures

From: Kevin Daudt <hidden>
Date: 2017-12-09 12:05:45 

Hello Hans Jerry,

Thank you for your contribution. I have soem remarks below.

On Sat, Dec 09, 2017 at 09:05:28AM +0000, Hans Jerry Illikainen wrote:
Verify the signature of the tip commit when `merge.verifySignatures` is
true.  This can be overridden with `--no-verify-signatures`.

Signed-off-by: Hans Jerry Illikainen <redacted>
I miss the motivation in the commit message. I imagine something like:

    git merge --verify-signatures can be used to verify that the tip
    commit of the branch being merged in is properly signed, but it's
    cumbersome to have to specify that every time.
    
    Add a configuration option that enables this behaviour by default,
    which can be overridden by --no-verify-signatures.
quoted hunk
---
 Documentation/merge-config.txt     |  7 +++++++
 builtin/merge.c                    |  2 ++
 t/t7612-merge-verify-signatures.sh | 43 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index df3ea3779..76571cd3b 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -26,6 +26,13 @@ merge.ff::
 	allowed (equivalent to giving the `--ff-only` option from the
 	command line).
 
+merge.verifySignatures::
+	Verify that the tip commit of the side branch being merged is
+	signed with a valid key, i.e. a key that has a valid uid: in the
+	default trust model, this means the signing key has been signed
+	by a trusted key. If the tip commit of the side branch is not
+	signed with a valid key, the merge is aborted.
+
This is a verbatim copy of the explenation of --verify-signatures. Would
it be an idea to refer to the explenation of merge --verify-signatures?
quoted hunk
 include::fmt-merge-msg-config.txt[]
 
 merge.renameLimit::
diff --git a/builtin/merge.c b/builtin/merge.c
index 612dd7bfb..30264cfd7 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
 		show_diffstat = git_config_bool(k, v);
+	else if (!strcmp(k, "merge.verifysignatures"))
+		verify_signatures = git_config_bool(k, v);
 	else if (!strcmp(k, "pull.twohead"))
 		return git_config_string(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh
index 8ae69a61c..f1a74a683 100755
--- a/t/t7612-merge-verify-signatures.sh
+++ b/t/t7612-merge-verify-signatures.sh
@@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with verification' '
 	test_i18ngrep "does not have a GPG signature" mergeerror
 '
 
+test_expect_success GPG 'merge unsigned commit with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only 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 bad signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only $(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 commit with untrusted signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only 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_i18ngrep "has a good GPG signature" mergeoutput &&
+	git checkout initial
This looks like a clean up step. If so, it's better to add
`test_when_finished 'git checkout initial'` at the beginning to clearly
mark it as a clean up step and make sure it's run even when the test
fails. Same counts for the other occurances.

+'
+
+test_expect_success GPG 'merge signed commit with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	git merge --verbose --ff-only side-signed >mergeoutput &&
+	test_i18ngrep "has a good GPG signature" mergeoutput &&
+	git checkout initial
 '
 
 test_expect_success GPG 'merge commit with bad signature without verification' '
-	git merge $(cat forged.commit)
+	git merge $(cat forged.commit) &&
+	git checkout initial
+'
+
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=false' '
+	test_config merge.verifySignatures false &&
+	git merge $(cat forged.commit) &&
+	git checkout initial
+'
+
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true and --no-verify-signatures' '
+	test_config merge.verifySignatures true &&
+	git merge --no-verify-signatures $(cat forged.commit) &&
+	git checkout initial
 '
 
 test_done
Kevin.

Re: [PATCH 2/3] t: add tests for pull --verify-signatures

From: Kevin Daudt <hidden>
Date: 2017-12-09 12:06:20 

On Sat, Dec 09, 2017 at 09:05:29AM +0000, Hans Jerry Illikainen wrote:
Add tests for `pull --verify-signatures` with untrusted, bad and no
signatures.  Previously the only test for `--verify-signatures` was to
make sure that `pull --rebase --verify-signatures` result in a warning
(t5520-pull.sh).
Nice!

Same thing regarding the `git checkout initial` commands counts
here too.
quoted hunk
Signed-off-by: Hans Jerry Illikainen <redacted>
---
 t/t5573-pull-verify-signatures.sh | 78 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100755 t/t5573-pull-verify-signatures.sh

diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh
new file mode 100755
index 000000000..700247910
--- /dev/null
+++ b/t/t5573-pull-verify-signatures.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='pull signature verification tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success GPG 'create repositories with signed commits' '
+	echo 1 >a && git add a &&
+	test_tick && git commit -m initial &&
+	git tag initial &&
+
+	git clone . signed &&
+	(
+		cd signed &&
+		echo 2 >b && git add b &&
+		test_tick && git commit -S -m "signed"
+	) &&
+
+	git clone . unsigned &&
+	(
+		cd unsigned &&
+		echo 3 >c && git add c &&
+		test_tick && git commit -m "unsigned"
+	) &&
+
+	git clone . bad &&
+	(
+		cd bad &&
+		echo 4 >d && git add d &&
+		test_tick && git commit -S -m "bad" &&
+		git cat-file commit HEAD >raw &&
+		sed -e "s/bad/forged bad/" raw >forged &&
+		git hash-object -w -t commit forged >forged.commit &&
+		git checkout $(cat forged.commit)
+	) &&
+
+	git clone . untrusted &&
+	(
+		cd untrusted &&
+		echo 5 >e && git add e &&
+		test_tick && git commit -SB7227189 -m "untrusted"
+	)
+'
+
+test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures unsigned 2>pullerror &&
+	test_i18ngrep "does not have a GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with bad signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
+	test_i18ngrep "has a bad GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull signed commit with --verify-signatures' '
+	git pull --verify-signatures signed >pulloutput &&
+	test_i18ngrep "has a good GPG signature" pulloutput &&
+	git checkout initial
+'
+
+test_expect_success GPG 'pull commit with bad signature without verification' '
+	git pull --ff-only bad 2>pullerror &&
+	git checkout initial
+'
+
+test_expect_success GPG 'pull commit with bad signature with --no-verify-signatures' '
+	test_config merge.verifySignatures true &&
+	test_config pull.verifySignatures true &&
+	git pull --ff-only --no-verify-signatures bad 2>pullerror &&
+	git checkout initial
+'
+
+test_done
-- 
2.11.0

Re: [PATCH 3/3] pull: add config option for verifySignatures

From: Kevin Daudt <hidden>
Date: 2017-12-09 12:06:30 

On Sat, Dec 09, 2017 at 09:05:30AM +0000, Hans Jerry Illikainen wrote:
Verify the signature of the tip commit when `pull.verifySignatures` is
true.  This option overrides `merge.verifySignatures` on pull, and can
be disabled with the option `--no-verify-signatures`.
Is there a reason why git pull would need a different behaviour from git
merge? Pull itself is just a convenience command for fetch +
merge/rebase.

One precedent for having a separate configuration option for pull
however is 'pull.ff', so there might be a usecase for it.

I guess your commit message could use a motivation on why you want to
set this differently from 'merge.verifySignature'.
quoted hunk
Signed-off-by: Hans Jerry Illikainen <redacted>
---
 Documentation/config.txt          |  8 ++++++++
 builtin/pull.c                    | 25 +++++++++++++++++++++++++
 t/t5520-pull.sh                   | 18 ++++++++++++++++++
 t/t5573-pull-verify-signatures.sh | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1598ee70..0cd2bc597 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2596,6 +2596,14 @@ pull.ff::
 	allowed (equivalent to giving the `--ff-only` option from the
 	command line). This setting overrides `merge.ff` when pulling.
 
+pull.verifySignatures::
+	Verify that the tip commit of the side branch being merged is
+	signed with a valid key, i.e. a key that has a valid uid: in the
+	default trust model, this means the signing key has been signed
+	by a trusted key. If the tip commit of the side branch is not
+	signed with a valid key, the merge is aborted. This setting
+	overrides `merge.verifySignatures` when pulling.
+
 pull.rebase::
 	When true, rebase branches on top of the fetched branch, instead
 	of merging the default branch from the default remote when "git
diff --git a/builtin/pull.c b/builtin/pull.c
index 166b777ed..791365915 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -300,6 +300,28 @@ static const char *config_get_ff(void)
 }
 
 /**
+ * If pull.verifySignatures is unset, returns NULL. If pull.verifySignatures is
+ * "true", returns "--verify-signatures". If pull.verifySignatures is "false",
+ * returns "--no-verify-signatures". Otherwise, die with an error.
+ */
+static const char *config_get_verify_signatures(void)
+{
+	const char *value;
+
+	if (git_config_get_value("pull.verifysignatures", &value))
+		return NULL;
+
+	switch (git_parse_maybe_bool(value)) {
+	case 0:
+		return "--no-verify-signatures";
+	case 1:
+		return "--verify-signatures";
+	default:
+		die(_("Invalid value for pull.verifysignatures: %s"), value);
+	}
+}
+
+/**
  * Returns the default configured value for --rebase. It first looks for the
  * value of "branch.$curr_branch.rebase", where $curr_branch is the current
  * branch, and if HEAD is detached or the configuration key does not exist,
@@ -849,6 +871,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (!opt_ff)
 		opt_ff = xstrdup_or_null(config_get_ff());
 
+	if (!opt_verify_signatures)
+		opt_verify_signatures = xstrdup_or_null(config_get_verify_signatures());
+
 	if (opt_rebase < 0)
 		opt_rebase = config_get_rebase();
 
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59c4b778d..cdf1fd213 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -416,6 +416,15 @@ test_expect_success "pull --rebase warns on --verify-signatures" '
 	test_i18ngrep "ignoring --verify-signatures for rebase" err
 '
 
+test_expect_success "pull --rebase warns on pull.verifySignatures=true" '
+	test_config pull.verifySignatures true &&
+	git reset --hard before-rebase &&
+	git pull --rebase . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep "ignoring --verify-signatures for rebase" err
+'
+
 test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
 	git reset --hard before-rebase &&
 	git pull --rebase --no-verify-signatures . copy 2>err &&
@@ -424,6 +433,15 @@ test_expect_success "pull --rebase does not warn on --no-verify-signatures" '
 	test_i18ngrep ! "verify-signatures" err
 '
 
+test_expect_success "pull --rebase does not warn on pull.verifySignatures=false" '
+	test_config pull.verifySignatures false &&
+	git reset --hard before-rebase &&
+	git pull --rebase . copy 2>err &&
+	test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" &&
+	test new = "$(git show HEAD:file2)" &&
+	test_i18ngrep ! "verify-signatures" err
+'
+
 # add a feature branch, keep-merge, that is merged into master, so the
 # test can try preserving the merge commit (or not) with various
 # --rebase flags/pull.rebase settings.
diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh
index 700247910..d1e8263d9 100755
--- a/t/t5573-pull-verify-signatures.sh
+++ b/t/t5573-pull-verify-signatures.sh
@@ -47,22 +47,54 @@ test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
 	test_i18ngrep "does not have a GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull unsigned commit with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only unsigned 2>pullerror &&
+	test_i18ngrep "does not have a GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull commit with bad signature with --verify-signatures' '
 	test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
 	test_i18ngrep "has a bad GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull commit with bad signature with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only bad 2>pullerror &&
+	test_i18ngrep "has a bad GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' '
 	test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror &&
 	test_i18ngrep "has an untrusted GPG signature" pullerror
 '
 
+test_expect_success GPG 'pull commit with untrusted signature with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with untrusted signature with pull.verifySignatures=true and merge.verifySignatures=false' '
+	test_config merge.verifySignatures false &&
+	test_config pull.verifySignatures true &&
+	test_must_fail git pull --ff-only untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
 test_expect_success GPG 'pull signed commit with --verify-signatures' '
 	git pull --verify-signatures signed >pulloutput &&
 	test_i18ngrep "has a good GPG signature" pulloutput &&
 	git checkout initial
 '
 
+test_expect_success GPG 'pull signed commit with pull.verifySignatures=true' '
+	test_config pull.verifySignatures true &&
+	git pull signed >pulloutput &&
+	test_i18ngrep "has a good GPG signature" pulloutput &&
+	git checkout initial
+'
+
 test_expect_success GPG 'pull commit with bad signature without verification' '
 	git pull --ff-only bad 2>pullerror &&
 	git checkout initial
-- 
2.11.0

Re: [PATCH 3/3] pull: add config option for verifySignatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-10 06:53:41 

On Sat, Dec 09, 2017 at 01:06:23PM +0100, Kevin Daudt wrote:
On Sat, Dec 09, 2017 at 09:05:30AM +0000, Hans Jerry Illikainen wrote:
quoted
Verify the signature of the tip commit when `pull.verifySignatures` is
true.  This option overrides `merge.verifySignatures` on pull, and can
be disabled with the option `--no-verify-signatures`.
Is there a reason why git pull would need a different behaviour from git
merge? Pull itself is just a convenience command for fetch +
merge/rebase.

One precedent for having a separate configuration option for pull
however is 'pull.ff', so there might be a usecase for it.

I guess your commit message could use a motivation on why you want to
set this differently from 'merge.verifySignature'.
Thanks for the review!  I wasn't sure whether pull.verifySignatures made
sense -- I included it to be consistent with pull.ff/merge.ff, but it's
scrapped in v2.

-- 
hji

[PATCH v2 2/2] t: add tests for pull --verify-signatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-10 06:53:44 

Add tests for pull --verify-signatures with untrusted, bad and no
signatures.  Previously the only test for --verify-signatures was to
make sure that pull --rebase --verify-signatures result in a warning
(t5520-pull.sh).

Signed-off-by: Hans Jerry Illikainen <redacted>
---
 t/t5573-pull-verify-signatures.sh | 78 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100755 t/t5573-pull-verify-signatures.sh

diff --git a/t/t5573-pull-verify-signatures.sh b/t/t5573-pull-verify-signatures.sh
new file mode 100755
index 000000000..8ae331f40
--- /dev/null
+++ b/t/t5573-pull-verify-signatures.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='pull signature verification tests'
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-gpg.sh"
+
+test_expect_success GPG 'create repositories with signed commits' '
+	echo 1 >a && git add a &&
+	test_tick && git commit -m initial &&
+	git tag initial &&
+
+	git clone . signed &&
+	(
+		cd signed &&
+		echo 2 >b && git add b &&
+		test_tick && git commit -S -m "signed"
+	) &&
+
+	git clone . unsigned &&
+	(
+		cd unsigned &&
+		echo 3 >c && git add c &&
+		test_tick && git commit -m "unsigned"
+	) &&
+
+	git clone . bad &&
+	(
+		cd bad &&
+		echo 4 >d && git add d &&
+		test_tick && git commit -S -m "bad" &&
+		git cat-file commit HEAD >raw &&
+		sed -e "s/bad/forged bad/" raw >forged &&
+		git hash-object -w -t commit forged >forged.commit &&
+		git checkout $(cat forged.commit)
+	) &&
+
+	git clone . untrusted &&
+	(
+		cd untrusted &&
+		echo 5 >e && git add e &&
+		test_tick && git commit -SB7227189 -m "untrusted"
+	)
+'
+
+test_expect_success GPG 'pull unsigned commit with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures unsigned 2>pullerror &&
+	test_i18ngrep "does not have a GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with bad signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures bad 2>pullerror &&
+	test_i18ngrep "has a bad GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull commit with untrusted signature with --verify-signatures' '
+	test_must_fail git pull --ff-only --verify-signatures untrusted 2>pullerror &&
+	test_i18ngrep "has an untrusted GPG signature" pullerror
+'
+
+test_expect_success GPG 'pull signed commit with --verify-signatures' '
+	test_when_finished "git checkout initial" &&
+	git pull --verify-signatures signed >pulloutput &&
+	test_i18ngrep "has a good GPG signature" pulloutput
+'
+
+test_expect_success GPG 'pull commit with bad signature without verification' '
+	test_when_finished "git checkout initial" &&
+	git pull --ff-only bad 2>pullerror
+'
+
+test_expect_success GPG 'pull commit with bad signature with --no-verify-signatures' '
+	test_when_finished "git checkout initial" &&
+	test_config merge.verifySignatures true &&
+	test_config pull.verifySignatures true &&
+	git pull --ff-only --no-verify-signatures bad 2>pullerror
+'
+
+test_done
-- 
2.15.1.356.g13e4cf275

[PATCH v2 1/2] merge: add config option for verifySignatures

From: Hans Jerry Illikainen <hidden>
Date: 2017-12-10 06:53:52 

git merge --verify-signatures can be used to verify that the tip commit
of the branch being merged in is properly signed, but it's cumbersome to
have to specify that every time.

Add a configuration option that enables this behaviour by default, which
can be overridden by --no-verify-signatures.

Signed-off-by: Hans Jerry Illikainen <redacted>
---
 Documentation/merge-config.txt     |  4 ++++
 builtin/merge.c                    |  2 ++
 t/t7612-merge-verify-signatures.sh | 39 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index df3ea3779..12b6bbf59 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -26,6 +26,10 @@ merge.ff::
 	allowed (equivalent to giving the `--ff-only` option from the
 	command line).
 
+merge.verifySignatures::
+	If true, this is equivalent to the --verify-signatures command
+	line option. See linkgit:git-merge[1] for details.
+
 include::fmt-merge-msg-config.txt[]
 
 merge.renameLimit::
diff --git a/builtin/merge.c b/builtin/merge.c
index 612dd7bfb..30264cfd7 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -567,6 +567,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 
 	if (!strcmp(k, "merge.diffstat") || !strcmp(k, "merge.stat"))
 		show_diffstat = git_config_bool(k, v);
+	else if (!strcmp(k, "merge.verifysignatures"))
+		verify_signatures = git_config_bool(k, v);
 	else if (!strcmp(k, "pull.twohead"))
 		return git_config_string(&pull_twohead, k, v);
 	else if (!strcmp(k, "pull.octopus"))
diff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh
index 8ae69a61c..2344995a1 100755
--- a/t/t7612-merge-verify-signatures.sh
+++ b/t/t7612-merge-verify-signatures.sh
@@ -39,23 +39,62 @@ test_expect_success GPG 'merge unsigned commit with verification' '
 	test_i18ngrep "does not have a GPG signature" mergeerror
 '
 
+test_expect_success GPG 'merge unsigned commit with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only 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 bad signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only $(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 commit with untrusted signature with merge.verifySignatures=true' '
+	test_config merge.verifySignatures true &&
+	test_must_fail git merge --ff-only side-untrusted 2>mergeerror &&
+	test_i18ngrep "has an untrusted GPG signature" mergeerror
+'
+
 test_expect_success GPG 'merge signed commit with verification' '
+	test_when_finished "git checkout initial" &&
 	git merge --verbose --ff-only --verify-signatures side-signed >mergeoutput &&
 	test_i18ngrep "has a good GPG signature" mergeoutput
 '
 
+test_expect_success GPG 'merge signed commit with merge.verifySignatures=true' '
+	test_when_finished "git checkout initial" &&
+	test_config merge.verifySignatures true &&
+	git merge --verbose --ff-only side-signed >mergeoutput &&
+	test_i18ngrep "has a good GPG signature" mergeoutput
+'
+
 test_expect_success GPG 'merge commit with bad signature without verification' '
+	test_when_finished "git checkout initial" &&
+	git merge $(cat forged.commit)
+'
+
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=false' '
+	test_when_finished "git checkout initial" &&
+	test_config merge.verifySignatures false &&
 	git merge $(cat forged.commit)
 '
 
+test_expect_success GPG 'merge commit with bad signature with merge.verifySignatures=true and --no-verify-signatures' '
+	test_when_finished "git checkout initial" &&
+	test_config merge.verifySignatures true &&
+	git merge --no-verify-signatures $(cat forged.commit)
+'
+
 test_done
-- 
2.15.1.356.g13e4cf275
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help