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

Reply via email to