[RFC] daemon whitelist handling (Re: git pull aborts in 50% of cases)

Having slept over the patch I am responding to, I tend to think
this is more trouble than its worth.  Validating the
directory enter_repo() chdir()'ed into and validated to be a
good git repository should be done on its canonical name as
getcwd() returns, not with a userland aliasing avoidance.

As an administrator, being able to say /pub/scm on the
whitelist, knowing /pub to be a symbolic link points at
somewhere today but maybe at different place tomorrow, and not
having to adjust the whitelist whenever that happens, is indeed
nice.  We do not allow the remote requestor to say /../ in the
path, so we trap him within the directories the whitelist
describes.

Not.

For example, I can by mistake create a symbolic link:

	ln -s /home /pub/scm/git/git.git/oops

now accesses /pub/scm/git/oops/hpa/secret.git/ is not
restricted.  We could hand-resolve the each level from the
request to see if no "funny" symbolic links are involved, but
what is the definition of "funny"?  When we see /pub pointing at
somewhere in /mnt/disk47/slice31, we should not complain.  When
we see "oops" under git in the above example, we would want to
complain.  These things are hard to get right.

I tend to say that the 0.99.9k (and the current master) rule to
make validation always work on what getcwd() gives back is
easier to understand (which generally means safer).  Can I talk
you into adjusting your whitelist on kernel.org machines?

Junio C Hamano wrote:

For example, I can by mistake create a symbolic link:

	ln -s /home /pub/scm/git/git.git/oops

now accesses /pub/scm/git/oops/hpa/secret.git/ is not
restricted.  We could hand-resolve the each level from the
request to see if no "funny" symbolic links are involved, but
what is the definition of "funny"?  When we see /pub pointing at
somewhere in /mnt/disk47/slice31, we should not complain.  When
we see "oops" under git in the above example, we would want to
complain.  These things are hard to get right.

Actually, it's a policy decision whether or not symlinks should be 
allowed to exit space like that; in Apache, for example, it's a 
configurable.

I tend to say that the 0.99.9k (and the current master) rule to
make validation always work on what getcwd() gives back is
easier to understand (which generally means safer).  Can I talk
you into adjusting your whitelist on kernel.org machines?

I'm not happy about it, but it's not a huge deal on kernel.org. 
However, I think it's the wrong thing, especially in the light of 
allowing user-relative paths.

At the very least, if you insist on using getcwd() names, you should 
pre-canonicalize the whitelist, too.

	-hpa

On Sat, 3 Dec 2005, H. Peter Anvin wrote:

At the very least, if you insist on using getcwd() names, you should
pre-canonicalize the whitelist, too.

That would probably solve the problem and sounds like the right 
user-friendly solution.

		Linus