Michael J Gruber <g...@drmicha.warpmail.net> writes:

> Currently, verify_signed_buffer() only checks the return code of gpg,
> and some callers implement additional unreliable checks for "Good
> signature" in the gpg output meant for the user.
>
> Use the status output instead and parse for a line beinning with
> "[GNUPG:] GOODSIG ". This is the only reliable way of checking for a
> good gpg signature.
>
> If needed we can change this easily to "[GNUPG:] VALIDSIG " if we want
> to take into account the trust model.

Thanks.  I didn't look beyond "man gpg" nor bother looking at
DETAILS file in its source, which the manpage refers to.

I think GOODSIG is a good starting point.  Depending on the context
(e.g. "%G?") we may also want to consider EXPSIG (but not EXPKEYSIG
or REVKEYSIG) acceptable, while reading "log --show-signature" on
ancient part of the history, no?

> Signed-off-by: Michael J Gruber <g...@drmicha.warpmail.net>
> ---
>  gpg-interface.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 4559033..c582b2e 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -96,15 +96,17 @@ int sign_buffer(struct strbuf *buffer, struct strbuf 
> *signature, const char *sig
>  /*
>   * 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 child_process gpg;
> -     const char *args_gpg[] = {NULL, "--verify", "FILE", "-", NULL};
> +     const char *args_gpg[] = {NULL, "--status-fd=1", "--verify", "FILE", 
> "-", NULL};
>       char path[PATH_MAX];
>       int fd, ret;
> +     struct strbuf buf = STRBUF_INIT;
>  
>       args_gpg[0] = gpg_program;
>       fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
> @@ -119,9 +121,10 @@ int verify_signed_buffer(const char *payload, size_t 
> payload_size,
>       memset(&gpg, 0, sizeof(gpg));
>       gpg.argv = args_gpg;
>       gpg.in = -1;
> +     gpg.out = -1;
>       if (gpg_output)
>               gpg.err = -1;
> -     args_gpg[2] = path;
> +     args_gpg[3] = path;
>       if (start_command(&gpg)) {
>               unlink(path);
>               return error(_("could not run gpg."));
> @@ -134,9 +137,15 @@ int verify_signed_buffer(const char *payload, size_t 
> payload_size,
>               strbuf_read(gpg_output, gpg.err, 0);
>               close(gpg.err);
>       }
> +     strbuf_read(&buf, gpg.out, 0);
> +     close(gpg.out);
> +
>       ret = finish_command(&gpg);
>  
>       unlink_or_warn(path);
>  
> +     ret |= !strstr(buf.buf, "\n[GNUPG:] GOODSIG ");
> +     strbuf_release(&buf);
> +
>       return ret;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to