Browse Source

Issue 51054 - Revise ACI target syntax checking

Bug Description:  The previous commit enforced a strict syntax that was previously
                  allowed.  This is causing regressions for customers and community
                  members.

Fix Description:  Reject ACI's that use more than one equal sign between the target
                  keyword and the value, but do not enforce that the values are
                  quoted.  A flag was added that we can turn on strict syntax at a
                  later date, but for now we will continue allow values without quotes.

relates: https://pagure.io/389-ds-base/issue/51054

Reviewed by:  firstyear & spichugi(Thanks!!)
Mark Reynolds 5 years ago
parent
commit
916d13bc23
2 changed files with 50 additions and 11 deletions
  1. 1 1
      dirsrvtests/tests/suites/acl/syntax_test.py
  2. 49 10
      ldap/servers/plugins/acl/aclparse.c

+ 1 - 1
dirsrvtests/tests/suites/acl/syntax_test.py

@@ -192,7 +192,7 @@ FAILED = [('test_targattrfilters_18',
            f'(all)userdn="ldap:///anyone";)'), ]
 
 
[email protected].skipif(ds_is_older('1.4.3'), reason="Not implemented")
[email protected].xfail(reason='https://bugzilla.redhat.com/show_bug.cgi?id=1691473')
 @pytest.mark.parametrize("real_value", [a[1] for a in FAILED],
                          ids=[a[0] for a in FAILED])
 def test_aci_invalid_syntax_fail(topo, real_value):

+ 49 - 10
ldap/servers/plugins/acl/aclparse.c

@@ -34,6 +34,10 @@ static int acl_verify_exactly_one_attribute(char *attr_name, Slapi_Filter *f);
 static int type_compare(Slapi_Filter *f, void *arg);
 static int acl_check_for_target_macro(aci_t *aci_item, char *value);
 static int get_acl_rights_as_int(char *strValue);
+
+/* Enforce strict aci syntax */
+#define STRICT_SYNTAX_CHECK 0
+
 /***************************************************************************
 *
 * acl_parse
@@ -306,11 +310,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
             if (NULL == tmpstr) {
                 return ACL_SYNTAX_ERR;
             }
+
             tmpstr++;
+            /* Consecutive equals are not allowed */
+            if (*tmpstr == '=') {
+                slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+                        "__aclp__parse_aci - target filter has an invalid syntax, "
+                        "do not use more than one '=' between the targetfilter keyword and its value: (%s)\n",
+                        str);
+                return ACL_SYNTAX_ERR;
+            }
             __acl_strip_leading_space(&tmpstr);
 
             /* The first character is expected to be a double quote */
-            if (*tmpstr != '"') {
+            if (STRICT_SYNTAX_CHECK && *tmpstr != '"') {
                 slapi_log_err(SLAPI_LOG_ERR, plugin_name,
                         "__aclp__parse_aci - target filter has an invalid value (%s)\n", str);
                 return ACL_SYNTAX_ERR;
@@ -355,11 +368,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
                 strncpy(s, single_space, 1);
             }
             if ((s = strchr(str, '=')) != NULL) {
-                value = s + 1;
+                s++;
+                if (*s == '=') {
+                    /* Consecutive equals are not allowed */
+                    slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+                            "__aclp__parse_aci - target to/from has an invalid syntax, "
+                            "do not use more than one '=' between the target to/from keyword and its value: (%s)\n",
+                            str);
+                    return ACL_SYNTAX_ERR;
+                }
+                value = s;
                 __acl_strip_leading_space(&value);
                 __acl_strip_trailing_space(value);
                 /* The first character is expected to be a double quote */
-                if (*value != '"') {
+                if (STRICT_SYNTAX_CHECK && *value != '"') {
                     slapi_log_err(SLAPI_LOG_ERR, plugin_name,
                             "__aclp__parse_aci - target to/from has an invalid value (%s)\n", str);
                     return ACL_SYNTAX_ERR;
@@ -414,11 +436,20 @@ __aclp__parse_aci(char *str, aci_t *aci_item, char **errbuf)
                 strncpy(s, single_space, 1);
             }
             if ((s = strchr(str, '=')) != NULL) {
-                value = s + 1;
+                s++;
+                if (*s == '=') {
+                    /* Consecutive equals are not allowed */
+                    slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+                            "__aclp__parse_aci - target has an invalid syntax, "
+                            "do not use more than one '=' between the target keyword and its value: (%s)\n",
+                            str);
+                    return ACL_SYNTAX_ERR;
+                }
+                value = s;
                 __acl_strip_leading_space(&value);
                 __acl_strip_trailing_space(value);
                 /* The first character is expected to be a double quote */
-                if (*value != '"') {
+                if (STRICT_SYNTAX_CHECK && *value != '"') {
                     slapi_log_err(SLAPI_LOG_ERR, plugin_name,
                             "__aclp__parse_aci - target has an invalid value (%s)\n", str);
                     return ACL_SYNTAX_ERR;
@@ -1520,14 +1551,22 @@ __aclp__init_targetattr(aci_t *aci, char *attr_val, char **errbuf)
         return ACL_SYNTAX_ERR;
     }
     s++;
+    if (*s == '=') {
+        /* Consecutive equals are not allowed */
+        slapi_log_err(SLAPI_LOG_ERR, plugin_name,
+                "__aclp__init_targetattr - targetattr has an invalid syntax, "
+                "do not use more than one '=' between the targetattr and its value: (%s)\n",
+                attr_val);
+        return ACL_SYNTAX_ERR;
+    }
     __acl_strip_leading_space(&s);
     __acl_strip_trailing_space(s);
     len = strlen(s);
-    /* Simple targetattr statements may not be quoted e.g.
-       targetattr=* or targetattr=userPassword
-       if it begins with a quote, it must end with one as well
-    */
+    /*
+     * If it begins with a quote, it must end with one as well
+     */
     if (*s == '"') {
+
         if (s[len - 1] == '"') {
             s[len - 1] = '\0'; /* trim trailing quote */
         } else {
@@ -1545,7 +1584,7 @@ __aclp__init_targetattr(aci_t *aci, char *attr_val, char **errbuf)
             return ACL_SYNTAX_ERR;
         }
         s++; /* skip leading quote */
-    } else {
+    } else if (STRICT_SYNTAX_CHECK) {
         /* The first character is expected to be a double quote */
         slapi_log_err(SLAPI_LOG_ERR, plugin_name,
                 "__aclp__init_targetattr - targetattr has an invalid value (%s)\n", attr_val);