Re: [PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.

From: Jeff King <peff@peff.net>
Date: Mar 24, 2016, 22:14 UTC
Message-ID: <20160324221457.GB17805@sigill.intra.peff.net>
In-Reply-To: <20160324215104.GC8830@LykOS>

On Thu, Mar 24, 2016 at 05:51:05PM -0400, Santiago Torres wrote:

> Sorry for the delay with this, I got caught up with coursework.

No problem. The project moves forward as contributor time permits.

> This is my first stab at this, in the dumbest/simplest way imaginable. I
> don't like that there is no code reuse (the run_gpg_verify function is
> repeated here and in the plumbing command). I would appreciate pointers
> on what would be the best way to avoid this.

It looks to me like you could factor the repeated code into a common verify_tag(), but maybe I am missing something.

Show 6 quoted lines

> I also spent quite some time figuring out what you meant with
> 
> > Do note the trickery with SIGPIPE in verify-tag, though. We probably
> > need to do the same here (in fact, I wonder if that should be pushed
> > down into the code that calls gpg).
> I don't see any explicit SIGPIPE trickery here. Any pointers?

There is a call to ignore SIGPIPE in builtin/verify-tag.c, line 100. Do we need to do be doing the same thing here?

There's some discussion in the thread starting at:

  http://thread.gmane.org/gmane.comp.version-control.git/53878/focus=53904

The claim there is that we get SIGPIPE and die early if we feed gpg a tag which isn't signed. We _should_ be catching that case already via parse_signature(), though I wonder if it can be fooled (e.g., something that looks like a signature, but when gpg parses it, it turns out to be bogus). So we should probably continue ignoring SIGPIPE to be on the safe side.

But I notice that we already handle SIGPIPE explicitly in sign_buffer() for similar reasons. What I was wondering earlier was whether we should teach other functions that call gpg (like verify_signed_buffer()) to ignore SIGPIPE, too, so that we can return a reasonable error value rather than just killing the whole program.

-Peff