You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2015/01/09 11:56:14 UTC

svn commit: r1650496 - in /subversion/branches/authzperf: BRANCH-README subversion/libsvn_repos/authz.c subversion/libsvn_repos/authz.h subversion/libsvn_repos/authz_info.c subversion/libsvn_repos/authz_parse.c subversion/tests/libsvn_repos/authz-test.c

Author: brane
Date: Fri Jan  9 10:56:13 2015
New Revision: 1650496

URL: http://svn.apache.org/r1650496
Log:
On the authzperf branch: Replace svn_repos_authz_access_t in the authz
implementation with a private enumeration that can be extended to
support lookup/travers access rights. Lookup access should not be
exposed in the public access check callback API.

* BRANCH-README: Update goals and todo/done lists.

* subversion/libsvn_repos/authz.h
  (authz_access_t): New enumeration; equivalent to, but intentionally
   incompatible with, svn_repos_authz_access_t.
  (authz_rights_t, authz_acl_t, authz_ace_t): Replace instances of
   svn_repos_authz_access_t with authz_access_t.

* subversion/libsvn_repos/authz_info.c,
  subversion/libsvn_repos/authz_parse.c:
   Replace instances of svn_repos_authz_access_t with authz_access_t.
* subversion/libsvn_repos/authz.c:
   Replace instances of svn_repos_authz_access_t with authz_access_t.
  (svn_repos_authz_check_access): Convert the flags from the required_access
   parameter to the internal authz_access_t representation.

Modified:
    subversion/branches/authzperf/BRANCH-README
    subversion/branches/authzperf/subversion/libsvn_repos/authz.c
    subversion/branches/authzperf/subversion/libsvn_repos/authz.h
    subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
    subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c
    subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c

Modified: subversion/branches/authzperf/BRANCH-README
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/BRANCH-README?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/BRANCH-README (original)
+++ subversion/branches/authzperf/BRANCH-README Fri Jan  9 10:56:13 2015
@@ -2,6 +2,10 @@ This branch (^/subversion/branches/authz
 search engine. Er ... that is, reimplement svn_authz_t & co. so that it
 allows much faster authz resolving and avoids the many full authz file scans.
 
+The new authz implementation on this branch will also be used for
+implementing lookup access rights (a.k.a. directory traversal rights),
+which are implicit in current released and trunk authz semantics.
+
 More information may be found here:
 https://wiki.apache.org/subversion/AuthzImprovements
 
@@ -9,6 +13,7 @@ TODO:
 
 * implement precedence rules
 * remove no-op escape sequences from wildcard rule segments
+* implement lookup access rights
 
 DONE:
 
@@ -31,3 +36,5 @@ DONE:
 * rewrite the filtered rules generator to use svn_authz_tng_t
 * rename svn_authz_tng_t to svn_authz_t
 * implement wildcard escaping in glob rules
+* in the implementation, replace the public access rights enum
+  with a private variant, in preparation for adding lookup access rights.

Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz.c?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz.c Fri Jan  9 10:56:13 2015
@@ -62,7 +62,7 @@ typedef struct access_t
   int sequence_number;
 
   /* Access rights of the respective user as defined by the rule set. */
-  svn_repos_authz_access_t rights;
+  authz_access_t rights;
 } access_t;
 
 /* Use this to indicate that no sequence ID has been assigned.
@@ -81,11 +81,11 @@ typedef struct limited_rights_t
 
   /* Minimal access rights that the user has on this or any other node in 
    * the sub-tree. */
-  svn_repos_authz_access_t min_rights;
+  authz_access_t min_rights;
 
   /* Maximal access rights that the user has on this or any other node in 
    * the sub-tree. */
-  svn_repos_authz_access_t max_rights;
+  authz_access_t max_rights;
 
 } limited_rights_t;
 
@@ -699,7 +699,7 @@ create_user_authz(svn_authz_t *authz,
   if (!has_local_rule(&root->rights))
     {
       root->rights.access.sequence_number = 0;
-      root->rights.access.rights = svn_authz_none;
+      root->rights.access.rights = authz_access_none;
     }
 
   /* Calculate recursive rights etc. */
@@ -707,8 +707,8 @@ create_user_authz(svn_authz_t *authz,
   finalize_up_tree(root, &root->rights.access, root, subpool);
 
   svn_pool_clear(subpool);
-  var_rights.max_rights = svn_authz_none;
-  var_rights.min_rights = svn_authz_read | svn_authz_write;
+  var_rights.max_rights = authz_access_none;
+  var_rights.min_rights = authz_access_write;
   finalize_down_tree(root, var_rights, subpool);
 
   /* Done. */
@@ -983,7 +983,7 @@ next_segment(svn_stringbuf_t *segment,
 static svn_boolean_t
 lookup(lookup_state_t *state,
        const char *path,
-       svn_repos_authz_access_t required,
+       authz_access_t required,
        svn_boolean_t recursive,
        apr_pool_t *scratch_pool)
 {
@@ -1028,14 +1028,14 @@ lookup(lookup_state_t *state,
       /* Initial state for this segment. */
       apr_array_clear(state->next);
       state->rights.access.sequence_number = NO_SEQUENCE_NUMBER;
-      state->rights.access.rights = svn_authz_none;
+      state->rights.access.rights = authz_access_none;
 
       /* These init values ensure that the first node's value will be used
        * when combined with them.  If there is no first node,
        * state->access.sequence_number remains unchanged and we will use
        * the parent's (i.e. inherited) access rights. */
-      state->rights.min_rights = svn_authz_read | svn_authz_write;
-      state->rights.max_rights = svn_authz_none;
+      state->rights.min_rights = authz_access_write;
+      state->rights.max_rights = authz_access_none;
 
       /* Update the PARENT_PATH member in STATE to match the nodes in
        * CURRENT at the end of this iteration, i.e. if and when NEXT
@@ -1504,6 +1504,10 @@ svn_repos_authz_check_access(svn_authz_t
                              svn_boolean_t *access_granted,
                              apr_pool_t *pool)
 {
+  const authz_access_t required =
+    ((required_access & svn_authz_read ? authz_access_read_flag : 0)
+     | (required_access & svn_authz_write ? authz_access_write_flag : 0));
+
   /* Pick or create the suitable pre-filtered path rule tree. */
   authz_user_rules_t *rules = get_filtered_tree(
       authz,
@@ -1513,19 +1517,23 @@ svn_repos_authz_check_access(svn_authz_t
   /* If PATH is NULL, check if the user has *any* access. */
   if (!path)
     {
-      svn_repos_authz_access_t required = required_access & ~svn_authz_recursive;
-      *access_granted = (rules->root->rights.max_rights & required) == required;
+      *access_granted =
+        ((rules->root->rights.max_rights & required) == required);
       return SVN_NO_ERROR;
     }
+  else
+    {
+      const svn_boolean_t recursive =
+        !!(required_access & svn_authz_recursive);
 
-  /* Sanity check. */
-  SVN_ERR_ASSERT(path[0] == '/');
+      /* Sanity check. */
+      SVN_ERR_ASSERT(path[0] == '/');
 
-  /* Determine the granted access for the requested path.
-   * PATH does not need to be normalized for lockup(). */
-  *access_granted = lookup(rules->lookup_state, path,
-                           required_access & ~svn_authz_recursive,
-                           required_access & svn_authz_recursive, pool);
+      /* Determine the granted access for the requested path.
+       * PATH does not need to be normalized for lockup(). */
+      *access_granted = lookup(rules->lookup_state, path,
+                               required, recursive, pool);
+    }
 
   return SVN_NO_ERROR;
 }

Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz.h
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz.h?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz.h (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz.h Fri Jan  9 10:56:13 2015
@@ -60,16 +60,60 @@ extern "C" {
 typedef struct authz_user_rules_t authz_user_rules_t;
 
 
+/* Access rights in an ACL.
+ *
+ * This enum is different from and incompatible with
+ * svn_repos_authz_access_t, because it has different semantics and
+ * encodes rights that are not and should never be exposed in the
+ * public API.
+ */
+typedef enum authz_access_t
+{
+  /*
+   * Individual access flags
+   */
+
+  /* TODO: Future extension for lookup/traverse access.
+  authz_access_lookup_flag = 0x10, */
+
+  /* Read access allows listing directory entries, reading file
+     contents and reading properties of files and directories. */
+  authz_access_read_flag = 0x20,
+
+  /* Write access allows adding, removing and renaming directory
+     entries, modifying file contents and adding, removing and
+     modifying properties of files and directories. */
+  authz_access_write_flag = 0x40,
+
+  /*
+   * Combined access flags
+   */
+
+  /* No access. */
+  authz_access_none = 0,
+
+
+  /* TODO: Lookup access is a synonym for the lookup flag.
+  authz_access_lookup = authz_access_lookup_flag, */
+
+  /* Read access (TODO: implies lookup access). */
+  authz_access_read = authz_access_read_flag /* TODO: | authz_access_lookup */,
+
+  /* Write access implies read (TODO: and lookup) access. */
+  authz_access_write = authz_access_write_flag | authz_access_read
+} authz_access_t;
+
+
 /* Accumulated rights for (user, repository). */
 typedef struct authz_rights_t
 {
   /* The lowest level of access that the user has to every
      path in the repository. */
-  svn_repos_authz_access_t min_access;
+  authz_access_t min_access;
 
   /* The highest level of access that the user has to
      any path in the repository. */
-  svn_repos_authz_access_t max_access;
+  authz_access_t max_access;
 } authz_rights_t;
 
 
@@ -207,11 +251,11 @@ typedef struct authz_acl_t
 
   /* Access rights for anonymous users */
   svn_boolean_t has_anon_access;
-  svn_repos_authz_access_t anon_access;
+  authz_access_t anon_access;
 
   /* Access rights for authenticated users */
   svn_boolean_t has_authn_access;
-  svn_repos_authz_access_t authn_access;
+  authz_access_t authn_access;
 
   /* All other user- or group-specific access rights.
      Aliases are replaced with their definitions, rules for the same
@@ -236,7 +280,7 @@ typedef struct authz_ace_t
   svn_boolean_t inverted;
 
   /* The access rights defined by this ACE. */
-  svn_repos_authz_access_t access;
+  authz_access_t access;
 } authz_ace_t;
 
 
@@ -281,7 +325,7 @@ svn_authz__compare_rules(const authz_rul
  * the user in this repository.
  */
 svn_boolean_t
-svn_authz__get_acl_access(svn_repos_authz_access_t *access,
+svn_authz__get_acl_access(authz_access_t *access,
                           const authz_acl_t *acl,
                           const char *user, const char *repos);
 

Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz_info.c Fri Jan  9 10:56:13 2015
@@ -32,11 +32,11 @@
 
 
 svn_boolean_t
-svn_authz__get_acl_access(svn_repos_authz_access_t *access_p,
+svn_authz__get_acl_access(authz_access_t *access_p,
                           const authz_acl_t *acl,
                           const char *user, const char *repos)
 {
-  svn_repos_authz_access_t access;
+  authz_access_t access;
   svn_boolean_t has_access;
   int i;
 
@@ -59,7 +59,7 @@ svn_authz__get_acl_access(svn_repos_auth
 
   /* Get the access rights for all authenticated users. */
   has_access = acl->has_authn_access;
-  access = (has_access ? acl->authn_access : svn_authz_none);
+  access = (has_access ? acl->authn_access : authz_access_none);
 
   /* Scan the ACEs in the ACL and merge the access rights. */
   for (i = 0; i < acl->user_access->nelts; ++i)
@@ -150,7 +150,7 @@ svn_authz__get_global_rights(authz_right
     }
 
   /* Fall-through: return the implicit rights, i.e., none. */
-  rights_p->min_access = svn_authz_none;
-  rights_p->max_access = svn_authz_none;
+  rights_p->min_access = authz_access_none;
+  rights_p->max_access = authz_access_none;
   return FALSE;
 }

Modified: subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c (original)
+++ subversion/branches/authzperf/subversion/libsvn_repos/authz_parse.c Fri Jan  9 10:56:13 2015
@@ -161,8 +161,8 @@ static const char authn_access_token[] =
    empty and are later bitwise-and'ed with actual rights. */
 static void init_rights(authz_rights_t *rights)
 {
-  rights->min_access = svn_authz_read | svn_authz_write;
-  rights->max_access = svn_authz_none;
+  rights->min_access = authz_access_write;
+  rights->max_access = authz_access_none;
  }
 
 /* Initialize a global rights structure.
@@ -187,9 +187,9 @@ insert_default_acl(ctor_baton_t *cb)
   acl->acl.rule.repos = interned_empty_string;
   acl->acl.rule.len = 0;
   acl->acl.rule.path = NULL;
-  acl->acl.anon_access = svn_authz_none;
+  acl->acl.anon_access = authz_access_none;
   acl->acl.has_anon_access = TRUE;
-  acl->acl.authn_access = svn_authz_none;
+  acl->acl.authn_access = authz_access_none;
   acl->acl.has_authn_access = TRUE;
   acl->acl.user_access = NULL;
   acl->aces = svn_hash__make(cb->parser_pool);
@@ -761,9 +761,9 @@ rules_open_section(void *baton, svn_stri
     }
 
   acl.acl.sequence_number = cb->parsed_acls->nelts;
-  acl.acl.anon_access = svn_authz_none;
+  acl.acl.anon_access = authz_access_none;
   acl.acl.has_anon_access = FALSE;
-  acl.acl.authn_access = svn_authz_none;
+  acl.acl.authn_access = authz_access_none;
   acl.acl.has_authn_access = FALSE;
   acl.acl.user_access = NULL;
 
@@ -821,7 +821,7 @@ add_access_entry(ctor_baton_t *cb, svn_s
   const svn_boolean_t inverted = (*name == '~');
   svn_boolean_t anonymous = FALSE;
   svn_boolean_t authenticated = FALSE;
-  svn_repos_authz_access_t access = svn_authz_none;
+  authz_access_t access = authz_access_none;
   authz_ace_t *ace;
   int i;
 
@@ -895,11 +895,11 @@ add_access_entry(ctor_baton_t *cb, svn_s
       switch (access_code)
         {
         case 'r':
-          access |= svn_authz_read;
+          access |= authz_access_read_flag;
           break;
 
         case 'w':
-          access |= svn_authz_write;
+          access |= authz_access_write_flag;
           break;
 
         default:
@@ -913,7 +913,7 @@ add_access_entry(ctor_baton_t *cb, svn_s
     }
 
   /* We do not support write-only access. */
-  if ((access & svn_authz_write) && !(access & svn_authz_read))
+  if ((access & authz_access_write_flag) && !(access & authz_access_read_flag))
     return svn_error_createf(
         SVN_ERR_AUTHZ_INVALID_CONFIG, NULL,
         _("Write-only access entry '%s' of rule [%s] is not valid"),
@@ -1174,7 +1174,7 @@ array_insert_ace(void *baton,
 /* Update accumulated RIGHTS from ACCESS. */
 static void
 update_rights(authz_rights_t *rights,
-              svn_repos_authz_access_t access)
+              authz_access_t access)
 {
   rights->min_access &= access;
   rights->max_access |= access;
@@ -1185,7 +1185,7 @@ update_rights(authz_rights_t *rights,
 static void
 update_global_rights(authz_global_rights_t *gr,
                      const char *repos,
-                     svn_repos_authz_access_t access)
+                     authz_access_t access)
 {
   update_rights(&gr->all_repos_rights, access);
   if (0 == strcmp(repos, AUTHZ_ANY_REPOSITORY))
@@ -1218,7 +1218,7 @@ update_user_rights(void *baton,
   const authz_acl_t *const acl = baton;
   const char *const user = key;
   authz_global_rights_t *const gr = value;
-  svn_repos_authz_access_t access;
+  authz_access_t access;
   svn_boolean_t has_access =
     svn_authz__get_acl_access(&access, acl, user, acl->rule.repos);
 

Modified: subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c?rev=1650496&r1=1650495&r2=1650496&view=diff
==============================================================================
--- subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c (original)
+++ subversion/branches/authzperf/subversion/tests/libsvn_repos/authz-test.c Fri Jan  9 10:56:13 2015
@@ -65,13 +65,13 @@ print_group(void *baton,
 
 
 static const char *
-access_string(svn_repos_authz_access_t access)
+access_string(authz_access_t access)
 {
-  switch (access & (svn_authz_read | svn_authz_write))
+  switch (access & authz_access_write)
     {
-    case svn_authz_none: return ""; break;
-    case svn_authz_read: return "r"; break;
-    case svn_authz_write: return "w"; break;
+    case authz_access_none: return ""; break;
+    case authz_access_read_flag: return "r"; break;
+    case authz_access_write_flag: return "w"; break;
     default:
       return "rw";
     }
@@ -225,9 +225,9 @@ test_authz_parse(const svn_test_opts_t *
   for (i = 0; i < authz->acls->nelts; ++i)
     {
       authz_acl_t *acl = &APR_ARRAY_IDX(authz->acls, i, authz_acl_t);
-      const svn_repos_authz_access_t all_access =
+      const authz_access_t all_access =
         (acl->anon_access & acl->authn_access);
-      svn_repos_authz_access_t access;
+      authz_access_t access;
       svn_boolean_t has_access =
         svn_authz__get_acl_access(&access, acl, check_user, check_repo);
       int j;