Opened 12 years ago
Closed 11 years ago
Last modified 10 years ago
#9976 closed defect (fixed)
Permission check of Repository Browser does not work
|Reported by:||anonymous||Owned by:||Christian Boos|
AuthzSourcePolicy slightly deviates from mod_authz_svn rules in order to show the parent folders of readable paths, so that one can actually find the latter by browsing.
Permission check of Repository Browser does not work correctly for me, is it a bug(security incident) or according to the specification?
I am using trac0.12.1 and Subversion in my project. I created a tag for my project, and set the access permissions for it. But the folders under the tag can be accessed with repository browser, even if the user doesn't have permission to access it.
And it worked well when I use svn access.
(1)create a tag named as "REL-1.0" svn copy http://host/svn/myproject/trunk/ http://host/svn/myproject/tags/REL-1.0 (2)set permissions for the tag This is part of my svnauth file after I finished setting the permissions: [myproject:/tags/REL-1.0] * = r admin = r [myproject:/tags/REL-1.0/secret] * = admin = r (3) login with a user (not admin) Access the secret folder by repository browser: http://host/trac/myproject/browser/tags/REL-1.0/secret The folders and files under "/tags/REL-1.0/secret" are displayed. I think this is not correct. (4) Verify by svn access (not admin) http://host/svn/myroject/tags/REL-2.0/secret/ The access was refused because there is not enough permission. I think this is correct
P.S. I have debugged the source code of "browser.py" and "perm.py". I found when I accessed "/tags/REL-1.0/secret" folder by repository browser, the permission of "/trunk/secret" was checked by the first line of _render_dir method in "browser.py". (node.resource) I hope the permission of "/tags/REL-1.0/secret" should be checked.
Change History (24)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Yes, I had added AuthzSourcePolicy to [trac] permission_policies. And I think I defined the repository name correctly.
comment:3 by , 12 years ago
Would you please check the following lines in browser.py?
line 540: req.perm(node.resource).require('BROWSER_VIEW') line 616: req.perm(node.resource).require('FILE_VIEW')
I found that in the object of "node.resource", there was no any information about the tag ("/tags/REL-1.0/secret") that I had accessed, instead of it, "/trunk/secret" was stored in it and was checked by the Permission check logic.(created_path)
I tried to change this by creating a new resorce object instead of "node.resource" such like:
resource = Resource('source', node.path, version=node.created_rev, parent=node.repos.resource)) req.perm(resource).require('BROWSER_VIEW')
It seemed work for me, but I don't know if the changes will affect the other programs.
comment:4 by , 12 years ago
I'm quite sure we changed to
created_path at some point for security concerns as well… ("I have
/trunk/secret, now I tagged it and everything below
/tags/1.0/secret is visible!"). Maybe we should just check both.
comment:5 by , 12 years ago
comment:6 by , 12 years ago
|Component:||general → version control/browser|
|Priority:||highest → normal|
comment:7 by , 12 years ago
("I have /trunk/secret, now I tagged it and everything below /tags/1.0/secret is visible!")
I think this is not a problem because it is the same behavior as subversion. Permission under "/tags/1.0" need to be set after it was created.
comment:8 by , 12 years ago
See #10110 for a similar use case.
comment:9 by , 12 years ago
Christian, I assume you wanted to grab this?
comment:10 by , 12 years ago
Have the same issue, but in slightly other way. Given the default model of development for Subversion I have /trunk, /branches and /tags in repository. Trunk and Branches are invisible to $anonymous users, so when software released - it is 'svn copy'ed to Tags - so anyone could grab it. Given the following AuthZ configuration:
[test:/] * = r [test:/trunk] $anonymous = [test:/branches] $anonymous =
It is impossible to access any file/folder copied from /trunk or /branches to /tags - as Node.get_perm() will check against (created_path, created_rev) and those reflects original /trunk or /branches paths - to which $anonymous does not have access.
For example we have folder
/trunk/src and have '
svn copy /trunk /tags/3.0' Here is some of debug output for accessing /tags/3.0 via "Browse Source":
Trac[main] DEBUG: Dispatching <Request "GET '/browser/test/tags/3.0'"> Trac[perm] DEBUG: AuthzSourcePolicy denies anonymous performing BROWSER_VIEW on <Resource u'repository:test, source:trunk/src@5'>
In sight of ticket:3340:
So my thinking was always to refactor the API to have:
- (rev,path) ⇒ the Node as currently seen and requested
- (created_rev, created_path) ⇒ the revision and path at which the Node got created (see above for when a Node gets created)
There should be no reason to check (created_rev, created_path) but check (rev,path) to get permissions of "Node as currently seen and requested"
Changing definition of Node's resource from:
resource = property(lambda self: Resource('source', self.created_path, version=self.created_rev, parent=self.repos.resource))
resource = property(lambda self: Resource('source', self.path, version=self.created_rev, parent=self.repos.resource))
fixes problem, but raise a question about "intended use of 'resource'" and possible security issues.
Looking forward for your comments about intended usage of 'created_path' with permission checking.
PS. trac-0.12.2, subversion-1.6.16, python-2.6.6
comment:11 by , 12 years ago
Ok, but I think that if no permissions has been set on (rev,path), then it makes sense to look at those of (created_rev, created_path).
comment:12 by , 12 years ago
I think that if no permissions has been set on (rev,path), then it makes sense to look at those of (created_rev, created_path)
Shouldn't behavior mimic the intended one for Subversion AuthZ ? If permissions set and checked on /tags - it will return the only that result and will not dig whether or when I copied path there from "secret" place. Also same behavior is crucial in having one configuration file for "Subversion AuthZ" and for "Browse Source" - you will be always sure that same user will see same resources thru SVN or thru Trac.
PS. as authz file always would have default permissions (in [module:/] or in /) you will always end up with some set of permissions for (rev, path)
comment:13 by , 12 years ago
Oh well, I think you're right: if we're trying to be "smarter" than svn authz, then you may indeed wrongly feel safe by seeing that such secret folder is not visible in Trac and forget to adjust your authz file to also get the same protection directly via svn…
follow-up: 15 comment:14 by , 12 years ago
We have tried to mimic Subversion as closely as possible, but there's at least one case where it doesn't make sense. With Subversion, you can forbid access to e.g.
/projects, and selectively allow access to
/projects/project2. Users are then able to checkout these two sub-folders, but neither
/projects/project3. If you browse
/projects on the repository, you get a "403 Forbidden".
If we copied this behavior in Trac, then
/projects wouldn't appear at all in the repository browser, and users would have no way to reach
/projects/project1. For this reason, we allow access to
/projects (the directory but not its content).
Of course, this is unrelated to the case here. It's just meant as an illustration why we don't copy Subversion completely.
follow-up: 16 comment:15 by , 12 years ago
Replying to rblank:
… If we copied this behavior in Trac, then
/projectswouldn't appear at all in the repository browser, and users would have no way to reach
/projects/project1. For this reason, we allow access to
/projects(the directory but not its content).
That's however not the behavior I observed, or maybe by "accident"…
For example if you use the trac test repository (svnrepos.dump), with the following svn access file content:
# -*- coding: utf-8 -*- [tractest:/tête/dir1] * = [tractest:/tête/dir1/dir2] * = r [tractest:/tags/v1.1/dir1] * = [tractest:/tags/v1.1/dir1/dir2] * = r
tractest/tête won't show you
tractest/tags/v1.1 will. A direct access to either
tractest/tags/v1.1/dir1 is rejected, but you can go to
If we fix the node resources to use
path instead of
created_path as suggested above in comment:10, then the observed behavior for
tags/v1.1 becomes the same as the one with
follow-up: 17 comment:16 by , 12 years ago
by , 12 years ago
fix svn authz to allow browsing a folder if there's any permission for a path below that one
comment:17 by , 12 years ago
The svn_authz-really-allow-access-to-all-parent-director.patch fixes this part.
comment:18 by , 12 years ago
comment:19 by , 11 years ago
comment:20 by , 11 years ago
This should also be fixed with 10208-requested-rev-r10910.patch:ticket:10208, and contains the suggestion from comment:10.
comment:21 by , 11 years ago
Patch applied in . Is there anything else that needs to be done here?
comment:22 by , 11 years ago
|Release Notes:||modified (diff)|
|Status:||new → closed|
comment:23 by , 10 years ago
Did you add AuthzSourcePolicy to
[trac] permission_policies, as mentioned on TracUpgrade and TracIni? Is your repository defined in Trac with the name "myproject"? The repository name must correspond to the module name used in the authz file.