[PATCH] Add anonymous user and inverted authz rule matching.

The following patch adds two new types of rule to Subversion's
authz system:

  * Rules with the text "$" or "anonymous" match *only* the
    anonymous user, and not authenticated users.
  * Rules preceded with a "~" are inverted, so that, for
    instance, "~anonymous" will match all authenticated users.

As part of the change, the code to test whether a rule applies to
the current user has been factored out into a new method in
authz.c called authz_line_applies_to_user().

The authz_validate_rule() method has also been updated to check
for "double negatives" -- inversion used repeatedly within a
single rule -- which I have explicitly denied to help assure
a modicum of sanity.

My e-mail client isn't very good at converting newlines and
such with plain text attachments, so I'm including the patch
text inline for review, and attaching the actual patch file as
a binary file (gzipped) to avoid posting messages with LF line
endings interspersed with CRLF line endings :-)

Send all comments, and (hopefully) enjoy,

Jonathan Gilbert

[[[
Add pure anonymous rules & inverted rules to the authz system.

* subversion/libsvn_repos/authz.c
  (authz_line_applies_to_user): New method factored out of
   authz_parse_line() and extended to support pure anonymous
   rules & inverted rules.
  (authz_validate_rule): Reject doubly-inverted rules ("double
   negatives").
]]]

Index: subversion/libsvn_repos/authz.c
===================================================================
--- subversion/libsvn_repos/authz.c (revision 21101)
+++ subversion/libsvn_repos/authz.c (working copy)
@@ -161,6 +161,70 @@
 }

+/* Determines whether an authz rule applies to the current
+ * user, given the name part of the rule's name-value pair
+ * and the authz_lookup_baton object with the username in
+ * question.
+ */
+static svn_boolean_t
+authz_line_applies_to_user(const char *rule_match_string,
+ struct authz_lookup_baton *b,
+ apr_pool_t *pool)
+{
+ /* Work out whether this ACL line applies to the user. */
+
+ /* Is this rule an inversion? */
+ if (rule_match_string[0] == '~')
+ {
+ svn_boolean_t recursion_result;
+ /* Double inversion? This rule is malformed, so it
+ * definitely does not apply!
+ */
+ if (rule_match_string[1] == '~')
+ return FALSE;
+
+ /* Recurse, and return the opposite. */
+ recursion_result = authz_line_applies_to_user(
+ &rule_match_string[1], b, pool);
+
+ return (recursion_result == FALSE);
+ }
+
+ /* Check for a pure-anonymous rule. */
+ if ((strcmp(rule_match_string, "$") == 0)
+ || (strcmp(rule_match_string, "anonymous") == 0))
+ return (b->user == NULL);
+
+ /* Check for a wildcard rule. */
+ if (strcmp(rule_match_string, "*") == 0)
+ return TRUE;
+
+ /* If we get here, then the rule is:
+ * - Not an inversion rule.
+ * - Not a pure-anonymous rule.
+ * - Not a wildcard rule.
+ *
+ * All that's left over is regular old-fashioned
+ * user or group specifications.
+ */
+
+ /* If the session is anonymous, then a user/group
+ * rule definitely won't match.
+ */
+ if (b->user == NULL)
+ return FALSE;
+
+ /* Process the rule depending on whether it is
+ * a user or group rule.
+ */
+ if (rule_match_string[0] == '@')
+ return authz_group_contains_user(
+ b->config, &rule_match_string[1], b->user, pool);
+ else
+ return (strcmp(b->user, rule_match_string) == 0);
+}
+
+
 /* Callback to parse one line of an authz file and update the
  * authz_baton accordingly.
  */
@@ -170,26 +234,10 @@
 {
   struct authz_lookup_baton *b = baton;

- /* Work out whether this ACL line applies to the user. */
- if (strcmp(name, "*") != 0)
- {
- /* Non-anon rule, anon user. Stop. */
- if (!b->user)
- return TRUE;
+ /* Always pass if the rule doesn't apply to this user. */
+ if (!authz_line_applies_to_user(name, b, pool))
+ return TRUE;

- /* Group rule and user not in group. Stop. */
- if (*name == '@')
- {
- if (!authz_group_contains_user(b->config, &name[1],
- b->user, pool))
- return TRUE;
- }
-
- /* User rule for wrong user. Stop. */
- else if (strcmp(name, b->user) != 0)
- return TRUE;
- }
-
   /* Set the access grants for the rule. */
   if (strchr(value, 'r'))
     b->allow |= svn_authz_read;
@@ -436,7 +484,7 @@
 /* Callback to check whether GROUP is a group name, and if so, whether
    the group definition exists. Return TRUE if the rule has no
    errors. Use BATON for context and error reporting. */
-static svn_boolean_t authz_validate_rule(const char *group,
+static svn_boolean_t authz_validate_rule(const char *rule_match_string,
                                          const char *value,
                                          void *baton,
                                          apr_pool_t *pool)
@@ -444,9 +492,22 @@
   const char *val;
   struct authz_validate_baton *b = baton;

+ /* Make sure the user isn't using double-negatives. */
+ if ((rule_match_string[0] == '~') && (rule_match_string[1] == '~'))
+ {
+ b->err = svn_error_create(SVN_ERR_AUTHZ_INVALID_CONFIG, NULL,
+ "An authz rule uses inverted "
+ "matching more than once; "
+ "double negatives are not "
+ "permitted.");
+ return FALSE;
+ }
+
   /* If the rule applies to a group, check its existence. */
- if (*group == '@')
+ if (*rule_match_string == '@')
     {
- svn_config_get(b->config, &val, "groups", &group[1], NULL);
+ const char *group = &rule_match_string[1];
+
+ svn_config_get(b->config, &val, "groups", group, NULL);
       /* Having a non-existent group in the ACL configuration might be
          the sign of a typo. Refuse to perform authz on uncertain
          rules. */
@@ -468,7 +529,8 @@
       if (*val != 'r' && *val != 'w' && ! svn_ctype_isspace(*val))
         b->err = svn_error_createf(SVN_ERR_AUTHZ_INVALID_CONFIG, NULL,
                                    "The character '%c' in rule '%s' is not "
- "allowed in authz rules", *val, group);
+ "allowed in authz rules", *val,
+ rule_match_string);

       ++val;
     }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org