Re: [PATCH 2/3] receive-pack: verify push options in cert
- From
- Brandon Williams <bmwill@google.com>
- Date
- May 6, 2017, 00:06 UTC
- Message-ID
- <20170506000608.GD55152@google.com>
- In-Reply-To
- <CAGZ79ka4vG1yd1JtK9bDBjWPUG0UCPMvw2XQUsfX_e_xFCpVLA@mail.gmail.com>
On 05/05, Stefan Beller wrote:
Show 24 quoted lines
> On Fri, May 5, 2017 at 4:46 PM, Jonathan Tan <jonathantanmy@google.com> 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