Merge branch 'fs/ssh-signing-fix'

gitster · gitster · commit ef1639145de7 · 2021-10-25T16:06:58.000-07:00
Fix-up for the other topic already in 'next'.

* fs/ssh-signing-fix:
  gpg-interface: fix leak of strbufs in get_ssh_key_fingerprint()
  gpg-interface: fix leak of "line" in parse_ssh_output()
  ssh signing: clarify trustlevel usage in docs
  ssh signing: fmt-merge-msg tests &amp; config parse
diff --git 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
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
@@ -9,13 +9,16 @@
 #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;
 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/gpg-interface.c b/gpg-interface.c
@@ -365,6 +365,7 @@ static int verify_gpg_signed_buffer(struct signature_check *sigc,
 static void parse_ssh_output(struct signature_check *sigc)
 {
 	const char *line, *principal, *search;
+	char *to_free;
 	char *key = NULL;
 
 	/*
@@ -383,7 +384,7 @@ static void parse_ssh_output(struct signature_check *sigc)
 	sigc->result = 'B';
 	sigc->trust_level = TRUST_NEVER;
 
-	line = xmemdupz(sigc->output, strcspn(sigc->output, "\n"));
+	line = to_free = xmemdupz(sigc->output, strcspn(sigc->output, "\n"));
 
 	if (skip_prefix(line, "Good \"git\" signature for ", &line)) {
 		/* Valid signature and known principal */
@@ -403,7 +404,7 @@ static void parse_ssh_output(struct signature_check *sigc)
 		sigc->result = 'G';
 		sigc->trust_level = TRUST_UNDEFINED;
 	} else {
-		return;
+		goto cleanup;
 	}
 
 	key = strstr(line, "key");
@@ -417,6 +418,9 @@ static void parse_ssh_output(struct signature_check *sigc)
 		 */
 		sigc->result = 'B';
 	}
+
+cleanup:
+	free(to_free);
 }
 
 static int verify_ssh_signed_buffer(struct signature_check *sigc,
@@ -707,6 +711,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
 	int ret = -1;
 	struct strbuf fingerprint_stdout = STRBUF_INIT;
 	struct strbuf **fingerprint;
+	char *fingerprint_ret;
 
 	/*
 	 * With SSH Signing this can contain a filename or a public key
@@ -733,7 +738,10 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
 		die_errno(_("failed to get the ssh fingerprint for key '%s'"),
 			  signing_key);
 
-	return strbuf_detach(fingerprint[1], NULL);
+	fingerprint_ret = strbuf_detach(fingerprint[1], NULL);
+	strbuf_list_free(fingerprint);
+	strbuf_release(&fingerprint_stdout);
+	return fingerprint_ret;
 }
 
 /* Returns the first public key from an ssh-agent to use for signing */
diff --git 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 &&
 
