Show 37 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> 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.
>