Skip to content

Commit

Permalink
Merge branch 'hi/gpg-prefer-check-signature'
Browse files Browse the repository at this point in the history 
The code to interface with GnuPG has been refactored.

* hi/gpg-prefer-check-signature:
  gpg-interface: prefer check_signature() for GPG verification
  t: increase test coverage of signature verification output
  • Loading branch information
@gitster
gitster committed Mar 27, 2020
2 parents f8cb64e + 6794898 commit fa82be9
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 78 deletions.
11 changes: 7 additions & 4 deletions builtin/fmt-merge-msg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
enum object_type type;
unsigned long size, len;
char *buf = read_object_file(oid, &type, &size);
struct signature_check sigc = { 0 };
struct strbuf sig = STRBUF_INIT;

if (!buf || type != OBJ_TAG)
Expand All @@ -502,10 +503,12 @@ static void fmt_merge_msg_sigs(struct strbuf *out)

if (size == len)
; /* merely annotated */
else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {
if (!sig.len)
strbuf_addstr(&sig, "gpg verification failed.\n");
}
else if (check_signature(buf, len, buf + len, size - len, &sigc) &&
!sigc.gpg_output)
strbuf_addstr(&sig, "gpg verification failed.\n");
else
strbuf_addstr(&sig, sigc.gpg_output);
signature_check_clear(&sigc);

if (!tag_number++) {
fmt_tag_signature(&tagbuf, &sig, buf, len);
Expand Down
97 changes: 49 additions & 48 deletions gpg-interface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,55 @@ static void parse_gpg_output(struct signature_check *sigc)
FREE_AND_NULL(sigc->key);
}

static int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t signature_size,
struct strbuf *gpg_output,
struct strbuf *gpg_status)
{
struct child_process gpg = CHILD_PROCESS_INIT;
struct gpg_format *fmt;
struct tempfile *temp;
int ret;
struct strbuf buf = STRBUF_INIT;

temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
if (!temp)
return error_errno(_("could not create temporary file"));
if (write_in_full(temp->fd, signature, signature_size) < 0 ||
close_tempfile_gently(temp) < 0) {
error_errno(_("failed writing detached signature to '%s'"),
temp->filename.buf);
delete_tempfile(&temp);
return -1;
}

fmt = get_format_by_sig(signature);
if (!fmt)
BUG("bad signature '%s'", signature);

argv_array_push(&gpg.args, fmt->program);
argv_array_pushv(&gpg.args, fmt->verify_args);
argv_array_pushl(&gpg.args,
"--status-fd=1",
"--verify", temp->filename.buf, "-",
NULL);

if (!gpg_status)
gpg_status = &buf;

sigchain_push(SIGPIPE, SIG_IGN);
ret = pipe_command(&gpg, payload, payload_size,
gpg_status, 0, gpg_output, 0);
sigchain_pop(SIGPIPE);

delete_tempfile(&temp);

ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
strbuf_release(&buf); /* no matter it was used or not */

return ret;
}

int check_signature(const char *payload, size_t plen, const char *signature,
size_t slen, struct signature_check *sigc)
{
Expand Down Expand Up @@ -418,51 +467,3 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig

return 0;
}

int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t signature_size,
struct strbuf *gpg_output, struct strbuf *gpg_status)
{
struct child_process gpg = CHILD_PROCESS_INIT;
struct gpg_format *fmt;
struct tempfile *temp;
int ret;
struct strbuf buf = STRBUF_INIT;

temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
if (!temp)
return error_errno(_("could not create temporary file"));
if (write_in_full(temp->fd, signature, signature_size) < 0 ||
close_tempfile_gently(temp) < 0) {
error_errno(_("failed writing detached signature to '%s'"),
temp->filename.buf);
delete_tempfile(&temp);
return -1;
}

fmt = get_format_by_sig(signature);
if (!fmt)
BUG("bad signature '%s'", signature);

argv_array_push(&gpg.args, fmt->program);
argv_array_pushv(&gpg.args, fmt->verify_args);
argv_array_pushl(&gpg.args,
"--status-fd=1",
"--verify", temp->filename.buf, "-",
NULL);

if (!gpg_status)
gpg_status = &buf;

sigchain_push(SIGPIPE, SIG_IGN);
ret = pipe_command(&gpg, payload, payload_size,
gpg_status, 0, gpg_output, 0);
sigchain_pop(SIGPIPE);

delete_tempfile(&temp);

ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
strbuf_release(&buf); /* no matter it was used or not */

return ret;
}
9 changes: 0 additions & 9 deletions gpg-interface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,6 @@ size_t parse_signature(const char *buf, size_t size);
int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
const char *signing_key);

/*
* Run "gpg" to see if the payload matches the detached signature.
* gpg_output, when set, receives the diagnostic output from GPG.
* gpg_status, when set, receives the status output from GPG.
*/
int verify_signed_buffer(const char *payload, size_t payload_size,
const char *signature, size_t signature_size,
struct strbuf *gpg_output, struct strbuf *gpg_status);

int git_gpg_config(const char *, const char *, void *);
void set_signing_key(const char *);
const char *get_signing_key(void);
Expand Down
34 changes: 17 additions & 17 deletions log-tree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,22 +449,21 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
{
struct strbuf payload = STRBUF_INIT;
struct strbuf signature = STRBUF_INIT;
struct strbuf gpg_output = STRBUF_INIT;
struct signature_check sigc = { 0 };
int status;

if (parse_signed_commit(commit, &payload, &signature) <= 0)
goto out;

status = verify_signed_buffer(payload.buf, payload.len,
signature.buf, signature.len,
&gpg_output, NULL);
if (status && !gpg_output.len)
strbuf_addstr(&gpg_output, "No signature\n");

show_sig_lines(opt, status, gpg_output.buf);
status = check_signature(payload.buf, payload.len, signature.buf,
signature.len, &sigc);
if (status && !sigc.gpg_output)
show_sig_lines(opt, status, "No signature\n");
else
show_sig_lines(opt, status, sigc.gpg_output);
signature_check_clear(&sigc);

out:
strbuf_release(&gpg_output);
strbuf_release(&payload);
strbuf_release(&signature);
}
Expand Down Expand Up @@ -497,8 +496,9 @@ static int show_one_mergetag(struct commit *commit,
struct object_id oid;
struct tag *tag;
struct strbuf verify_message;
struct signature_check sigc = { 0 };
int status, nth;
size_t payload_size, gpg_message_offset;
size_t payload_size;

hash_object_file(the_hash_algo, extra->value, extra->len,
type_name(OBJ_TAG), &oid);
Expand All @@ -520,19 +520,19 @@ static int show_one_mergetag(struct commit *commit,
else
strbuf_addf(&verify_message,
"parent #%d, tagged '%s'\n", nth + 1, tag->tag);
gpg_message_offset = verify_message.len;

payload_size = parse_signature(extra->value, extra->len);
status = -1;
if (extra->len > payload_size) {
/* could have a good signature */
if (!verify_signed_buffer(extra->value, payload_size,
extra->value + payload_size,
extra->len - payload_size,
&verify_message, NULL))
status = 0; /* good */
else if (verify_message.len <= gpg_message_offset)
status = check_signature(extra->value, payload_size,
extra->value + payload_size,
extra->len - payload_size, &sigc);
if (sigc.gpg_output)
strbuf_addstr(&verify_message, sigc.gpg_output);
else
strbuf_addstr(&verify_message, "No signature\n");
signature_check_clear(&sigc);
/* otherwise we couldn't verify, which is shown as bad */
}

Expand Down
105 changes: 105 additions & 0 deletions t/t4202-log.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1627,6 +1627,66 @@ test_expect_success GPG 'log --graph --show-signature for merged tag in shallow
grep "tag signed_tag_shallow names a non-parent $hash" actual
'

test_expect_success GPG 'log --graph --show-signature for merged tag with missing key' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b plain-nokey master &&
echo aaa >bar &&
git add bar &&
git commit -m bar_commit &&
git checkout -b tagged-nokey master &&
echo bbb >baz &&
git add baz &&
git commit -m baz_commit &&
git tag -s -m signed_tag_msg signed_tag_nokey &&
git checkout plain-nokey &&
git merge --no-ff -m msg signed_tag_nokey &&
GNUPGHOME=. git log --graph --show-signature -n1 plain-nokey >actual &&
grep "^|\\\ merged tag" actual &&
grep "^| | gpg: Signature made" actual &&
grep "^| | gpg: Can'"'"'t check signature: \(public key not found\|No public key\)" actual
'

test_expect_success GPG 'log --graph --show-signature for merged tag with bad signature' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b plain-bad master &&
echo aaa >bar &&
git add bar &&
git commit -m bar_commit &&
git checkout -b tagged-bad master &&
echo bbb >baz &&
git add baz &&
git commit -m baz_commit &&
git tag -s -m signed_tag_msg signed_tag_bad &&
git cat-file tag signed_tag_bad >raw &&
sed -e "s/signed_tag_msg/forged/" raw >forged &&
git hash-object -w -t tag forged >forged.tag &&
git checkout plain-bad &&
git merge --no-ff -m msg "$(cat forged.tag)" &&
git log --graph --show-signature -n1 plain-bad >actual &&
grep "^|\\\ merged tag" actual &&
grep "^| | gpg: Signature made" actual &&
grep "^| | gpg: BAD signature from" actual
'

test_expect_success GPG 'log --show-signature for merged tag with GPG failure' '
test_when_finished "git reset --hard && git checkout master" &&
git checkout -b plain-fail master &&
echo aaa >bar &&
git add bar &&
git commit -m bar_commit &&
git checkout -b tagged-fail master &&
echo bbb >baz &&
git add baz &&
git commit -m baz_commit &&
git tag -s -m signed_tag_msg signed_tag_fail &&
git checkout plain-fail &&
git merge --no-ff -m msg signed_tag_fail &&
TMPDIR="$(pwd)/bogus" git log --show-signature -n1 plain-fail >actual &&
grep "^merged tag" actual &&
grep "^No signature" actual &&
! grep "^gpg: Signature made" actual
'

test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
test_when_finished "git reset --hard && git checkout master" &&
test_config gpg.format x509 &&
Expand All @@ -1648,6 +1708,51 @@ test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
grep "^| | gpgsm: Good signature" actual
'

test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 missing key' '
test_when_finished "git reset --hard && git checkout master" &&
test_config gpg.format x509 &&
test_config user.signingkey $GIT_COMMITTER_EMAIL &&
git checkout -b plain-x509-nokey master &&
echo aaa >bar &&
git add bar &&
git commit -m bar_commit &&
git checkout -b tagged-x509-nokey master &&
echo bbb >baz &&
git add baz &&
git commit -m baz_commit &&
git tag -s -m signed_tag_msg signed_tag_x509_nokey &&
git checkout plain-x509-nokey &&
git merge --no-ff -m msg signed_tag_x509_nokey &&
GNUPGHOME=. git log --graph --show-signature -n1 plain-x509-nokey >actual &&
grep "^|\\\ merged tag" actual &&
grep "^| | gpgsm: certificate not found" actual
'

test_expect_success GPGSM 'log --graph --show-signature for merged tag x509 bad signature' '
test_when_finished "git reset --hard && git checkout master" &&
test_config gpg.format x509 &&
test_config user.signingkey $GIT_COMMITTER_EMAIL &&
git checkout -b plain-x509-bad master &&
echo aaa >bar &&
git add bar &&
git commit -m bar_commit &&
git checkout -b tagged-x509-bad master &&
echo bbb >baz &&
git add baz &&
git commit -m baz_commit &&
git tag -s -m signed_tag_msg signed_tag_x509_bad &&
git cat-file tag signed_tag_x509_bad >raw &&
sed -e "s/signed_tag_msg/forged/" raw >forged &&
git hash-object -w -t tag forged >forged.tag &&
git checkout plain-x509-bad &&
git merge --no-ff -m msg "$(cat forged.tag)" &&
git log --graph --show-signature -n1 plain-x509-bad >actual &&
grep "^|\\\ merged tag" actual &&
grep "^| | gpgsm: Signature made" actual &&
grep "^| | gpgsm: invalid signature" actual
'


test_expect_success GPG '--no-show-signature overrides --show-signature' '
git log -1 --show-signature --no-show-signature signed >actual &&
! grep "^gpg:" actual
Expand Down
23 changes: 23 additions & 0 deletions t/t6200-fmt-merge-msg.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
test_description='fmt-merge-msg test'

. ./test-lib.sh
. "$TEST_DIRECTORY/lib-gpg.sh"

test_expect_success setup '
echo one >one &&
Expand Down Expand Up @@ -73,6 +74,10 @@ test_expect_success setup '
apos="'\''"
'

test_expect_success GPG 'set up a signed tag' '
git tag -s -m signed-tag-msg signed-good-tag left
'

test_expect_success 'message for merging local branch' '
echo "Merge branch ${apos}left${apos}" >expected &&
Expand All @@ -83,6 +88,24 @@ test_expect_success 'message for merging local branch' '
test_cmp expected actual
'

test_expect_success GPG 'message for merging local tag signed by good key' '
git checkout master &&
git fetch . signed-good-tag &&
git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
grep "^Merge tag ${apos}signed-good-tag${apos}" actual &&
grep "^# gpg: Signature made" actual &&
grep "^# gpg: Good signature from" actual
'

test_expect_success GPG 'message for merging local tag signed by unknown key' '
git checkout master &&
git fetch . signed-good-tag &&
GNUPGHOME=. git fmt-merge-msg <.git/FETCH_HEAD >actual 2>&1 &&
grep "^Merge tag ${apos}signed-good-tag${apos}" actual &&
grep "^# gpg: Signature made" actual &&
grep "^# gpg: Can${apos}t check signature: \(public key not found\|No public key\)" actual
'

test_expect_success 'message for merging external branch' '
echo "Merge branch ${apos}left${apos} of $(pwd)" >expected &&
Expand Down

0 comments on commit fa82be9

Please sign in to comment.