Revert "gpg-interface: prefer check_signature() for GPG verification"

This reverts commit 72b006f4bfd30b7c5037c163efaf279ab65bea9c, which
breaks the end-user experience when merging a signed tag without
having the public key.  We should report "can't check because we
have no public key", but the code with this change claimed that
there was no signature.

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index f7ed102d8b37294f992c3edf25156ce4f3e20c69..a4615587fd7929e8fb49e65dbda30a40b8599cfd 100644 (file)
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -495,7 +495,6 @@ 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)
@@ -504,12 +503,10 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 
                if (size == len)
                        ; /* merely annotated */
-               else if (!check_signature(buf, len, buf + len, size - len,
-                                         &sigc)) {
-                       strbuf_addstr(&sig, sigc.gpg_output);
-                       signature_check_clear(&sigc);
-               } else
-                       strbuf_addstr(&sig, "gpg verification failed.\n");
+               else if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {
+                       if (!sig.len)
+                               strbuf_addstr(&sig, "gpg verification failed.\n");
+               }
 
                if (!tag_number++) {
                        fmt_tag_signature(&tagbuf, &sig, buf, len);

diff --git a/gpg-interface.c b/gpg-interface.c
index 5134ce27806866c41d5eda19cf01110cbf7849f2..131e7d529e9bdb480c88a01a33a3dd8339d219cd 100644 (file)
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -207,55 +207,6 @@ found_duplicate_status:
        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)
 {
@@ -400,3 +351,51 @@ 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 93cc3aff5c93ce8fa19aadea909960f9267edbdc..3e624ec289ab5f46b686f9eb2489c997bca23232 100644 (file)
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -46,6 +46,15 @@ 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 aa6b038adb1f2bf58ceba1e4a367a7e3bb6553e0..1e56df62a79c0d221fa55ba351b6b16f8027d12b 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -448,22 +448,22 @@ static void show_signature(struct rev_info *opt, struct commit *commit)
 {
        struct strbuf payload = STRBUF_INIT;
        struct strbuf signature = STRBUF_INIT;
-       struct signature_check sigc = { 0 };
+       struct strbuf gpg_output = STRBUF_INIT;
        int status;
 
        if (parse_signed_commit(commit, &payload, &signature) <= 0)
                goto out;
 
-       status = check_signature(payload.buf, payload.len, signature.buf,
-                                signature.len, &sigc);
-       if (status && sigc.result == 'N')
-               show_sig_lines(opt, status, "No signature\n");
-       else {
-               show_sig_lines(opt, status, sigc.gpg_output);
-               signature_check_clear(&sigc);
-       }
+       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);
 
  out:
+       strbuf_release(&gpg_output);
        strbuf_release(&payload);
        strbuf_release(&signature);
 }
@@ -496,7 +496,6 @@ 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;
 
@@ -525,13 +524,12 @@ static int show_one_mergetag(struct commit *commit,
        status = -1;
        if (extra->len > payload_size) {
                /* could have a good signature */
-               if (!check_signature(extra->value, payload_size,
-                                    extra->value + payload_size,
-                                    extra->len - payload_size, &sigc)) {
-                       strbuf_addstr(&verify_message, sigc.gpg_output);
-                       signature_check_clear(&sigc);
+               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)
+               else if (verify_message.len <= gpg_message_offset)
                        strbuf_addstr(&verify_message, "No signature\n");
                /* otherwise we couldn't verify, which is shown as bad */
        }