You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <lg...@mobsol.be> on 2006/02/19 17:28:16 UTC
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.
---------------------------------------------------------------------
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.