8:46 a.m.
This is an automated email from the git hooks/post-receive script.
mreynolds pushed a commit to branch 389-ds-base-1.4.3
in repository 389-ds-base.
The following commit(s) were added to refs/heads/389-ds-base-1.4.3 by this push:
new f2e9961 Issue 51054 - Revise ACI target syntax checking
f2e9961 is described below
commit f2e9961f1acc657b13bc901bf1c3f58f735d1400
Author: Mark Reynolds <mreynolds(a)redhat.com>
AuthorDate: Thu Apr 30 11:47:26 2020 -0400
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!!)
---
dirsrvtests/tests/suites/acl/syntax_test.py | 2 +-
ldap/servers/plugins/acl/aclparse.c | 59 ++++++++++++++++++++++++-----
2 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/dirsrvtests/tests/suites/acl/syntax_test.py
b/dirsrvtests/tests/suites/acl/syntax_test.py
index cf08c65..c143ff7 100644
--- a/dirsrvtests/tests/suites/acl/syntax_test.py
+++ b/dirsrvtests/tests/suites/acl/syntax_test.py
@@ -192,7 +192,7 @@ FAILED = [('test_targattrfilters_18',
f'(all)userdn="ldap:///anyone";)'), ]
-(a)pytest.mark.skipif(ds_is_older('1.4.3'), reason="Not implemented")
+@pytest.mark.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):
diff --git a/ldap/servers/plugins/acl/aclparse.c b/ldap/servers/plugins/acl/aclparse.c
index d5edaf5..577251e 100644
--- a/ldap/servers/plugins/acl/aclparse.c
+++ b/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);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.