Ticket 48170 - Parse nsIndexType correctly

Bug Description:  When parsing the nsIndexType attribute value, we were
                  only checking the length of the value to compare.  If
                  there were additional characters, or index types, they
                  would be silently ignored.

Fix Description:  Properly check the value of nsIndexType, and return an
                  error where appropriate, and improve error logging to
                  include index entry

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

Reviewed by: rmeggins(Thanks!)

dirsrvtests/tickets/ticket48170_test.py

@@ -0,0 +1,88 @@
+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 *
+
+logging.getLogger(__name__).setLevel(logging.DEBUG)
+log = logging.getLogger(__name__)
+
+installation1_prefix = None
+
+
+class TopologyStandalone(object):
+    def __init__(self, standalone):
+        standalone.open()
+        self.standalone = standalone
+
+
[email protected](scope="module")
+def topology(request):
+    global installation1_prefix
+    if installation1_prefix:
+        args_instance[SER_DEPLOYED_DIR] = installation1_prefix
+
+    # Creating standalone instance ...
+    standalone = DirSrv(verbose=False)
+    args_instance[SER_HOST] = HOST_STANDALONE
+    args_instance[SER_PORT] = PORT_STANDALONE
+    args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE
+    args_instance[SER_CREATION_SUFFIX] = DEFAULT_SUFFIX
+    args_standalone = args_instance.copy()
+    standalone.allocate(args_standalone)
+    instance_standalone = standalone.exists()
+    if instance_standalone:
+        standalone.delete()
+    standalone.create()
+    standalone.open()
+
+    # Clear out the tmp dir
+    standalone.clearTmpDir(__file__)
+
+    return TopologyStandalone(standalone)
+
+
+def test_ticket48170(topology):
+    '''
+    Attempt to add a nsIndexType wikth an invalid value:  "eq,pres"
+    '''
+
+    INDEX_DN = 'cn=cn,cn=index,cn=userroot,cn=ldbm database,cn=plugins,cn=config'
+    REJECTED = False
+    try:
+        topology.standalone.modify_s(INDEX_DN, [(ldap.MOD_ADD, 'nsINdexType', 'eq,pres')])
+    except ldap.UNWILLING_TO_PERFORM:
+        log.info('Index update correctly rejected')
+        REJECTED = True
+
+    if not REJECTED:
+        log.fatal('Invalid nsIndexType value was incorrectly accepted.')
+        assert False
+
+    log.info('Test complete')
+
+
+def test_ticket48170_final(topology):
+    topology.standalone.delete()
+    log.info('Testcase PASSED')
+
+
+def run_isolated():
+    global installation1_prefix
+    installation1_prefix = None
+
+    topo = topology(True)
+    test_ticket48170(topo)
+    test_ticket48170_final(topo)
+
+
+if __name__ == '__main__':
+    run_isolated()
+

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

@@ -654,7 +654,7 @@ attr_index_idlistsize_config(Slapi_Entry *e, struct attrinfo *ai, char *returnte
 	return rc;
 }
 
-void
+int
 attr_index_config(
     backend *be,
     char		*fname,
@@ -685,7 +685,7 @@ attr_index_config(
 		 attrValue = slapi_value_get_berval(sval);
 	} else {
 		LDAPDebug(LDAP_DEBUG_ANY, "attr_index_config: Missing indexing arguments\n", 0, 0, 0);
-		return;
+		return -1;
 	}
 
 	a = attrinfo_new();
@@ -707,19 +707,19 @@ attr_index_config(
 		for (j = slapi_attr_first_value(attr, &sval); j != -1;j = slapi_attr_next_value(attr, j, &sval)) {
 			hasIndexType = 1;
 			attrValue = slapi_value_get_berval(sval);
-			if ( strncasecmp( attrValue->bv_val, "pres", 4 ) == 0 ) {
+			if ( strcasecmp( attrValue->bv_val, "pres" ) == 0 ) {
 				a->ai_indexmask |= INDEX_PRESENCE;
-			} else if ( strncasecmp( attrValue->bv_val, "eq", 2 ) == 0 ) {
+			} else if ( strcasecmp( attrValue->bv_val, "eq" ) == 0 ) {
 				a->ai_indexmask |= INDEX_EQUALITY;
-			} else if ( strncasecmp( attrValue->bv_val, "approx", 6 ) == 0 ) {
+			} else if ( strcasecmp( attrValue->bv_val, "approx" ) == 0 ) {
 				a->ai_indexmask |= INDEX_APPROX;
-			} else if ( strncasecmp( attrValue->bv_val, "subtree", 7 ) == 0 ) {
+			} else if ( strcasecmp( attrValue->bv_val, "subtree" ) == 0 ) {
 				/* subtree should be located before "sub" */
 				a->ai_indexmask |= INDEX_SUBTREE;
 				a->ai_dup_cmp_fn = entryrdn_compare_dups;
-			} else if ( strncasecmp( attrValue->bv_val, "sub", 3 ) == 0 ) {
+			} else if ( strcasecmp( attrValue->bv_val, "sub" ) == 0 ) {
 				a->ai_indexmask |= INDEX_SUB;
-			} else if ( strncasecmp( attrValue->bv_val, "none", 4 ) == 0 ) {
+			} else if ( strcasecmp( attrValue->bv_val, "none" ) == 0 ) {
 				if ( a->ai_indexmask != 0 ) {
 					LDAPDebug(LDAP_DEBUG_ANY,
 						"%s: line %d: index type \"none\" cannot be combined with other types\n",
@@ -727,19 +727,19 @@ attr_index_config(
 				}
 				a->ai_indexmask = INDEX_OFFLINE; /* note that the index isn't available */
 			} else {
-				LDAPDebug(LDAP_DEBUG_ANY,
-					"%s: line %d: unknown index type \"%s\" (ignored)\n",
-					fname, lineno, attrValue->bv_val);
-				LDAPDebug(LDAP_DEBUG_ANY,
+				slapi_log_error(SLAPI_LOG_FATAL, "attr_index_config",
+					"%s: line %d: unknown index type \"%s\" (ignored) in entry (%s), "
 					"valid index types are \"pres\", \"eq\", \"approx\", or \"sub\"\n",
-					0, 0, 0);
+					fname, lineno, attrValue->bv_val, slapi_entry_get_dn(e));
+				attrinfo_delete(&a);
+				return -1;
 			}
 		}
 		if(hasIndexType == 0){
 			/* indexType missing, error out */
 			LDAPDebug(LDAP_DEBUG_ANY, "attr_index_config: Missing index type\n", 0, 0, 0);
 			attrinfo_delete(&a);
-			return;
+			return -1;
 		}
 	}
 
@@ -929,6 +929,8 @@ attr_index_config(
 		/* duplicate - existing version updated */
 		attrinfo_delete(&a);
 	}
+
+	return 0;
 }
 
 /*

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

@@ -96,7 +96,10 @@ static int ldbm_index_parse_entry(ldbm_instance *inst, Slapi_Entry *e,
     }
 
     /* ok the entry is good to process, pass it to attr_index_config */
-    attr_index_config(inst->inst_be, (char *)trace_string, 0, e, 0, 0);
+    if (attr_index_config(inst->inst_be, (char *)trace_string, 0, e, 0, 0)){
+        slapi_ch_free_string(index_name);
+        return LDAP_OPERATIONS_ERROR;
+    }
 
     return LDAP_SUCCESS;
 }
@@ -247,7 +250,10 @@ ldbm_instance_index_config_modify_callback(Slapi_PBlock *pb, Slapi_Entry *e,
     	 return SLAPI_DSE_CALLBACK_ERROR;
     }
 
-    attr_index_config(inst->inst_be, "from DSE modify", 0, entryAfter, 0, 0);
+    if(attr_index_config(inst->inst_be, "from DSE modify", 0, entryAfter, 0, 0)){
+        *returncode = LDAP_UNWILLING_TO_PERFORM;
+        return SLAPI_DSE_CALLBACK_ERROR;
+    }
 
     return SLAPI_DSE_CALLBACK_OK;
 }

ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

@@ -55,7 +55,7 @@ void attr_masks( backend *be, char *type, int *indexmask,
  int *syntaxmask );
 void attr_masks_ex( backend *be, char *type, int *indexmask,
  int *syntaxmask, struct attrinfo **at );
-void attr_index_config( backend *be, char *fname, int lineno,
+int attr_index_config( backend *be, char *fname, int lineno,
  Slapi_Entry *e, int init, int none );
 int ldbm_compute_init();
 void attrinfo_deletetree(ldbm_instance *inst);