On Mon, Nov 12, 2012 at 4:23 PM, Ivan Zhakov <i...@visualsvn.com> wrote: > On Mon, Nov 12, 2012 at 2:28 AM, Thomas Åkesson > <thomas.akes...@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.akes...@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
