Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 14, 2013, 17:22 UTC
- Message-ID
- <7vzjz624kj.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <e0f3c48e474c64e33c67c772468bbf7bba0169af.1360857415.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
Show 10 quoted lines
> 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?
Show 56 quoted lines
> Signed-off-by: Michael J Gruber <git@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;
> }