Re: [RFC/PATCH] upload-pack: disable object filtering when disabled by config

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 28, 2018, 22:12 UTC
Message-ID: <xmqqo9j77sub.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180328203303.GA260688@aiede.svl.corp.google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

Show 26 quoted lines

> When upload-pack gained partial clone support (v2.17.0-rc0~132^2~12,
> 2017-12-08), it was guarded by the uploadpack.allowFilter config item
> to allow server operators to control when they start supporting it.
>
> That config item didn't go far enough, though: it controls whether the
> 'filter' capability is advertised, but if a (custom) client ignores
> the capability advertisement and passes a filter specification anyway,
> the server would handle that despite allowFilter being false.
>
> This is particularly significant if a security bug is discovered in
> this new experimental partial clone code.  Installations without
> uploadpack.allowFilter ought not to be affected since they don't
> intend to support partial clone, but they would be swept up into being
> vulnerable.
>
> Simplify and limit the attack surface by making uploadpack.allowFilter
> disable the feature, not just the advertisement of it.
>
> NEEDSWORK: tests
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Noticed while reviewing the corresponding JGit code.
>
> If this change seems like a good idea, I can add tests and re-send it
> for real.

Yup. The names of the static variables tell almost the whole story to convince me why this is a good change ;-). It is not about advertising a feature alone, but if the feature is actually enabled, so advertisement and handling of the feature should be either both enabled or disabled.

Thanks.