You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Thomas Åkesson <th...@akesson.cc> on 2012/10/18 12:06:56 UTC
Authz on Collection of Repositories (was: Expansion of authz policy name leak)
There was a discussion in April 2010 regarding the "fix" for issue 2753.
http://svn.haxx.se/dev/archive-2010-04/0277.shtml
Unfortunately the discussion died due to lack of other opinions. I think Mike had some very important input here and I believe that this concluding statement is incorrect:
From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Tue, 13 Apr 2010 00:35:11 +0530
> Whoever wanted SVNListParentPath to work with authz prior to this commit was using a workaround of <Location /svn/> which was leaking the same info.
Status in Subversion 1.6
- I have never seen the leak in 1.6 that Kamesh is referring to.
- It was possible to use SVNListParentPath and authz on location /svn/ with a simple RedirectMatch ensuring that browsers end up att /svn/ rather than /svn. I might have been impossible when hosting SVNListParentPath at server root, not sure.
- It was possible to restrict access to "Collection of Repositories" by controlling access to [/], while access to individual repositories were controlled by [repoN:/]. This might not have been by design, bit still a very useful feature.
- It was NOT possible to display a "Collection of Repositories" filtered to display the repositories where the user has access.
Status in Subversion 1.7
- The fix for issue 2753 presumably enables SVNListParentPath to work with authz on server root. By completely removing authz on Collection of Repositories (?).
- It is no longer possible to protect "Collection of Repositories" from groups of users. When having "Satisfy Any", required for anonymous access (* = r), the "Collection of Repositories" is open for anonymous access. Otherwise requires authn but not authz.
Use cases suffering from regression in 1.7
- The "Collection of Repositories" is leaked to all users, including for instance external consultants with access to a subset of information. There might be separate repositories for unreleased products, where leaking the repo name is undesirable.
- Servers hosting multiple repositories for different customers will leak which customers are on the same server. Can likely be worked around by explicitly blocking access to "Collection of Repositories" in Apache config.
- Difficult/impossible to combine anonymous access to a subset of information and requiring authentication to view "Collection of Repositories".
There might be workarounds for some of the use cases, but that would involve using groups in Apache config which creates an additional location where authz must be maintained.
During the 2010 discussion Mike suggested something that we (Simonsoft) would be very happy to see implemented:
> In a perfect world, I would expect that requests to the parent directory
> would not be authz-denied, but that each repository in the listing of
> repositories would be authz-checked against the authz configuration. In
> other words, say I have a parent-path with three repositories: calc, watch,
> lamp. And say I have an authz file like so:
> [lamp:/]
> * =
> I would expect that a request to the parent directory would yield a listing
> that included the 'calc' and 'watch' repositories, but not the 'lamp' one.
>
Given that we now also have AuthzSVNReposRelativeAccessFile, there is no obvious location to define access to "Collection of Repositories". By always allowing access to "Collection of Repositories" but filtering based on whether the user has access to each repository root, there is no need to explicitly set access to "Collection of Repositories". One less piece of information to maintain.
Hope this summary can spark some fresh discussion!
Regards,
Thomas Å.
Re: Authz on Collection of Repositories (was: Expansion of authz policy name leak)
Posted by Thomas Åkesson <th...@akesson.cc>.
On 24 okt 2012, at 15:37, Roderich Schupp wrote:
> On Wed, Oct 24, 2012 at 6:09 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>> Daniel Shahaf wrote on Wed, Oct 24, 2012 at 06:07:45 +0200:
>>> I can't reproduce this. 'curl -s https://svn.apache.org/repos/private/'
>> Since I didn't pass -u, in both cases I was browsing as an anonymous user.
>>> That server runs 1.7.0.
>
> SVN 1.7.7 here and I tested as an authenticated user (<Location /svn>
> does "require valid-user"). I'm not listed in the
> AuthzSVNAccessFile and it doesn't contain any * or $special rules either.
I was able to reproduce after reconfiguring from SVNParentPath to SVNPath.
- With Satisfy All (default): curl -s http://... -u username
- With Satisfy Any: curl -s http://...
Got similar output to what Roderich posted when authz configured such that user has no access.
If this behaviour appeared in SVN 1.7, then it seems likely that we are seeing another side effect of the very change that this thread started from: i.e. the "fix" for issue 2753. That fix presumably removes authz when accessing "/" regardless whether that means the list of repositories or the root of a single repository.
At minimum, this should be changed such that removal of authz on "/" only applies if referring to actual server root. That would enable hosting SVNParentPath directly on server root while reverting to 1.6 authz for the other cases: SVNParentPath in /somepath/ and SVNPath configs. Somewhat half baked since the "leak" will remain when hosting SVNParentPath directly on server root but ok for other configs.
A better solution is the earlier suggestion to filter list of repositories based on actual access to the individual repositories. That would remove the non-obvious behaviours which is somewhat important with authz.
regards,
Thomas Å.
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Roderich Schupp <ro...@gmail.com>.
On Wed, Oct 24, 2012 at 6:09 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Daniel Shahaf wrote on Wed, Oct 24, 2012 at 06:07:45 +0200:
>> I can't reproduce this. 'curl -s https://svn.apache.org/repos/private/'
> Since I didn't pass -u, in both cases I was browsing as an anonymous user.
>> That server runs 1.7.0.
SVN 1.7.7 here and I tested as an authenticated user (<Location /svn>
does "require valid-user"). I'm not listed in the
AuthzSVNAccessFile and it doesn't contain any * or $special rules either.
wget --user XXX --password YYY -O - https://liintrab.muc:4756/svn/addons/
<html><head><title>addons - Revision 149: /</title></head>
<body>
<h2>addons - Revision 149: /</h2>
<ul>
</ul>
Cheers, Roderich
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Wed, Oct 24, 2012 at 06:07:45 +0200:
> Roderich Schupp wrote on Wed, Oct 24, 2012 at 00:54:07 +0200:
> > On Wed, Oct 24, 2012 at 12:08 AM, Thomas Åkesson <th...@akesson.cc> wrote:
> > > Are you saying that SVN 1.7 always allows browsing the root but it is empty
> > > when the user lacks authz?
> >
> > Yes - for a "standalone" repository (i.e. one specified with SVNPath,
> > _not_ with SVNParentPath)
>
> I can't reproduce this. 'curl -s https://svn.apache.org/repos/private/'
> gives a directory listing that shows one world-readable directory, but
> the same command on a sibling repository (which does not contain any
> world-readable directories whatsoever) gives a 401 Unauthorized error.
> Both <Location>s use SVNPath.
>
Since I didn't pass -u, in both cases I was browsing as an anonymous user.
> That server runs 1.7.0.
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Roderich Schupp wrote on Wed, Oct 24, 2012 at 00:54:07 +0200:
> On Wed, Oct 24, 2012 at 12:08 AM, Thomas Åkesson <th...@akesson.cc> wrote:
> > Are you saying that SVN 1.7 always allows browsing the root but it is empty
> > when the user lacks authz?
>
> Yes - for a "standalone" repository (i.e. one specified with SVNPath,
> _not_ with SVNParentPath)
I can't reproduce this. 'curl -s https://svn.apache.org/repos/private/'
gives a directory listing that shows one world-readable directory, but
the same command on a sibling repository (which does not contain any
world-readable directories whatsoever) gives a 401 Unauthorized error.
Both <Location>s use SVNPath.
That server runs 1.7.0.
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Roderich Schupp <ro...@gmail.com>.
On Wed, Oct 24, 2012 at 12:08 AM, Thomas Åkesson <th...@akesson.cc> wrote:
> Are you saying that SVN 1.7 always allows browsing the root but it is empty
> when the user lacks authz?
Yes - for a "standalone" repository (i.e. one specified with SVNPath,
_not_ with SVNParentPath)
Cheers, Roderich
Re: Authz on Collection of Repositories (was: Expansion of authz policy name leak)
Posted by Thomas Åkesson <th...@akesson.cc>.
On 23 okt 2012, at 14:22, roderich.schupp@gmail.com wrote:
> I'm working on the patch to list only readable repositories. There is
> already TODO comment in the code by cmpilato:
> subversion\mod_dav_svn\repos.c:3461
>
Thanks Ivan for looking into it. Let's see if it is feasible to address.
> Please keep in mind that the problem is not restricted to parent-path collections
> of repositories: Since SVN 1.7 any user can "list" the root of a "standalone"
> repository even if she has no access grants whatsoever. Of course, the listing
> will be empty in this case (but the head revision is leaked).
Are you saying that SVN 1.7 always allows browsing the root but it is empty when the user lacks authz? When I follow a link from the parentpath repository list into a repository where I do not have access, I get a 403.
Perhaps it is possible to confirm the existence of a repository by specifically requesting the head revision from arbitrary repository names. That is not ideal but requires significantly more determination to figure out than just looking at a list.
regards,
Thomas Å.
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by "roderich.schupp@gmail.com" <ro...@gmail.com>.
>
> I'm working on the patch to list only readable repositories. There is
>
already TODO comment in the code by cmpilato:
> subversion\mod_dav_svn\repos.c:3461
>
Please keep in mind that the problem is not restricted to parent-path
collections
of repositories: Since SVN 1.7 any user can "list" the root of a
"standalone"
repository even if she has no access grants whatsoever. Of course, the
listing
will be empty in this case (but the head revision is leaked).
Cheers, Roderich
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/23/2012 08:48 AM, Stefan Sperling wrote:
> On Tue, Oct 23, 2012 at 04:29:51PM +0400, Ivan Zhakov wrote:
>>>> I'm working on the patch to list only readable repositories. There is
>>>> already TODO comment in the code by cmpilato:
>>>> subversion\mod_dav_svn\repos.c:3461
>>>> [[[
>>>> /* ### TODO: We could test for readability of the root
>>>> directory of each repository and hide those that
>>>> the user can't see. */
>
>> I'm going to create small patch to just fix this problem and probably
>> refactor later in separate commit.
>
> What about users who are allowed to see a subtree of the repository but
> not the root? Shouldn't such users be allowed to list the repository?
That would be ideal in a universe where Subversion's overall authz policy
was designed to accommodate it, but would today be entirely inconsistent
with our handling of in-repos paths. What would the repository root name
link to? A directory view they'd get 403'd on? Sorry, but at this time I
would oppose that (questionably) feature creep.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Branko Čibej <br...@wandisco.com>.
On 23.10.2012 13:48, Stefan Sperling wrote:
> On Tue, Oct 23, 2012 at 04:29:51PM +0400, Ivan Zhakov wrote:
>>>> I'm working on the patch to list only readable repositories. There is
>>>> already TODO comment in the code by cmpilato:
>>>> subversion\mod_dav_svn\repos.c:3461
>>>> [[[
>>>> /* ### TODO: We could test for readability of the root
>>>> directory of each repository and hide those that
>>>> the user can't see. */
>> I'm going to create small patch to just fix this problem and probably
>> refactor later in separate commit.
> What about users who are allowed to see a subtree of the repository but
> not the root? Shouldn't such users be allowed to list the repository?
Maybe. The access grants involved are usually called "directory
traversal" and "directory read". Anyone who has any kind of at least
read access to any subtree in the repository should implicitly have
permission to traverse the tree to the root of that subtree, but not to
list directory contents.
What you're suggesting means that directory traversal permission on the
repository root implies partial, non-inheritable directory read
permission on the virtual one level above root. We could treat this as
an exception to an otherwise more usual permission model.
The trouble I see with modelling this is that, in most access control
models, the name of a node is a property of its parent, not of the node;
therefore, "read" permission for a directory implies being allowed to
list the names of all its children, but not their attributes (e.g., in
this case, the HEAD revision of each repository). I'd actually prefer to
stick with a more or less standard access control model than invent our own.
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Oct 23, 2012 at 04:29:51PM +0400, Ivan Zhakov wrote:
> >> I'm working on the patch to list only readable repositories. There is
> >> already TODO comment in the code by cmpilato:
> >> subversion\mod_dav_svn\repos.c:3461
> >> [[[
> >> /* ### TODO: We could test for readability of the root
> >> directory of each repository and hide those that
> >> the user can't see. */
> I'm going to create small patch to just fix this problem and probably
> refactor later in separate commit.
What about users who are allowed to see a subtree of the repository but
not the root? Shouldn't such users be allowed to list the repository?
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>> I'm working on the patch to list only readable repositories. There is
>> already TODO comment in the code by cmpilato:
>> subversion\mod_dav_svn\repos.c:3461
>> [[[
>> /* ### TODO: We could test for readability of the root
>> directory of each repository and hide those that
>> the user can't see. */
>> ]]]
>
> I, too, started looking into this, Ivan, but I realized that I was probably
> about to run into a whole mess of code refactoring that I wasn't really up
> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
> I'm happy to review any work you do on this issue, though.
>
I'm going to create small patch to just fix this problem and probably
refactor later in separate commit.
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by Branko Čibej <br...@wandisco.com>.
On 02.11.2012 15:25, C. Michael Pilato wrote:
> On 11/02/2012 09:50 AM, Mark Phippard wrote:
>> On Fri, Nov 2, 2012 at 4:13 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> Looking forward for your review. Thanks!
>> + /* Build a Public Resource uri representing repository root. */
>> + uri = svn_urlpath__join(dav_svn__get_root_dir(r),
>> + svn_path_uri_encode(repos_name, pool), pool);
>> +
>> + /* Check if GET would work against this uri. */
>> + subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
>>
>> Just a drive-by, so if I am way-off just say so.
>>
>> I am assuming that since this is doing a GET, the server will have to
>> fully process it as if it would for a web browser making the same
>> request. So on a repository like the ASF or Wordpress where there are
>> a lot of top level folders then the server might have to do a fair
>> amount of work to process the request and return. I assume we do not
>> care about the content of the response, just the success or failure.
>>
>> So I am just wondering if there is a lighter weight HTTP request we
>> could do that would still trigger the authz check? Something like
>> OPTIONS or PROPFIND. Whatever would make sense and be quick to
>> process.
> I think HEAD[1] request would be the appropriate request here. (And I
> wonder, in retrospect, why we aren't using it for our regular in-repos
> path-based authz...)
>
> -- C-Mike
>
> [1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4
If you read that section in the RFC carefully, it implies that there's
no guarantee that the server would have to perform less work for a HEAD
request compared to a GET; e.g., if the GET response would contain a
content-size header, and getting the size was expensive, HEAD would
still have to calculate it. All you really gain is response bandwidth.
-- Brane
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Nov 2, 2012 at 10:09 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> So on a repository like the ASF or Wordpress where there are
>> a lot of top level folders then the server might have to do a fair
>> amount of work to process the request and return. I assume we do not
>> care about the content of the response, just the success or failure.
>>
> We already use sub requests for authorization checks in repository
> folder listing and log.
Sure, but in those cases the user is looking in the repository. Just
speculating on a worst case scenario where you have 100 repositories
that are all like the Wordpress repository and how much work it would
be to bring up the list of repositories.
>> So I am just wondering if there is a lighter weight HTTP request we
>> could do that would still trigger the authz check? Something like
>> OPTIONS or PROPFIND. Whatever would make sense and be quick to
>> process.
>>
> Another option is HEAD.
That sounds good to me.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, Nov 2, 2012 at 5:50 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Fri, Nov 2, 2012 at 4:13 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cm...@collab.net> wrote:
>>> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>>>> I'm working on the patch to list only readable repositories. There is
>>>> already TODO comment in the code by cmpilato:
>>>> subversion\mod_dav_svn\repos.c:3461
>>>> [[[
>>>> /* ### TODO: We could test for readability of the root
>>>> directory of each repository and hide those that
>>>> the user can't see. */
>>>> ]]]
>>>
>>> I, too, started looking into this, Ivan, but I realized that I was probably
>>> about to run into a whole mess of code refactoring that I wasn't really up
>>> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
>>> I'm happy to review any work you do on this issue, though.
>>>
>> Hi Mike,
>>
>> Please find attached patch to hide unreadable repositories in
>> "Collection of Repositories":
>> [[[
>> mod_dav_svn: Hide repositories from list that are not accessible for user.
>>
>> * subversion/mod_dav_svn/authz.c
>> * subversion/mod_dav_svn/dav_svn.h
>> (dav_svn__allow_list_repos): New.
>>
>> * subversion/mod_dav_svn/repos.c
>> (deliver): Check for readability of the root directory of each
>> repository and hide those that the user can't see.
>> ]]]
>>
>> Code in deliver() method is not best now, but I was trying to minimize
>> changes in my patch. I'm going to refactor code later after committing
>> my patch.
>>
>> Looking forward for your review. Thanks!
>
> + /* Build a Public Resource uri representing repository root. */
> + uri = svn_urlpath__join(dav_svn__get_root_dir(r),
> + svn_path_uri_encode(repos_name, pool), pool);
> +
> + /* Check if GET would work against this uri. */
> + subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
>
> Just a drive-by, so if I am way-off just say so.
>
> I am assuming that since this is doing a GET, the server will have to
> fully process it as if it would for a web browser making the same
> request.
I didn't details, but I assume that actual sub request execution
happens on ap_run_sub_req() call.
> So on a repository like the ASF or Wordpress where there are
> a lot of top level folders then the server might have to do a fair
> amount of work to process the request and return. I assume we do not
> care about the content of the response, just the success or failure.
>
We already use sub requests for authorization checks in repository
folder listing and log.
> So I am just wondering if there is a lighter weight HTTP request we
> could do that would still trigger the authz check? Something like
> OPTIONS or PROPFIND. Whatever would make sense and be quick to
> process.
>
Another option is HEAD.
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/16/2013 03:07 PM, Thomas Åkesson wrote:
> I think you have improved this complicated piece.
Good, 'cause that's what I committed. :-)
> Btw, I tried to convey the difficulty of combining Anonymous and
> authenticated access (you wrote about that long ago) in the Note under
> Example 2. Hope you find that description accurate.
Yup. I committed that up, too, as you hopefully saw. (Though, I broke your
three INSTALL hunks into separate commits.)
Thanks so much for your contribution!
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
On 16 jan 2013, at 20:44, C. Michael Pilato wrote:
> On 01/16/2013 02:27 PM, Thomas Åkesson wrote:
>>
>> On 16 jan 2013, at 20:15, C. Michael Pilato wrote:
>>
>>> On 01/16/2013 01:54 PM, Thomas Åkesson wrote:
>>>> Hi Ivan,
>>>>
>>>> I committed to drafting some change notes for this change quite some time
>>>> ago.
>>>>
>>>> - Below is a draft of a section to include in Release Notes. I suggest
>>>> just after "In repository authz". - Patch contains line for CHANGES -
>>>> Patch contains clarification and new example for mod_authz_svn INSTALL
>>>> file.
>>>>
>>>> Hope I got the patch right.
>>>
>>> Thanks, Thomas. I like the release notes, and will incorporate them in just
>>> a few minutes.
>>
>> Good.
>
> Actually, I have a quick question for you. Your release notes say:
>
> {{{
> The access to "Collection of Repositories" is not restricted by
> mod_authz_svn. In order to require authentication on this location, the
> location should have "Satisfy All" (default). See examples in INSTALL for
> mod_authz_svn for additional details.
> }}}
Hmm, yes... complicated.
>
> I *think* I understood what meant in calling out that this is "not
> restricted by mod_authz_svn",
Since Subversion 1.7, mod_authz_svn always returns OK for CoR, regardless what's in the authz file. Before 1.6 it was possible to control access to CoR with [/] section.
So, if the CoR should not be anonymously accessible, we must do Satisfy All + Require valid-user.
> and wordsmithed that section to read like this
> instead:
>
> {{{
> NOTE: Access to "Collection of Repositories" is not restricted by
> mod_authz_svn, but is instead managed by mod_dav_svn itself. In order to
> require authentication on this location, the location should have "Satisfy
> All" (which is the default value of this directive). See examples in
> mod_authz_svn's INSTALL document for additional details.
> }}
>
> Is this still accurate?
I think you have improved this complicated piece.
> but is instead managed by mod_dav_svn itself
Is it technically managed by mod_dav_svn? Or is it... Apache core? Probably doesn't matter.
Btw, I tried to convey the difficulty of combining Anonymous and authenticated access (you wrote about that long ago) in the Note under Example 2. Hope you find that description accurate.
/Thomas Å.
Re: Authz on Collection of Repositories
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/16/2013 02:27 PM, Thomas Åkesson wrote:
>
> On 16 jan 2013, at 20:15, C. Michael Pilato wrote:
>
>> On 01/16/2013 01:54 PM, Thomas Åkesson wrote:
>>> Hi Ivan,
>>>
>>> I committed to drafting some change notes for this change quite some time
>>> ago.
>>>
>>> - Below is a draft of a section to include in Release Notes. I suggest
>>> just after "In repository authz". - Patch contains line for CHANGES -
>>> Patch contains clarification and new example for mod_authz_svn INSTALL
>>> file.
>>>
>>> Hope I got the patch right.
>>
>> Thanks, Thomas. I like the release notes, and will incorporate them in just
>> a few minutes.
>
> Good.
Actually, I have a quick question for you. Your release notes say:
{{{
The access to "Collection of Repositories" is not restricted by
mod_authz_svn. In order to require authentication on this location, the
location should have "Satisfy All" (default). See examples in INSTALL for
mod_authz_svn for additional details.
}}}
I *think* I understood what meant in calling out that this is "not
restricted by mod_authz_svn", and wordsmithed that section to read like this
instead:
{{{
NOTE: Access to "Collection of Repositories" is not restricted by
mod_authz_svn, but is instead managed by mod_dav_svn itself. In order to
require authentication on this location, the location should have "Satisfy
All" (which is the default value of this directive). See examples in
mod_authz_svn's INSTALL document for additional details.
}}
Is this still accurate?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
On 16 jan 2013, at 20:15, C. Michael Pilato wrote:
> On 01/16/2013 01:54 PM, Thomas Åkesson wrote:
>> Hi Ivan,
>>
>> I committed to drafting some change notes for this change quite some time
>> ago.
>>
>> - Below is a draft of a section to include in Release Notes. I suggest
>> just after "In repository authz". - Patch contains line for CHANGES -
>> Patch contains clarification and new example for mod_authz_svn INSTALL
>> file.
>>
>> Hope I got the patch right.
>
> Thanks, Thomas. I like the release notes, and will incorporate them in just
> a few minutes.
Good.
>
> The CHANGES addition is misplaced -- this is a server-side bugfix, not a
> client-side one. But don't worry about submitting another patch for that alone.
Sorry, I actually intended server-side but I slipped...
Cheers,
Thomas Å.
Re: Authz on Collection of Repositories
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 01/16/2013 01:54 PM, Thomas Åkesson wrote:
> Hi Ivan,
>
> I committed to drafting some change notes for this change quite some time
> ago.
>
> - Below is a draft of a section to include in Release Notes. I suggest
> just after "In repository authz". - Patch contains line for CHANGES -
> Patch contains clarification and new example for mod_authz_svn INSTALL
> file.
>
> Hope I got the patch right.
Thanks, Thomas. I like the release notes, and will incorporate them in just
a few minutes.
The CHANGES addition is misplaced -- this is a server-side bugfix, not a
client-side one. But don't worry about submitting another patch for that alone.
And I'll review your INSTALL patch as well.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
Hi Ivan,
I committed to drafting some change notes for this change quite some time ago.
- Below is a draft of a section to include in Release Notes. I suggest just after "In repository authz".
- Patch contains line for CHANGES
- Patch contains clarification and new example for mod_authz_svn INSTALL file.
Hope I got the patch right.
Thanks,
Thomas Å.
Release notes below:
Filtering "Collection of Repositories" based on authz
When Apache is configured with the SVNParentPath directive, the "Collection of Repositories" list will now be filtered based on read access to the root of each repository. Up to now, all repositories were included in the list even if navigating to a repository would be forbidden. The "Collection of Repositories" will now be consistent with the directory lists within repositories. This provides an improved user experience by only displaying the repositories of interest to the user.
The access to "Collection of Repositories" is not restricted by mod_authz_svn. In order to require authentication on this location, the location should have "Satisfy All" (default). See examples in INSTALL for mod_authz_svn for additional details.
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
On 14 nov 2012, at 11:53, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>
>>> Confirmed as far as my testing goes (did not test short_circuit). I suggest
>>> committing the patch with GET subrequest and potentially change all to
>>> HEAD in a separate commit if there is consensus.
>> Committed in r1408184.
> I doubt about backporting this fix to 1.7.x.
>
> Pro:
> * This is regression from 1.6.x: It was possible to restrict access
> to "Collection of Repositories" by controlling access to [/], while
> access to individual repositories were controlled by [repoN:/]. This
> might not have been by design, bit still a very useful feature.
>
> * We already ported similar fix to hide unreadable dirs to 1.6.x (r996884)
>
> Cons:
> * Security behavior changes in patches is not good thing from my point view
>
>
> Any opinions?
I think it makes sense to release in 1.8 (no backport). Provides a better opportunity to explain the change. Admins on 1.6 who can not have open access to Collection of Repositories will have to skip 1.7.
I can try to draft something for the change notes, next week.
/Thomas Å.
Re: Authz on Collection of Repositories
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Nov 12, 2012 at 4:23 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Mon, Nov 12, 2012 at 2:28 AM, Thomas Åkesson
> <th...@simonsoft.se> wrote:
>>
>> On 9 nov 2012, at 18:45, Ivan Zhakov wrote:
>>
>>> On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
>>> <th...@simonsoft.se> wrote:
>>>>
>>>> Parentpath on /svn/ and Satisfy Any:
>>>>
>>>> - Access without auth displays repositories with anonymous access, auth is not requested.
>>>> - Access with auth displays filtered list. Works well when browser has previously
>>>> been on an authenticated path. This is the situation when Satisfy Any and filtered
>>>> Collection of Repositories does not work well.
>>> That's why mixing anonymous and authenticated access is not good thing.
>>
>> Yes, I am just trying to cover all bases including the possibility that people are depending on the inconsistency that we are addressing.
>>
>>>
>>>> - Did a test with AuthzSVNAnonymous Off, which gave the quite surprising result
>>>> that all content was listed both on Collection of Repositories and within the
>>>> repositories. I doubt this is the intended behaviour?!?
>>> I agree, this is really strange behavior. Could you check this
>>> behavior with my patch? It's very low chance that my patch changes
>>> this behavior.
>>
>> I have tested both with and without your patch. As expected, the patch has no impact on the AuthzSVNAnonymous issue.
>>
>> There seems to be an issue when "AuthzSVNAnonymous Off" is combined with "Satisfy Any"; opens up the fort completely. Neither authn nor authz is required.
>>
>> I think the problem is with access_checker, perhaps this part (has changed a few times during the years):
>> if (!conf->anonymous
>> || (! (conf->access_file || conf->repo_relative_access_file)))
>> return DECLINED;
>>
>> I am not quite sure how a DECLINE manages to bypass "Require valid-user" though. I understand how an OK would though.
>>
>>
>>>> - What is going on with AuthzSVNAnonymous Off? I will do more analysis of the
>>>> code (focusing on access_checker in mod_authz_svn.c) but it would be great if
>>>> someone could elaborate a bit on the intent.
>>>>
>>> It would be nice if you confirm that my patch does not change
>>> AuthzSVNAnonymous Off behavior in this case I'll commit my patch and
>>> we may focus on this issue.
>>
>> Confirmed as far as my testing goes (did not test short_circuit). I suggest
>> committing the patch with GET subrequest and potentially change all to
>> HEAD in a separate commit if there is consensus.
>>
> Committed in r1408184.
>
I doubt about backporting this fix to 1.7.x.
Pro:
* This is regression from 1.6.x: It was possible to restrict access
to "Collection of Repositories" by controlling access to [/], while
access to individual repositories were controlled by [repoN:/]. This
might not have been by design, bit still a very useful feature.
* We already ported similar fix to hide unreadable dirs to 1.6.x (r996884)
Cons:
* Security behavior changes in patches is not good thing from my point view
Any opinions?
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Nov 12, 2012 at 2:28 AM, Thomas Åkesson
<th...@simonsoft.se> wrote:
>
> On 9 nov 2012, at 18:45, Ivan Zhakov wrote:
>
>> On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
>> <th...@simonsoft.se> wrote:
>>>
>>> Parentpath on /svn/ and Satisfy Any:
>>>
>>> - Access without auth displays repositories with anonymous access, auth is not requested.
>>> - Access with auth displays filtered list. Works well when browser has previously
>>> been on an authenticated path. This is the situation when Satisfy Any and filtered
>>> Collection of Repositories does not work well.
>> That's why mixing anonymous and authenticated access is not good thing.
>
> Yes, I am just trying to cover all bases including the possibility that people are depending on the inconsistency that we are addressing.
>
>>
>>> - Did a test with AuthzSVNAnonymous Off, which gave the quite surprising result
>>> that all content was listed both on Collection of Repositories and within the
>>> repositories. I doubt this is the intended behaviour?!?
>> I agree, this is really strange behavior. Could you check this
>> behavior with my patch? It's very low chance that my patch changes
>> this behavior.
>
> I have tested both with and without your patch. As expected, the patch has no impact on the AuthzSVNAnonymous issue.
>
> There seems to be an issue when "AuthzSVNAnonymous Off" is combined with "Satisfy Any"; opens up the fort completely. Neither authn nor authz is required.
>
> I think the problem is with access_checker, perhaps this part (has changed a few times during the years):
> if (!conf->anonymous
> || (! (conf->access_file || conf->repo_relative_access_file)))
> return DECLINED;
>
> I am not quite sure how a DECLINE manages to bypass "Require valid-user" though. I understand how an OK would though.
>
>
>>> - What is going on with AuthzSVNAnonymous Off? I will do more analysis of the
>>> code (focusing on access_checker in mod_authz_svn.c) but it would be great if
>>> someone could elaborate a bit on the intent.
>>>
>> It would be nice if you confirm that my patch does not change
>> AuthzSVNAnonymous Off behavior in this case I'll commit my patch and
>> we may focus on this issue.
>
> Confirmed as far as my testing goes (did not test short_circuit). I suggest
> committing the patch with GET subrequest and potentially change all to
> HEAD in a separate commit if there is consensus.
>
Committed in r1408184.
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
On 9 nov 2012, at 18:45, Ivan Zhakov wrote:
> On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
> <th...@simonsoft.se> wrote:
>>
>> Parentpath on /svn/ and Satisfy Any:
>>
>> - Access without auth displays repositories with anonymous access, auth is not requested.
>> - Access with auth displays filtered list. Works well when browser has previously
>> been on an authenticated path. This is the situation when Satisfy Any and filtered
>> Collection of Repositories does not work well.
> That's why mixing anonymous and authenticated access is not good thing.
Yes, I am just trying to cover all bases including the possibility that people are depending on the inconsistency that we are addressing.
>
>> - Did a test with AuthzSVNAnonymous Off, which gave the quite surprising result
>> that all content was listed both on Collection of Repositories and within the
>> repositories. I doubt this is the intended behaviour?!?
> I agree, this is really strange behavior. Could you check this
> behavior with my patch? It's very low chance that my patch changes
> this behavior.
I have tested both with and without your patch. As expected, the patch has no impact on the AuthzSVNAnonymous issue.
There seems to be an issue when "AuthzSVNAnonymous Off" is combined with "Satisfy Any"; opens up the fort completely. Neither authn nor authz is required.
I think the problem is with access_checker, perhaps this part (has changed a few times during the years):
if (!conf->anonymous
|| (! (conf->access_file || conf->repo_relative_access_file)))
return DECLINED;
I am not quite sure how a DECLINE manages to bypass "Require valid-user" though. I understand how an OK would though.
>> - What is going on with AuthzSVNAnonymous Off? I will do more analysis of the
>> code (focusing on access_checker in mod_authz_svn.c) but it would be great if
>> someone could elaborate a bit on the intent.
>>
> It would be nice if you confirm that my patch does not change
> AuthzSVNAnonymous Off behavior in this case I'll commit my patch and
> we may focus on this issue.
Confirmed as far as my testing goes (did not test short_circuit). I suggest committing the patch with GET subrequest and potentially change all to HEAD in a separate commit if there is consensus.
Thanks again,
Thomas Å.
Re: Authz on Collection of Repositories
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
<th...@simonsoft.se> wrote:
> On 5 nov 2012, at 00:21, Thomas Åkesson wrote:
>>
Hi Thomas,
Thank you for comprehensive testing! See my reply inline.
>> I have meant to set up a test server with our reference configuration to validate the patch under realistic circumstances. Unfortunately, the SLES activation servers have been down for several hours (we don't have dev tools on our VM Appliance by default). I will do some tests with parentpath under "/svn/" and both variations of Satisfy as soon as possible.
>
> Right, it took a while to get that test server up and running with the dev setup. I had to refresh some knowledge.
>
> I have performed the following tests with patch 2012-11-02. All tests with access file configured and "Require valid-user".
>
> Parentpath on /svn/ and Satisfy Any:
>
> - Access without auth displays repositories with anonymous access, auth is not requested.
> - Access with auth displays filtered list. Works well when browser has previously
> been on an authenticated path. This is the situation when Satisfy Any and filtered
> Collection of Repositories does not work well.
That's why mixing anonymous and authenticated access is not good thing.
> - Did a test with AuthzSVNAnonymous Off, which gave the quite surprising result
> that all content was listed both on Collection of Repositories and within the
> repositories. I doubt this is the intended behaviour?!?
I agree, this is really strange behavior. Could you check this
behavior with my patch? It's very low chance that my patch changes
this behavior.
>
>
> Parentpath on /svn/ and Satisfy All:
>
> - Authentication is required everywhere and the Collection of Repositories is beautifully filtered. Works very well with improved user experience on many installations.
>
> AuthzSVNAnonymous seems to have no effect in this case, which is expected.
>
>
> Parentpath on /:
>
> Tested both Satisfy Any/All with same results as on /svn/. Good, I had some
> concerns since there have historically been issues.
Good.
> The remaining concerns I have:
> - The combination of this patch with Satisfy Any. I am a bit more concerned than I was initially.
> - What is going on with AuthzSVNAnonymous Off? I will do more analysis of the
> code (focusing on access_checker in mod_authz_svn.c) but it would be great if
> someone could elaborate a bit on the intent.
>
It would be nice if you confirm that my patch does not change
AuthzSVNAnonymous Off behavior in this case I'll commit my patch and
we may focus on this issue.
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@simonsoft.se>.
On 5 nov 2012, at 00:21, Thomas Åkesson wrote:
>
> I have meant to set up a test server with our reference configuration to validate the patch under realistic circumstances. Unfortunately, the SLES activation servers have been down for several hours (we don't have dev tools on our VM Appliance by default). I will do some tests with parentpath under "/svn/" and both variations of Satisfy as soon as possible.
Right, it took a while to get that test server up and running with the dev setup. I had to refresh some knowledge.
I have performed the following tests with patch 2012-11-02. All tests with access file configured and "Require valid-user".
Parentpath on /svn/ and Satisfy Any:
- Access without auth displays repositories with anonymous access, auth is not requested.
- Access with auth displays filtered list. Works well when browser has previously been on an authenticated path. This is the situation when Satisfy Any and filtered Collection of Repositories does not work well.
- Did a test with AuthzSVNAnonymous Off, which gave the quite surprising result that all content was listed both on Collection of Repositories and within the repositories. I doubt this is the intended behaviour?!?
Parentpath on /svn/ and Satisfy All:
- Authentication is required everywhere and the Collection of Repositories is beautifully filtered. Works very well with improved user experience on many installations.
AuthzSVNAnonymous seems to have no effect in this case, which is expected.
Parentpath on /:
Tested both Satisfy Any/All with same results as on /svn/. Good, I had some concerns since there have historically been issues.
The remaining concerns I have:
- The combination of this patch with Satisfy Any. I am a bit more concerned than I was initially.
- What is going on with AuthzSVNAnonymous Off? I will do more analysis of the code (focusing on access_checker in mod_authz_svn.c) but it would be great if someone could elaborate a bit on the intent.
Thanks,
Thomas Å.
Re: Authz on Collection of Repositories
Posted by Lieven Govaerts <lg...@mobsol.be>.
Mark,
On Mon, Nov 5, 2012 at 2:12 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Nov 5, 2012 at 8:07 AM, Mark Phippard <ma...@gmail.com> wrote:
>> On Mon, Nov 5, 2012 at 8:01 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
>>> On Mon, Nov 5, 2012 at 12:02 PM, Mark Phippard <ma...@gmail.com> wrote:
>>>> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>>>>
>>>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>>>>
>>>>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>>>>
>>>>> Which is precisely what I was talking about in my other post. Such HEAD
>>>>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>>>>
>>>>> -- Brane
>>>>
>>>> I thought that Serf already issues HEAD requests? Not sure about Neon.
>>>>
>>> No it doesn't, serf only sends the requests provided by svn. (except
>>> when setting up an ssl tunnel, but that's not relevant here).
>>
>> Are you talking about trunk specifically? When 1.7 was released, a
>> checkout/update done with a Serf client would issue a series of HEAD
>> and PROPFIND requests to the server. I think the changes that
>> cmpilato made to include the properties in the REPORT response removed
>> this on trunk, but I thought it was still true with 1.7.
>
> Hmm, I do not see this with current 1.7 client so maybe that was all
> changed before we released 1.7. I do see some mailing list threads
> that referenced it but maybe we were able to remove all those requests
> from the code.
>
> I recall this came up when I was raising the issue about the number of
> requests that Serf makes to the server so it is possible someone
> removed them.
>
My statement actually concerns the serf library, you are talking about ra_serf.
Sorry for the misunderstanding.
Lieven
Re: Authz on Collection of Repositories
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Nov 5, 2012 at 8:12 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Nov 5, 2012 at 8:07 AM, Mark Phippard <ma...@gmail.com> wrote:
>> On Mon, Nov 5, 2012 at 8:01 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
>>> On Mon, Nov 5, 2012 at 12:02 PM, Mark Phippard <ma...@gmail.com> wrote:
>>>> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>>>>
>>>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>>>>
>>>>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>>>>
>>>>> Which is precisely what I was talking about in my other post. Such HEAD
>>>>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>>>>
>>>>> -- Brane
>>>>
>>>> I thought that Serf already issues HEAD requests? Not sure about Neon.
>>>>
>>> No it doesn't, serf only sends the requests provided by svn. (except
>>> when setting up an ssl tunnel, but that's not relevant here).
>>
>> Are you talking about trunk specifically? When 1.7 was released, a
>> checkout/update done with a Serf client would issue a series of HEAD
>> and PROPFIND requests to the server. I think the changes that
>> cmpilato made to include the properties in the REPORT response removed
>> this on trunk, but I thought it was still true with 1.7.
>
> Hmm, I do not see this with current 1.7 client so maybe that was all
> changed before we released 1.7. I do see some mailing list threads
> that referenced it but maybe we were able to remove all those requests
> from the code.
>
> I recall this came up when I was raising the issue about the number of
> requests that Serf makes to the server so it is possible someone
> removed them.
Yep, this is the issue that removed the HEAD requests from Serf:
http://svn.apache.org/viewvc?view=revision&revision=1348131
http://subversion.tigris.org/issues/show_bug.cgi?id=4179
Anyway, my only point was that our server was already responding to
HEAD requests. Perhaps the response did not conform to the spec but
it was handling the requests already.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Authz on Collection of Repositories
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Nov 5, 2012 at 8:07 AM, Mark Phippard <ma...@gmail.com> wrote:
> On Mon, Nov 5, 2012 at 8:01 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
>> On Mon, Nov 5, 2012 at 12:02 PM, Mark Phippard <ma...@gmail.com> wrote:
>>> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>>>
>>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>>>
>>>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>>>
>>>> Which is precisely what I was talking about in my other post. Such HEAD
>>>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>>>
>>>> -- Brane
>>>
>>> I thought that Serf already issues HEAD requests? Not sure about Neon.
>>>
>> No it doesn't, serf only sends the requests provided by svn. (except
>> when setting up an ssl tunnel, but that's not relevant here).
>
> Are you talking about trunk specifically? When 1.7 was released, a
> checkout/update done with a Serf client would issue a series of HEAD
> and PROPFIND requests to the server. I think the changes that
> cmpilato made to include the properties in the REPORT response removed
> this on trunk, but I thought it was still true with 1.7.
Hmm, I do not see this with current 1.7 client so maybe that was all
changed before we released 1.7. I do see some mailing list threads
that referenced it but maybe we were able to remove all those requests
from the code.
I recall this came up when I was raising the issue about the number of
requests that Serf makes to the server so it is possible someone
removed them.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Authz on Collection of Repositories
Posted by Mark Phippard <ma...@gmail.com>.
On Mon, Nov 5, 2012 at 8:01 AM, Lieven Govaerts <lg...@mobsol.be> wrote:
> On Mon, Nov 5, 2012 at 12:02 PM, Mark Phippard <ma...@gmail.com> wrote:
>> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>>
>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>>
>>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>>
>>> Which is precisely what I was talking about in my other post. Such HEAD
>>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>>
>>> -- Brane
>>
>> I thought that Serf already issues HEAD requests? Not sure about Neon.
>>
> No it doesn't, serf only sends the requests provided by svn. (except
> when setting up an ssl tunnel, but that's not relevant here).
Are you talking about trunk specifically? When 1.7 was released, a
checkout/update done with a Serf client would issue a series of HEAD
and PROPFIND requests to the server. I think the changes that
cmpilato made to include the properties in the REPORT response removed
this on trunk, but I thought it was still true with 1.7.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Authz on Collection of Repositories
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Mon, Nov 5, 2012 at 12:02 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>
>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>
>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>
>> Which is precisely what I was talking about in my other post. Such HEAD
>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>
>> -- Brane
>
> I thought that Serf already issues HEAD requests? Not sure about Neon.
>
No it doesn't, serf only sends the requests provided by svn. (except
when setting up an ssl tunnel, but that's not relevant here).
Lieven
Re: Authz on Collection of Repositories
Posted by Branko Čibej <br...@wandisco.com>.
On 05.11.2012 12:02, Mark Phippard wrote:
> On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
>
>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>
>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>> Which is precisely what I was talking about in my other post. Such HEAD
>> responses are invalid. If we implement HEAD, we have to do it correctly.
> I thought that Serf already issues HEAD requests? Not sure about Neon.
It might do. But the problem is server-side, not client-side. I suspect
we're intentionally busting the HEAD response to save CPU cycles on the
server.
-- Brane
Re: Authz on Collection of Repositories
Posted by Mark Phippard <ma...@gmail.com>.
On Nov 5, 2012, at 3:11 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 05.11.2012 00:21, Thomas Åkesson wrote:
>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>
>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>
> Which is precisely what I was talking about in my other post. Such HEAD
> responses are invalid. If we implement HEAD, we have to do it correctly.
>
> -- Brane
I thought that Serf already issues HEAD requests? Not sure about Neon.
Mark
Re: Authz on Collection of Repositories
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Nov 5, 2012 at 5:06 PM, Lieven Govaerts <lg...@mobsol.be> wrote:
> On Mon, Nov 5, 2012 at 1:15 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Mon, Nov 5, 2012 at 12:11 PM, Branko Čibej <br...@wandisco.com> wrote:
>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>>
>>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>>
>>> Which is precisely what I was talking about in my other post. Such HEAD
>>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>>
>> I believe we use chunked responses and I assume they do not require
>> Content-Length header.
>>
>
> Not anymore.
>
> Serf uses chunked encoding for requests if the content length of the
> request body isn't known up front.
> Svn explicitly sets the content length for requests when it is in
> HTTP/1.0 mode, which is the default for a new connection.
> (see the call to serf_bucket_request_set_CL in util.c:678)
>
> So the first request on a connection will always have the content length set:
>
I meant Content-Length response header, not request none. Sorry for confusion.
--
Ivan Zhakov
Re: Authz on Collection of Repositories
Posted by Lieven Govaerts <lg...@mobsol.be>.
On Mon, Nov 5, 2012 at 1:15 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Mon, Nov 5, 2012 at 12:11 PM, Branko Čibej <br...@wandisco.com> wrote:
>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>>
>>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>>
>> Which is precisely what I was talking about in my other post. Such HEAD
>> responses are invalid. If we implement HEAD, we have to do it correctly.
>>
> I believe we use chunked responses and I assume they do not require
> Content-Length header.
>
Not anymore.
Serf uses chunked encoding for requests if the content length of the
request body isn't known up front.
Svn explicitly sets the content length for requests when it is in
HTTP/1.0 mode, which is the default for a new connection.
(see the call to serf_bucket_request_set_CL in util.c:678)
So the first request on a connection will always have the content length set:
OPTIONS /kerbsvn/repos HTTP/1.1
Host: lgo-ubuntu.local
User-Agent: SVN/1.8.0-dev serf/2.0.0
Content-Type: text/xml
Connection: keep-alive
DAV: http://subversion.tigris.org/xmlns/dav/svn/depth
DAV: http://subversion.tigris.org/xmlns/dav/svn/mergeinfo
DAV: http://subversion.tigris.org/xmlns/dav/svn/log-revprops
Content-Length: 131
<?xml version="1.0" encoding="utf-8"?><D:options
xmlns:D="DAV:"><D:activity-collection-set></D:activity-collection-set></D:options>
> --
> Ivan Zhakov
Lieven
Re: Authz on Collection of Repositories
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Mon, Nov 5, 2012 at 12:11 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 05.11.2012 00:21, Thomas Åkesson wrote:
>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>
>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>
> Which is precisely what I was talking about in my other post. Such HEAD
> responses are invalid. If we implement HEAD, we have to do it correctly.
>
I believe we use chunked responses and I assume they do not require
Content-Length header.
--
Ivan Zhakov
Re: Content-Length in HEAD responses
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sun, Nov 11, 2012 at 2:28 AM, Justin Erenkrantz
<ju...@erenkrantz.com> wrote:
> On Sat, Nov 10, 2012 at 3:25 PM, Thomas Åkesson <th...@akesson.cc> wrote:
>>
>> I suppose this means that it would be a significant optimization to
>> perform HEAD rather than GET when discovering ACLs for every subdirectory in
>> a directory listing?
>
>
> Probably - doing the HEAD request will run the full authn and authz checks,
> but it won't produce the bodies - you'll also save not having to send the
> responses on the wire - but you won't know what the directory listing is
> unless you do a GET in the first place. So, it might help at the leaf nodes
> in the tree. (But how would you know it's a leaf! Fun.)
>
I've checked in debugger and it seems creating sub request performs
auth/authz without
until call to ap_subreq_run().
--
Ivan Zhakov
Re: Content-Length in HEAD responses
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, Nov 10, 2012 at 3:25 PM, Thomas Åkesson <th...@akesson.cc> wrote:
> I suppose this means that it would be a significant optimization to
> perform HEAD rather than GET when discovering ACLs for every subdirectory
> in a directory listing?
>
Probably - doing the HEAD request will run the full authn and authz checks,
but it won't produce the bodies - you'll also save not having to send the
responses on the wire - but you won't know what the directory listing is
unless you do a GET in the first place. So, it might help at the leaf
nodes in the tree. (But how would you know it's a leaf! Fun.)
> Branko's concern is still interesting... because this behaviour (omitting
> CL for HEAD requests) does seem to violate the HTTP RFC, but for good
> reason. Given that mod_autoindex as well as mod_php (at least on the config
> I tested) also omits CL for HEAD I suppose it is a well accepted
> optimization in practice.
>
IIRC, there is an out in the RFC if the content is dynamic - this also may
be something cleaned up in the httpbis RFC clarifications as forcing the
server to generate the content only to throw it away (when it can't be
pre-computed) is kinda pointless. -- justin
Re: Content-Length in HEAD responses
Posted by Thomas Åkesson <th...@akesson.cc>.
Thanks Justin for clarifying.
On 10 nov 2012, at 12:54, Justin Erenkrantz wrote:
> On Sat, Nov 10, 2012 at 6:49 AM, Justin Erenkrantz <ju...@erenkrantz.com> wrote:
> There is a C-L header...so, I don't know what the original poster is seeing, but we're already doing the right thing... -- justin
>
> I bet the OP is trying to do HEAD on a directory as there was talk elsethread about trying to discover ACLs on a directories...that's not going to generate a C-L.
I am not sure if Original Poster refers to Branko (who raised concerns about using HEAD requests for ACLs) or myself who did some tests in response to that concern.
Yes, we are doing HEAD on directories since that's the situation where we were considering doing HEAD rather than GET requests.
> And, C-L is not meaningful in anyway on directories - only actual resources. mod_autoindex doesn't do it either:
>
> ...
>
> (The only way to generate the C-L would be to run through the response...which we don't do for HEAD.)
I suppose this means that it would be a significant optimization to perform HEAD rather than GET when discovering ACLs for every subdirectory in a directory listing?
Branko's concern is still interesting... because this behaviour (omitting CL for HEAD requests) does seem to violate the HTTP RFC, but for good reason. Given that mod_autoindex as well as mod_php (at least on the config I tested) also omits CL for HEAD I suppose it is a well accepted optimization in practice.
Thanks,
Thomas Å.
Re: Content-Length in HEAD responses
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Sat, Nov 10, 2012 at 6:49 AM, Justin Erenkrantz <ju...@erenkrantz.com>wrote:
> There is a C-L header...so, I don't know what the original poster is
> seeing, but we're already doing the right thing... -- justin
>
I bet the OP is trying to do HEAD on a directory as there was talk
elsethread about trying to discover ACLs on a directories...that's not
going to generate a C-L. And, C-L is not meaningful in anyway on
directories - only actual resources. mod_autoindex doesn't do it either:
% curl --head https://archive.apache.org/dist/
HTTP/1.1 200 OK
Date: Sat, 10 Nov 2012 11:53:36 GMT
Server: Apache/2.4.1 (Unix) OpenSSL/1.0.0g
Content-Type: text/html;charset=ISO-8859-1
(The only way to generate the C-L would be to run through the
response...which we don't do for HEAD.)
HTH. -- justin
Re: Content-Length in HEAD responses
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Fri, Nov 9, 2012 at 11:37 PM, Branko Čibej <br...@wandisco.com> wrote:
> That would imply that, if content-length doesn't get set on a HEAD
> response, but Transfer-Encoding: chunked does, then everything is
> correct, right? If somewhat inefficient.
>
The chunked header is only sent on non-HEAD responses. r->chunked is set
as a side-effect in a conditional in httpd and that conditional gets
short-circuited earlier if we are a HEAD request.
BTW, if I do:
% curl --head
https://svn.apache.org/repos/asf/subversion/trunk/subversion/mod_dav_svn/repos.c
HTTP/1.1 200 OK
Date: Sat, 10 Nov 2012 11:48:26 GMT
Server: Apache/2.2.17 (Unix) mod_ssl/2.2.17 OpenSSL/1.0.0c DAV/2
mod_wsgi/3.1 Python/2.6.6 SVN/1.7.0
Last-Modified: Fri, 19 Oct 2012 14:11:49 GMT
ETag: "1400104//subversion/trunk/subversion/mod_dav_svn/repos.c"
Accept-Ranges: bytes
Content-Length: 157969
Vary: Accept-Encoding
Content-Type: text/plain; charset=ISO-8859-1
There is a C-L header...so, I don't know what the original poster is
seeing, but we're already doing the right thing... -- justin
Re: Content-Length in HEAD responses
Posted by Branko Čibej <br...@wandisco.com>.
On 10.11.2012 04:59, Justin Erenkrantz wrote:
> On Fri, Nov 9, 2012 at 10:15 AM, C. Michael Pilato <cm...@collab.net>wrote:
>
>> request, so I'm wondering ... does Apache just handle a HEAD as a GET
>> under-the-hood and then discard the resulting response body? The comment
>>
> Unless the handler does something special, yes, httpd will treat HEAD as a
> GET until it is time to send the response body...and simply drops the
> response body.
>
That would imply that, if content-length doesn't get set on a HEAD
response, but Transfer-Encoding: chunked does, then everything is
correct, right? If somewhat inefficient.
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: Content-Length in HEAD responses
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Fri, Nov 9, 2012 at 10:15 AM, C. Michael Pilato <cm...@collab.net>wrote:
> request, so I'm wondering ... does Apache just handle a HEAD as a GET
> under-the-hood and then discard the resulting response body? The comment
>
Unless the handler does something special, yes, httpd will treat HEAD as a
GET until it is time to send the response body...and simply drops the
response body. -- justin
Re: Content-Length in HEAD responses
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/08/2012 11:42 AM, Daniel Shahaf wrote:
> Thomas Åkesson wrote on Thu, Nov 08, 2012 at 15:15:03 +0100:
>>
>> On 5 nov 2012, at 09:11, Branko Čibej wrote:
>>
>>> On 05.11.2012 00:21, Thomas Åkesson wrote:
>>>> I did some tests with curl --head just as a sanity check. It seems
>>>> to be a good choice for access control. I primarily wanted to see
>>>> that HEAD requests were not allowed in situations where GET is not
>>>> (e.g. when user has access in directories below).
>>>>
>>>> The HEAD requests I performed (minimal curl command) did not cause
>>>> the server to provide Content-Length when returning "200 OK".
>>>
>>> Which is precisely what I was talking about in my other post. Such
>>> HEAD responses are invalid. If we implement HEAD, we have to do it
>>> correctly.
>>
>> Right, I was just confirming that.
>>
>> I think this is approaching off-topic for this thread. The server
>> (mod_dav_svn) currently does respond to HEAD requests without
>> Content-Length, which appears to be invalid. Perhaps a separate
>> issue/thread should discuss whether the HEAD response should be changed
>> to conform with the specification.
>>
>
> We could also add Content-Length if it's not required but cheap to
> compute. (svn_fs_file_length())
To date, I find myself unable through code inspection to see where "we" do
anything about HEAD requests. mod_dav itself doesn't explicitly handle that
request, so I'm wondering ... does Apache just handle a HEAD as a GET
under-the-hood and then discard the resulting response body? The comment
above the #defines for request types in httpd.h leads me to believe this is
likely:
[...]
* These constants are used in bit shifting masks of size int, so it is
* unsafe to have more methods than bits in an int. HEAD == M_GET.
[...]
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Content-Length in HEAD responses (was: Re: Authz on Collection of
Repositories)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Thomas Åkesson wrote on Thu, Nov 08, 2012 at 15:15:03 +0100:
>
> On 5 nov 2012, at 09:11, Branko Čibej wrote:
>
> > On 05.11.2012 00:21, Thomas Åkesson wrote:
> >> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
> >>
> >> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
> >
> > Which is precisely what I was talking about in my other post. Such HEAD
> > responses are invalid. If we implement HEAD, we have to do it correctly.
>
> Right, I was just confirming that.
>
> I think this is approaching off-topic for this thread. The server
> (mod_dav_svn) currently does respond to HEAD requests without
> Content-Length, which appears to be invalid. Perhaps a separate
> issue/thread should discuss whether the HEAD response should be
> changed to conform with the specification.
>
We could also add Content-Length if it's not required but cheap to
compute. (svn_fs_file_length())
> On-topic, looking at the HTTP RFC, the HEAD request makes a lot of
> sense for access control purposes and that gives the server a chance
> to optimize the response even if, worst case, only the response
> bandwidth would be gained.
>
> /Thomas Å.
>
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@akesson.cc>.
On 5 nov 2012, at 09:11, Branko Čibej wrote:
> On 05.11.2012 00:21, Thomas Åkesson wrote:
>> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>>
>> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
>
> Which is precisely what I was talking about in my other post. Such HEAD
> responses are invalid. If we implement HEAD, we have to do it correctly.
Right, I was just confirming that.
I think this is approaching off-topic for this thread. The server (mod_dav_svn) currently does respond to HEAD requests without Content-Length, which appears to be invalid. Perhaps a separate issue/thread should discuss whether the HEAD response should be changed to conform with the specification.
On-topic, looking at the HTTP RFC, the HEAD request makes a lot of sense for access control purposes and that gives the server a chance to optimize the response even if, worst case, only the response bandwidth would be gained.
/Thomas Å.
Re: Authz on Collection of Repositories
Posted by Branko Čibej <br...@wandisco.com>.
On 05.11.2012 00:21, Thomas Åkesson wrote:
> I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
>
> The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
Which is precisely what I was talking about in my other post. Such HEAD
responses are invalid. If we implement HEAD, we have to do it correctly.
-- Brane
Re: Authz on Collection of Repositories
Posted by Thomas Åkesson <th...@akesson.cc>.
Thanks Ivan for your work. I have very little experience with the svn codebase so my review is probably not very valuable. Anyway. looks good to me.
I have meant to set up a test server with our reference configuration to validate the patch under realistic circumstances. Unfortunately, the SLES activation servers have been down for several hours (we don't have dev tools on our VM Appliance by default). I will do some tests with parentpath under "/svn/" and both variations of Satisfy as soon as possible.
On 2 nov 2012, at 15:25, C. Michael Pilato wrote:
> I think HEAD[1] request would be the appropriate request here. (And I
> wonder, in retrospect, why we aren't using it for our regular in-repos
> path-based authz...)
I did some tests with curl --head just as a sanity check. It seems to be a good choice for access control. I primarily wanted to see that HEAD requests were not allowed in situations where GET is not (e.g. when user has access in directories below).
The HEAD requests I performed (minimal curl command) did not cause the server to provide Content-Length when returning "200 OK".
/Thomas Å.
Re: Authz on Collection of Repositories
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/02/2012 09:50 AM, Mark Phippard wrote:
> On Fri, Nov 2, 2012 at 4:13 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> Looking forward for your review. Thanks!
>
> + /* Build a Public Resource uri representing repository root. */
> + uri = svn_urlpath__join(dav_svn__get_root_dir(r),
> + svn_path_uri_encode(repos_name, pool), pool);
> +
> + /* Check if GET would work against this uri. */
> + subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
>
> Just a drive-by, so if I am way-off just say so.
>
> I am assuming that since this is doing a GET, the server will have to
> fully process it as if it would for a web browser making the same
> request. So on a repository like the ASF or Wordpress where there are
> a lot of top level folders then the server might have to do a fair
> amount of work to process the request and return. I assume we do not
> care about the content of the response, just the success or failure.
>
> So I am just wondering if there is a lighter weight HTTP request we
> could do that would still trigger the authz check? Something like
> OPTIONS or PROPFIND. Whatever would make sense and be quick to
> process.
I think HEAD[1] request would be the appropriate request here. (And I
wonder, in retrospect, why we aren't using it for our regular in-repos
path-based authz...)
-- C-Mike
[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.4
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Nov 2, 2012 at 4:13 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cm...@collab.net> wrote:
>> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>>> I'm working on the patch to list only readable repositories. There is
>>> already TODO comment in the code by cmpilato:
>>> subversion\mod_dav_svn\repos.c:3461
>>> [[[
>>> /* ### TODO: We could test for readability of the root
>>> directory of each repository and hide those that
>>> the user can't see. */
>>> ]]]
>>
>> I, too, started looking into this, Ivan, but I realized that I was probably
>> about to run into a whole mess of code refactoring that I wasn't really up
>> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
>> I'm happy to review any work you do on this issue, though.
>>
> Hi Mike,
>
> Please find attached patch to hide unreadable repositories in
> "Collection of Repositories":
> [[[
> mod_dav_svn: Hide repositories from list that are not accessible for user.
>
> * subversion/mod_dav_svn/authz.c
> * subversion/mod_dav_svn/dav_svn.h
> (dav_svn__allow_list_repos): New.
>
> * subversion/mod_dav_svn/repos.c
> (deliver): Check for readability of the root directory of each
> repository and hide those that the user can't see.
> ]]]
>
> Code in deliver() method is not best now, but I was trying to minimize
> changes in my patch. I'm going to refactor code later after committing
> my patch.
>
> Looking forward for your review. Thanks!
+ /* Build a Public Resource uri representing repository root. */
+ uri = svn_urlpath__join(dav_svn__get_root_dir(r),
+ svn_path_uri_encode(repos_name, pool), pool);
+
+ /* Check if GET would work against this uri. */
+ subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
Just a drive-by, so if I am way-off just say so.
I am assuming that since this is doing a GET, the server will have to
fully process it as if it would for a web browser making the same
request. So on a repository like the ASF or Wordpress where there are
a lot of top level folders then the server might have to do a fair
amount of work to process the request and return. I assume we do not
care about the content of the response, just the success or failure.
So I am just wondering if there is a lighter weight HTTP request we
could do that would still trigger the authz check? Something like
OPTIONS or PROPFIND. Whatever would make sense and be quick to
process.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cm...@collab.net> wrote:
> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>> I'm working on the patch to list only readable repositories. There is
>> already TODO comment in the code by cmpilato:
>> subversion\mod_dav_svn\repos.c:3461
>> [[[
>> /* ### TODO: We could test for readability of the root
>> directory of each repository and hide those that
>> the user can't see. */
>> ]]]
>
> I, too, started looking into this, Ivan, but I realized that I was probably
> about to run into a whole mess of code refactoring that I wasn't really up
> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
> I'm happy to review any work you do on this issue, though.
>
Hi Mike,
Please find attached patch to hide unreadable repositories in
"Collection of Repositories":
[[[
mod_dav_svn: Hide repositories from list that are not accessible for user.
* subversion/mod_dav_svn/authz.c
* subversion/mod_dav_svn/dav_svn.h
(dav_svn__allow_list_repos): New.
* subversion/mod_dav_svn/repos.c
(deliver): Check for readability of the root directory of each
repository and hide those that the user can't see.
]]]
Code in deliver() method is not best now, but I was trying to minimize
changes in my patch. I'm going to refactor code later after committing
my patch.
Looking forward for your review. Thanks!
--
Ivan Zhakov
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
> I'm working on the patch to list only readable repositories. There is
> already TODO comment in the code by cmpilato:
> subversion\mod_dav_svn\repos.c:3461
> [[[
> /* ### TODO: We could test for readability of the root
> directory of each repository and hide those that
> the user can't see. */
> ]]]
I, too, started looking into this, Ivan, but I realized that I was probably
about to run into a whole mess of code refactoring that I wasn't really up
for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
I'm happy to review any work you do on this issue, though.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Oct 18, 2012 at 2:06 PM, Thomas Åkesson <th...@akesson.cc> wrote:
> There was a discussion in April 2010 regarding the "fix" for issue 2753.
> http://svn.haxx.se/dev/archive-2010-04/0277.shtml
>
[...]
>
> During the 2010 discussion Mike suggested something that we (Simonsoft)
> would be very happy to see implemented:
>
> In a perfect world, I would expect that requests to the parent directory
> would not be authz-denied, but that each repository in the listing of
> repositories would be authz-checked against the authz configuration. In
> other words, say I have a parent-path with three repositories: calc, watch,
> lamp. And say I have an authz file like so:
>
> [lamp:/]
> * =
>
> I would expect that a request to the parent directory would yield a listing
> that included the 'calc' and 'watch' repositories, but not the 'lamp' one.
>
>
Hi Thomas,
I'm working on the patch to list only readable repositories. There is
already TODO comment in the code by cmpilato:
subversion\mod_dav_svn\repos.c:3461
[[[
/* ### TODO: We could test for readability of the root
directory of each repository and hide those that
the user can't see. */
]]]
--
Ivan Zhakov
Re: Authz on Collection of Repositories (was: Expansion of authz
policy name leak)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Thomas Åkesson wrote on Mon, Oct 22, 2012 at 17:20:44 +0200:
> On 19 okt 2012, at 02:07, Daniel Shahaf wrote:
> > This is complicated by:
> >
> > - THe DAV protocol does not prompt for authentication for resources
> > readable by anonymous (for this, see cmpilato's old "foo-no-anon"
> > blog post).
>
> Hmm, I failed to get any hit on that blog post (tried both Google and DuckDuckGo).
>
http://blogs.collab.net/subversion/2007/03/authz_and_anon_/
Re: Authz on Collection of Repositories (was: Expansion of authz policy name leak)
Posted by Thomas Åkesson <th...@akesson.cc>.
To clarify what this issue is about:
Subversion 1.7 leaks repository names when configured with SVNListParentPath and AuthzSVNAccessFile. It might have been unintentional, but with Subversion 1.6 (and earlier) it was possible to control access to the repository list (Collection of Repositories) using the following section in access file: [/]
This is a quite unexpected change when upgrading an existing server to version 1.7, which is not mentioned in changes. It will suddenly provide unauthenticated access to the repository list (when Satisfy Any) or provide access to users that were not intended to have access (Satisfy All).
On 19 okt 2012, at 02:07, Daniel Shahaf wrote:
> I agree it is reasonable to want the SVNParentPath URL to display only
> repositories one has some access to.
>
> This is complicated by:
>
> - THe DAV protocol does not prompt for authentication for resources
> readable by anonymous (for this, see cmpilato's old "foo-no-anon"
> blog post).
Hmm, I failed to get any hit on that blog post (tried both Google and DuckDuckGo).
Right, previously it was possible to request authentication when accessing the repository list via the access file, e.g.:
[/]
@GroupA = r
The requirement to hide/filter the repository list will be most prevalent with server configurations that always requires authentication, typically corporate subversion servers or in the hosting use case. Anonymous access on the other hand is more prevalent in the open-source use case where the feature to hide repositories is likely very rare.
The Apache directive Satisfy Any is required in order to combine access control and anonymous access (* = r). When removing Satisfy Any (i.e. Satisfy All) Apache ensures that clients are required to authenticate in order to view the repository list. This would ensure that the repository list can be correctly filtered by mod_authz_svn.
In summary, it would not be possible to have both anonymous access and a filtered repository list on the same Apache location, but that is a relatively rare requirement and not a regression. (well, perhaps it is possible with some creative Apache config).
>
> - It's possible that someone would have r or rw access to ^/bar/baz,
> but no access at all to ^/ or ^/foo/. When this is the case, should
> that repository be listed in the SVNParentPath screen? And if it's
> listed, what should happen when the link is followed?
No, it should not be listed. In this case, the user cannot browse from repository list down to the location were access is granted anyway (since he has not access to ^/bar/).
> And if it's listed, what should happen when the link is followed?
The repository is listed in both Svn 1.7, 1.6 and earlier. Access is denied if followed. The difference is that with filtering, the link will not even be displayed. An improvement if you ask me and more consistent with how access control in the repository works.
> These are my thoughts at a high level --- i.e., user-facing level, not
> implementation level. I'm not extremely familiar with the details of
> the thread, the parentpath code, or issue #2753.
I am fairly familiar with Apache config and the typical requirements of corporate installations, but not with the code. By looking at svn log C-Mike, Philip and Kamesh might be best suited to comment on the code.
regards,
Thomas Å.