Subject: Re: [PATCH 2/3] receive-pack: verify push options in cert

From: Brandon Williams <[email-masked]>
Date: Fri, 5 May 2017 17:06:08 -0700

On 05/05, Stefan Beller wrote:
> On Fri, May 5, 2017 at 4:46 PM, Jonathan Tan <[email-masked]> wrote:
> > In commit f6a4e61 ("push: accept push options", 2016-07-14), send-pack
> > was taught to include push options both within the signed cert (if the
> > push is a signed push) and outside the signed cert; however,
> > receive-pack ignores push options within the cert, only handling push
> > options outside the cert.
> >
> > Teach receive-pack, in the case that push options are provided for a
> > signed push, to verify that the push options both within the cert and
> > outside the cert are consistent, and to provide the results of that
> > verification to the receive hooks as an environment variable (just like
> > it currently does for the nonce verification).
> >
> > This sets in stone the requirement that send-pack redundantly send its
> > push options in 2 places, but I think that this is better than the
> > alternatives. Sending push options only within the cert is
> > backwards-incompatible with existing Git servers (which read push
> > options only from outside the cert), and sending push options only
> > outside the cert means that the push options are not signed for.
> 
> As the combination of push certs and push options are broken
> (at least when using JGit as well, which are used in Gerrit
> installations), I would not feel too bad about actually going
> the backwards-incompatible way.

yeah just think of the bits that could be saved!

But in all seriousness, I'd agree that doing the backwards-incompatible
way would be cleaner in the longer run (esp since they are broken
currently).

-- 
Brandon Williams