Re: [PATCH/RFC] builtin/tag.c: move PGP verification inside builtin.
- From
Santiago Torres <santiago@nyu.edu>
- Date
- Mar 24, 2016, 22:32 UTC
- Message-ID
- <20160324223257.GE8830@LykOS>
- In-Reply-To
- <20160324221457.GB17805@sigill.intra.peff.net>
Show 7 quoted lines
> > 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.
I see, yes. This looks like the way forward.
Show 28 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. >
Now I get it I think this should be easy to achieve by moving verify_tag() to tag.c, along with the static run_gpg_verify functions. I could move the SIGPIPE call inside the verify-tag command and patch up everything accordingly. Does this sound ok?
Thanks, -Santiago.