{"thread":{"id":"56689","subject":"[PATCH 0/2] ssh signing: fix merging signed tags & docs","startedAt":"2021-10-12T09:22:57Z","lastAt":"2021-10-12T17:36:20Z","messageCount":5,"participants":["Fabian Stelzer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"438523","messageId":"20211012092236.619822-1-fs@gigacodes.de","threadId":"56689","inReplyTo":null,"subject":"[PATCH 0/2] ssh signing: fix merging signed tags & docs","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-12T09:22:34Z","receivedAt":"2021-10-12T09:22:57Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"Two small follow up patches on top of 1bfb57f642d. fmt-merge-msg needs\nto load gpg config to be able to verify merged tags. load it and add\nsome tests. And i forgot to adjust the docs when we changed some\nbehaviour of the original patch during review.\n\nSince this is my first incremental patch onto a next topic via send-email\n(not sure if can do this really via gitgitgadget) i apologize for any\nprocess mistakes in advance.\n\nFabian Stelzer (2):\n  ssh signing: fmt-merge-msg tests & config parse\n  fixup! ssh signing: verify signatures using ssh-keygen\n\n Documentation/config/gpg.txt |  4 +---\n fmt-merge-msg.c              |  6 ++++++\n t/t6200-fmt-merge-msg.sh     | 28 ++++++++++++++++++++++++++++\n 3 files changed, 35 insertions(+), 3 deletions(-)\n\n-- \n2.31.1\n\n"},{"id":"438524","messageId":"20211012092236.619822-2-fs@gigacodes.de","threadId":"56689","inReplyTo":"20211012092236.619822-1-fs@gigacodes.de","subject":"[PATCH 1/2] ssh signing: fmt-merge-msg tests & config parse","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-12T09:22:35Z","receivedAt":"2021-10-12T09:22:58Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"When merging a signed tag fmt-merge-msg was unable to verify its\nvalidity missing the necessary ssh allowedSignersFile config.\n\nAdds gpg config parsing to fmt-merge-msg.\nAdds tests for ssh signed tags to fmt-merge-msg tests.\n\nSigned-off-by: Fabian Stelzer <fs@gigacodes.de>\n---\n fmt-merge-msg.c          |  6 ++++++\n t/t6200-fmt-merge-msg.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex fb300bb4b6..2d49584bf5 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -9,6 +9,7 @@\n #include \"branch.h\"\n #include \"fmt-merge-msg.h\"\n #include \"commit-reach.h\"\n+#include \"gpg-interface.h\"\n \n static int use_branch_desc;\n static int suppress_dest_pattern_seen;\n@@ -16,6 +17,8 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n \n int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n {\n+\tint status = 0;\n+\n \tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n \t\tint is_bool;\n \t\tmerge_log_config = git_config_bool_or_int(key, value, &is_bool);\n@@ -34,6 +37,9 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)\n \t\t\tstring_list_append(&suppress_dest_patterns, value);\n \t\tsuppress_dest_pattern_seen = 1;\n \t} else {\n+\t\tstatus = git_gpg_config(key, value, NULL);\n+\t\tif (status)\n+\t\t\treturn status;\n \t\treturn git_default_config(key, value, cb);\n \t}\n \treturn 0;\ndiff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh\nindex 44f55d93fe..06c5fb5615 100755\n--- a/t/t6200-fmt-merge-msg.sh\n+++ b/t/t6200-fmt-merge-msg.sh\n@@ -81,6 +81,16 @@ test_expect_success GPG 'set up a signed tag' '\n \tgit tag -s -m signed-tag-msg signed-good-tag left\n '\n \n+test_expect_success GPGSSH 'created ssh signed commit and tag' '\n+\ttest_config gpg.format ssh &&\n+\tgit checkout -b signed-ssh &&\n+\ttouch file &&\n+\tgit add file &&\n+\tgit commit -m \"ssh signed\" -S\"${GPGSSH_KEY_PRIMARY}\" &&\n+\tgit tag -s -u\"${GPGSSH_KEY_PRIMARY}\" -m signed-ssh-tag-msg signed-good-ssh-tag left &&\n+\tgit tag -s -u\"${GPGSSH_KEY_UNTRUSTED}\" -m signed-ssh-tag-msg-untrusted signed-untrusted-ssh-tag left\n+'\n+\n test_expect_success 'message for merging local branch' '\n \techo \"Merge branch ${apos}left${apos}\" >expected &&\n \n@@ -109,6 +119,24 @@ test_expect_success GPG 'message for merging local tag signed by unknown key' '\n \tgrep -E \"^# gpg: Can${apos}t check signature: (public key not found|No public key)\" actual\n '\n \n+test_expect_success GPGSSH 'message for merging local tag signed by good ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . signed-good-ssh-tag &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_TRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual\n+'\n+\n+test_expect_success GPGSSH 'message for merging local tag signed by unknown ssh key' '\n+\ttest_config gpg.ssh.allowedSignersFile \"${GPGSSH_ALLOWED_SIGNERS}\" &&\n+\tgit checkout main &&\n+\tgit fetch . signed-untrusted-ssh-tag &&\n+\tgit fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&\n+\tgrep \"${GPGSSH_GOOD_SIGNATURE_UNTRUSTED}\" actual &&\n+\t! grep \"${GPGSSH_BAD_SIGNATURE}\" actual &&\n+\tgrep \"${GPGSSH_KEY_NOT_TRUSTED}\" actual\n+'\n test_expect_success 'message for merging external branch' '\n \techo \"Merge branch ${apos}left${apos} of $(pwd)\" >expected &&\n \n-- \n2.31.1\n\n"},{"id":"438525","messageId":"20211012092236.619822-3-fs@gigacodes.de","threadId":"56689","inReplyTo":"20211012092236.619822-1-fs@gigacodes.de","subject":"[PATCH 2/2] fixup! ssh signing: verify signatures using ssh-keygen","fromName":"Fabian Stelzer","fromEmail":"fs@gigacodes.de","sentAt":"2021-10-12T09:22:36Z","receivedAt":"2021-10-12T09:23:03Z","isPatch":true,"sender":{"key":"fs@gigacodes.de","avatar":"https://avatars.githubusercontent.com/u/564858?v=4"},"body":"This behaviour changed during patch review and documentation no longer\nmatched it.\n---\n Documentation/config/gpg.txt | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt\nindex 51a756b2f1..4f30c7dbdd 100644\n--- a/Documentation/config/gpg.txt\n+++ b/Documentation/config/gpg.txt\n@@ -52,9 +52,7 @@ gpg.ssh.allowedSignersFile::\n SSH has no concept of trust levels like gpg does. To be able to differentiate\n between valid signatures and trusted signatures the trust level of a signature\n verification is set to `fully` when the public key is present in the allowedSignersFile.\n-Therefore to only mark fully trusted keys as verified set gpg.minTrustLevel to `fully`.\n-Otherwise valid but untrusted signatures will still verify but show no principal\n-name of the signer.\n+Otherwise the trust level is `undefined` and git verify-commit/tag will fail.\n +\n This file can be set to a location outside of the repository and every developer\n maintains their own trust store. A central repository server could generate this\n-- \n2.31.1\n\n"},{"id":"438573","messageId":"xmqqv922405g.fsf@gitster.g","threadId":"56689","inReplyTo":"20211012092236.619822-1-fs@gigacodes.de","subject":"Re: [PATCH 0/2] ssh signing: fix merging signed tags & docs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-12T17:34:51Z","receivedAt":"2021-10-12T17:34:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer <fs@gigacodes.de> writes:\n\n> Two small follow up patches on top of 1bfb57f642d. fmt-merge-msg needs\n> to load gpg config to be able to verify merged tags. load it and add\n> some tests. And i forgot to adjust the docs when we changed some\n> behaviour of the original patch during review.\n\nSigh, why does this series have to come _after_ the problematic\ncommit that needs fixup! have finally hit 'next', after lingering in\n'seen' for very long time, instead of much earlier?\n\nIn any case, the damage has been made.  We don't revert a topic out\nof 'next' very lightly, so let's have the 'fixup!' one as a true\nincremental.  We pretend as if we have already shipped what is in\n'next' as part of a stable version to customers, and write a bugfix\npatch that says \"Commit X made Y to do Z, which is wrong for such\nand such reasons.  Make Y do W instead.\"\n\nThanks.\n"},{"id":"438575","messageId":"xmqqr1cq4036.fsf@gitster.g","threadId":"56689","inReplyTo":"20211012092236.619822-3-fs@gigacodes.de","subject":"Re: [PATCH 2/2] fixup! ssh signing: verify signatures using ssh-keygen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-12T17:36:13Z","receivedAt":"2021-10-12T17:36:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Stelzer <fs@gigacodes.de> writes:\n\n> This behaviour changed during patch review and documentation no longer\n> matched it.\n> ---\n\nThanks, you'd need to sign-off, to make this a standalone bugfix\npatch.\n\n>  Documentation/config/gpg.txt | 4 +---\n>  1 file changed, 1 insertion(+), 3 deletions(-)\n>\n> diff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt\n> index 51a756b2f1..4f30c7dbdd 100644\n> --- a/Documentation/config/gpg.txt\n> +++ b/Documentation/config/gpg.txt\n> @@ -52,9 +52,7 @@ gpg.ssh.allowedSignersFile::\n>  SSH has no concept of trust levels like gpg does. To be able to differentiate\n>  between valid signatures and trusted signatures the trust level of a signature\n>  verification is set to `fully` when the public key is present in the allowedSignersFile.\n> -Therefore to only mark fully trusted keys as verified set gpg.minTrustLevel to `fully`.\n> -Otherwise valid but untrusted signatures will still verify but show no principal\n> -name of the signer.\n> +Otherwise the trust level is `undefined` and git verify-commit/tag will fail.\n>  +\n>  This file can be set to a location outside of the repository and every developer\n>  maintains their own trust store. A central repository server could generate this\n"}]}