git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 0/2] ssh signing: fix merging signed tags & docs
@ 2021-10-13  7:51 Fabian Stelzer
  2021-10-13  7:51 ` [PATCH v2 1/2] ssh signing: fmt-merge-msg tests & config parse Fabian Stelzer
  2021-10-13  7:51 ` [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs Fabian Stelzer
  0 siblings, 2 replies; 4+ messages in thread
From: Fabian Stelzer @ 2021-10-13  7:51 UTC (permalink / raw)
  To: git; +Cc: Fabian Stelzer

Two small follow up patches on top of 1bfb57f642d. fmt-merge-msg needs
to load gpg config to be able to verify merged tags. load it and add
some tests. And i forgot to adjust the docs when we changed some
behaviour of the original patch during review.

Sorry for sending this after the merge to next. I didn't want to send a
full reroll of the big patch series for these small changes. Should i
do that next time or can/should i send these to the list as single
patches on top of my existing patch series? I used gitgitgadget for the
series and i'm not sure how sth like this would work with it (if it does
at all).

Fabian Stelzer (2):
  ssh signing: fmt-merge-msg tests & config parse
  ssh signing: clarify trustlevel usage in docs

 Documentation/config/gpg.txt |  4 +---
 fmt-merge-msg.c              |  6 ++++++
 t/t6200-fmt-merge-msg.sh     | 28 ++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 1/2] ssh signing: fmt-merge-msg tests & config parse
  2021-10-13  7:51 [PATCH v2 0/2] ssh signing: fix merging signed tags & docs Fabian Stelzer
@ 2021-10-13  7:51 ` Fabian Stelzer
  2021-10-13  7:51 ` [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs Fabian Stelzer
  1 sibling, 0 replies; 4+ messages in thread
From: Fabian Stelzer @ 2021-10-13  7:51 UTC (permalink / raw)
  To: git; +Cc: Fabian Stelzer

When merging a signed tag fmt-merge-msg was unable to verify its
validity missing the necessary ssh allowedSignersFile config.

Adds gpg config parsing to fmt-merge-msg.
Adds tests for ssh signed tags to fmt-merge-msg tests.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
 fmt-merge-msg.c          |  6 ++++++
 t/t6200-fmt-merge-msg.sh | 28 ++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index fb300bb4b6..2d49584bf5 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -9,6 +9,7 @@
 #include "branch.h"
 #include "fmt-merge-msg.h"
 #include "commit-reach.h"
+#include "gpg-interface.h"
 
 static int use_branch_desc;
 static int suppress_dest_pattern_seen;
@@ -16,6 +17,8 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
 
 int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 {
+	int status = 0;
+
 	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
 		int is_bool;
 		merge_log_config = git_config_bool_or_int(key, value, &is_bool);
@@ -34,6 +37,9 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 			string_list_append(&suppress_dest_patterns, value);
 		suppress_dest_pattern_seen = 1;
 	} else {
+		status = git_gpg_config(key, value, NULL);
+		if (status)
+			return status;
 		return git_default_config(key, value, cb);
 	}
 	return 0;
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 44f55d93fe..06c5fb5615 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -81,6 +81,16 @@ test_expect_success GPG 'set up a signed tag' '
 	git tag -s -m signed-tag-msg signed-good-tag left
 '
 
+test_expect_success GPGSSH 'created ssh signed commit and tag' '
+	test_config gpg.format ssh &&
+	git checkout -b signed-ssh &&
+	touch file &&
+	git add file &&
+	git commit -m "ssh signed" -S"${GPGSSH_KEY_PRIMARY}" &&
+	git tag -s -u"${GPGSSH_KEY_PRIMARY}" -m signed-ssh-tag-msg signed-good-ssh-tag left &&
+	git tag -s -u"${GPGSSH_KEY_UNTRUSTED}" -m signed-ssh-tag-msg-untrusted signed-untrusted-ssh-tag left
+'
+
 test_expect_success 'message for merging local branch' '
 	echo "Merge branch ${apos}left${apos}" >expected &&
 
@@ -109,6 +119,24 @@ test_expect_success GPG 'message for merging local tag signed by unknown key' '
 	grep -E "^# gpg: Can${apos}t check signature: (public key not found|No public key)" actual
 '
 
+test_expect_success GPGSSH 'message for merging local tag signed by good ssh key' '
+	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	git checkout main &&
+	git fetch . signed-good-ssh-tag &&
+	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+	grep "${GPGSSH_GOOD_SIGNATURE_TRUSTED}" actual &&
+	! grep "${GPGSSH_BAD_SIGNATURE}" actual
+'
+
+test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh key' '
+	test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	git checkout main &&
+	git fetch . signed-untrusted-ssh-tag &&
+	git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
+	grep "${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}" actual &&
+	! grep "${GPGSSH_BAD_SIGNATURE}" actual &&
+	grep "${GPGSSH_KEY_NOT_TRUSTED}" actual
+'
 test_expect_success 'message for merging external branch' '
 	echo "Merge branch ${apos}left${apos} of $(pwd)" >expected &&
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs
  2021-10-13  7:51 [PATCH v2 0/2] ssh signing: fix merging signed tags & docs Fabian Stelzer
  2021-10-13  7:51 ` [PATCH v2 1/2] ssh signing: fmt-merge-msg tests & config parse Fabian Stelzer
@ 2021-10-13  7:51 ` Fabian Stelzer
  2021-10-13 17:03   ` Junio C Hamano
  1 sibling, 1 reply; 4+ messages in thread
From: Fabian Stelzer @ 2021-10-13  7:51 UTC (permalink / raw)
  To: git; +Cc: Fabian Stelzer

facca53ac added verification for ssh signatures but incorrectly
described the usage of gpg.minTrustLevel. While the verifications
trustlevel is stil set to fully or undefined depending on if the key is
known or not it has no effect on the verification result. Unknown keys
will always fail verification. This commit updates the docs to match
this behaviour.

Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
---
 Documentation/config/gpg.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt
index 51a756b2f1..4f30c7dbdd 100644
--- a/Documentation/config/gpg.txt
+++ b/Documentation/config/gpg.txt
@@ -52,9 +52,7 @@ gpg.ssh.allowedSignersFile::
 SSH has no concept of trust levels like gpg does. To be able to differentiate
 between valid signatures and trusted signatures the trust level of a signature
 verification is set to `fully` when the public key is present in the allowedSignersFile.
-Therefore to only mark fully trusted keys as verified set gpg.minTrustLevel to `fully`.
-Otherwise valid but untrusted signatures will still verify but show no principal
-name of the signer.
+Otherwise the trust level is `undefined` and git verify-commit/tag will fail.
 +
 This file can be set to a location outside of the repository and every developer
 maintains their own trust store. A central repository server could generate this
-- 
2.31.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs
  2021-10-13  7:51 ` [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs Fabian Stelzer
@ 2021-10-13 17:03   ` Junio C Hamano
  0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2021-10-13 17:03 UTC (permalink / raw)
  To: Fabian Stelzer; +Cc: git

Fabian Stelzer <fs@gigacodes.de> writes:

>  SSH has no concept of trust levels like gpg does. To be able to differentiate
>  between valid signatures and trusted signatures the trust level of a signature
>  verification is set to `fully` when the public key is present in the allowedSignersFile.
> -Therefore to only mark fully trusted keys as verified set gpg.minTrustLevel to `fully`.
> -Otherwise valid but untrusted signatures will still verify but show no principal
> -name of the signer.
> +Otherwise the trust level is `undefined` and git verify-commit/tag will fail.
>  +
>  This file can be set to a location outside of the repository and every developer
>  maintains their own trust store. A central repository server could generate this

Perfect.  Thanks.

Will queue.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-10-13 17:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13  7:51 [PATCH v2 0/2] ssh signing: fix merging signed tags & docs Fabian Stelzer
2021-10-13  7:51 ` [PATCH v2 1/2] ssh signing: fmt-merge-msg tests & config parse Fabian Stelzer
2021-10-13  7:51 ` [PATCH v2 2/2] ssh signing: clarify trustlevel usage in docs Fabian Stelzer
2021-10-13 17:03   ` Junio C Hamano

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).