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!)

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
+
+
[email protected](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()

ldap/servers/plugins/acl/acl.c

@@ -1450,9 +1450,7 @@ 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; 
 			}

ldap/servers/plugins/replication/cl5_config.c

@@ -357,15 +357,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 )
 				{

ldap/servers/plugins/replication/repl5_agmtlist.c

@@ -498,8 +498,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. */

ldap/servers/plugins/replication/repl5_replica_config.c

@@ -534,8 +534,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;
                 }

ldap/servers/slapd/add.c

@@ -916,8 +916,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));

ldap/servers/slapd/back-ldbm/ldbm_config.c

@@ -1849,10 +1849,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;

ldap/servers/slapd/configdse.c

@@ -116,10 +116,9 @@ ignore_attr_type(const char *attr_type)
 		 (strcasecmp (attr_type, "aci") == 0) ||
 		 (strcasecmp (attr_type, "objectclass") == 0) ||
 		 (strcasecmp (attr_type, "numsubordinates") == 0) ||
-		 (strcasecmp (attr_type, "internalModifiersname") == 0) ||
 		 (strcasecmp (attr_type, "internalCreatorsname") == 0) ||
-		 (strcasecmp (attr_type, "modifytimestamp") == 0) ||
-		 (strcasecmp (attr_type, "modifiersname") == 0)) {
+		 slapi_attr_is_last_mod((char *)attr_type))
+	{
 		return 1;
 	}
 

ldap/servers/slapd/opshared.c

@@ -217,6 +217,23 @@ modify_update_last_modified_attr(Slapi_PBlock *pb, Slapi_Mods *smods)
                            "modifytimestamp", bvals);
 }
 
+/*
+ * 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. 

ldap/servers/slapd/result.c

@@ -1059,6 +1059,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)
 

ldap/servers/slapd/slapi-plugin.h

@@ -5278,6 +5278,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.

ldap/servers/slapd/task.c

@@ -821,8 +821,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;

ldap/servers/slapd/tools/mmldif.c

@@ -1009,9 +1009,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)) {
@@ -1066,15 +1064,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;
         }