Skip to content

Commit

Permalink
gpg: centralize signature check
Browse files Browse the repository at this point in the history
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
brian m. carlson authored and Junio C Hamano committed Jun 22, 2015
1 parent 8e98e5f commit 434060e
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 10 deletions.
5 changes: 3 additions & 2 deletions builtin/verify-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions builtin/verify-tag.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -1227,20 +1227,24 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header
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;
}


Expand Down
2 changes: 1 addition & 1 deletion commit.h
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,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);

Expand Down
4 changes: 3 additions & 1 deletion gpg-interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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';
}

/*
Expand Down
2 changes: 1 addition & 1 deletion gpg-interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion t/t7510-signed-commit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down

0 comments on commit 434060e

Please sign in to comment.