Re: Authz on Collection of Repositories

On Mon, Nov 12, 2012 at 4:23 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On Mon, Nov 12, 2012 at 2:28 AM, Thomas Åkesson
> <thomas.akesson_at_simonsoft.se> wrote:
>>
>> On 9 nov 2012, at 18:45, Ivan Zhakov wrote:
>>
>>> On Thu, Nov 8, 2012 at 6:49 PM, Thomas Åkesson
>>> <thomas.akesson_at_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