On 3/28/2018 6:12 PM, Junio C Hamano wrote:
Jonathan Nieder <jrnieder@xxxxxxxxx> 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@xxxxxxxxx>
---
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.
I agree. Thanks.
