git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/7] gpg: centralize signature check

From
brian m. carlson <sandals@crustytoothpaste.net>
Date
Jun 21, 2015, 23:14 UTC
Message-ID
<1434928483-105916-5-git-send-email-sandals@crustytoothpaste.net>
In-Reply-To
<1434928483-105916-1-git-send-email-sandals@crustytoothpaste.net>

verify-commit and verify-tag both share a central codepath for verifying commits: check_signature. However, verify-tag exited successfully for untrusted signature, while verify-commit exited unsuccessfully. Centralize this signature check and make verify-commit adopt the older verify-tag behavior. This behavior is more logical anyway, as the signature is in fact valid, whether or not there's a path of trust to the author.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/verify-commit.c  | 5 +++--
 builtin/verify-tag.c     | 5 +++--
 commit.c                 | 8 ++++++--
 commit.h                 | 2 +-
 gpg-interface.c          | 4 +++-
 gpg-interface.h          | 2 +-
 t/t7510-signed-commit.sh | 2 +-
 7 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index ec0c4e3..e30f7cf 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -21,10 +21,11 @@ static const char * const verify_commit_usage[] = {
 static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned long size, int verbose)
 {
 	struct signature_check signature_check;
+	int ret;
 
 	memset(&signature_check, 0, sizeof(signature_check));
 
-	check_commit_signature(lookup_commit(sha1), &signature_check);
+	ret = check_commit_signature(lookup_commit(sha1), &signature_check);
 
 	if (verbose && signature_check.payload)
 		fputs(signature_check.payload, stdout);
@@ -33,7 +34,7 @@ static int run_gpg_verify(const unsigned char *sha1, const char *buf, unsigned l
 		fputs(signature_check.gpg_output, stderr);
 
 	signature_check_clear(&signature_check);
-	return signature_check.result != 'G';
+	return ret;
 }
 
 static int verify_commit(const char *name, int verbose)
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index e1eb341..8750bef 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -22,6 +22,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 {
 	struct signature_check sigc;
 	int len;
+	int ret;
 
 	memset(&sigc, 0, sizeof(sigc));
 
@@ -32,11 +33,11 @@ static int run_gpg_verify(const char *buf, unsigned long size, int verbose)
 	if (size == len)
 		return error("no signature found");
 
-	check_signature(buf, len, buf + len, size - len, &sigc);
+	ret = check_signature(buf, len, buf + len, size - len, &sigc);
 	fputs(sigc.gpg_output, stderr);
 
 	signature_check_clear(&sigc);
-	return sigc.result != 'G' && sigc.result != 'U';
+	return ret;
 }
 
 static int verify_tag(const char *name, int verbose)
diff --git a/commit.c b/commit.c
index 1e2e144..62223a3 100644
--- a/commit.c
+++ b/commit.c
@@ -1232,20 +1232,24 @@ free_return:
 	free(buf);
 }
 
-void check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
 {
 	struct strbuf payload = STRBUF_INIT;
 	struct strbuf signature = STRBUF_INIT;
+	int ret = 1;
 
 	sigc->result = 'N';
 
 	if (parse_signed_commit(commit, &payload, &signature) <= 0)
 		goto out;
-	check_signature(payload.buf, payload.len, signature.buf, signature.len, sigc);
+	ret = check_signature(payload.buf, payload.len, signature.buf,
+		signature.len, sigc);
 
  out:
 	strbuf_release(&payload);
 	strbuf_release(&signature);
+
+	return ret;
 }
 
 
diff --git a/commit.h b/commit.h
index 9a1fa96..4983bdd 100644
--- a/commit.h
+++ b/commit.h
@@ -379,7 +379,7 @@ extern void print_commit_list(struct commit_list *list,
  * 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);
+extern int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
 
 int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
 
diff --git a/gpg-interface.c b/gpg-interface.c
index 66dbee2..77a4da6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -60,7 +60,7 @@ void parse_gpg_output(struct signature_check *sigc)
 	}
 }
 
-void check_signature(const char *payload, size_t plen, const char *signature,
+int check_signature(const char *payload, size_t plen, const char *signature,
 	size_t slen, struct signature_check *sigc)
 {
 	struct strbuf gpg_output = STRBUF_INIT;
@@ -81,6 +81,8 @@ void check_signature(const char *payload, size_t plen, const char *signature,
  out:
 	strbuf_release(&gpg_status);
 	strbuf_release(&gpg_output);
+
+	return sigc->result != 'G' && sigc->result != 'U';
 }
 
 /*
diff --git a/gpg-interface.h b/gpg-interface.h
index 043bcaa..e2aabde 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -27,7 +27,7 @@ extern int verify_signed_buffer(const char *payload, size_t payload_size, const
 extern int git_gpg_config(const char *, const char *, void *);
 extern void set_signing_key(const char *);
 extern const char *get_signing_key(void);
-extern void check_signature(const char *payload, size_t plen,
+extern int check_signature(const char *payload, size_t plen,
 	const char *signature, size_t slen, struct signature_check *sigc);
 
 #endif
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 4fc376e..796138f 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -81,7 +81,7 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
-test_expect_failure GPG 'verify-commit exits success on untrusted signature' '
+test_expect_success GPG 'verify-commit exits success on untrusted signature' '
 	git verify-commit eighth-signed-alt 2>actual &&
 	grep "Good signature from" actual &&
 	! grep "BAD signature from" actual &&

-- 
2.4.0
Previous: brian m. carlsonNext: brian m. carlson
Message 5 of 9 in “Raw gpg output support for verify-commit and verify-tag”
  1. 0/7 Raw gpg output support for verify-commit and verify-tagbrian m. carlson, Jun 21, 2015
  2. 1/7 verify-tag: add testsbrian m. carlson, Jun 21, 2015
  3. 2/7 verify-tag: share code with verify-commitbrian m. carlson, Jun 21, 2015
  4. 3/7 verify-commit: add test for exit status on untrusted signaturebrian m. carlson, Jun 21, 2015
  5. 4/7 gpg: centralize signature checkbrian m. carlson, Jun 21, 2015
  6. 5/7 gpg: centralize printing signature buffersbrian m. carlson, Jun 21, 2015
  7. 6/7 verify-commit: add option to print raw gpg status informationbrian m. carlson, Jun 21, 2015
  8. 7/7 verify-tag: add option to print raw gpg status informationbrian m. carlson, Jun 21, 2015
  9. Junio C HamanoJun 22, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.