You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@red-bean.com> on 2006/01/22 21:14:25 UTC
Re: [Issue 2486] New - Svnserve 1.3 authz: writing on subfolder requires read access on repository root
> A check in the code, and a short discussion with David Anderson on irc learned that:
> - svnserve requires read access to the repository root for commits.
> - this behaviour is by design.
> - mod_authz_svn doesn't have this behaviour, making both implementations of the
> same authz model incompatible
Before jumping into a patch, I'd like to understand *why* there are
these two different behaviors in our servers.
David Anderson: why is the new behavior intentional?
Sander Striker: why does mod_authz_svn *not* have this behavior? is
it intentional?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 2486] New - Svnserve 1.3 authz: writing on subfolder requires
read access on repository root
Posted by Max Bowsher <ma...@ukf.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
David Anderson wrote:
> Ben Collins-Sussman wrote:
>>> A check in the code, and a short discussion with David Anderson on irc learned that:
>>> - svnserve requires read access to the repository root for commits.
>>> - this behaviour is by design.
>>> - mod_authz_svn doesn't have this behaviour, making both implementations of the
>>> same authz model incompatible
>>
>> Before jumping into a patch, I'd like to understand *why* there are
>> these two different behaviors in our servers.
>>
>> David Anderson: why is the new behavior intentional?
>
> It is intentional because, lacking any execution cap in authz, it made sense to
> me to require read access for traversing the repository prior to writing to it.
> How can someone write to a path which he couldn't actually read? Now of
> course, in the light of this bug, I realize that this point of view is flawed.
>
> I had asked at the time of modifying the commit editor about this ("Should the
> commit editor require read access to the root?"), and was told that yes, the
> commit editor should enforce this. I won't play pass the buck, as this problem
> is likely ascribable to my relatively short experience with both authz and svn
> editors when I wrote that change in.
>
> I think that the difference here is that I took 'r' to mean 'rx', whereas
> mod_authz_svn takes 'x' for granted and means 'r' litterally. In the former
> case, svnserve's behaviour is valid; in the latter case, mod_authz_svn's
> behaviour is valid.
>
> So, my take on all this is that svnserve's implementation is indeed faulty, as
> it was supposed to copy mod_authz_svn's implementation. The solution is to
> either correct svnserve (I believe the fix is a two-liner - remove read access
> check on opening directories in the commit editor), or introduce an 'x' bit that
> explicitely identifies the right to traverse directories.
Let's do the minimum fix to make svnserve behave the same as mod_*_svn
for now, and get that into 1.3.1.
Then, we can consider the need for traverse checking more carefully -
special-casing the root seems wrong to me, as surely any situation which
could apply there could also apply at any deeper place in the hierarchy.
Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
iD8DBQFD1hKbfFNSmcDyxYARAuRhAJ9PvJmeXTq4ZLvk+28zHxTC7n9YXgCgk7La
t4+16z3Z7lCwaf2JVTay7Dc=
=tLUe
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz: guidance requested)
Posted by Lieven Govaerts <lg...@mobsol.be>.
Quoting "Peter N. Lundblad" <pe...@famlundblad.se>:
> >
> The fact that it is not on trunk *now* does *not* mean it couldn't go
> into 1.3.2. Is the patch attached or referenced from the issue?
>
> Best,
> //Peter
The patch was attached to a previous mail in this thread:
http://svn.haxx.se/dev/archive-2006-03/0075.shtml
Lieven.
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder
requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz:
guidance requested)
Posted by Danny van Heumen <da...@hccnet.nl>.
Peter N. Lundblad wrote:
> The fact that it is not on trunk *now* does *not* mean it couldn't go
> into 1.3.2. Is the patch attached or referenced from the issue?
Too bad that it can't go into 1.3.1 anymore (but I completely understand
that it's too late now :-) )
I hope you guys can take a look at this patch, 'cause I'd really like to
see it in 1.3.2. (And of course it wouldn't be okay if someone went
through all the trouble to make a patch fast only to let it age forever :P)
Danny
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz: guidance requested)
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Lieven Govaerts writes:
> Quoting Danny van Heumen <da...@hccnet.nl>:
>
> It was my goal to have it fixed for 1.3.1, as so many people have been asking
> for it on the users list.
> The patch is finished but not yet committed on the trunk, so I'm afraid it will
> not be included in 1.3.2 either.
>
The fact that it is not on trunk *now* does *not* mean it couldn't go
into 1.3.2. Is the patch attached or referenced from the issue?
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz: guidance requested)
Posted by Lieven Govaerts <lg...@mobsol.be>.
Quoting Danny van Heumen <da...@hccnet.nl>:
> Lieven Govaerts wrote:
> > hereby attached is my patch for issue 2486. Referring to previous
> > discussions concerning this issue:
> > http://svn.haxx.se/dev/archive-2006-03/0003.shtml and
> > http://svn.haxx.se/dev/archive-2006-01/0704.shtml
>
> Could anyone confirm whether or not this patch is included in the
> upcoming 1.3.1 version of svn?
> The bugtracker still says 'Status: STARTED', so I'm a bit confused.
> (http://subversion.tigris.org/issues/show_bug.cgi?id=2486)
>
> I really hope it is :)
>
> Tnx,
>
> Danny
It was my goal to have it fixed for 1.3.1, as so many people have been asking
for it on the users list.
The patch is finished but not yet committed on the trunk, so I'm afraid it will
not be included in 1.3.2 either.
Lieven.
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder
requires read access on repository root (was [Issue 2486] Svnserve 1.3
authz: guidance requested)
Posted by eg <eg...@gmail.com>.
Danny van Heumen wrote:
> Lieven Govaerts wrote:
>> hereby attached is my patch for issue 2486. Referring to previous
>> discussions concerning this issue:
>> http://svn.haxx.se/dev/archive-2006-03/0003.shtml and
>> http://svn.haxx.se/dev/archive-2006-01/0704.shtml
>
> Could anyone confirm whether or not this patch is included in the
> upcoming 1.3.1 version of svn?
> The bugtracker still says 'Status: STARTED', so I'm a bit confused.
> (http://subversion.tigris.org/issues/show_bug.cgi?id=2486)
>
> I really hope it is :)
>
Take a look at: http://svn.collab.net/repos/svn/branches/1.3.x/CHANGES
An RC for 1.3.1 is in the process of being built... I dont see this
issue listed in the Changes for 1.3.1
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder
requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz:
guidance requested)
Posted by Danny van Heumen <da...@hccnet.nl>.
Lieven Govaerts wrote:
> hereby attached is my patch for issue 2486. Referring to previous
> discussions concerning this issue:
> http://svn.haxx.se/dev/archive-2006-03/0003.shtml and
> http://svn.haxx.se/dev/archive-2006-01/0704.shtml
Could anyone confirm whether or not this patch is included in the
upcoming 1.3.1 version of svn?
The bugtracker still says 'Status: STARTED', so I'm a bit confused.
(http://subversion.tigris.org/issues/show_bug.cgi?id=2486)
I really hope it is :)
Tnx,
Danny
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
[PATCH] issue 2486: Svnserve 1.3 authz: writing on subfolder requires read access on repository root (was [Issue 2486] Svnserve 1.3 authz: guidance requested)
Posted by Lieven Govaerts <lg...@mobsol.be>.
Hi,
hereby attached is my patch for issue 2486. Referring to previous
discussions concerning this issue:
http://svn.haxx.se/dev/archive-2006-03/0003.shtml and
http://svn.haxx.se/dev/archive-2006-01/0704.shtml
This patch contains these changes:
- libsvn_repos/commit.c: removed unneeded checks for read-access in
open_root and open_directory;
- tests/cmdline/authz-tests.py: new tests for this issue, test on open_root
and open_directory.
- repos-test.c: removed now obsolete white-box test
To avoid introducing functional or security issues, I did following tests:
- ran repos-test. I had to remove part of a test that calls open_directory
on a folder with no read-access expecting an error there. That isn't working
anymore, so I removed that part of the test(!).
- tested the error-messages returned when trying to access both denied (*= )
and not-existing folders to check for path-existance leaks. My tests:
repo structure /A/B/E where B is '*='.
Tested 'svn ls svn://localhost/repos/A/B/E' -> svn: Authorization failed
Tested 'svn ls svn://localhost/repos/A/B/XYZ' -> svn: Authorization failed
Tested 'svn mkdir svn://localhost/repos/A/B/E/q' -> svn: Access denied
Tested 'svn mkdir svn://localhost/repos/A/B/E/XYZ/q' -> svn: Access denied
- added the new Python authz-tests.py as a reproduction of the issue. When
run, the tests will:
* be skipped for localhost,
* succeed for http ( I tested that as well ) and
* for svnserve they succeed when the patch is applied and fail without.
These are the situations I tested but I'm sure this patch should be
thoroughly reviewed on the security part.
This issue was reported a lot on the users list since the release of svn
1.3, so I consider it important to have a fix in 1.3.1.
regards,
Lieven.
[Issue 2486] Svnserve 1.3 authz: guidance requested
Posted by Lieven Govaerts <lg...@mobsol.be>.
Hi,
I'm working on fixing issue #2486: "Svnserve 1.3 authz: writing on subfolder
requires read access on repository root".
First of all, the added patch contains a new Python test authz_tests.py
which tests on the two root causes of this issue ( open_root &
open_directory function in libsvn_repos/commit.c ). These two tests will
fail when run on svnserve 1.3, succeed with mod_authz_svn ( not tested yet )
and skip when run on local repository.
An easy way to solve this issue is by removing the check for read access in
open_root and open_directory. These were added when adding path-based authz
in svnserve and are clearly to restrictive. However, in a private email
David Anderson expressed concerns on a possible security issue with this
solution, by leaking the existance of paths (no error returned == directory
exists). These concerns are not yet included in the python tests scripts. In
other words, when I remove the read access test from open_root &
open_directory, both tests will succeed.
I'm not sure how to test for these possible security issues, so I bring this
issue back on the list, and ask your opinions on this solution.
Lieven.
> -----Original Message-----
> From: Lieven Govaerts [mailto:lgo@mobsol.be]
> Sent: zondag 19 februari 2006 18:28
> To: 'David Anderson'
> Cc: 'Ben Collins-Sussman'; dev@subversion.tigris.org; 'Sander Striker'
> Subject: RE: [Issue 2486] New - Svnserve 1.3 authz: writing
> on subfolder requires read access on repository root
>
> David,
>
> I'd like to fix this issue so it can be included in svn
> 1.3.1. I know you're busy these days, so I just need some
> information from you so I can provide a patch myself.
>
> > -----Original Message-----
> > From: David Anderson [mailto:david.anderson@calixo.net]
> > Sent: dinsdag 24 januari 2006 9:55
> ...
> >
> > So, my take on all this is that svnserve's implementation is indeed
> > faulty, as it was supposed to copy mod_authz_svn's implementation.
> > The solution is to either correct svnserve (I believe the fix is a
> > two-liner - remove read access check on opening directories in the
> > commit editor), or introduce an 'x' bit that explicitely identifies
> > the right to traverse directories.
> >
> > - Dave.
>
> If I understand you correctly, you propose to remove these lines in
> open_root():
>
> /* Check read access to root */
> SVN_ERR(check_authz (eb, "/", eb->txn_root, svn_authz_read, pool));
>
> from svn_repos/commit.c right? Let's keep adding the 'x' bit
> for later ( issue 2298 is already available for that purpose ).
>
> I know you added these lines when implementing authz for
> svnserve 1.3, so removing then will probably not have impact
> of other usage scenario's. I'll provide some test scripts to
> validate that anyhow.
>
> regards,
>
> Lieven.
RE: [Issue 2486] New - Svnserve 1.3 authz: writing on subfolder requires read access on repository root
Posted by Lieven Govaerts <lg...@mobsol.be>.
David,
I'd like to fix this issue so it can be included in svn 1.3.1. I know you're
busy these days, so I just need some information from you so I can provide a
patch myself.
> -----Original Message-----
> From: David Anderson [mailto:david.anderson@calixo.net]
> Sent: dinsdag 24 januari 2006 9:55
...
>
> So, my take on all this is that svnserve's implementation is
> indeed faulty, as it was supposed to copy mod_authz_svn's
> implementation. The solution is to either correct svnserve
> (I believe the fix is a two-liner - remove read access check
> on opening directories in the commit editor), or introduce an
> 'x' bit that explicitely identifies the right to traverse directories.
>
> - Dave.
If I understand you correctly, you propose to remove these lines in
open_root():
/* Check read access to root */
SVN_ERR(check_authz (eb, "/", eb->txn_root, svn_authz_read, pool));
from svn_repos/commit.c right? Let's keep adding the 'x' bit for later (
issue 2298 is already available for that purpose ).
I know you added these lines when implementing authz for svnserve 1.3, so
removing then will probably not have impact of other usage scenario's. I'll
provide some test scripts to validate that anyhow.
regards,
Lieven.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Issue 2486] New - Svnserve 1.3 authz: writing on subfolder requires
read access on repository root
Posted by David Anderson <da...@calixo.net>.
Ben Collins-Sussman wrote:
>>A check in the code, and a short discussion with David Anderson on irc learned that:
>>- svnserve requires read access to the repository root for commits.
>>- this behaviour is by design.
>>- mod_authz_svn doesn't have this behaviour, making both implementations of the
>>same authz model incompatible
>
>
> Before jumping into a patch, I'd like to understand *why* there are
> these two different behaviors in our servers.
>
> David Anderson: why is the new behavior intentional?
It is intentional because, lacking any execution cap in authz, it made sense to
me to require read access for traversing the repository prior to writing to it.
How can someone write to a path which he couldn't actually read? Now of
course, in the light of this bug, I realize that this point of view is flawed.
I had asked at the time of modifying the commit editor about this ("Should the
commit editor require read access to the root?"), and was told that yes, the
commit editor should enforce this. I won't play pass the buck, as this problem
is likely ascribable to my relatively short experience with both authz and svn
editors when I wrote that change in.
I think that the difference here is that I took 'r' to mean 'rx', whereas
mod_authz_svn takes 'x' for granted and means 'r' litterally. In the former
case, svnserve's behaviour is valid; in the latter case, mod_authz_svn's
behaviour is valid.
So, my take on all this is that svnserve's implementation is indeed faulty, as
it was supposed to copy mod_authz_svn's implementation. The solution is to
either correct svnserve (I believe the fix is a two-liner - remove read access
check on opening directories in the commit editor), or introduce an 'x' bit that
explicitely identifies the right to traverse directories.
- Dave.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org