Junio C Hamano venit, vidit, dixit 14.02.2013 18:22: > 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?
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. >> 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