Doug Robinson: Would you please cast your eye over this, as an affected
party? Basically I'd ask if you want to offer your take on fixing this
authz semantics regression?
I haven't been following this issue; I've just noticed it. On quick
inspection it seems to me after two LTS releases with changed semantics,
it might not be appropriate to change it back unconditionally in point
releases, as this could be just as bad for those who have begun relying
on the new semantics. Instead it would seem better to seek some way to
take account of both groups of users, those who are relying on each
version of the semantics. But I am not familiar with the nuances of
this particular change and how it may affect users in practice, to judge
what is best.
References:
* 2018-07-04 users@ email "svn 1.10: mod_authz no longer combines ACL
entries"
https://lists.apache.org/thread.html/1aef30b70b4063ecfa24f17be0e1c9998837b4f8b3f7e8b03de4e240@%3Cusers.subversion.apache.org%3E
* 2018-07-10 issue 4762 "authz doesn't combine global and repository rules"
https://subversion.apache.org/issue/4762
* 2020-10-01 "Fix issue #4762 ..."
http://svn.apache.org/r1882186
(Subject line changed for visibility; cc added.)
Daniel Shahaf wrote:
> stsp_at_apache.org wrote on Thu, 01 Oct 2020 19:36 +00:00:
>> Fix issue #4762 "authz doesn't combine global and repository rules"
>>
>> The new authz implementation of SVN 1.10 introduced an incompatible change
>> in the interpretation of authz rules: Global rules access were not
>> considered if per-repository access rules were also supplied.
>>
>> This change seems entirely unnecessary
>
> I'm wary of this justification. The semantics being changed here were
> covered by tests and, according to your very log message, were likely
> implemented deliberately. Reverting such semantics requires better
> arguments than "they seem unnecessary". That argument is exactly how
> the Debian OpenSSL bug was introduced.
>
> I think we should more seriously investigate the possibility that _this_
> change, r1882186, will break some use-case or another.
>
> (To be clear, I'm disagreeing only with the _justification_ for the
> change, not with the change itself. I'm not commenting on the
> change itself.)
>
>> and is still causing problems today
>> for deployments which upgrade from earlier versions, such as from SVN 1.8.
>> Existing authz files no longer produce expected results and adjusting
>> large existing authz rule files to avoid this problem is a lot of work.
>
> Sounds like this change merits an entry in the 1.14 release notes (if
> it's backported) or in the 1.15 release notes if it's not backported.
> Would you please add a placeholder (just a section header or a ToC
> link) or file a corresponding issue, so we don't forget to document
> this change?
>
>> * subversion/libsvn_repos/authz.c
>> (authz_check_access): New helper function, extracted from ...
>> (svn_repos_authz_check_access): ... here. Always consider the global
>> rule set in addition to the repository-specific one. The results
>> from both rulesets are now merged as was the case before SVN 1.10.
>>
>> * subversion/tests/cmdline/svnauthz_tests.py
>> (svnauthz_accessof_file_test,
>> svnauthz_accessof_repo_test,
>> svnauthz_accessof_groups_file_test,
>> svnauthz_accessof_groups_repo_test,
>> svnauthz_accessof_is_file_test,
>> svnauthz_accessof_is_repo_test): Adjust test expectations accordingly.
>>
>> * subversion/tests/libsvn_repos/authz-test.c
>> (reposful_reposless_stanzas_inherit): Remove XFAIL marker.
>>
>> * subversion/tests/libsvn_repos/repos-test.c
>> (test_authz_prefixes): Adjust test expectations. This test shows that
>> the behaviour change was likely deliberate. This test assumed that
>> global rules would only apply if listed after per-repository rules.
>> Change test expectations such that global rules are always taken
>> into account.
Received on 2020-10-06 13:21:30 CEST