You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2019/03/06 04:00:19 UTC
svn commit: r1854886 - in /subversion/branches/1.11.x: ./ STATUS
subversion/libsvn_repos/authz.h subversion/libsvn_repos/authz_info.c
subversion/libsvn_repos/authz_parse.c subversion/tests/cmdline/authz_tests.py
subversion/tests/cmdline/svnauthz_tests.py
Author: svn-role
Date: Wed Mar 6 04:00:19 2019
New Revision: 1854886
URL: http://svn.apache.org/viewvc?rev=1854886&view=rev
Log:
Merge the r1847598 group from trunk:
* r1847598, r1847697, r1847922, r1847924, r1847946
Fix SVN-4793: authz rights from inverted access selectors were not
accounted for at the global level, causing wrong authz check results.
Justification:
Fixes a regression in the new authz parser and resolver.
Notes:
- r1847598 and r1847697 are only test suite changes, but by including
them we can avoids creating a backport branch.
- r1847924 and r1847946 are tiny but IMO relevant comment tweaks.
Votes:
+1: brane, stefan2, stsp
Modified:
subversion/branches/1.11.x/ (props changed)
subversion/branches/1.11.x/STATUS
subversion/branches/1.11.x/subversion/libsvn_repos/authz.h
subversion/branches/1.11.x/subversion/libsvn_repos/authz_info.c
subversion/branches/1.11.x/subversion/libsvn_repos/authz_parse.c
subversion/branches/1.11.x/subversion/tests/cmdline/authz_tests.py
subversion/branches/1.11.x/subversion/tests/cmdline/svnauthz_tests.py
Propchange: subversion/branches/1.11.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 6 04:00:19 2019
@@ -100,4 +100,4 @@
/subversion/branches/verify-at-commit:1462039-1462408
/subversion/branches/verify-keep-going:1439280-1546110
/subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1850348,1850621
+/subversion/trunk:1840990-1840991,1840995,1840997,1841059,1841079,1841091,1841098,1841136,1841180,1841272,1841481,1841524-1841525,1841567,1841600-1841602,1841606,1841719,1841725,1841731,1841736,1841742-1841743,1841753-1841754,1841822,1841850,1841867,1842090,1842222-1842223,1842334,1842814,1842827,1842829,1842877,1843888,1844882,1844987,1845204,1845261,1845408,1845555-1845556,1845559,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847377,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621
Modified: subversion/branches/1.11.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/STATUS?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/STATUS (original)
+++ subversion/branches/1.11.x/STATUS Wed Mar 6 04:00:19 2019
@@ -93,18 +93,6 @@ Veto-blocked changes:
Approved changes:
=================
- * r1847598, r1847697, r1847922, r1847924, r1847946
- Fix SVN-4793: authz rights from inverted access selectors were not
- accounted for at the global level, causing wrong authz check results.
- Justification:
- Fixes a regression in the new authz parser and resolver.
- Notes:
- - r1847598 and r1847697 are only test suite changes, but by including
- them we can avoids creating a backport branch.
- - r1847924 and r1847946 are tiny but IMO relevant comment tweaks.
- Votes:
- +1: brane, stefan2, stsp
-
* r1851676, r1851687, r1851791
Allow the use of empty groups in authz rules.
Justification:
Modified: subversion/branches/1.11.x/subversion/libsvn_repos/authz.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/libsvn_repos/authz.h?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/libsvn_repos/authz.h (original)
+++ subversion/branches/1.11.x/subversion/libsvn_repos/authz.h Wed Mar 6 04:00:19 2019
@@ -139,6 +139,10 @@ typedef struct authz_full_t
svn_boolean_t has_authn_rights;
authz_global_rights_t authn_rights;
+ /* Globally accumulated rights from inverted selectors. */
+ svn_boolean_t has_neg_rights;
+ authz_global_rights_t neg_rights;
+
/* Globally accumulated rights, for all concrete users mentioned
in the authz file. The key is the user name, the value is
an authz_global_rights_t*. */
@@ -257,14 +261,19 @@ typedef struct authz_acl_t
/* The parsed rule. */
authz_rule_t rule;
- /* Access rights for anonymous users */
+
+ /* Access rights for anonymous users. */
svn_boolean_t has_anon_access;
authz_access_t anon_access;
- /* Access rights for authenticated users */
+ /* Access rights for authenticated users. */
svn_boolean_t has_authn_access;
authz_access_t authn_access;
+ /* Access rights from inverted selectors. */
+ svn_boolean_t has_neg_access;
+ authz_access_t neg_access;
+
/* All other user- or group-specific access rights.
Aliases are replaced with their definitions, rules for the same
user or group are merged. */
Modified: subversion/branches/1.11.x/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/libsvn_repos/authz_info.c?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/branches/1.11.x/subversion/libsvn_repos/authz_info.c Wed Mar 6 04:00:19 2019
@@ -148,37 +148,50 @@ svn_authz__get_global_rights(authz_right
{
/* Check if we have explicit rights for anonymous access. */
if (authz->has_anon_rights)
- return resolve_global_rights(rights_p, &authz->anon_rights, repos);
+ {
+ return resolve_global_rights(rights_p, &authz->anon_rights, repos);
+ }
+ else
+ {
+ /* Return the implicit rights, i.e., none. */
+ rights_p->min_access = authz_access_none;
+ rights_p->max_access = authz_access_none;
+ return FALSE;
+ }
}
else
{
+ svn_boolean_t combine_user_rights = FALSE;
+ svn_boolean_t access = FALSE;
+
/* Check if we have explicit rights for this user. */
const authz_global_rights_t *const user_rights =
svn_hash_gets(authz->user_rights, user);
if (user_rights)
{
- svn_boolean_t explicit
- = resolve_global_rights(rights_p, user_rights, repos);
-
- /* Rights given to _any_ authenticated user may apply, too. */
- if (authz->has_authn_rights)
- {
- authz_rights_t authn;
- explicit |= resolve_global_rights(&authn, &authz->authn_rights,
- repos);
- combine_rights(rights_p, rights_p, &authn);
- }
- return explicit;
+ access = resolve_global_rights(rights_p, user_rights, repos);
+ combine_user_rights = TRUE;
+ }
+ else if (authz->has_neg_rights)
+ {
+ /* Check if inverted-rule rights apply */
+ access = resolve_global_rights(rights_p, &authz->neg_rights, repos);
+ combine_user_rights = TRUE;
}
- /* Check if we have explicit rights for authenticated access. */
+ /* Rights given to _any_ authenticated user may apply, too. */
if (authz->has_authn_rights)
- return resolve_global_rights(rights_p, &authz->authn_rights, repos);
- }
+ {
+ authz_rights_t authn;
+ access |= resolve_global_rights(&authn, &authz->authn_rights, repos);
- /* Fall-through: return the implicit rights, i.e., none. */
- rights_p->min_access = authz_access_none;
- rights_p->max_access = authz_access_none;
- return FALSE;
+ if (combine_user_rights)
+ combine_rights(rights_p, rights_p, &authn);
+ else
+ *rights_p = authn;
+ }
+
+ return access;
+ }
}
Modified: subversion/branches/1.11.x/subversion/libsvn_repos/authz_parse.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/libsvn_repos/authz_parse.c?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/libsvn_repos/authz_parse.c (original)
+++ subversion/branches/1.11.x/subversion/libsvn_repos/authz_parse.c Wed Mar 6 04:00:19 2019
@@ -154,6 +154,8 @@ static const char anon_access_token[] =
/* The authenticated access token. */
static const char authn_access_token[] = "$authenticated";
+/* Fake token for inverted rights. */
+static const char neg_access_token[] = "~~$inverted";
/* Initialize a rights structure.
The minimum rights start with all available access and are later
@@ -191,6 +193,8 @@ insert_default_acl(ctor_baton_t *cb)
acl->acl.has_anon_access = TRUE;
acl->acl.authn_access = authz_access_none;
acl->acl.has_authn_access = TRUE;
+ acl->acl.neg_access = authz_access_none;
+ acl->acl.has_neg_access = TRUE;
acl->acl.user_access = NULL;
acl->aces = svn_hash__make(cb->parser_pool);
acl->alias_aces = svn_hash__make(cb->parser_pool);
@@ -208,6 +212,7 @@ create_ctor_baton(apr_pool_t *result_poo
authz_full_t *const authz = apr_pcalloc(result_pool, sizeof(*authz));
init_global_rights(&authz->anon_rights, anon_access_token, result_pool);
init_global_rights(&authz->authn_rights, authn_access_token, result_pool);
+ init_global_rights(&authz->neg_rights, neg_access_token, result_pool);
authz->user_rights = svn_hash__make(result_pool);
authz->pool = result_pool;
@@ -758,6 +763,8 @@ rules_open_section(void *baton, svn_stri
acl.acl.has_anon_access = FALSE;
acl.acl.authn_access = authz_access_none;
acl.acl.has_authn_access = FALSE;
+ acl.acl.neg_access = authz_access_none;
+ acl.acl.has_neg_access = FALSE;
acl.acl.user_access = NULL;
acl.aces = svn_hash__make(cb->parser_pool);
@@ -958,6 +965,14 @@ add_access_entry(ctor_baton_t *cb, svn_s
if (!aliased && *ace->name != '@')
prepare_global_rights(cb, ace->name);
}
+
+ /* Propagate rights for inverted selectors to the global rights, otherwise
+ an access check can bail out early. See: SVN-4793 */
+ if (inverted)
+ {
+ acl->acl.has_neg_access = TRUE;
+ acl->acl.neg_access |= access;
+ }
}
return SVN_NO_ERROR;
@@ -1271,6 +1286,12 @@ expand_acl_callback(void *baton,
update_global_rights(&cb->authz->authn_rights,
acl->rule.repos, acl->authn_access);
}
+ if (acl->has_neg_access)
+ {
+ cb->authz->has_neg_rights = TRUE;
+ update_global_rights(&cb->authz->neg_rights,
+ acl->rule.repos, acl->neg_access);
+ }
SVN_ERR(svn_iter_apr_hash(NULL, cb->authz->user_rights,
update_user_rights, acl, scratch_pool));
return SVN_NO_ERROR;
Modified: subversion/branches/1.11.x/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/tests/cmdline/authz_tests.py?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/branches/1.11.x/subversion/tests/cmdline/authz_tests.py Wed Mar 6 04:00:19 2019
@@ -1663,6 +1663,53 @@ def remove_access_after_commit(sbox):
expected_status,
[], True)
+@Issue(4793)
+@Skip(svntest.main.is_ra_type_file)
+def inverted_group_membership(sbox):
+ "access rights for user in inverted group"
+
+ sbox.build(create_wc = False)
+
+ svntest.actions.enable_revprop_changes(sbox.repo_dir)
+ write_restrictive_svnserve_conf(sbox.repo_dir)
+ write_authz_file(sbox,
+ {"/" : ("$anonymous =\n"
+ "~@readonly = rw\n"
+ "@readonly = r\n")},
+ {"groups": "readonly = %s\n" % svntest.main.wc_author2})
+
+ expected_output = svntest.verify.UnorderedOutput(['A/\n', 'iota\n'])
+
+ # User mentioned in the @readonly group can read ...
+ svntest.actions.run_and_verify_svn(expected_output, [],
+ 'list',
+ '--username', svntest.main.wc_author2,
+ sbox.repo_url)
+
+ # ... but the access control entry for the inverted group isn't applied.
+ svntest.actions.run_and_verify_svn(expected_output, [],
+ 'list',
+ '--username', svntest.main.wc_author,
+ sbox.repo_url)
+
+@Skip(svntest.main.is_ra_type_file)
+def group_member_empty_string(sbox):
+ "group definition ignores with empty member"
+
+ sbox.build(create_wc = False)
+
+ write_restrictive_svnserve_conf(sbox.repo_dir)
+ write_authz_file(sbox,
+ {"/" : ("$anonymous =\n"
+ "@readonly = r\n")},
+ {"groups": "readonly = , %s\n" % svntest.main.wc_author})
+
+ expected_output = svntest.verify.UnorderedOutput(['A/\n', 'iota\n'])
+ svntest.actions.run_and_verify_svn(expected_output, [],
+ 'list',
+ '--username', svntest.main.wc_author,
+ sbox.repo_url)
+
########################################################################
# Run the tests
@@ -1700,6 +1747,8 @@ test_list = [ None,
authz_file_external_to_authz,
authz_log_censor_revprops,
remove_access_after_commit,
+ inverted_group_membership,
+ group_member_empty_string,
]
serial_only = True
Modified: subversion/branches/1.11.x/subversion/tests/cmdline/svnauthz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.11.x/subversion/tests/cmdline/svnauthz_tests.py?rev=1854886&r1=1854885&r2=1854886&view=diff
==============================================================================
--- subversion/branches/1.11.x/subversion/tests/cmdline/svnauthz_tests.py (original)
+++ subversion/branches/1.11.x/subversion/tests/cmdline/svnauthz_tests.py Wed Mar 6 04:00:19 2019
@@ -896,6 +896,75 @@ def svnauthz_compat_mode_repo_test(sbox)
repo_url + "/zilch"
)
+
+@Issue(4793)
+def svnauthz_inverted_selector_test(sbox):
+ "test inverted selector rights"
+
+ # build an authz file
+ authz_content = ("[groups]\n"
+ "grouped = grouper\n"
+
+ "[aliases]\n"
+ "aliased = aliaser\n"
+
+ "[A:/]\n"
+ "@grouped = r\n"
+ "~@grouped = rw\n"
+
+ "[B:/]\n"
+ "&aliased = r\n"
+ "~&aliased = rw\n"
+
+ "[C:/]\n"
+ "user = r\n"
+ "~user = rw\n"
+
+ "[D:/]\n"
+ "~@grouped = rw\n"
+ "not-grouper = r\n"
+
+ "[E:/]\n"
+ "~&aliased = rw\n"
+ "not-aliaser = r\n"
+
+ "[F:/]\n"
+ "~user = rw\n"
+ "not-user = r\n")
+
+ (authz_fd, authz_path) = tempfile.mkstemp()
+ svntest.main.file_write(authz_path, authz_content)
+
+ def accessof(repos, user, expected_stdout):
+ return svntest.actions.run_and_verify_svnauthz(
+ expected_stdout, None, 0, False, 'accessof',
+ '--repository', repos, '--username', user, authz_path)
+
+ accessof('A', 'grouper', 'r')
+ accessof('A', 'not-grouper', 'rw')
+
+ accessof('B', 'aliaser', 'r')
+ accessof('B', 'not-aliaser', 'rw')
+
+ accessof('C', 'user', 'r')
+ accessof('C', 'not-user', 'rw')
+
+ accessof('D', 'grouper', 'no')
+ accessof('D', 'not-grouper', 'r')
+ accessof('D', 'other', 'rw')
+
+ accessof('E', 'aliaser', 'no')
+ accessof('E', 'not-aliaser', 'r')
+ accessof('E', 'other', 'rw')
+
+ accessof('F', 'user', 'no')
+ accessof('F', 'not-user', 'r')
+ accessof('F', 'other', 'rw')
+
+ os.close(authz_fd)
+ os.remove(authz_path)
+
+
########################################################################
# Run the tests
@@ -914,6 +983,7 @@ test_list = [ None,
svnauthz_accessof_txn_test,
svnauthz_compat_mode_file_test,
svnauthz_compat_mode_repo_test,
+ svnauthz_inverted_selector_test,
]
if __name__ == '__main__':