From 075e22b5f19eea42f7985aafb25055919b6ec2d8 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Tue, 18 Nov 2014 09:24:21 -0500 Subject: [PATCH 275/305] Ticket 47950 - Bind DN tracking unable to write to internalModifiersName without special permissions Bug Description: When a non-rootDN entry makes an update when plugin bind dn tracking is enabled, it is incorrectly rejected with an error 50. Fix Description: Create a function to check if an attribute is one of last mod attributes, including the internalModfieresname/internalModifytimestamp. Use this new function wherever we are checking for last mod attributes. https://fedorahosted.org/389/ticket/47950 Reviewed by: rmeggins(Thanks!) (cherry picked from commit c973e7150cf1e7fb3f61a76cf1baf3c0fa91f756) Conflicts: ldap/servers/plugins/acl/acl.c ldap/servers/slapd/configdse.c (cherry picked from commit 0c47dfb627ab4fc5f7d8d067028cc74c90f61dad) --- dirsrvtests/tickets/ticket47950_test.py | 273 +++++++++++++++++++++ ldap/servers/plugins/acl/acl.c | 6 +- ldap/servers/plugins/replication/cl5_config.c | 9 +- ldap/servers/plugins/replication/repl5_agmtlist.c | 3 +- .../plugins/replication/repl5_replica_config.c | 3 +- ldap/servers/slapd/add.c | 4 +- ldap/servers/slapd/back-ldbm/ldbm_config.c | 3 +- ldap/servers/slapd/configdse.c | 5 +- ldap/servers/slapd/opshared.c | 17 ++ ldap/servers/slapd/result.c | 2 + ldap/servers/slapd/slapi-plugin.h | 2 + ldap/servers/slapd/task.c | 4 +- ldap/servers/slapd/tools/mmldif.c | 12 +- 13 files changed, 313 insertions(+), 30 deletions(-) create mode 100644 dirsrvtests/tickets/ticket47950_test.py diff --git a/dirsrvtests/tickets/ticket47950_test.py b/dirsrvtests/tickets/ticket47950_test.py new file mode 100644 index 0000000..976f964 --- /dev/null +++ b/dirsrvtests/tickets/ticket47950_test.py @@ -0,0 +1,273 @@ +import os +import sys +import time +import ldap +import logging +import socket +import pytest +from lib389 import DirSrv, Entry, tools, tasks +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from constants import * + +log = logging.getLogger(__name__) + +installation_prefix = None + +USER1_DN = "uid=user1,%s" % DEFAULT_SUFFIX +USER2_DN = "uid=user2,%s" % DEFAULT_SUFFIX + + +class TopologyStandalone(object): + def __init__(self, standalone): + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + ''' + This fixture is used to standalone topology for the 'module'. + At the beginning, It may exists a standalone instance. + It may also exists a backup for the standalone instance. + + Principle: + If standalone instance exists: + restart it + If backup of standalone exists: + create/rebind to standalone + + restore standalone instance from backup + else: + Cleanup everything + remove instance + remove backup + Create instance + Create backup + ''' + global installation_prefix + + if installation_prefix: + args_instance[SER_DEPLOYED_DIR] = installation_prefix + + standalone = DirSrv(verbose=False) + + # Args for the standalone instance + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + + # Get the status of the backups + backup_standalone = standalone.checkBackupFS() + + # Get the status of the instance and restart it if it exists + instance_standalone = standalone.exists() + if instance_standalone: + # assuming the instance is already stopped, just wait 5 sec max + standalone.stop(timeout=5) + standalone.start(timeout=10) + + if backup_standalone: + # The backup exist, assuming it is correct + # we just re-init the instance with it + if not instance_standalone: + standalone.create() + # Used to retrieve configuration information (dbdir, confdir...) + standalone.open() + + # restore standalone instance from backup + standalone.stop(timeout=10) + standalone.restoreFS(backup_standalone) + standalone.start(timeout=10) + + else: + # We should be here only in two conditions + # - This is the first time a test involve standalone instance + # - Something weird happened (instance/backup destroyed) + # so we discard everything and recreate all + + # Remove the backup. So even if we have a specific backup file + # (e.g backup_standalone) we clear backup that an instance may have created + if backup_standalone: + standalone.clearBackupFS() + + # Remove the instance + if instance_standalone: + standalone.delete() + + # Create the instance + standalone.create() + + # Used to retrieve configuration information (dbdir, confdir...) + standalone.open() + + # Time to create the backups + standalone.stop(timeout=10) + standalone.backupfile = standalone.backupFS() + standalone.start(timeout=10) + + # clear the tmp directory + standalone.clearTmpDir(__file__) + + # + # Here we have standalone instance up and running + # Either coming from a backup recovery + # or from a fresh (re)init + # Time to return the topology + return TopologyStandalone(standalone) + + +def test_ticket47950(topology): + """ + Testing nsslapd-plugin-binddn-tracking does not cause issues around + access control and reconfiguring replication/repl agmt. + """ + + log.info('Testing Ticket 47950 - Testing nsslapd-plugin-binddn-tracking') + + # + # Turn on bind dn tracking + # + try: + topology.standalone.modify_s("cn=config", [(ldap.MOD_REPLACE, 'nsslapd-plugin-binddn-tracking', 'on')]) + log.info('nsslapd-plugin-binddn-tracking enabled.') + except ldap.LDAPError, e: + log.error('Failed to enable bind dn tracking: ' + e.message['desc']) + assert False + + # + # Add two users + # + try: + topology.standalone.add_s(Entry((USER1_DN, { + 'objectclass': "top person inetuser".split(), + 'userpassword': "password", + 'sn': "1", + 'cn': "user 1"}))) + log.info('Added test user %s' % USER1_DN) + except ldap.LDAPError, e: + log.error('Failed to add %s: %s' % (USER1_DN, e.message['desc'])) + assert False + + try: + topology.standalone.add_s(Entry((USER2_DN, { + 'objectclass': "top person inetuser".split(), + 'sn': "2", + 'cn': "user 2"}))) + log.info('Added test user %s' % USER2_DN) + except ldap.LDAPError, e: + log.error('Failed to add user1: ' + e.message['desc']) + assert False + + # + # Add an aci + # + try: + acival = '(targetattr ="cn")(version 3.0;acl "Test bind dn tracking"' + \ + ';allow (all) (userdn = "ldap:///%s");)' % USER1_DN + + topology.standalone.modify_s(DEFAULT_SUFFIX, [(ldap.MOD_ADD, 'aci', acival)]) + log.info('Added aci') + except ldap.LDAPError, e: + log.error('Failed to add aci: ' + e.message['desc']) + assert False + + # + # Make modification as user + # + try: + topology.standalone.simple_bind_s(USER1_DN, "password") + log.info('Bind as user %s successful' % USER1_DN) + except ldap.LDAPError, e: + log.error('Failed to bind as user1: ' + e.message['desc']) + assert False + + try: + topology.standalone.modify_s(USER2_DN, [(ldap.MOD_REPLACE, 'cn', 'new value')]) + log.info('%s successfully modified user %s' % (USER1_DN, USER2_DN)) + except ldap.LDAPError, e: + log.error('Failed to update user2: ' + e.message['desc']) + assert False + + # + # Setup replica and create a repl agmt + # + try: + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + log.info('Bind as %s successful' % DN_DM) + except ldap.LDAPError, e: + log.error('Failed to bind as rootDN: ' + e.message['desc']) + assert False + + try: + topology.standalone.replica.enableReplication(suffix=DEFAULT_SUFFIX, role=REPLICAROLE_MASTER, + replicaId=REPLICAID_MASTER) + log.info('Successfully enabled replication.') + except ValueError: + log.error('Failed to enable replication') + assert False + + properties = {RA_NAME: r'test plugin internal bind dn', + RA_BINDDN: defaultProperties[REPLICATION_BIND_DN], + RA_BINDPW: defaultProperties[REPLICATION_BIND_PW], + RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD], + RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]} + + try: + repl_agreement = topology.standalone.agreement.create(suffix=DEFAULT_SUFFIX, host="127.0.0.1", + port="7777", properties=properties) + log.info('Successfully created replication agreement') + except InvalidArgumentError, e: + log.error('Failed to create replication agreement: ' + e.message['desc']) + assert False + + # + # modify replica + # + try: + properties = {REPLICA_ID: "7"} + topology.standalone.replica.setProperties(DEFAULT_SUFFIX, None, None, properties) + log.info('Successfully modified replica') + except ldap.LDAPError, e: + log.error('Failed to update replica config: ' + e.message['desc']) + assert False + + # + # modify repl agmt + # + try: + properties = {RA_CONSUMER_PORT: "8888"} + topology.standalone.agreement.setProperties(None, repl_agreement, None, properties) + log.info('Successfully modified replication agreement') + except ValueError: + log.error('Failed to update replica agreement: ' + repl_agreement) + assert False + + # We passed + log.info("Test Passed.") + + +def test_ticket47953_final(topology): + topology.standalone.stop(timeout=10) + + +def run_isolated(): + ''' + run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..) + To run isolated without py.test, you need to + - edit this file and comment '@pytest.fixture' line before 'topology' function. + - set the installation prefix + - run this program + ''' + global installation_prefix + installation_prefix = None + + topo = topology(True) + test_ticket47950(topo) + +if __name__ == '__main__': + run_isolated() \ No newline at end of file diff --git a/ldap/servers/plugins/acl/acl.c b/ldap/servers/plugins/acl/acl.c index 598601b..ad929c5 100644 --- a/ldap/servers/plugins/acl/acl.c +++ b/ldap/servers/plugins/acl/acl.c @@ -1383,9 +1383,9 @@ acl_check_mods( if (be != NULL) slapi_pblock_get ( pb, SLAPI_BE_LASTMOD, &lastmod ); } - if (lastmod && - (strcmp (mod->mod_type, "modifiersname")== 0 || - strcmp (mod->mod_type, "modifytimestamp")== 0)) { + + if (lastmod && slapi_attr_is_last_mod(mod->mod_type)) { + /* skip pseudo attr(s) */ continue; } diff --git a/ldap/servers/plugins/replication/cl5_config.c b/ldap/servers/plugins/replication/cl5_config.c index 900cfc0..f99fde9 100644 --- a/ldap/servers/plugins/replication/cl5_config.c +++ b/ldap/servers/plugins/replication/cl5_config.c @@ -355,15 +355,10 @@ changelog5_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr config_attr = (char *) mods[i]->mod_type; config_attr_value = (char *) mods[i]->mod_bvalues[j]->bv_val; -#define ATTR_MODIFIERSNAME "modifiersname" -#define ATTR_MODIFYTIMESTAMP "modifytimestamp" - - if ( strcasecmp ( config_attr, ATTR_MODIFIERSNAME ) == 0 ) { - continue; - } - if ( strcasecmp ( config_attr, ATTR_MODIFYTIMESTAMP ) == 0 ) { + if ( slapi_attr_is_last_mod(config_attr)){ continue; } + /* replace existing value */ if ( strcasecmp (config_attr, CONFIG_CHANGELOG_DIR_ATTRIBUTE ) == 0 ) { diff --git a/ldap/servers/plugins/replication/repl5_agmtlist.c b/ldap/servers/plugins/replication/repl5_agmtlist.c index 70f71a8..d37704d 100644 --- a/ldap/servers/plugins/replication/repl5_agmtlist.c +++ b/ldap/servers/plugins/replication/repl5_agmtlist.c @@ -488,8 +488,7 @@ agmtlist_modify_callback(Slapi_PBlock *pb, Slapi_Entry *entryBefore, Slapi_Entry repl5_set_debug_timeout(val); slapi_ch_free_string(&val); } - else if (strcasecmp (mods[i]->mod_type, "modifytimestamp") == 0 || - strcasecmp (mods[i]->mod_type, "modifiersname") == 0 || + else if (slapi_attr_is_last_mod(mods[i]->mod_type) || strcasecmp (mods[i]->mod_type, "description") == 0) { /* ignore modifier's name and timestamp attributes and the description. */ diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c index b8dd605..94ab71b 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/ldap/servers/plugins/replication/repl5_replica_config.c @@ -484,8 +484,7 @@ replica_config_modify (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* } } /* ignore modifiers attributes added by the server */ - else if (strcasecmp (config_attr, "modifytimestamp") == 0 || - strcasecmp (config_attr, "modifiersname") == 0) + else if (slapi_attr_is_last_mod(config_attr)) { *returncode = LDAP_SUCCESS; } diff --git a/ldap/servers/slapd/add.c b/ldap/servers/slapd/add.c index 5b3b5ee..e03bc6c 100644 --- a/ldap/servers/slapd/add.c +++ b/ldap/servers/slapd/add.c @@ -865,8 +865,8 @@ static int check_rdn_for_created_attrs(Slapi_Entry *e) int i, rc = 0; Slapi_RDN *rdn = NULL; char *value = NULL; - char *type[] = {SLAPI_ATTR_UNIQUEID, "modifytimestamp", "createtimestamp", - "creatorsname", "modifiersname", 0}; + char *type[] = {SLAPI_ATTR_UNIQUEID, "modifytimestamp", "modifiersname", "internalmodifytimestamp", + "internalmodifiersname", "createtimestamp", "creatorsname", 0}; if ((rdn = slapi_rdn_new())) { slapi_rdn_init_dn(rdn, slapi_entry_get_dn_const(e)); diff --git a/ldap/servers/slapd/back-ldbm/ldbm_config.c b/ldap/servers/slapd/back-ldbm/ldbm_config.c index b35004a..11aa42d 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_config.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_config.c @@ -1731,10 +1731,9 @@ int ldbm_config_ignored_attr(char *attr_name) if (!strcasecmp("objectclass", attr_name) || !strcasecmp("cn", attr_name) || !strcasecmp("creatorsname", attr_name) || - !strcasecmp("modifiersname", attr_name) || !strcasecmp("createtimestamp", attr_name) || !strcasecmp(LDBM_NUMSUBORDINATES_STR, attr_name) || - !strcasecmp("modifytimestamp", attr_name)) { + slapi_attr_is_last_mod(attr_name)){ return 1; } else { return 0; diff --git a/ldap/servers/slapd/configdse.c b/ldap/servers/slapd/configdse.c index 9265b98..fff7353 100644 --- a/ldap/servers/slapd/configdse.c +++ b/ldap/servers/slapd/configdse.c @@ -117,7 +117,10 @@ ignore_attr_type(const char *attr_type) (strcasecmp (attr_type, "numsubordinates") == 0) || (strcasecmp (attr_type, "internalModifiersname") == 0) || (strcasecmp (attr_type, "modifytimestamp") == 0) || - (strcasecmp (attr_type, "modifiersname") == 0)) { + (strcasecmp (attr_type, "modifiersname") == 0) || + (strcasecmp (attr_type, "internalCreatorsname") == 0) || + slapi_attr_is_last_mod((char *)attr_type)) + { return 1; } diff --git a/ldap/servers/slapd/opshared.c b/ldap/servers/slapd/opshared.c index 65fcea6..7a0b544 100644 --- a/ldap/servers/slapd/opshared.c +++ b/ldap/servers/slapd/opshared.c @@ -224,6 +224,23 @@ modify_update_last_modified_attr(Slapi_PBlock *pb, Slapi_Mods *smods) } /* + * If the attribute is one of the last mod attributes return 1, + * otherwise return 0; + */ +int +slapi_attr_is_last_mod(char *attr) +{ + if(strcasecmp (attr, "modifytimestamp") == 0 || + strcasecmp (attr, "modifiersname") == 0 || + strcasecmp (attr, "internalmodifytimestamp") == 0 || + strcasecmp (attr, "internalmodifiersname") == 0) + { + return 1; + } + return 0; +} + +/* * Returns: 0 - if the operation is successful * < 0 - if operation fails. * Note that an operation is considered "failed" if a result is sent diff --git a/ldap/servers/slapd/result.c b/ldap/servers/slapd/result.c index 5463592..993dc9e 100644 --- a/ldap/servers/slapd/result.c +++ b/ldap/servers/slapd/result.c @@ -805,6 +805,8 @@ encode_attr( #define LASTMODATTR( x ) (strcasecmp( x, "modifytimestamp" ) == 0 \ || strcasecmp( x, "modifiersname" ) == 0 \ + || strcasecmp( x, "internalmodifytimestamp" ) == 0 \ + || strcasecmp( x, "internalmodifiersname" ) == 0 \ || strcasecmp( x, "createtimestamp" ) == 0 \ || strcasecmp( x, "creatorsname" ) == 0) diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 8fff870..e02127d 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -4978,6 +4978,8 @@ int slapi_filter_compare(struct slapi_filter *f1, struct slapi_filter *f2); Slapi_Filter *slapi_filter_dup(Slapi_Filter *f); int slapi_filter_changetype(Slapi_Filter *f, const char *newtype); +int slapi_attr_is_last_mod(char *attr); + /** * Normalize in-place the given filter. Normalizes the attribute types always. * If norm_values is true, will also normalize the values. diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c index 2fe1de7..0488640 100644 --- a/ldap/servers/slapd/task.c +++ b/ldap/servers/slapd/task.c @@ -779,8 +779,8 @@ static int task_modify(Slapi_PBlock *pb, Slapi_Entry *e, * stuck in by the server */ if ((strcasecmp(mods[i]->mod_type, "ttl") != 0) && (strcasecmp(mods[i]->mod_type, "nsTaskCancel") != 0) && - (strcasecmp(mods[i]->mod_type, "modifiersName") != 0) && - (strcasecmp(mods[i]->mod_type, "modifyTimestamp") != 0)) { + !slapi_attr_is_last_mod(mods[i]->mod_type)) + { /* you aren't allowed to change this! */ *returncode = LDAP_UNWILLING_TO_PERFORM; return SLAPI_DSE_CALLBACK_ERROR; diff --git a/ldap/servers/slapd/tools/mmldif.c b/ldap/servers/slapd/tools/mmldif.c index 4213e65..ebadf13 100644 --- a/ldap/servers/slapd/tools/mmldif.c +++ b/ldap/servers/slapd/tools/mmldif.c @@ -1017,9 +1017,7 @@ addnew(FILE * edf3, const char *changetype, record_t * first) for (attnum = 1, att = &first->data; attnum <= first->nattrs; attnum++, att = attribnext(att)) { - if (!stricmp(attribname(att), "modifytimestamp")) - continue; - if (!stricmp(attribname(att), "modifiersname")) + if (slapi_attr_is_last_mod(attribname(att))) continue; if (!putvalue(edf3, NULL, attribname(att), att->namelen, attribvalue(att), att->valuelen)) { @@ -1074,15 +1072,11 @@ addmodified(FILE * edf3, attrib1_t * attrib, record_t * first) */ while (a != NULL || num_b <= tot_b) { /* ignore operational attrs */ - if (num_b <= tot_b && - (stricmp(attribname(b), "modifytimestamp") == 0 || - stricmp(attribname(b), "modifiersname") == 0)) { + if ( num_b <= tot_b && slapi_attr_is_last_mod(attribname(b)) ){ b = attribnext(b); num_b++; continue; } - if (a != NULL && - (stricmp(a->name, "modifytimestamp") == 0 || - stricmp(a->name, "modifiersname") == 0)) { + if (a != NULL && slapi_attr_is_last_mod(a->name)) { a = a->next; continue; } -- 1.9.3