From 9d18e60d5299e440fe77b0126accaeea7272477e Mon Sep 17 00:00:00 2001 From: James Chapman Date: Fri, 30 Apr 2021 15:08:30 +0100 Subject: [PATCH] Issue 4701 - RFE - Exclude attributes from retro changelog (#4723) (#4746) Description: When the retro changelog plugin is enabled it writes the added/modified values to the "cn-changelog" suffix. In some cases an entries attribute values can be of a sensitive nature and should be excluded. This RFE adds functionality that will allow an admin exclude certain attributes from the retro changelog DB. Relates: https://github.com/389ds/389-ds-base/issues/4701 Reviewed by: mreynolds389, droideck (Thanks folks) --- .../tests/suites/retrocl/basic_test.py | 292 ++++++++++++++++++ ldap/servers/plugins/retrocl/retrocl.c | 53 +++- ldap/servers/plugins/retrocl/retrocl.h | 2 + ldap/servers/plugins/retrocl/retrocl_po.c | 15 + .../lib389/cli_conf/plugins/retrochangelog.py | 6 +- 5 files changed, 366 insertions(+), 2 deletions(-) create mode 100644 dirsrvtests/tests/suites/retrocl/basic_test.py diff --git a/dirsrvtests/tests/suites/retrocl/basic_test.py b/dirsrvtests/tests/suites/retrocl/basic_test.py new file mode 100644 index 0000000000..112c73cb9a --- /dev/null +++ b/dirsrvtests/tests/suites/retrocl/basic_test.py @@ -0,0 +1,292 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2021 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- + +import logging +import ldap +import time +import pytest +from lib389.topologies import topology_st +from lib389.plugins import RetroChangelogPlugin +from lib389._constants import * +from lib389.utils import * +from lib389.tasks import * +from lib389.cli_base import FakeArgs, connect_instance, disconnect_instance +from lib389.cli_base.dsrc import dsrc_arg_concat +from lib389.cli_conf.plugins.retrochangelog import retrochangelog_add +from lib389.idm.user import UserAccount, UserAccounts, nsUserAccounts + +pytestmark = pytest.mark.tier1 + +USER1_DN = 'uid=user1,ou=people,'+ DEFAULT_SUFFIX +USER2_DN = 'uid=user2,ou=people,'+ DEFAULT_SUFFIX +USER_PW = 'password' +ATTR_HOMEPHONE = 'homePhone' +ATTR_CARLICENSE = 'carLicense' + +log = logging.getLogger(__name__) + +def test_retrocl_exclude_attr_add(topology_st): + """ Test exclude attribute feature of the retrocl plugin for add operation + + :id: 3481650f-2070-45ef-9600-2500cfc51559 + + :setup: Standalone instance + + :steps: + 1. Enable dynamic plugins + 2. Confige retro changelog plugin + 3. Add an entry + 4. Ensure entry attrs are in the changelog + 5. Exclude an attr + 6. Add another entry + 7. Ensure excluded attr is not in the changelog + + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Success + 5. Success + 6. Success + 7. Success + """ + + st = topology_st.standalone + + log.info('Enable dynamic plugins') + try: + st.config.set('nsslapd-dynamic-plugins', 'on') + except ldap.LDAPError as e: + ldap.error('Failed to enable dynamic plugins ' + e.args[0]['desc']) + assert False + + log.info('Configure retrocl plugin') + rcl = RetroChangelogPlugin(st) + rcl.disable() + rcl.enable() + rcl.replace('nsslapd-attribute', 'nsuniqueid:targetUniqueId') + + log.info('Restarting instance') + try: + st.restart() + except ldap.LDAPError as e: + ldap.error('Failed to restart instance ' + e.args[0]['desc']) + assert False + + users = UserAccounts(st, DEFAULT_SUFFIX) + + log.info('Adding user1') + try: + user1 = users.create(properties={ + 'sn': '1', + 'cn': 'user 1', + 'uid': 'user1', + 'uidNumber': '11', + 'gidNumber': '111', + 'givenname': 'user1', + 'homePhone': '0861234567', + 'carLicense': '131D16674', + 'mail': 'user1@whereever.com', + 'homeDirectory': '/home/user1', + 'userpassword': USER_PW}) + except ldap.ALREADY_EXISTS: + pass + except ldap.LDAPError as e: + log.error("Failed to add user1") + + log.info('Verify homePhone and carLicense attrs are in the changelog changestring') + try: + cllist = st.search_s(RETROCL_SUFFIX, ldap.SCOPE_SUBTREE, '(targetDn=%s)' % USER1_DN) + except ldap.LDAPError as e: + log.fatal("Changelog search failed, error: " +str(e)) + assert False + assert len(cllist) > 0 + if cllist[0].hasAttr('changes'): + clstr = (cllist[0].getValue('changes')).decode() + assert ATTR_HOMEPHONE in clstr + assert ATTR_CARLICENSE in clstr + + log.info('Excluding attribute ' + ATTR_HOMEPHONE) + args = FakeArgs() + args.connections = [st.host + ':' + str(st.port) + ':' + DN_DM + ':' + PW_DM] + args.instance = 'standalone1' + args.basedn = None + args.binddn = None + args.starttls = False + args.pwdfile = None + args.bindpw = None + args.prompt = False + args.exclude_attrs = ATTR_HOMEPHONE + args.func = retrochangelog_add + dsrc_inst = dsrc_arg_concat(args, None) + inst = connect_instance(dsrc_inst, False, args) + result = args.func(inst, None, log, args) + disconnect_instance(inst) + assert result is None + + log.info("5s delay for retrocl plugin to restart") + time.sleep(5) + + log.info('Adding user2') + try: + user2 = users.create(properties={ + 'sn': '2', + 'cn': 'user 2', + 'uid': 'user2', + 'uidNumber': '22', + 'gidNumber': '222', + 'givenname': 'user2', + 'homePhone': '0879088363', + 'carLicense': '04WX11038', + 'mail': 'user2@whereever.com', + 'homeDirectory': '/home/user2', + 'userpassword': USER_PW}) + except ldap.ALREADY_EXISTS: + pass + except ldap.LDAPError as e: + log.error("Failed to add user2") + + log.info('Verify homePhone attr is not in the changelog changestring') + try: + cllist = st.search_s(RETROCL_SUFFIX, ldap.SCOPE_SUBTREE, '(targetDn=%s)' % USER2_DN) + assert len(cllist) > 0 + if cllist[0].hasAttr('changes'): + clstr = (cllist[0].getValue('changes')).decode() + assert ATTR_HOMEPHONE not in clstr + assert ATTR_CARLICENSE in clstr + except ldap.LDAPError as e: + log.fatal("Changelog search failed, error: " +str(e)) + assert False + +def test_retrocl_exclude_attr_mod(topology_st): + """ Test exclude attribute feature of the retrocl plugin for mod operation + + :id: f6bef689-685b-4f86-a98d-f7e6b1fcada3 + + :setup: Standalone instance + + :steps: + 1. Enable dynamic plugins + 2. Confige retro changelog plugin + 3. Add user1 entry + 4. Ensure entry attrs are in the changelog + 5. Exclude an attr + 6. Modify user1 entry + 7. Ensure excluded attr is not in the changelog + + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Success + 5. Success + 6. Success + 7. Success + """ + + st = topology_st.standalone + + log.info('Enable dynamic plugins') + try: + st.config.set('nsslapd-dynamic-plugins', 'on') + except ldap.LDAPError as e: + ldap.error('Failed to enable dynamic plugins ' + e.args[0]['desc']) + assert False + + log.info('Configure retrocl plugin') + rcl = RetroChangelogPlugin(st) + rcl.disable() + rcl.enable() + rcl.replace('nsslapd-attribute', 'nsuniqueid:targetUniqueId') + + log.info('Restarting instance') + try: + st.restart() + except ldap.LDAPError as e: + ldap.error('Failed to restart instance ' + e.args[0]['desc']) + assert False + + users = UserAccounts(st, DEFAULT_SUFFIX) + + log.info('Adding user1') + try: + user1 = users.create(properties={ + 'sn': '1', + 'cn': 'user 1', + 'uid': 'user1', + 'uidNumber': '11', + 'gidNumber': '111', + 'givenname': 'user1', + 'homePhone': '0861234567', + 'carLicense': '131D16674', + 'mail': 'user1@whereever.com', + 'homeDirectory': '/home/user1', + 'userpassword': USER_PW}) + except ldap.ALREADY_EXISTS: + pass + except ldap.LDAPError as e: + log.error("Failed to add user1") + + log.info('Verify homePhone and carLicense attrs are in the changelog changestring') + try: + cllist = st.search_s(RETROCL_SUFFIX, ldap.SCOPE_SUBTREE, '(targetDn=%s)' % USER1_DN) + except ldap.LDAPError as e: + log.fatal("Changelog search failed, error: " +str(e)) + assert False + assert len(cllist) > 0 + if cllist[0].hasAttr('changes'): + clstr = (cllist[0].getValue('changes')).decode() + assert ATTR_HOMEPHONE in clstr + assert ATTR_CARLICENSE in clstr + + log.info('Excluding attribute ' + ATTR_CARLICENSE) + args = FakeArgs() + args.connections = [st.host + ':' + str(st.port) + ':' + DN_DM + ':' + PW_DM] + args.instance = 'standalone1' + args.basedn = None + args.binddn = None + args.starttls = False + args.pwdfile = None + args.bindpw = None + args.prompt = False + args.exclude_attrs = ATTR_CARLICENSE + args.func = retrochangelog_add + dsrc_inst = dsrc_arg_concat(args, None) + inst = connect_instance(dsrc_inst, False, args) + result = args.func(inst, None, log, args) + disconnect_instance(inst) + assert result is None + + log.info("5s delay for retrocl plugin to restart") + time.sleep(5) + + log.info('Modify user1 carLicense attribute') + try: + st.modify_s(USER1_DN, [(ldap.MOD_REPLACE, ATTR_CARLICENSE, b"123WX321")]) + except ldap.LDAPError as e: + log.fatal('test_retrocl_exclude_attr_mod: Failed to update user1 attribute: error ' + e.message['desc']) + assert False + + log.info('Verify carLicense attr is not in the changelog changestring') + try: + cllist = st.search_s(RETROCL_SUFFIX, ldap.SCOPE_SUBTREE, '(targetDn=%s)' % USER1_DN) + assert len(cllist) > 0 + # There will be 2 entries in the changelog for this user, we are only + #interested in the second one, the modify operation. + if cllist[1].hasAttr('changes'): + clstr = (cllist[1].getValue('changes')).decode() + assert ATTR_CARLICENSE not in clstr + except ldap.LDAPError as e: + log.fatal("Changelog search failed, error: " +str(e)) + assert False + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE) diff --git a/ldap/servers/plugins/retrocl/retrocl.c b/ldap/servers/plugins/retrocl/retrocl.c index 8d6135dad2..2a620301c1 100644 --- a/ldap/servers/plugins/retrocl/retrocl.c +++ b/ldap/servers/plugins/retrocl/retrocl.c @@ -44,9 +44,11 @@ int retrocl_nattributes = 0; char **retrocl_attributes = NULL; char **retrocl_aliases = NULL; int retrocl_log_deleted = 0; +int retrocl_nexclude_attrs = 0; static Slapi_DN **retrocl_includes = NULL; static Slapi_DN **retrocl_excludes = NULL; +static char **retrocl_exclude_attrs = NULL; /* ----------------------------- Retrocl Plugin */ @@ -390,6 +392,28 @@ retrocl_start(Slapi_PBlock *pb) return -1; } + /* Get the exclude attributes */ + values = slapi_entry_attr_get_charray_ext(e, CONFIG_CHANGELOG_EXCLUDE_ATTRS, &num_vals); + if (values) { + retrocl_nexclude_attrs = num_vals; + retrocl_exclude_attrs = (char **)slapi_ch_calloc(num_vals + 1, sizeof(char *)); + + for (size_t i = 0; i < num_vals; i++) { + char *value = values[i]; + size_t length = strlen(value); + + char *pos = strchr(value, ':'); + if (pos == NULL) { + retrocl_exclude_attrs[i] = slapi_ch_strdup(value); + } else { + retrocl_exclude_attrs[i] = slapi_ch_malloc(pos - value + 1); + strncpy(retrocl_exclude_attrs[i], value, pos - value); + retrocl_exclude_attrs[i][pos - value] = '\0'; + } + slapi_log_err(SLAPI_LOG_INFO, RETROCL_PLUGIN_NAME,"retrocl_start - retrocl_exclude_attrs (%s).\n", retrocl_exclude_attrs[i]); + } + slapi_ch_array_free(values); + } /* Get the exclude suffixes */ values = slapi_entry_attr_get_charray_ext(e, CONFIG_CHANGELOG_EXCLUDE_SUFFIX, &num_vals); if (values) { @@ -500,7 +524,6 @@ retrocl_start(Slapi_PBlock *pb) slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME, " - %s\n", retrocl_attributes[i]); - } else { retrocl_attributes[i] = slapi_ch_malloc(pos - value + 1); strncpy(retrocl_attributes[i], value, pos - value); @@ -599,6 +622,8 @@ retrocl_stop(Slapi_PBlock *pb __attribute__((unused))) retrocl_attributes = NULL; slapi_ch_array_free(retrocl_aliases); retrocl_aliases = NULL; + slapi_ch_array_free(retrocl_exclude_attrs); + retrocl_exclude_attrs = NULL; while (retrocl_excludes && retrocl_excludes[i]) { slapi_sdn_free(&retrocl_excludes[i]); @@ -684,3 +709,29 @@ retrocl_plugin_init(Slapi_PBlock *pb) legacy_initialised = 1; return rc; } + +/* + * Function: retrocl_attr_in_exclude_attrs + * + * Return 1 if attribute exists in the retrocl_exclude_attrs list, else return 0. + * + * Arguments: attribute string, attribute length. + * + * Description: Check if an attribute is in the global exclude attribute list. + * + */ +int +retrocl_attr_in_exclude_attrs(char *attr, int attrlen) +{ + int i = 0; + if (attr && attrlen > 0 && retrocl_nexclude_attrs > 0) { + while (retrocl_exclude_attrs[i]) { + if (strncmp(retrocl_exclude_attrs[i], attr, attrlen) == 0) { + slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME,"retrocl_attr_in_exclude_attrs - excluding attr (%s).\n", attr); + return 1; + } + i++; + } + } + return 0; +} diff --git a/ldap/servers/plugins/retrocl/retrocl.h b/ldap/servers/plugins/retrocl/retrocl.h index 2ce76fcec6..ade9c00f1b 100644 --- a/ldap/servers/plugins/retrocl/retrocl.h +++ b/ldap/servers/plugins/retrocl/retrocl.h @@ -69,6 +69,7 @@ typedef struct _cnumRet #define CONFIG_CHANGELOG_DIRECTORY_ATTRIBUTE "nsslapd-changelogdir" #define CONFIG_CHANGELOG_INCLUDE_SUFFIX "nsslapd-include-suffix" #define CONFIG_CHANGELOG_EXCLUDE_SUFFIX "nsslapd-exclude-suffix" +#define CONFIG_CHANGELOG_EXCLUDE_ATTRS "nsslapd-exclude-attrs" #define RETROCL_CHANGELOG_DN "cn=changelog" #define RETROCL_MAPPINGTREE_DN "cn=\"cn=changelog\",cn=mapping tree,cn=config" @@ -144,5 +145,6 @@ extern void retrocl_stop_trimming(void); extern char *retrocl_get_config_str(const char *attrt); int retrocl_entry_in_scope(Slapi_Entry *e); +int retrocl_attr_in_exclude_attrs(char *attr, int attrlen); #endif /* _H_RETROCL */ diff --git a/ldap/servers/plugins/retrocl/retrocl_po.c b/ldap/servers/plugins/retrocl/retrocl_po.c index e1488f56b1..e447ccdfb1 100644 --- a/ldap/servers/plugins/retrocl/retrocl_po.c +++ b/ldap/servers/plugins/retrocl/retrocl_po.c @@ -59,6 +59,11 @@ make_changes_string(LDAPMod **ldm, const char **includeattrs) l = lenstr_new(); for (i = 0; ldm[i] != NULL; i++) { + /* Skip if we find an excluded attribute */ + if (retrocl_attr_in_exclude_attrs(ldm[i]->mod_type, strlen(ldm[i]->mod_type))) { + slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME, "make_changes_string - excluding attr (%s).\n", ldm[i]->mod_type); + continue; + } /* If a list of explicit attributes was given, only add those */ if (NULL != includeattrs) { skip = 1; @@ -408,6 +413,7 @@ entry2reple(Slapi_Entry *e, Slapi_Entry *oe, int optype) struct berval *vals[2]; struct berval val; int len; + Slapi_Attr *attrs = NULL; vals[0] = &val; vals[1] = NULL; @@ -424,6 +430,15 @@ entry2reple(Slapi_Entry *e, Slapi_Entry *oe, int optype) } slapi_entry_add_values(e, retrocl_changetype, vals); + /* Does this entry contain any excluded attributes */ + for (attrs = oe->e_attrs; attrs != NULL; attrs = attrs->a_next) { + if (retrocl_attr_in_exclude_attrs(attrs->a_type, strlen(attrs->a_type))) { + slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME, "entry2reple - excluding attr (%s).\n", attrs->a_type); + attrlist_delete(&oe->e_attrs, attrs->a_type); + attrs = attrs->a_next; + } + } + estr = slapi_entry2str(oe, &len); p = estr; /* Skip over the dn: line */ diff --git a/src/lib389/lib389/cli_conf/plugins/retrochangelog.py b/src/lib389/lib389/cli_conf/plugins/retrochangelog.py index 2fcaf20ef0..9940c65323 100644 --- a/src/lib389/lib389/cli_conf/plugins/retrochangelog.py +++ b/src/lib389/lib389/cli_conf/plugins/retrochangelog.py @@ -14,7 +14,8 @@ 'attribute': 'nsslapd-attribute', 'directory': 'nsslapd-changelogdir', 'max_age': 'nsslapd-changelogmaxage', - 'exclude_suffix': 'nsslapd-exclude-suffix' + 'exclude_suffix': 'nsslapd-exclude-suffix', + 'exclude_attrs': 'nsslapd-exclude-attrs' } @@ -40,6 +41,9 @@ def _add_parser_args(parser): parser.add_argument('--exclude-suffix', help='This attribute specifies the suffix which will be excluded ' 'from the scope of the plugin (nsslapd-exclude-suffix)') + parser.add_argument('--exclude-attrs', + help='This attribute specifies the attributes which will be excluded ' + 'from the scope of the plugin (nsslapd-exclude-attrs)') def create_parser(subparsers):