Previously, signature verification for commits had a number of helper
functions with slightly different behaviors:
- builtin/verify-commit.c had a file-local helper, so it wasn't reusable
outside of the verify-commit builtin.
- commit.c had verify_merge_signature() that die()d on errors, making it
difficult to reuse in parts of Git that mustn't (immediately) die on
failure.
- commit.c also had check_commit_signature(). It's flexible enough to
be used anywhere, but it isn't as nice as gpg_verify_tag(). More
specifically, it doesn't take care of printing the result; nor does it
accept and parse arbitrary object IDs.
This commit changes check_commit_signature() to file-local scope. It
also introduces a gpg_verify_commit() function modelled after
gpg_verify_tag(). It is written with the intent of removing the need to
implement local helpers for operations that verifies commit signatures.
This should hopefully make the code paths to the GPG interface easier to
follow.
Signed-off-by: Hans Jerry Illikainen <hji@xxxxxxxxxxxx>
---
builtin/merge.c | 16 ++++++++------
builtin/pull.c | 18 +++++-----------
builtin/verify-commit.c | 25 +---------------------
commit.c | 47 ++++++++++++++++++++++++++++++-----------
commit.h | 31 ++++++---------------------
gpg-interface.h | 1 +
pretty.c | 3 ++-
7 files changed, 60 insertions(+), 81 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index d127d2225f..e472f17738 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -62,8 +62,8 @@ static int show_diffstat = 1, shortlog_len = -1, squash;
static int option_commit = -1;
static int option_edit = -1;
static int allow_trivial = 1, have_message, verify_signatures;
-static int check_trust_level = 1;
static int overwrite_ignore = 1;
+static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;
static struct strbuf merge_msg = STRBUF_INIT;
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
@@ -633,7 +633,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)
sign_commit = git_config_bool(k, v) ? "" : NULL;
return 0;
} else if (!strcmp(k, "gpg.mintrustlevel")) {
- check_trust_level = 0;
+ gpg_flags ^= GPG_VERIFY_COMPAT;
}
status = fmt_merge_msg_config(k, v, cb);
@@ -1399,9 +1399,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (remoteheads->next)
die(_("Can merge only exactly one commit into empty head"));
- if (verify_signatures)
- verify_merge_signature(remoteheads->item, verbosity,
- check_trust_level);
+ if (verify_signatures &&
+ gpg_verify_commit(&remoteheads->item->object.oid, NULL,
+ NULL, gpg_flags))
+ die(_("Signature verification failed"));
remote_head_oid = &remoteheads->item->object.oid;
read_empty(remote_head_oid, 0);
@@ -1424,8 +1425,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
if (verify_signatures) {
for (p = remoteheads; p; p = p->next) {
- verify_merge_signature(p->item, verbosity,
- check_trust_level);
+ if (gpg_verify_commit(&p->item->object.oid, NULL, NULL,
+ gpg_flags))
+ die(_("Signature verification failed"));
}
}
diff --git a/builtin/pull.c b/builtin/pull.c
index d4e3e77c8e..e41c4032ae 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -107,11 +107,11 @@ static char *opt_ff;
static char *opt_verify_signatures;
static int opt_autostash = -1;
static int config_autostash;
-static int check_trust_level = 1;
static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
static char *opt_gpg_sign;
static int opt_allow_unrelated_histories;
+static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;
/* Options passed to git-fetch */
static char *opt_all;
@@ -366,7 +366,7 @@ static int git_pull_config(const char *var, const char *value, void *cb)
RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
return 0;
} else if (!strcmp(var, "gpg.mintrustlevel")) {
- check_trust_level = 0;
+ gpg_flags ^= GPG_VERIFY_COMPAT;
}
status = git_gpg_config(var, value, cb);
@@ -589,17 +589,9 @@ static int run_fetch(const char *repo, const char **refspecs)
static int pull_into_void(const struct object_id *merge_head,
const struct object_id *curr_head)
{
- if (opt_verify_signatures) {
- struct commit *commit;
-
- commit = lookup_commit(the_repository, merge_head);
- if (!commit)
- die(_("unable to access commit %s"),
- oid_to_hex(merge_head));
-
- verify_merge_signature(commit, opt_verbosity,
- check_trust_level);
- }
+ if (opt_verify_signatures)
+ if (gpg_verify_commit(merge_head, NULL, NULL, gpg_flags))
+ return 1;
/*
* Two-way merge: we treat the index as based on an empty tree,
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index acc01a7be9..3f392654dd 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -20,37 +20,14 @@ static const char * const verify_commit_usage[] = {
NULL
};
-static int run_gpg_verify(struct commit *commit, unsigned flags)
-{
- struct signature_check signature_check;
- int ret;
-
- memset(&signature_check, 0, sizeof(signature_check));
-
- ret = check_commit_signature(commit, &signature_check);
- print_signature_buffer(&commit->object.oid, &signature_check, ret,
- flags);
-
- signature_check_clear(&signature_check);
- return ret;
-}
-
static int verify_commit(const char *name, unsigned flags)
{
struct object_id oid;
- struct object *obj;
if (get_oid(name, &oid))
return error("commit '%s' not found.", name);
- obj = parse_object(the_repository, &oid);
- if (!obj)
- return error("%s: unable to read file.", name);
- if (obj->type != OBJ_COMMIT)
- return error("%s: cannot verify a non-commit object of type %s.",
- name, type_name(obj->type));
-
- return run_gpg_verify((struct commit *)obj, flags);
+ return gpg_verify_commit(&oid, NULL, NULL, flags);
}
static int git_verify_commit_config(const char *var, const char *value, void *cb)
diff --git a/commit.c b/commit.c
index 519599469b..f8339916a6 100644
--- a/commit.c
+++ b/commit.c
@@ -1116,7 +1116,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header
free(buf);
}
-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
+static int check_commit_signature(const struct commit *commit,
+ struct signature_check *sigc)
{
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
@@ -1136,21 +1137,43 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
return ret;
}
-void verify_merge_signature(struct commit *commit, int verbosity,
- int check_trust)
+int gpg_verify_commit(const struct object_id *oid, const char *name_to_report,
+ struct signature_check *sigc, unsigned flags)
{
- struct signature_check signature_check;
+ struct object *obj;
+ struct signature_check tmp = { 0 };
int ret;
- memset(&signature_check, 0, sizeof(signature_check));
- ret = check_commit_signature(commit, &signature_check);
- ret |= check_trust && signature_check.trust_level < TRUST_MARGINAL;
- print_signature_buffer(&commit->object.oid, &signature_check, ret,
- GPG_VERIFY_SHORT);
- if (ret)
- die(_("Signature verification failed"));
+ if (!sigc)
+ sigc = &tmp;
- signature_check_clear(&signature_check);
+ obj = parse_object(the_repository, oid);
+ if (!obj)
+ return error("%s: unable to read file.",
+ name_to_report ?
+ name_to_report :
+ find_unique_abbrev(oid, DEFAULT_ABBREV));
+
+ if (obj->type != OBJ_COMMIT)
+ return error("%s: cannot verify a non-commit object of type %s.",
+ name_to_report ?
+ name_to_report :
+ find_unique_abbrev(oid, DEFAULT_ABBREV),
+ type_name(obj->type));
+
+ ret = check_commit_signature((struct commit *)obj, sigc);
+ /*
+ * Merge operations has historically required a trust level of
+ * 'marginal' or higher as a default. Backward-compatibility is
+ * maintained here -- however, new users of this function should
+ * delegate trust level verification to the GPG interface.
+ */
+ ret |= flags & GPG_VERIFY_COMPAT && sigc->trust_level < TRUST_MARGINAL;
+
+ print_signature_buffer(oid, sigc, ret, flags);
+ signature_check_clear(&tmp);
+
+ return ret;
}
void append_merge_tag_headers(struct commit_list *parents,
diff --git a/commit.h b/commit.h
index 008a0fa4a0..a31aaa7304 100644
--- a/commit.h
+++ b/commit.h
@@ -364,13 +364,14 @@ int parse_signed_commit(const struct commit *commit,
int remove_signature(struct strbuf *buf);
/*
- * Check the signature of the given commit. The result of the check is stored
- * in sig->check_result, 'G' for a good signature, 'U' for a good signature
- * from an untrusted signer, 'B' for a bad signature and 'N' for no signature
- * at all. This may allocate memory for sig->gpg_output, sig->gpg_status,
- * sig->signer and sig->key.
+ * Check the signature of the given commit. The result is stored in sigc if it
+ * is non-NULL. However, note that a return code of 0 from this function
+ * should be used to determine if the signature verified successfully, because
+ * multiple members in the signature_check structure are needed to sufficiently
+ * determine the outcome.
*/
-int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
+int gpg_verify_commit(const struct object_id *oid, const char *name_to_report,
+ struct signature_check *sigc, unsigned flags);
/* record author-date for each commit object */
struct author_date_slab;
@@ -378,24 +379,6 @@ void record_author_date(struct author_date_slab *author_date,
struct commit *commit);
int compare_commits_by_author_date(const void *a_, const void *b_, void *unused);
-
-/*
- * Verify a single commit with check_commit_signature() and die() if it is not
- * a good signature. This isn't really suitable for general use, but is a
- * helper to implement consistent logic for pull/merge --verify-signatures.
- *
- * The check_trust parameter is meant for backward-compatibility. The GPG
- * interface verifies key trust with a default trust level that is below the
- * default trust level for merge operations. Its value should be non-zero if
- * the user hasn't set a minimum trust level explicitly in their configuration.
- *
- * If the user has set a minimum trust level, then that value should be obeyed
- * and check_trust should be zero, even if the configured trust level is below
- * the default trust level for merges.
- */
-void verify_merge_signature(struct commit *commit, int verbose,
- int check_trust);
-
int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);
int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused);
diff --git a/gpg-interface.h b/gpg-interface.h
index 2f349f8ed5..3f619cce0e 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -7,6 +7,7 @@ struct strbuf;
#define GPG_VERIFY_RAW (1 << 1)
#define GPG_VERIFY_FULL (1 << 2)
#define GPG_VERIFY_SHORT (1 << 3)
+#define GPG_VERIFY_COMPAT (1 << 4)
enum signature_trust_level {
TRUST_UNDEFINED,
diff --git a/pretty.c b/pretty.c
index f5fbbc5ffb..c19620ffe6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1302,7 +1302,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (placeholder[0] == 'G') {
if (!c->signature_check.result)
- check_commit_signature(c->commit, &(c->signature_check));
+ gpg_verify_commit(&c->commit->object.oid, NULL,
+ &(c->signature_check), 0);
switch (placeholder[1]) {
case 'G':
if (c->signature_check.gpg_output)
--
2.25.0.rc1.302.gc71d20beed
