You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2010/01/14 22:16:35 UTC
Multiple matching lines in authz file
What should happen when a username matches multiple lines in an authz
file? The svnbook states that the first matching line applies:
[groups]
paint-developers = frank, sally, jane
[paint:/projects/paint]
jane = r
@paint-developers = rw
Another important fact is that the first matching rule is the one
which gets applied to a user. In the prior example, even though
Jane is a member of the paint-developers group (which has
read/write access), the jane = r rule will be discovered and
matched before the group rule, thus denying Jane write access.
That's not what happens when I try it. I've stepped through in the
debugger and all lines that match get considered and the resulting
authz is the combination of all the permissions. In the above example
jane will have read/write access. Even simple authz files behave this
way:
[/]
philip =
* = r
The book implies that philip would be denied access but when I try it
read access is allowed. Is this a book bug or a Subversion bug?
If it's a Subversion bug can we fix it? Do admins want this
behaviour? Can we change the behaviour of existing authz files? I
suppose we could introduce a some sort of directive to control the
matching behaviour:
[directives]
match = first
--
Philip
Re: Multiple matching lines in authz file
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jan 27, 2010 at 01:49:18PM +0200, Mark Cammidge wrote:
> Stefan Sperling wrote:
>
> >> Does anybody object if I create an issue in the documentation issue
> >> tracker? I'll create a patch to the English documentation to go with
> >> the issue.
> >
> > Please go ahead and file an issue.
>
> http://code.google.com/p/svnbook/issues/detail?id=71
FYI, I've fixed this in the book and closed the above issue.
http://code.google.com/p/svnbook/source/detail?r=3728
Stefan
Re: Multiple matching lines in authz file
Posted by Mark Cammidge <ma...@peralex.com>.
Stefan Sperling wrote:
>> Does anybody object if I create an issue in the documentation issue
>> tracker? I'll create a patch to the English documentation to go with
>> the issue.
>
> Please go ahead and file an issue.
http://code.google.com/p/svnbook/issues/detail?id=71
Disclaimer: http://www.peralex.com/disclaimer.html
Re: Multiple matching lines in authz file
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jan 26, 2010 at 11:36:13AM +0200, Mark Cammidge wrote:
> Danil Shopyrin wrote:
>
> >> Do they? Perhaps they write them by trial-and-error, testing to see
> >> if they get the behaviour they want. In that case we will break authz
> >> files that work.
> >
> > Agree on this. From our experience, people configure access
> > permissions by trial-and-error approach. At least, they do not
> > consider the svnbook.
> >
>
> I would agree that the documentation rather than the code should be
> updated. I experienced this error in the documentation when creating
> authz rules (as did others) and found something that worked correctly.
OK then.
> Does anybody object if I create an issue in the documentation issue
> tracker? I'll create a patch to the English documentation to go with
> the issue.
Please go ahead and file an issue. Thanks!
> Disclaimer: http://www.peralex.com/disclaimer.html
This is so much nicer than putting the boilerplate into email directly.
I wish more companies feeling they have to have the boilerplate would
follow your example!
Stefan
Re: Multiple matching lines in authz file
Posted by Mark Cammidge <ma...@peralex.com>.
Danil Shopyrin wrote:
>> Do they? Perhaps they write them by trial-and-error, testing to see
>> if they get the behaviour they want. In that case we will break authz
>> files that work.
>
> Agree on this. From our experience, people configure access
> permissions by trial-and-error approach. At least, they do not
> consider the svnbook.
>
I would agree that the documentation rather than the code should be
updated. I experienced this error in the documentation when creating
authz rules (as did others) and found something that worked correctly.
Does anybody object if I create an issue in the documentation issue
tracker? I'll create a patch to the English documentation to go with
the issue.
Mark
Disclaimer: http://www.peralex.com/disclaimer.html
Re: Multiple matching lines in authz file
Posted by Danil Shopyrin <da...@visualsvn.com>.
>> Most people have probably written their authz rules according to the
>> book, so their authz rules may not work as expected.
>
> Do they? Perhaps they write them by trial-and-error, testing to see
> if they get the behaviour they want. In that case we will break authz
> files that work.
Agree on this. From our experience, people configure access
permissions by trial-and-error approach. At least, they do not
consider the svnbook.
BTW, there is a KB article for VisualSVN Server users that describes
the current behavior of Subversion authz files:
http://www.visualsvn.com/support/topic/00033/
> Can we change the behaviour of existing authz files?
I think, it's not a very good idea to change the behavior of existing
authz files. At least, it should not be done implicitly.
--
With best regards,
Danil Shopyrin
VisualSVN Team
Re: Multiple matching lines in authz file
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> This is quite a dangerous bug isn't it?
> Anyone relying on authz for confidentially would rightfully regard
> this as a security issue. I think we should fix the code.
> Most people have probably written their authz rules according to the
> book, so their authz rules may not work as expected.
Do they? Perhaps they write them by trial-and-error, testing to see
if they get the behaviour they want. In that case we will break authz
files that work.
> Can you test if the diff below fixes this?
>
> Index: subversion/libsvn_repos/authz.c
> ===================================================================
> --- subversion/libsvn_repos/authz.c (revision 899439)
> +++ subversion/libsvn_repos/authz.c (working copy)
> @@ -271,7 +271,7 @@ authz_parse_line(const char *name, const char *val
> else
> b->deny |= svn_authz_write;
>
> - return TRUE;
> + return FALSE;
> }
It doesn't appear to. I wrote the following test to verify the
current behaviour, it continues to PASS with your patch.
Index: subversion/tests/cmdline/authz_tests.py
===================================================================
--- subversion/tests/cmdline/authz_tests.py (revision 901125)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -903,6 +903,39 @@
root_url + '/A-copy/B/E/beta',
root_url + '/A-copy/C')
+
+def multiple_matches(sbox):
+ "multiple lines matching a user"
+
+ sbox.build(create_wc = False)
+ root_url = sbox.repo_url
+ write_restrictive_svnserve_conf(sbox.repo_dir)
+ if sbox.repo_url.startswith("http"):
+ expected_err = ".*[Ff]orbidden.*"
+ else:
+ expected_err = ".*svn: Authorization failed.*"
+
+ # Disable access and commit fails
+ write_authz_file(sbox, {'/': 'jrandom ='})
+ svntest.actions.run_and_verify_svn(None, None, expected_err,
+ 'cp', '-m', 'fail copy',
+ root_url, root_url + '/fail')
+
+ # At present if multiple lines match the permissions of all the
+ # matching lines are amalgamated. So jrandom gets access regardless
+ # of the line excluding access and regardless of the order of the
+ # lines. This might be a bug.
+
+ write_authz_file(sbox, {'/': 'jrandom =' + '\n' + '* = rw'})
+ svntest.main.run_svn(None, 'cp',
+ '-m', 'first copy',
+ root_url, root_url + '/first')
+
+ write_authz_file(sbox, {'/': '* = rw' + '\n' + 'jrandom ='})
+ svntest.main.run_svn(None, 'cp',
+ '-m', 'second copy',
+ root_url, root_url + '/second')
+
########################################################################
# Run the tests
@@ -928,6 +961,7 @@
svntest.main.is_ra_type_file)),
Skip(authz_access_required_at_repo_root,
svntest.main.is_ra_type_file),
+ Skip(multiple_matches, svntest.main.is_ra_type_file),
]
if __name__ == '__main__':
--
Philip
Re: Multiple matching lines in authz file
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jan 14, 2010 at 10:16:35PM +0000, Philip Martin wrote:
> What should happen when a username matches multiple lines in an authz
> file? The svnbook states that the first matching line applies:
>
> [groups]
> paint-developers = frank, sally, jane
>
> [paint:/projects/paint]
> jane = r
> @paint-developers = rw
>
> Another important fact is that the first matching rule is the one
> which gets applied to a user. In the prior example, even though
> Jane is a member of the paint-developers group (which has
> read/write access), the jane = r rule will be discovered and
> matched before the group rule, thus denying Jane write access.
>
> That's not what happens when I try it. I've stepped through in the
> debugger and all lines that match get considered and the resulting
> authz is the combination of all the permissions. In the above example
> jane will have read/write access. Even simple authz files behave this
> way:
>
> [/]
> philip =
> * = r
>
> The book implies that philip would be denied access but when I try it
> read access is allowed. Is this a book bug or a Subversion bug?
>
> If it's a Subversion bug can we fix it? Do admins want this
> behaviour? Can we change the behaviour of existing authz files?
This is quite a dangerous bug isn't it?
Anyone relying on authz for confidentially would rightfully regard
this as a security issue. I think we should fix the code.
Most people have probably written their authz rules according to the
book, so their authz rules may not work as expected.
Can you test if the diff below fixes this?
> I suppose we could introduce a some sort of directive to control the
> matching behaviour:
>
> [directives]
> match = first
Sure, but not in a patch release.
Stefan
[[[
* subversion/libsvn_repos/authz.c
(authz_parse_line): Stop parsing the authz config file after
the first line which matched the current user.
]]]
Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 899439)
+++ subversion/libsvn_repos/authz.c (working copy)
@@ -271,7 +271,7 @@ authz_parse_line(const char *name, const char *val
else
b->deny |= svn_authz_write;
- return TRUE;
+ return FALSE;
}
Re: Multiple matching lines in authz file
Posted by Eric Lee <pg...@gmail.com>.
On Fri, Jan 15, 2010 at 6:16 AM, Philip Martin <ph...@codematters.co.uk>wrote:
> What should happen when a username matches multiple lines in an authz
> file? The svnbook states that the first matching line applies:
>
> [groups]
> paint-developers = frank, sally, jane
>
> [paint:/projects/paint]
> jane = r
> @paint-developers = rw
>
> Another important fact is that the first matching rule is the one
> which gets applied to a user. In the prior example, even though
> Jane is a member of the paint-developers group (which has
> read/write access), the jane = r rule will be discovered and
> matched before the group rule, thus denying Jane write access.
>
> That's not what happens when I try it. I've stepped through in the
> debugger and all lines that match get considered and the resulting
> authz is the combination of all the permissions. In the above example
> jane will have read/write access. Even simple authz files behave this
> way:
>
> [/]
> philip =
> * = r
>
> The book implies that philip would be denied access but when I try it
> read access is allowed. Is this a book bug or a Subversion bug?
>
> If it's a Subversion bug can we fix it? Do admins want this
> behaviour? Can we change the behaviour of existing authz files? I
> suppose we could introduce a some sort of directive to control the
> matching behaviour:
>
> [directives]
> match = first
>
>
Yes, you are right. it actual permission given out was the r , w but not r
only.
I have been experience this issue staring from 1.6.4. ( I have't yet tested
it on later version)
May I ask when a patch will come up for fixing this ?
Eric,
Regards,
--
> Philip
>