about summary refs log tree commit
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2020-03-26 17:11:20 -0700
committerJunio C Hamano <gitster@pobox.com>2020-03-26 17:11:20 -0700
commitfa82be982dfc5b463a125991a2d381f1cd0ad9eb (patch)
tree6e67608a69b0c1a51a540c3c70e0b03c25646fd1
parentf8cb64e3d4d512a86c1b7b3aa584f11740b3d038 (diff)
parent67948981983b336eab2fa7e6a0e125d529391dfc (diff)
downloadgit-fa82be982dfc5b463a125991a2d381f1cd0ad9eb.tar.gz
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
-rw-r--r--builtin/fmt-merge-msg.c11
-rw-r--r--gpg-interface.c97
-rw-r--r--gpg-interface.h9
-rw-r--r--log-tree.c34
-rwxr-xr-xt/t4202-log.sh105
-rwxr-xr-xt/t6200-fmt-merge-msg.sh23
6 files changed, 201 insertions, 78 deletions
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 736f666f64..172dfbd852 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -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)
@@ -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);
diff --git a/gpg-interface.c b/gpg-interface.c
index 165274d74a..2d538bcd6e 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -256,6 +256,55 @@ error:
         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)
 {
@@ -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;
-}
diff --git a/gpg-interface.h b/gpg-interface.h
index 796571e9e9..f4e9b4f371 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -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);
diff --git a/log-tree.c b/log-tree.c
index 52127427ff..897a90233e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -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);
 }
@@ -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);
@@ -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 */
         }
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 0f766ba65f..5eeb739f3e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -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 &&
@@ -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
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 8a72b4c43a..b15582a7a2 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -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 &&
@@ -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 &&
 
@@ -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 &&