Ticket 48916 - DNA Threshold set to 0 causes SIGFPE

Bug Description:  If the DNA threshold was set to 0, a divide by zero would
occur when requesting ranges.

Fix Description:  Prevent the config from setting a value of 0 for dna threshold.

If an existing site has a threshold of 0, we guard the divide operation, and
return an operations error instead.

https://fedorahosted.org/389/ticket/48916

Author: wibrown

Review by: nhosoi, mreynolds (Thank you!)

.gitignore

@@ -5,6 +5,7 @@ autom4te.cache
 .cproject
 .project
 .settings
+.cache
 *.a
 *.dirstamp
 *.la

dirsrvtests/tests/tickets/ticket48916_test.py

@@ -0,0 +1,253 @@
+import os
+import sys
+import time
+import ldap
+import logging
+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 lib389.utils import *
+
+DEBUGGING = False
+
+if DEBUGGING:
+    logging.getLogger(__name__).setLevel(logging.DEBUG)
+else:
+    logging.getLogger(__name__).setLevel(logging.INFO)
+
+
+log = logging.getLogger(__name__)
+
+
+class TopologyReplication(object):
+    """The Replication Topology Class"""
+    def __init__(self, master1, master2):
+        """Init"""
+        master1.open()
+        self.master1 = master1
+        master2.open()
+        self.master2 = master2
+
+
[email protected](scope="module")
+def topology(request):
+    """Create Replication Deployment"""
+
+    # Creating master 1...
+    if DEBUGGING:
+        master1 = DirSrv(verbose=True)
+    else:
+        master1 = DirSrv(verbose=False)
+    args_instance[SER_HOST] = HOST_MASTER_1
+    args_instance[SER_PORT] = PORT_MASTER_1
+    args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_1
+    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
+    args_master = args_instance.copy()
+    master1.allocate(args_master)
+    instance_master1 = master1.exists()
+    if instance_master1:
+        master1.delete()
+    master1.create()
+    master1.open()
+    master1.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_1)
+
+    # Creating master 2...
+    if DEBUGGING:
+        master2 = DirSrv(verbose=True)
+    else:
+        master2 = DirSrv(verbose=False)
+    args_instance[SER_HOST] = HOST_MASTER_2
+    args_instance[SER_PORT] = PORT_MASTER_2
+    args_instance[SER_SERVERID_PROP] = SERVERID_MASTER_2
+    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
+    args_master = args_instance.copy()
+    master2.allocate(args_master)
+    instance_master2 = master2.exists()
+    if instance_master2:
+        master2.delete()
+    master2.create()
+    master2.open()
+    master2.replica.enableReplication(suffix=SUFFIX, role=REPLICAROLE_MASTER, replicaId=REPLICAID_MASTER_2)
+
+    #
+    # Create all the agreements
+    #
+    # Creating agreement from master 1 to master 2
+    properties = {RA_NAME: 'meTo_' + master2.host + ':' + str(master2.port),
+                  RA_BINDDN: defaultProperties[REPLICATION_BIND_DN],
+                  RA_BINDPW: defaultProperties[REPLICATION_BIND_PW],
+                  RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD],
+                  RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]}
+    m1_m2_agmt = master1.agreement.create(suffix=SUFFIX, host=master2.host, port=master2.port, properties=properties)
+    if not m1_m2_agmt:
+        log.fatal("Fail to create a master -> master replica agreement")
+        sys.exit(1)
+    log.debug("%s created" % m1_m2_agmt)
+
+    # Creating agreement from master 2 to master 1
+    properties = {RA_NAME: 'meTo_' + master1.host + ':' + str(master1.port),
+                  RA_BINDDN: defaultProperties[REPLICATION_BIND_DN],
+                  RA_BINDPW: defaultProperties[REPLICATION_BIND_PW],
+                  RA_METHOD: defaultProperties[REPLICATION_BIND_METHOD],
+                  RA_TRANSPORT_PROT: defaultProperties[REPLICATION_TRANSPORT]}
+    m2_m1_agmt = master2.agreement.create(suffix=SUFFIX, host=master1.host, port=master1.port, properties=properties)
+    if not m2_m1_agmt:
+        log.fatal("Fail to create a master -> master replica agreement")
+        sys.exit(1)
+    log.debug("%s created" % m2_m1_agmt)
+
+    # Allow the replicas to get situated with the new agreements...
+    time.sleep(5)
+
+    #
+    # Initialize all the agreements
+    #
+    master1.agreement.init(SUFFIX, HOST_MASTER_2, PORT_MASTER_2)
+    master1.waitForReplInit(m1_m2_agmt)
+
+    # Check replication is working...
+    if master1.testReplication(DEFAULT_SUFFIX, master2):
+        log.info('Replication is working.')
+    else:
+        log.fatal('Replication is not working.')
+        assert False
+
+    def fin():
+        """If we are debugging just stop the instances, otherwise remove
+        them
+        """
+        if DEBUGGING:
+            master1.stop()
+            master2.stop()
+        else:
+            master1.delete()
+            master2.delete()
+
+    request.addfinalizer(fin)
+
+    # Clear out the tmp dir
+    master1.clearTmpDir(__file__)
+
+    return TopologyReplication(master1, master2)
+
+
+def _create_user(inst, idnum):
+    inst.add_s(Entry(
+        ('uid=user%s,ou=People,%s' % (idnum, DEFAULT_SUFFIX), {
+            'objectClass' : 'top account posixAccount'.split(' '),
+            'cn' : 'user',
+            'uid' : 'user%s' % idnum,
+            'homeDirectory' : '/home/user%s' % idnum,
+            'loginShell' : '/bin/nologin',
+            'gidNumber' : '-1',
+            'uidNumber' : '-1',
+        })
+    ))
+
+def test_ticket48916(topology):
+    """
+    https://bugzilla.redhat.com/show_bug.cgi?id=1353629
+
+    This is an issue with ID exhaustion in DNA causing a crash.
+
+    To access each DirSrv instance use:  topology.master1, topology.master2,
+        ..., topology.hub1, ..., topology.consumer1,...
+
+
+    """
+
+    if DEBUGGING:
+        # Add debugging steps(if any)...
+        pass
+
+    # Enable the plugin on both servers
+
+    dna_m1 = topology.master1.plugins.get('Distributed Numeric Assignment Plugin')
+    dna_m2 = topology.master2.plugins.get('Distributed Numeric Assignment Plugin')
+
+    # Configure it
+    # Create the container for the ranges to go into.
+
+    topology.master1.add_s(Entry(
+        ('ou=Ranges,%s' % DEFAULT_SUFFIX, {
+            'objectClass' : 'top organizationalUnit'.split(' '),
+            'ou' : 'Ranges',
+        })
+    ))
+
+    # Create the dnaAdmin?
+
+    # For now we just pinch the dn from the dna_m* types, and add the relevant child config
+    # but in the future, this could be a better plugin template type from lib389
+
+    config_dn = dna_m1.dn
+
+    topology.master1.add_s(Entry(
+        ('cn=uids,%s' % config_dn, {
+            'objectClass' : 'top dnaPluginConfig'.split(' '),
+            'cn': 'uids',
+            'dnatype': 'uidNumber gidNumber'.split(' '),
+            'dnafilter': '(objectclass=posixAccount)',
+            'dnascope': '%s' % DEFAULT_SUFFIX,
+            'dnaNextValue': '1',
+            'dnaMaxValue': '50',
+            'dnasharedcfgdn': 'ou=Ranges,%s' % DEFAULT_SUFFIX,
+            'dnaThreshold': '0',
+            'dnaRangeRequestTimeout': '60',
+            'dnaMagicRegen': '-1',
+            'dnaRemoteBindDN': 'uid=dnaAdmin,ou=People,%s' % DEFAULT_SUFFIX,
+            'dnaRemoteBindCred': 'secret123',
+            'dnaNextRange': '80-90'
+        })
+    ))
+
+    topology.master2.add_s(Entry(
+        ('cn=uids,%s' % config_dn, {
+            'objectClass' : 'top dnaPluginConfig'.split(' '),
+            'cn': 'uids',
+            'dnatype': 'uidNumber gidNumber'.split(' '),
+            'dnafilter': '(objectclass=posixAccount)',
+            'dnascope': '%s' % DEFAULT_SUFFIX,
+            'dnaNextValue': '61',
+            'dnaMaxValue': '70',
+            'dnasharedcfgdn': 'ou=Ranges,%s' % DEFAULT_SUFFIX,
+            'dnaThreshold': '2',
+            'dnaRangeRequestTimeout': '60',
+            'dnaMagicRegen': '-1',
+            'dnaRemoteBindDN': 'uid=dnaAdmin,ou=People,%s' % DEFAULT_SUFFIX,
+            'dnaRemoteBindCred': 'secret123',
+        })
+    ))
+
+
+    # Enable the plugins
+    dna_m1.enable()
+    dna_m2.enable()
+    
+    # Restart the instances
+    topology.master1.restart(60)
+    topology.master2.restart(60)
+
+    # Wait for a replication .....
+    time.sleep(40)
+
+    # Allocate the 10 members to exhaust
+
+    for i in range(1,11):
+        _create_user(topology.master2, i)
+
+    # Allocate the 11th
+    _create_user(topology.master2, 11)
+
+    log.info('Test PASSED')
+
+
+if __name__ == '__main__':
+    # Run isolated
+    # -s for DEBUG mode
+    CURRENT_FILE = os.path.realpath(__file__)
+    pytest.main("-s %s" % CURRENT_FILE)
+

ldap/servers/plugins/dna/dna.c

@@ -1244,6 +1244,12 @@ dna_parse_config_entry(Slapi_PBlock *pb, Slapi_Entry * e, int apply)
         slapi_log_error(SLAPI_LOG_CONFIG, DNA_PLUGIN_SUBSYSTEM,
                         "----------> %s [%s]\n", DNA_THRESHOLD, value);
 
+        if (entry->threshold <= 0) {
+            entry->threshold = 1;
+            slapi_log_error(SLAPI_LOG_FATAL, DNA_PLUGIN_SUBSYSTEM,
+                            "----------> %s too low, setting to [%s]\n", DNA_THRESHOLD, value);
+        }
+
         slapi_ch_free_string(&value);
     } else {
         entry->threshold = 1;
@@ -2171,7 +2177,7 @@ static int dna_dn_is_config(char *dn)
     int ret = 0;
 
     slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
-                    "--> dna_is_config\n");
+                    "--> dna_is_config %s\n", dn);
 
     if (slapi_dn_issuffix(dn, getPluginDN())) {
         ret = 1;
@@ -3404,18 +3410,21 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
 
             /* Did we already service all of these configured types? */
             if (dna_list_contains_types(generated_types, config_entry->types)) {
+                slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    no types to act upon.\n");
                 goto next;
             }
 
             /* is the entry in scope? */
             if (config_entry->scope &&
                 !slapi_dn_issuffix(dn, config_entry->scope)) {
+                slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    dn not in scope\n");
                 goto next;
             }
 
             /* is this entry in an excluded scope? */
             for (i = 0; config_entry->excludescope && config_entry->excludescope[i]; i++) {
                 if (slapi_dn_issuffix(dn, slapi_sdn_get_dn(config_entry->excludescope[i]))) {
+                    slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    dn in excluded scope\n");
                     goto next;
                 }
             }
@@ -3424,7 +3433,8 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
             if (config_entry->slapi_filter) {
                 ret = slapi_vattr_filter_test(pb, e, config_entry->slapi_filter, 0);
                 if (LDAP_SUCCESS != ret) {
-                        goto next;
+                    slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    dn does not match filter\n");
+                    goto next;
                 }
             }
 
@@ -3454,6 +3464,8 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
             }
 
             if (types_to_generate && types_to_generate[0]) {
+
+                slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    adding %s to %s as -2\n", types_to_generate[0], dn);
                 /* add - add to entry */
                 for (i = 0; types_to_generate && types_to_generate[i]; i++) {
                     slapi_entry_attr_set_charptr(e, types_to_generate[i],
@@ -3492,6 +3504,7 @@ _dna_pre_op_add(Slapi_PBlock *pb, Slapi_Entry *e, char **errstr)
                 slapi_lock_mutex(config_entry->lock);
 
                 ret = dna_first_free_value(config_entry, &setval);
+                slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    retrieved value %" PRIu64 " ret %d\n", setval, ret);
                 if (LDAP_SUCCESS != ret) {
                     /* check if we overflowed the configured range */
                     if (setval > config_entry->maxval) {
@@ -4022,18 +4035,22 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)
                     "--> dna_be_txn_pre_op\n");
 
     if (!slapi_plugin_running(pb)) {
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, " --x bailing, plugin not running\n");
         goto bail;
     }
 
     if (0 == (dn = dna_get_dn(pb))) {
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, " --x bailing, is dna dn\n");
         goto bail;
     }
 
     if (dna_dn_is_config(dn)) {
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, " --x bailing is dna config dn\n");
         goto bail;
     }
 
     if (dna_isrepl(pb)) {
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, " --x bailing replicated operation\n");
         /* if repl, the dna values should be already in the entry. */
         goto bail;
     }
@@ -4045,6 +4062,7 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)
     }
 
     if (e == NULL) {
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, " --x bailing entry is NULL\n");
         goto bail;
     } else if (LDAP_CHANGETYPE_MODIFY == modtype) {
         slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods);
@@ -4056,32 +4074,39 @@ static int dna_be_txn_pre_op(Slapi_PBlock *pb, int modtype)
 
     if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
         list = PR_LIST_HEAD(dna_global_config);
+        slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, "     using global config...\n");
 
         while (list != dna_global_config && LDAP_SUCCESS == ret) {
             config_entry = (struct configEntry *) list;
 
             /* Did we already service all of these configured types? */
             if (dna_list_contains_types(generated_types, config_entry->types)) {
+                slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM, "    All types already serviced\n");
                 goto next;
             }
 
             /* is the entry in scope? */
             if (config_entry->scope) {
-                if (!slapi_dn_issuffix(dn, config_entry->scope))
+                if (!slapi_dn_issuffix(dn, config_entry->scope)) {
+                    slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    Entry not in scope of dnaScope!\n");
                     goto next;
+                }
             }
 
             /* is this entry in an excluded scope? */
             for (i = 0; config_entry->excludescope && config_entry->excludescope[i]; i++) {
                 if (slapi_dn_issuffix(dn, slapi_sdn_get_dn(config_entry->excludescope[i]))) {
+                    slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    Entry in excluded scope, next\n");
                     goto next;
                 }
             }
-            
+
             /* does the entry match the filter? */
             if (config_entry->slapi_filter) {
-                if(LDAP_SUCCESS != slapi_vattr_filter_test(pb,e,config_entry->slapi_filter, 0))
+                if(LDAP_SUCCESS != slapi_vattr_filter_test(pb,e,config_entry->slapi_filter, 0)) {
+                    slapi_log_error(SLAPI_LOG_PLUGIN, DNA_PLUGIN_SUBSYSTEM, "    Entry does not match filter\n");
                     goto next;
+                }
             }
 
             if (LDAP_CHANGETYPE_ADD == modtype) {
@@ -4526,6 +4551,11 @@ dna_release_range(char *range_dn, PRUint64 *lower, PRUint64 *upper)
              * it instead of from the active range */
             if (config_entry->next_range_lower != 0) {
                 /* Release up to half of our values from the next range. */
+                if (config_entry->threshold == 0) {
+                    ret = LDAP_UNWILLING_TO_PERFORM;
+                    goto bail;
+                }
+
                 release = (((config_entry->next_range_upper - config_entry->next_range_lower + 1) /
                            2) / config_entry->threshold) * config_entry->threshold;