Re: [PATCH 1/5] gpg-interface: check good signature in a reliable way
- From
Michael J Gruber <git@drmicha.warpmail.net>
- Date
- Feb 15, 2013, 08:27 UTC
- Message-ID
- <511DF174.4090301@drmicha.warpmail.net>
- In-Reply-To
- <7vzjz624kj.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 14.02.2013 18:22:
Show 20 quoted lines
> Michael J Gruber <git@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?
Yes, we could certainly return a more detailed status to the callers. Currently, "0" is OK (GOODSIG) and everything else is a fail. We would need to change the callers to allow more details on the "fail" as well as the "OK" so that they can decide what is good enough, say:
-1: fail for technical reasons (no sig, can't run gpg etc.) 0: sig present bad (cryptographically) BAD 1: REVKEYSIG 2: EXPKEYSIG 3: EXPSIG 4: GOODSIG 5: VALIDSIG
I'd have to recheck whether a bitmask or ordered values make more sense.
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;
>> }