[PATCH] httprepo: long arguments support (issue2126)

Augie Fackler durin42 at gmail.com
Sun Mar 27 18:13:50 CDT 2011


On Mar 27, 2011, at 5:51 PM, Laurens Holst wrote:
> 
> Op 28-3-2011 0:18, Laurens Holst schreef:
>> Op 27-3-2011 11:21, Steven Brown schreef:
>>> On 21 March 2011 22:54, Laurens Holst<laurens.nospam at grauw.nl>  wrote:
>>>> Op 21-03-11 08:30, Dirkjan Ochtman schreef:
>>>>> On Mon, Mar 21, 2011 at 02:39, Matt Mackall<mpm at selenic.com>    wrote:
>>>>>> Let's try to get more discussion on whether POST is acceptable and
>>>>>> anyone is using POST filtering.
>>>>> AFAICT restricting push access by filtering out POST requests is a
>>>>> fairly common setup.
>>>> Yes I do that too, it was the setup described on the wiki. And fairly
>>>> convenient I must say (and properly RESTful :)).
>>>> 
>>>> ~Laurens
>>>> 
>>> It would still be possible to authenticate on push like this:
>>> 
>>> RewriteEngine on
>>> RewriteCond %{QUERY_STRING} cmd=unbundle
>>> RewriteRule .* - [E=hg_auth:1]
>>> 
>>> <Location /hg>
>>>     Order Allow,Deny
>>>     Allow from env=!hg_auth
>>>     AuthType Basic
>>>     AuthName "Mercurial repositories"
>>>     AuthUserFile /home/user/hg/hgusers
>>>     Require valid-user
>>>     Satisfy Any
>>> </Location>
>> 
>> If I understand this correctly, you’re basically saying, POST is not authenticated unless it’s an unbundle command?
>> 
>> I don’t think this is good. This means that if you want to add a new command that alters the server, all users would have to update their servers or they would be insecure. Effectively this means that you can’t add new commands, and you’re putting a severe restriction on Mercurial’s future extensibility.
> 
> Also it is relatively easy to introduce actual security problems: what if the server configuration doesn’t decode the query strings before processing but Python or Mercurial does, I could easily unbundle without getting authenticated by writing cmd=un%62undle. Or, maybe the server does a case-sensitive match while Mercurial does an insensitive one? cmd=unBundle. Are the key or value names trimmed? cmd%20=%20unbundle. Null characters treated specially? cmd%00=%00unbundle.
> 
> Lots of edge cases. Are you sure you can cover them all? Maybe by looking at the Mercurial code and doing a bunch of experiments you can provide a well-tested Apache configuration, but what if people need to make modifications, or have a different server technology and need to create their own authentication rules…
> 
> Just trying to illustrate why I think this looks like a bit of bad idea to me :).
> 
> ~Laurens

Tossing out an idea: if we allow use of base64.urlsafe_b64encode on nodes they're 28 bytes instead of 40. That buys about 10 heads per kilobyte if I did the math right (which is always questionable), and it feels to me (without blowing lots of time on investigation) like we should be able to squeeze a little more out of the available  encoding space without too much creativity (one obvious thing is encoding many shas in a single base64 block to allow sharing of padding.)

Doesn't (completely) solve the problem, but at least buys some more space before resorting to nonstandard use of HTTP verbs.



More information about the Mercurial-devel mailing list