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:10 UTC
svn commit: r1854882 - in /subversion/branches/1.10.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:10 2019
New Revision: 1854882
URL: http://svn.apache.org/viewvc?rev=1854882&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.10.x/ (props changed)
subversion/branches/1.10.x/STATUS
subversion/branches/1.10.x/subversion/libsvn_repos/authz.h
subversion/branches/1.10.x/subversion/libsvn_repos/authz_info.c
subversion/branches/1.10.x/subversion/libsvn_repos/authz_parse.c
subversion/branches/1.10.x/subversion/tests/cmdline/authz_tests.py
subversion/branches/1.10.x/subversion/tests/cmdline/svnauthz_tests.py
Propchange: subversion/branches/1.10.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Mar 6 04:00:10 2019
@@ -103,4 +103,4 @@
/subversion/branches/verify-at-commit:1462039-1462408
/subversion/branches/verify-keep-going:1439280-1546110
/subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1817837,1817856,1818577-1818578,1818584,1818651,1818662,1818727,1818801,1818803,1818807,1818868,1818871,1819036-1819037,1819043,1819049,1819052,1819093,1819146,1819162,1819444,1819556-1819557,1819603,1819804,1819911,1820044,1820046-1820047,1820518,1820627,1820718,1820778,1821183,1821224,1821621,1821678,1822401,1822587,1822591,1822996,1823202-1823203,1823211,1823327,1823791,1823966,1823989,1824033,1825024,1825045,1825215,1825266,1825306,1825709,1825711,1825721,1825736,1825778,1825783,1825787-1825788,1825979,1826720-1826721,1826747,1826811,1826814,1826877,1826907,1826971,1827105,1827114,1827191,1827562,1827574,1827670,1828613,1829012,1829015,1829241,1829260,1829344,1830083,1830882-1830883,1830885,1830900-1830901,1831110,1831112,1831540,1833465,1833621,1833836,1833842,1833864,1833866,1833895,1833897,1833899,1833901,1835760,1836306,1836762,1836802,1836960,1836963,1836968,1836976,1837037,1837790,1838813,1839662,1839703,1839734,1839739,1840991,1842260,1842262,1842264,184
3888,1844882,1844987,1845204,1845261,1845408,1845555,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847572,1847596,1850348,1850621
+/subversion/trunk:1817837,1817856,1818577-1818578,1818584,1818651,1818662,1818727,1818801,1818803,1818807,1818868,1818871,1819036-1819037,1819043,1819049,1819052,1819093,1819146,1819162,1819444,1819556-1819557,1819603,1819804,1819911,1820044,1820046-1820047,1820518,1820627,1820718,1820778,1821183,1821224,1821621,1821678,1822401,1822587,1822591,1822996,1823202-1823203,1823211,1823327,1823791,1823966,1823989,1824033,1825024,1825045,1825215,1825266,1825306,1825709,1825711,1825721,1825736,1825778,1825783,1825787-1825788,1825979,1826720-1826721,1826747,1826811,1826814,1826877,1826907,1826971,1827105,1827114,1827191,1827562,1827574,1827670,1828613,1829012,1829015,1829241,1829260,1829344,1830083,1830882-1830883,1830885,1830900-1830901,1831110,1831112,1831540,1833465,1833621,1833836,1833842,1833864,1833866,1833895,1833897,1833899,1833901,1835760,1836306,1836762,1836802,1836960,1836963,1836968,1836976,1837037,1837790,1838813,1839662,1839703,1839734,1839739,1840991,1842260,1842262,1842264,184
3888,1844882,1844987,1845204,1845261,1845408,1845555,1845577,1846299,1846403,1846406,1846704,1847181-1847182,1847188,1847264,1847572,1847596,1847598,1847697,1847922,1847924,1847946,1850348,1850621
Modified: subversion/branches/1.10.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/STATUS?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/STATUS (original)
+++ subversion/branches/1.10.x/STATUS Wed Mar 6 04:00:10 2019
@@ -87,18 +87,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.10.x/subversion/libsvn_repos/authz.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_repos/authz.h?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_repos/authz.h (original)
+++ subversion/branches/1.10.x/subversion/libsvn_repos/authz.h Wed Mar 6 04:00:10 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.10.x/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_repos/authz_info.c?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/branches/1.10.x/subversion/libsvn_repos/authz_info.c Wed Mar 6 04:00:10 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.10.x/subversion/libsvn_repos/authz_parse.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/libsvn_repos/authz_parse.c?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/libsvn_repos/authz_parse.c (original)
+++ subversion/branches/1.10.x/subversion/libsvn_repos/authz_parse.c Wed Mar 6 04:00:10 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.10.x/subversion/tests/cmdline/authz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/tests/cmdline/authz_tests.py?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/tests/cmdline/authz_tests.py (original)
+++ subversion/branches/1.10.x/subversion/tests/cmdline/authz_tests.py Wed Mar 6 04:00:10 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.10.x/subversion/tests/cmdline/svnauthz_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/1.10.x/subversion/tests/cmdline/svnauthz_tests.py?rev=1854882&r1=1854881&r2=1854882&view=diff
==============================================================================
--- subversion/branches/1.10.x/subversion/tests/cmdline/svnauthz_tests.py (original)
+++ subversion/branches/1.10.x/subversion/tests/cmdline/svnauthz_tests.py Wed Mar 6 04:00:10 2019
@@ -898,6 +898,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
@@ -916,6 +985,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__':