[PATCH 3 of 7] dispatch: rework the serve --stdio safe argument checks

[Augie Fackler]

> On Jun 26, 2018, at 07:53, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> On Mon, 25 Jun 2018 19:51:52 +0200, Paul Morelle wrote:
>> On 22/06/18 13:15, Yuya Nishihara wrote:
>>> On Thu, 21 Jun 2018 19:46:24 +0200, Paul Morelle wrote:
>> Doing this at the authorized_keys level is not an option for us. In one
>> of our target environment for this kind of debugging, any change to the
>> ssh script requires validation by admin people that can take from a
>> couple of days to a couple of weeks, making it impossible to turn
>> debugging on for a couple of commands, or easily compare various settings.
> 
> IMHO, that doesn't justify why we have to put everything in hg core.
> "hg serve" itself is an SSH script in security POV.

Agreed. It's also a strong argument for infrastructure-as-code so you can test configuration changes like this on a disposable VM, then apply to the real environment once it's right.

> 
>> The ability to control the used debug options from the client is very
>> valuable in this case. We have already gathered many useful information
>> from it!
>> 
>> I agree that ad-hoc argument parsing is less than optimal. Handling
>> three extra flags (--debug, --profile and --traceback) is not too awful,
>> but then we need to pass some associated configuration option (e.g.
>> profiling.time-track=real) which will make things even more verbose.
>> 
>> Maybe we could leverage the contents of `mercurial/configitems.py` by
>> adding a "serversafe" attribute to the configuration items? Such config
>> item could be specified on the client side (if the user is allowed to).
> 
> No, that sounds worse than the original patch. IIUC, the reason why we have
> hard-coded check for req.args is that our option parser isn't strict and is
> likely to allow command-line injection. Making the whitelist fully configurable
> would be dangerous.

Yep. I'm dubious we'd ever take such a patch.

> 
>>>> I don't think that moving the permission handling outside of Mercurial
>>>> would be a good move : implementing similar features for HTTP would
>>>> still require some Mercurial-side restrictions and permissions handling
>>>> anyway. It seems more consistent to implement it on the Mercurial side
>>>> for all protocols.
>>> I agree it should be handled at protocol level, and I think it should be
>>> a wire command/option rather than the command argument hack. IMHO, it's
>>> way safer than constructing/parsing shell command arguments carefully,
>>> and HTTP will need that anyway.
>> The life cycle of the server is quite different in ssh and http, and
>> this has different implications.
>> For the ssh client, there are multiple initializations and cachings that
>> are worth tracking, so tracking the full run is what is most appropriate
>> for our needs.
>> For http, the arguments will have to be processed at the hgweb level
>> (instead of the protocol level).
> 
> I know the situation of HTTP and SSH is slightly different, but how important
> it is to catch the early-stage thingy by extending the SSH command-line
> interface, which we have to support forever? I think wire-protocol level
> implementation will be good enough, and better in the long run.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel