From 918df0a60a9cf1e3a836165f1044ea63c88bdd72 Mon Sep 17 00:00:00 2001 From: William Brown Date: Fri, 6 Dec 2019 14:04:45 +1000 Subject: [PATCH] Ticket 50727 - correct mistaken options in filter validation patch Bug Description: Because William of the past missed (forgot) to make some agreed upon changes, we shipped the feature for filter validation in a state that was a bit unclear for users. Fix Description: Fix the options to now be clearer in what is expected/demaned from admins. We now have 4 possible states for the value of the config: * reject-invalid (prev on) * process-safe (prev warn) * warn-invalid (new!) * off (prev off) These behave as: * reject-invalid - reject queries that contain unknown attributes * process-safe - log a notes=F that an attr is missing, and idl_alloc(0) the missing attribute for RFC4511 compliance. * warn-invalid - log a notes=F that an attr is missing, and process as ALLIDS (the legacy behaviour) * off - process as ALLIDs (the legacy behaviour) The default is "process-safe". https://pagure.io/389-ds-base/issue/50727 Author: William Brown Review by: tbordaz, lkrispen (thanks) --- .../suites/filter/schema_validation_test.py | 112 ++++++++++++++++-- ldap/servers/slapd/back-ldbm/filterindex.c | 28 +++-- ldap/servers/slapd/libglobs.c | 78 +++++++----- ldap/servers/slapd/schema.c | 26 ++-- ldap/servers/slapd/slap.h | 21 ++-- ldap/servers/slapd/slapi-plugin.h | 1 + ldap/servers/slapd/slapi-private.h | 3 +- src/lib389/lib389/extensibleobject.py | 53 +++++++++ 8 files changed, 258 insertions(+), 64 deletions(-) create mode 100644 src/lib389/lib389/extensibleobject.py diff --git a/dirsrvtests/tests/suites/filter/schema_validation_test.py b/dirsrvtests/tests/suites/filter/schema_validation_test.py index 4ac9fa8ff..d0d67ca95 100644 --- a/dirsrvtests/tests/suites/filter/schema_validation_test.py +++ b/dirsrvtests/tests/suites/filter/schema_validation_test.py @@ -9,14 +9,29 @@ import pytest import ldap -from lib389.topologies import topology_st +from lib389.topologies import topology_st as topology_st_pre from lib389.dirsrv_log import DirsrvAccessLog from lib389._mapped_object import DSLdapObjects from lib389._constants import DEFAULT_SUFFIX +from lib389.extensibleobject import UnsafeExtensibleObjects -def _check_value(inst_cfg, value): +def _check_value(inst_cfg, value, exvalue=None): + if exvalue is None: + exvalue = value inst_cfg.set('nsslapd-verify-filter-schema', value) - assert(inst_cfg.get_attr_val_utf8('nsslapd-verify-filter-schema') == value) + assert(inst_cfg.get_attr_val_utf8('nsslapd-verify-filter-schema') == exvalue) + +@pytest.fixture(scope="module") +def topology_st(topology_st_pre): + raw_objects = UnsafeExtensibleObjects(topology_st_pre.standalone, basedn=DEFAULT_SUFFIX) + # Add an object that won't be able to be queried due to invalid attrs. + raw_objects.create(properties = { + "cn": "test_obj", + "a": "a", + "b": "b", + "uid": "foo" + }) + return topology_st_pre @pytest.mark.ds50349 @@ -51,8 +66,14 @@ def test_filter_validation_config(topology_st): initial_value = inst_cfg.get_attr_val_utf8('nsslapd-verify-filter-schema') - _check_value(inst_cfg, "on") - _check_value(inst_cfg, "warn") + # Check legacy values that may have been set + _check_value(inst_cfg, "on", "reject-invalid") + _check_value(inst_cfg, "warn", "process-safe") + _check_value(inst_cfg, "off") + # Check the more descriptive values + _check_value(inst_cfg, "reject-invalid") + _check_value(inst_cfg, "process-safe") + _check_value(inst_cfg, "warn-invalid") _check_value(inst_cfg, "off") # This should fail @@ -85,7 +106,7 @@ def test_filter_validation_enabled(topology_st): inst = topology_st.standalone # In case the default has changed, we set the value to warn. - inst.config.set("nsslapd-verify-filter-schema", "on") + inst.config.set("nsslapd-verify-filter-schema", "reject-invalid") raw_objects = DSLdapObjects(inst, basedn=DEFAULT_SUFFIX) # Check a good query has no errors. @@ -104,9 +125,9 @@ def test_filter_validation_enabled(topology_st): @pytest.mark.ds50349 -def test_filter_validation_warning(topology_st): +def test_filter_validation_warn_safe(topology_st): """Test that queries which are invalid, are correctly marked as "notes=F" in - the access log. + the access log, and return no entries or partial sets. :id: 8b2b23fe-d878-435c-bc84-8c298be4ca1f :setup: Standalone instance @@ -122,7 +143,7 @@ def test_filter_validation_warning(topology_st): inst = topology_st.standalone # In case the default has changed, we set the value to warn. - inst.config.set("nsslapd-verify-filter-schema", "warn") + inst.config.set("nsslapd-verify-filter-schema", "process-safe") # Set the access log to un-buffered so we get it immediately. inst.config.set("nsslapd-accesslog-logbuffering", "off") @@ -139,20 +160,93 @@ def test_filter_validation_warning(topology_st): # Check a good query has no warnings. r = raw_objects.filter("(objectClass=*)") + assert(len(r) > 0) r_s1 = access_log.match(".*notes=F.*") # Should be the same number of log lines IE 0. assert(len(r_init) == len(r_s1)) # Check a bad one DOES emit a warning. r = raw_objects.filter("(a=a)") + assert(len(r) == 0) r_s2 = access_log.match(".*notes=F.*") # Should be the greate number of log lines IE +1 assert(len(r_init) + 1 == len(r_s2)) # Check a bad complex one does emit a warning. r = raw_objects.filter("(&(a=a)(b=b)(objectClass=*))") + assert(len(r) == 0) r_s3 = access_log.match(".*notes=F.*") # Should be the greate number of log lines IE +2 assert(len(r_init) + 2 == len(r_s3)) + # Check that we can still get things when partial + r = raw_objects.filter("(|(a=a)(b=b)(uid=foo))") + assert(len(r) == 1) + r_s4 = access_log.match(".*notes=F.*") + # Should be the greate number of log lines IE +2 + assert(len(r_init) + 3 == len(r_s4)) + + +@pytest.mark.ds50349 +def test_filter_validation_warn_unsafe(topology_st): + """Test that queries which are invalid, are correctly marked as "notes=F" in + the access log, and uses the legacy query behaviour to return unsafe sets. + + :id: 8b2b23fe-d878-435c-bc84-8c298be4ca1f + :setup: Standalone instance + :steps: + 1. Search a well formed query + 2. Search a poorly formed query + 3. Search a poorly formed complex (and/or) query + :expectedresults: + 1. No warnings + 2. notes=F is present + 3. notes=F is present + """ + inst = topology_st.standalone + + # In case the default has changed, we set the value to warn. + inst.config.set("nsslapd-verify-filter-schema", "warn-invalid") + # Set the access log to un-buffered so we get it immediately. + inst.config.set("nsslapd-accesslog-logbuffering", "off") + + # Setup the query object. + # Now we don't care if there are any results, we only care about good/bad queries. + # To do this we have to bypass some of the lib389 magic, and just emit raw queries + # to check them. Turns out lib389 is well designed and this just works as expected + # if you use a single DSLdapObjects and filter. :) + raw_objects = DSLdapObjects(inst, basedn=DEFAULT_SUFFIX) + + # Find any initial notes=F + access_log = DirsrvAccessLog(inst) + r_init = access_log.match(".*notes=(U,)?F.*") + + # Check a good query has no warnings. + r = raw_objects.filter("(objectClass=*)") + assert(len(r) > 0) + r_s1 = access_log.match(".*notes=(U,)?F.*") + # Should be the same number of log lines IE 0. + assert(len(r_init) == len(r_s1)) + + # Check a bad one DOES emit a warning. + r = raw_objects.filter("(a=a)") + assert(len(r) == 1) + # NOTE: Unlike warn-process-safely, these become UNINDEXED and show in the logs. + r_s2 = access_log.match(".*notes=(U,)?F.*") + # Should be the greate number of log lines IE +1 + assert(len(r_init) + 1 == len(r_s2)) + + # Check a bad complex one does emit a warning. + r = raw_objects.filter("(&(a=a)(b=b)(objectClass=*))") + assert(len(r) == 1) + r_s3 = access_log.match(".*notes=(U,)?F.*") + # Should be the greate number of log lines IE +2 + assert(len(r_init) + 2 == len(r_s3)) + + # Check that we can still get things when partial + r = raw_objects.filter("(|(a=a)(b=b)(uid=foo))") + assert(len(r) == 1) + r_s4 = access_log.match(".*notes=(U,)?F.*") + # Should be the greate number of log lines IE +2 + assert(len(r_init) + 3 == len(r_s4)) diff --git a/ldap/servers/slapd/back-ldbm/filterindex.c b/ldap/servers/slapd/back-ldbm/filterindex.c index 7e65f73ca..8a79848c3 100644 --- a/ldap/servers/slapd/back-ldbm/filterindex.c +++ b/ldap/servers/slapd/back-ldbm/filterindex.c @@ -223,13 +223,15 @@ ava_candidates( switch (ftype) { case LDAP_FILTER_GE: - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { idl = range_candidates(pb, be, type, bval, NULL, err, &sattr, allidslimit); @@ -239,13 +241,15 @@ ava_candidates( goto done; break; case LDAP_FILTER_LE: - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { idl = range_candidates(pb, be, type, NULL, bval, err, &sattr, allidslimit); @@ -293,13 +297,15 @@ ava_candidates( ptr[1] = NULL; ivals = ptr; - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { slapi_attr_assertion2keys_ava_sv(&sattr, &tmp, (Slapi_Value ***)&ivals, LDAP_FILTER_EQUALITY_FAST); @@ -326,13 +332,15 @@ ava_candidates( slapi_ch_free((void **)&ivals); } } else { - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { slapi_value_init_berval(&sv, bval); @@ -382,13 +390,15 @@ presence_candidates( } slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { idl = index_read_ext_allids(pb, be, type, indextype_PRESENCE, @@ -485,13 +495,15 @@ extensible_candidates( slapi_pblock_get(pb, SLAPI_PLUGIN_MR_KEYS, &keys)) { /* something went wrong. bail. */ break; - } else if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + } else if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else if (keys == NULL || keys[0] == NULL) { /* no keys */ @@ -986,13 +998,15 @@ substring_candidates( * look up each key in the index, ANDing the resulting * IDLists together. */ - if (f->f_flags & SLAPI_FILTER_INVALID_ATTR) { + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_WARN) { /* * REMEMBER: this flag is only set on WARN levels. If the filter verify * is on strict, we reject in search.c, if we ar off, the flag will NOT * be set on the filter at all! */ slapi_pblock_set_flag_operation_notes(pb, SLAPI_OP_NOTE_FILTER_INVALID); + } + if (f->f_flags & SLAPI_FILTER_INVALID_ATTR_UNDEFINE) { idl = idl_alloc(0); } else { slapi_pblock_get(pb, SLAPI_TXN, &txn.back_txn_txn); diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c index b9cdb6b37..66170ebc6 100644 --- a/ldap/servers/slapd/libglobs.c +++ b/ldap/servers/slapd/libglobs.c @@ -166,7 +166,7 @@ typedef enum { CONFIG_SPECIAL_VALIDATE_CERT_SWITCH, /* maps strings to an enumeration */ CONFIG_SPECIAL_UNHASHED_PW_SWITCH, /* unhashed pw: on/off/nolog */ CONFIG_SPECIAL_TLS_CHECK_CRL, /* maps enum tls_check_crl_t to char * */ - CONFIG_ON_OFF_WARN, /* maps to a config on/warn/off enum */ + CONFIG_SPECIAL_FILTER_VERIFY, /* maps to a config strict/warn-strict/warn/off enum */ } ConfigVarType; static int32_t config_set_onoff(const char *attrname, char *value, int32_t *configvalue, char *errorbuf, int apply); @@ -256,7 +256,7 @@ slapi_int_t init_malloc_mmap_threshold; slapi_onoff_t init_extract_pem; slapi_onoff_t init_ignore_vattrs; slapi_onoff_t init_enable_upgrade_hash; -slapi_onwarnoff_t init_verify_filter_schema; +slapi_special_filter_verify_t init_verify_filter_schema; static int isInt(ConfigVarType type) @@ -1248,7 +1248,7 @@ static struct config_get_and_set {CONFIG_VERIFY_FILTER_SCHEMA, config_set_verify_filter_schema, NULL, 0, (void **)&global_slapdFrontendConfig.verify_filter_schema, - CONFIG_ON_OFF_WARN, (ConfigGetFunc)config_get_verify_filter_schema, + CONFIG_SPECIAL_FILTER_VERIFY, (ConfigGetFunc)config_get_verify_filter_schema, &init_verify_filter_schema}, /* End config */ }; @@ -7659,18 +7659,21 @@ config_initvalue_to_onoff(struct config_get_and_set *cgas, char *initvalbuf, siz } static char * -config_initvalue_to_onwarnoff(struct config_get_and_set *cgas, char *initvalbuf, size_t initvalbufsize) { +config_initvalue_to_special_filter_verify(struct config_get_and_set *cgas, char *initvalbuf, size_t initvalbufsize) { char *retval = NULL; - if (cgas->config_var_type == CONFIG_ON_OFF_WARN) { - slapi_onwarnoff_t *value = (slapi_onwarnoff_t *)(intptr_t)cgas->initvalue; + if (cgas->config_var_type == CONFIG_SPECIAL_FILTER_VERIFY) { + slapi_special_filter_verify_t *value = (slapi_special_filter_verify_t *)(intptr_t)cgas->initvalue; if (value != NULL) { - if (*value == SLAPI_ON) { - PR_snprintf(initvalbuf, initvalbufsize, "%s", "on"); + if (*value == SLAPI_STRICT) { + PR_snprintf(initvalbuf, initvalbufsize, "%s", "reject-invalid"); retval = initvalbuf; - } else if (*value == SLAPI_WARN) { - PR_snprintf(initvalbuf, initvalbufsize, "%s", "warn"); + } else if (*value == SLAPI_WARN_SAFE) { + PR_snprintf(initvalbuf, initvalbufsize, "%s", "process-safe"); retval = initvalbuf; - } else if (*value == SLAPI_OFF) { + } else if (*value == SLAPI_WARN_UNSAFE) { + PR_snprintf(initvalbuf, initvalbufsize, "%s", "warn-invalid"); + retval = initvalbuf; + } else if (*value == SLAPI_OFF_UNSAFE) { PR_snprintf(initvalbuf, initvalbufsize, "%s", "off"); retval = initvalbuf; } @@ -7680,7 +7683,7 @@ config_initvalue_to_onwarnoff(struct config_get_and_set *cgas, char *initvalbuf, } static int32_t -config_set_onoffwarn(slapdFrontendConfig_t *slapdFrontendConfig, slapi_onwarnoff_t *target, const char *attrname, char *value, char *errorbuf, int apply) { +config_set_specialfilterverify(slapdFrontendConfig_t *slapdFrontendConfig, slapi_special_filter_verify_t *target, const char *attrname, char *value, char *errorbuf, int apply) { if (target == NULL) { return LDAP_OPERATIONS_ERROR; } @@ -7691,15 +7694,23 @@ config_set_onoffwarn(slapdFrontendConfig_t *slapdFrontendConfig, slapi_onwarnoff slapi_special_filter_verify_t p_val = SLAPI_WARN_UNSAFE; + /* on/warn/off retained for legacy reasons due to wbrown making terrible mistakes :( :( */ if (strcasecmp(value, "on") == 0) { - p_val = SLAPI_ON; + p_val = SLAPI_STRICT; } else if (strcasecmp(value, "warn") == 0) { - p_val = SLAPI_WARN; + p_val = SLAPI_WARN_SAFE; + /* The new fixed/descriptive names */ + } else if (strcasecmp(value, "reject-invalid") == 0) { + p_val = SLAPI_STRICT; + } else if (strcasecmp(value, "process-safe") == 0) { + p_val = SLAPI_WARN_SAFE; + } else if (strcasecmp(value, "warn-invalid") == 0) { + p_val = SLAPI_WARN_UNSAFE; } else if (strcasecmp(value, "off") == 0) { - p_val = SLAPI_OFF; + p_val = SLAPI_OFF_UNSAFE; } else { slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, - "%s: invalid value \"%s\". Valid values are \"on\", \"warn\" or \"off\".", attrname, value); + "%s: invalid value \"%s\". Valid values are \"reject-invalid\", \"process-safe\", \"warn-invalid\" or \"off\". If in doubt, choose \"process-safe\"", attrname, value); return LDAP_OPERATIONS_ERROR; } @@ -7718,14 +7729,14 @@ config_set_onoffwarn(slapdFrontendConfig_t *slapdFrontendConfig, slapi_onwarnoff int32_t config_set_verify_filter_schema(const char *attrname, char *value, char *errorbuf, int apply) { slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); - slapi_onwarnoff_t *target = &(slapdFrontendConfig->verify_filter_schema); - return config_set_onoffwarn(slapdFrontendConfig, target, attrname, value, errorbuf, apply); + slapi_special_filter_verify_t *target = &(slapdFrontendConfig->verify_filter_schema); + return config_set_specialfilterverify(slapdFrontendConfig, target, attrname, value, errorbuf, apply); } Slapi_Filter_Policy config_get_verify_filter_schema() { - slapi_onwarnoff_t retVal; + slapi_special_filter_verify_t retVal; slapdFrontendConfig_t *slapdFrontendConfig = getFrontendConfig(); CFG_LOCK_READ(slapdFrontendConfig); retVal = slapdFrontendConfig->verify_filter_schema; @@ -7733,10 +7744,13 @@ config_get_verify_filter_schema() /* Now map this to a policy that the fns understand. */ switch (retVal) { - case SLAPI_ON: + case SLAPI_STRICT: return FILTER_POLICY_STRICT; break; - case SLAPI_WARN: + case SLAPI_WARN_SAFE: + return FILTER_POLICY_PROTECT; + break; + case SLAPI_WARN_UNSAFE: return FILTER_POLICY_WARNING; break; default: @@ -7794,8 +7808,8 @@ config_set(const char *attr, struct berval **values, char *errorbuf, int apply) void *initval = cgas->initvalue; if (cgas->config_var_type == CONFIG_ON_OFF) { initval = (void *)config_initvalue_to_onoff(cgas, initvalbuf, sizeof(initvalbuf)); - } else if (cgas->config_var_type == CONFIG_ON_OFF_WARN) { - initval = (void *)config_initvalue_to_onwarnoff(cgas, initvalbuf, sizeof(initvalbuf)); + } else if (cgas->config_var_type == CONFIG_SPECIAL_FILTER_VERIFY) { + initval = (void *)config_initvalue_to_special_filter_verify(cgas, initvalbuf, sizeof(initvalbuf)); } if (cgas->setfunc) { retval = (cgas->setfunc)(cgas->attr_name, initval, errorbuf, apply); @@ -8021,20 +8035,24 @@ config_set_value( break; - case CONFIG_ON_OFF_WARN: + case CONFIG_SPECIAL_FILTER_VERIFY: /* Is this the right default here? */ if (!value) { - slapi_entry_attr_set_charptr(e, cgas->attr_name, "off"); + slapi_entry_attr_set_charptr(e, cgas->attr_name, "process-safe"); break; } - if (*((slapi_onwarnoff_t *)value) == SLAPI_ON) { - slapi_entry_attr_set_charptr(e, cgas->attr_name, "on"); - } else if (*((slapi_onwarnoff_t *)value) == SLAPI_WARN) { - slapi_entry_attr_set_charptr(e, cgas->attr_name, "warn"); + if (*((slapi_special_filter_verify_t *)value) == SLAPI_STRICT) { + slapi_entry_attr_set_charptr(e, cgas->attr_name, "reject-invalid"); + } else if (*((slapi_special_filter_verify_t *)value) == SLAPI_WARN_SAFE) { + slapi_entry_attr_set_charptr(e, cgas->attr_name, "process-safe"); + } else if (*((slapi_special_filter_verify_t *)value) == SLAPI_WARN_UNSAFE) { + slapi_entry_attr_set_charptr(e, cgas->attr_name, "warn-invalid"); + } else if (*((slapi_special_filter_verify_t *)value) == SLAPI_OFF_UNSAFE) { + slapi_entry_attr_set_charptr(e, cgas->attr_name, "off"); } else { /* Default to safe warn-proccess-safely */ - slapi_entry_attr_set_charptr(e, cgas->attr_name, "warn-invalid"); + slapi_entry_attr_set_charptr(e, cgas->attr_name, "process-safe"); } break; diff --git a/ldap/servers/slapd/schema.c b/ldap/servers/slapd/schema.c index 6e853fc7c..d7c3c139e 100644 --- a/ldap/servers/slapd/schema.c +++ b/ldap/servers/slapd/schema.c @@ -698,7 +698,7 @@ out: } static Slapi_Filter_Result -slapi_filter_schema_check_inner(Slapi_Filter *f) { +slapi_filter_schema_check_inner(Slapi_Filter *f, slapi_filter_flags flags) { /* * Default response to Ok. If any more severe things happen we * alter this to reflect it. IE we bubble up more severe errors @@ -712,26 +712,26 @@ slapi_filter_schema_check_inner(Slapi_Filter *f) { case LDAP_FILTER_LE: case LDAP_FILTER_APPROX: if (!attr_syntax_exist_by_name_nolock(f->f_avtype)) { - f->f_flags |= SLAPI_FILTER_INVALID_ATTR; + f->f_flags |= flags; r = FILTER_SCHEMA_WARNING; } break; case LDAP_FILTER_PRESENT: if (!attr_syntax_exist_by_name_nolock(f->f_type)) { - f->f_flags |= SLAPI_FILTER_INVALID_ATTR; + f->f_flags |= flags; r = FILTER_SCHEMA_WARNING; } break; case LDAP_FILTER_SUBSTRINGS: if (!attr_syntax_exist_by_name_nolock(f->f_sub_type)) { - f->f_flags |= SLAPI_FILTER_INVALID_ATTR; + f->f_flags |= flags; r = FILTER_SCHEMA_WARNING; } break; case LDAP_FILTER_EXTENDED: /* I don't have any examples of this, so I'm not 100% on how to check it */ if (!attr_syntax_exist_by_name_nolock(f->f_mr_type)) { - f->f_flags |= SLAPI_FILTER_INVALID_ATTR; + f->f_flags |= flags; r = FILTER_SCHEMA_WARNING; } break; @@ -740,7 +740,7 @@ slapi_filter_schema_check_inner(Slapi_Filter *f) { case LDAP_FILTER_NOT: /* Recurse and check all elemments of the filter */ for (Slapi_Filter *f_child = f->f_list; f_child != NULL; f_child = f_child->f_next) { - Slapi_Filter_Result ri = slapi_filter_schema_check_inner(f_child); + Slapi_Filter_Result ri = slapi_filter_schema_check_inner(f_child, flags); if (ri > r) { r = ri; } @@ -769,12 +769,24 @@ slapi_filter_schema_check(Slapi_Filter *f, Slapi_Filter_Policy fp) { return FILTER_SCHEMA_SUCCESS; } + /* + * There are two possible warning types - it's not up to us to warn into + * the logs, that's the backends job. So we have to flag a hint into the + * filter about what it should do. This is why there are two FILTER_INVALID + * types in filter_flags, one for logging it, and one for actually doing + * the rejection. + */ + slapi_filter_flags flags = SLAPI_FILTER_INVALID_ATTR_WARN; + if (fp == FILTER_POLICY_PROTECT) { + flags |= SLAPI_FILTER_INVALID_ATTR_UNDEFINE; + } + /* * Filters are nested, recursive structures, so we actually have to call an inner * function until we have a result! */ attr_syntax_read_lock(); - Slapi_Filter_Result r = slapi_filter_schema_check_inner(f); + Slapi_Filter_Result r = slapi_filter_schema_check_inner(f, flags); attr_syntax_unlock_read(); /* If any warning occured, ensure we fail it. */ diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 8a2748519..d73e9aaae 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -457,12 +457,12 @@ typedef enum _tls_check_crl_t { TLS_CHECK_ALL = 2, } tls_check_crl_t; -typedef enum _slapi_onwarnoff_t { - SLAPI_OFF = 0, - SLAPI_WARN = 1, - SLAPI_ON = 2, -} slapi_onwarnoff_t; - +typedef enum _slapi_special_filter_verify_t { + SLAPI_STRICT = 0, + SLAPI_WARN_SAFE = 1, + SLAPI_WARN_UNSAFE = 2, + SLAPI_OFF_UNSAFE = 3, +} slapi_special_filter_verify_t; struct subfilt { @@ -2547,11 +2547,12 @@ typedef struct _slapdFrontendConfig slapi_onoff_t enable_upgrade_hash; /* If on, upgrade hashes for PW at bind */ /* * Do we verify the filters we recieve by schema? - * on - yes, and reject if attribute not found - * warn - yes, and warn that the attribute is unknown and unindexed - * off - no, do whatever (old status-quo) + * reject-invalid - reject filter if there is anything invalid + * process-safe - allow the filter, warn about what's invalid, and then idl_alloc(0) with rfc compliance + * warn-invalid - allow the filter, warn about the invalid, and then do a ALLIDS (may lead to full table scan) + * off - don't warn, just allow anything. This is the legacy behaviour. */ - slapi_onwarnoff_t verify_filter_schema; + slapi_special_filter_verify_t verify_filter_schema; } slapdFrontendConfig_t; /* possible values for slapdFrontendConfig_t.schemareplace */ diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 50b8d12c8..40b5c911a 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -1571,6 +1571,7 @@ int slapi_entry_syntax_check(Slapi_PBlock *pb, Slapi_Entry *e, int override); typedef enum { FILTER_POLICY_OFF, FILTER_POLICY_WARNING, + FILTER_POLICY_PROTECT, FILTER_POLICY_STRICT, } Slapi_Filter_Policy; diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h index 04c045532..1f17eda12 100644 --- a/ldap/servers/slapd/slapi-private.h +++ b/ldap/servers/slapd/slapi-private.h @@ -54,7 +54,8 @@ typedef enum _slapi_filter_flags_t { SLAPI_FILTER_RUV = 4, SLAPI_FILTER_NORMALIZED_TYPE = 8, SLAPI_FILTER_NORMALIZED_VALUE = 16, - SLAPI_FILTER_INVALID_ATTR = 32, + SLAPI_FILTER_INVALID_ATTR_UNDEFINE = 32, + SLAPI_FILTER_INVALID_ATTR_WARN = 64, } slapi_filter_flags; #define SLAPI_ENTRY_LDAPSUBENTRY 2 diff --git a/src/lib389/lib389/extensibleobject.py b/src/lib389/lib389/extensibleobject.py new file mode 100644 index 000000000..8fe37f980 --- /dev/null +++ b/src/lib389/lib389/extensibleobject.py @@ -0,0 +1,53 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2019, William Brown +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- + +from lib389._mapped_object import DSLdapObject, DSLdapObjects +from lib389.utils import ensure_str + +class UnsafeExtensibleObject(DSLdapObject): + """A single instance of an extensible object. Extensible object by it's + nature is unsafe, eliminating rules around attribute checking. It may + cause unsafe or other unknown behaviour if not handled correctly. + + :param instance: An instance + :type instance: lib389.DirSrv + :param dn: Entry DN + :type dn: str + """ + + def __init__(self, instance, dn=None): + super(UnsafeExtensibleObject, self).__init__(instance, dn) + self._rdn_attribute = "cn" + # Can I generate these from schema? + self._must_attributes = [] + self._create_objectclasses = [ + 'top', + 'extensibleObject', + ] + self._protected = False + +class UnsafeExtensibleObjects(DSLdapObjects): + """DSLdapObjects that represents all extensible objects. Extensible Objects + are unsafe in their nature, disabling many checks around schema and attribute + handling. You should really really REALLY not use this unless you have specific + needs for testing. + + :param instance: An instance + :type instance: lib389.DirSrv + :param basedn: Base DN for all group entries below + :type basedn: str + """ + + def __init__(self, instance, basedn): + super(UnsafeExtensibleObjects, self).__init__(instance) + self._objectclasses = [ + 'extensibleObject', + ] + self._filterattrs = ["cn"] + self._childobject = UnsafeExtensibleObject + self._basedn = ensure_str(basedn) -- 2.21.1