Re: Git Patch
Brought to you by:
xystrus
From: Derek M. <co...@pi...> - 2011-08-30 19:11:42
|
On Fri, Aug 26, 2011 at 03:36:53PM +0200, richard -rw- weinberger wrote: > On Fri, Aug 26, 2011 at 1:39 PM, Matthias Witte <wi...@ne...> wrote: > > Hi, > > > > Richard Weinberger has written feature patch, to include git in rssh: > > > > Website: http://www.nod.at/~richard/ > > Patch: http://www.nod.at/~richard/alltag/rssh-git.patch > > It's always nice to meet old and crappy code. ;-) I'm not sure I would call it crappy... though there are a couple of points. One is that in general, patching RSSH involves inherent badness due to the design of the config file. It wasn't really meant to be patched... Existing patches typically just update existing cases using the same numbers/bit fields, so they conflict. To be more extensible, it really needs a configuration language to replace the existing config file format. But I didn't want that... it was really meant to strictly limit the cases of access to keep the security implications manageable. Past failures have convinced me that this was the right decision; if I ever do release another version it will very probably remove support for everything other than scp/sftp, as was originally intended. This, however, is unlikely. There does seem to be a demand for such a beast though; if I ever get it in my head to experiment with a more flexible configuration parser, I may split rssh into rssh and some other thing that's more extensible. Or, I may just steal code from sudo. But at least for the moment, I have no interest in doing any of that. The other badness that immediately makes itself evident is the hard- coding of the paths of the GIT binaries. Nothing else does this, and the git support shouldn't either. Lastly, validate_access() already takes too many args; the access types should be converted to either a bit mask or a struct, probably the latter. But this again is more a "flaw" in RSSH that was somewhat included by design, than it is a weakness of the patch. Other than that -- codewise -- it basically looks fine to me. I nevertheless discourage its use without taking the time to carefully consider the security implications of using git in this fashion, which I have no intention of doing. =8^) If you aren't an expert git user/developer with 100% familiarity with its user interfaces, and understand every possible way that it might invoke some other program, then you're probably not qualified to make that judgement. My largely uneducated (with respect to git) guess is that this patch will foil casual "unlocked door" exploits, but won't keep out someone determined to get a shell on your machine. It's typical for RCS software to have hooks that can be controlled by the user which can run arbitrary programs, which tends to beat any sort of restricted shell program. -- Derek D. Martin http://www.pizzashack.org/ GPG Key ID: 0x81CFE75D |