From f7ab63f9b9c1b12e710608c4729a310cd15b27fb Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Tue, 24 Oct 2017 15:38:57 -0400 Subject: [PATCH] Ticket 48393 - Improve replication config validation Bug Description: There was inconsistent behavior when modifying and adding replication configurations and agreements. There were also a few places where unsigned ints were used for values which made checking for negative values impossible. Fix Description: Added a new function to properly check "number" attribute values. Also forced failure on the actual update if an invalid value was used (previously we would log an error and use some default value). https://pagure.io/389-ds-base/issue/48393 Reviewed by: ? --- .../suites/replication/replica_config_test.py | 396 +++++++++++++++++++++ ldap/schema/01core389.ldif | 3 +- ldap/servers/plugins/replication/repl5.h | 4 + ldap/servers/plugins/replication/repl5_agmt.c | 74 ++-- ldap/servers/plugins/replication/repl5_replica.c | 172 +++++---- .../plugins/replication/repl5_replica_config.c | 156 ++++---- ldap/servers/plugins/replication/replutil.c | 26 ++ 7 files changed, 658 insertions(+), 173 deletions(-) create mode 100644 dirsrvtests/tests/suites/replication/replica_config_test.py diff --git a/dirsrvtests/tests/suites/replication/replica_config_test.py b/dirsrvtests/tests/suites/replication/replica_config_test.py new file mode 100644 index 000000000..3b8c631aa --- /dev/null +++ b/dirsrvtests/tests/suites/replication/replica_config_test.py @@ -0,0 +1,396 @@ +import logging +import pytest +import copy +import os +import ldap +from lib389._constants import * +from lib389 import Entry +from lib389.topologies import topology_st as topo + +DEBUGGING = os.getenv("DEBUGGING", default=False) +if DEBUGGING: + logging.getLogger(__name__).setLevel(logging.DEBUG) +else: + logging.getLogger(__name__).setLevel(logging.INFO) +log = logging.getLogger(__name__) + +REPLICA_DN = 'cn=replica,cn="dc=example,dc=com",cn=mapping tree,cn=config' +AGMT_DN = 'cn=test_agreement,cn=replica,cn="dc=example,dc=com",cn=mapping tree,cn=config' +notnum = 'invalid' +too_big = '9223372036854775807' +overflow = '9999999999999999999999999999999999999999999999999999999999999999999' + +replica_dict = {'objectclass': 'top nsDS5Replica'.split(), + 'nsDS5ReplicaRoot': 'dc=example,dc=com', + 'nsDS5ReplicaType': '3', + 'nsDS5Flags': '1', + 'nsDS5ReplicaId': '65535', + 'nsds5ReplicaPurgeDelay': '604800', + 'nsDS5ReplicaBindDN': 'cn=u', + 'cn': 'replica'} + +agmt_dict = {'objectClass': 'top nsDS5ReplicationAgreement'.split(), + 'cn': 'test_agreement', + 'nsDS5ReplicaRoot': 'dc=example,dc=com', + 'nsDS5ReplicaHost': 'localhost.localdomain', + 'nsDS5ReplicaPort': '5555', + 'nsDS5ReplicaBindDN': 'uid=tester', + 'nsds5ReplicaCredentials': 'password', + 'nsDS5ReplicaTransportInfo': 'LDAP', + 'nsDS5ReplicaBindMethod': 'SIMPLE'} + + +repl_add_attrs = [('nsDS5ReplicaType', '-1', '4', overflow, notnum, '1'), + ('nsDS5Flags', '-1', '2', overflow, notnum, '1'), + ('nsDS5ReplicaId', '0', '65536', overflow, notnum, '1'), + ('nsds5ReplicaPurgeDelay', '-2', too_big, overflow, notnum, '1'), + ('nsDS5ReplicaBindDnGroupCheckInterval', '-2', too_big, overflow, notnum, '1'), + ('nsds5ReplicaTombstonePurgeInterval', '-2', too_big, overflow, notnum, '1'), + ('nsds5ReplicaProtocolTimeout', '-1', too_big, overflow, notnum, '1'), + ('nsds5ReplicaReleaseTimeout', '-1', too_big, overflow, notnum, '1'), + ('nsds5ReplicaBackoffMin', '0', too_big, overflow, notnum, '3'), + ('nsds5ReplicaBackoffMax', '0', too_big, overflow, notnum, '6')] + +repl_mod_attrs = [('nsDS5Flags', '-1', '2', overflow, notnum, '1'), + ('nsds5ReplicaPurgeDelay', '-2', too_big, overflow, notnum, '1'), + ('nsDS5ReplicaBindDnGroupCheckInterval', '-2', too_big, overflow, notnum, '1'), + ('nsds5ReplicaTombstonePurgeInterval', '-2', too_big, overflow, notnum, '1'), + ('nsds5ReplicaProtocolTimeout', '-1', too_big, overflow, notnum, '1'), + ('nsds5ReplicaReleaseTimeout', '-1', too_big, overflow, notnum, '1'), + ('nsds5ReplicaBackoffMin', '0', too_big, overflow, notnum, '3'), + ('nsds5ReplicaBackoffMax', '0', too_big, overflow, notnum, '6')] + +agmt_attrs = [('nsds5ReplicaTimeout', '-1', too_big, overflow, notnum, '6'), + ('nsds5ReplicaBusyWaitTime', '-1', too_big, overflow, notnum, '6'), + ('nsds5ReplicaSessionPauseTime', '-1', too_big, overflow, notnum, '6'), + ('nsds5ReplicaFlowControlWindow', '-1', too_big, overflow, notnum, '6'), + ('nsds5ReplicaFlowControlPause', '-1', too_big, overflow, notnum, '6'), + ('nsds5ReplicaProtocolTimeout', '-1', too_big, overflow, notnum, '6')] + + +def replica_setup(topo): + """Add a valid replica config entry to modify + """ + try: + topo.standalone.delete_s(REPLICA_DN) + except: + pass + + try: + topo.standalone.add_s(Entry((REPLICA_DN, replica_dict))) + except ldap.LDAPError as e: + log.fatal("Failed to add replica entry: " + str(e)) + assert False + + +def replica_reset(topo): + try: + topo.standalone.delete_s(REPLICA_DN) + except: + pass + + +def agmt_setup(topo): + """Add a valid replica config entry to modify + """ + try: + topo.standalone.delete_s(AGMT_DN) + except: + pass + + try: + topo.standalone.add_s(Entry((AGMT_DN, agmt_dict))) + except ldap.LDAPError as e: + log.fatal("Failed to add agreement entry: " + str(e)) + assert False + + +def agmt_reset(topo): + try: + topo.standalone.delete_s(AGMT_DN) + except: + pass + + +@pytest.mark.parametrize("attr, too_small, too_big, overflow, notnum, valid", repl_add_attrs) +def test_replica_num_add(topo, attr, too_small, too_big, overflow, notnum, valid): + """Test all the number values you can set for a replica config entry + + :id: a8b47d4a-a089-4d70-8070-e6181209bf92 + :setup: standalone instance + :steps: + 1. Use a value that is too small + 2. Use a value that is too big + 3. Use a value that overflows the int + 4. Use a value with character value (not a number) + 5. Use a valid value + :expectedresults: + 1. Add is rejected + 2. Add is rejected + 3. Add is rejected + 4. Add is rejected + 5. Add is allowed + """ + + replica_reset(topo) + + # Test too small + my_replica = copy.deepcopy(replica_dict) + my_replica[attr] = too_small + try: + topo.standalone.add_s(Entry((REPLICA_DN, my_replica))) + log.fatal("Incorrectly allowed to add replica entry with {}:{}".format(attr, too_small)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add replica entry with {}:{} error: {}".format(attr, too_small, str(e))) + + # Test too big + my_replica = copy.deepcopy(replica_dict) + my_replica[attr] = too_big + try: + topo.standalone.add_s(Entry((REPLICA_DN, my_replica))) + log.fatal("Incorrectly allowed to add replica entry with {}:{}".format(attr, too_big)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add replica entry with {}:{} error: {}".format(attr, too_big, str(e))) + + # Test overflow + my_replica = copy.deepcopy(replica_dict) + my_replica[attr] = overflow + try: + topo.standalone.add_s(Entry((REPLICA_DN, my_replica))) + log.fatal("Incorrectly allowed to add replica entry with {}:{}".format(attr, overflow)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add replica entry with {}:{} error: {}".format(attr, overflow, str(e))) + + # test not a number + my_replica = copy.deepcopy(replica_dict) + my_replica[attr] = notnum + try: + topo.standalone.add_s(Entry((REPLICA_DN, my_replica))) + log.fatal("Incorrectly allowed to add replica entry with {}:{}".format(attr, notnum)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add replica entry with {}:{} error: {}".format(attr, notnum, str(e))) + + # Test valid value + my_replica = copy.deepcopy(replica_dict) + my_replica[attr] = valid + try: + topo.standalone.add_s(Entry((REPLICA_DN, my_replica))) + log.info("Correctly allowed to add replica entry with {}: {}".format(attr, valid)) + except ldap.LDAPError as e: + log.fatal("Incorrectly failed to add replica entry with {}: {} error: {}".format(attr, valid, str(e))) + assert False + + +@pytest.mark.parametrize("attr, too_small, too_big, overflow, notnum, valid", repl_mod_attrs) +def test_replica_num_modify(topo, attr, too_small, too_big, overflow, notnum, valid): + """Test all the number values you can set for a replica config entry + + :id: a8b47d4a-a089-4d70-8070-e6181209bf93 + :setup: standalone instance + :steps: + 1. Replace a value that is too small + 2. Repalce a value that is too big + 3. Replace a value that overflows the int + 4. Replace a value with character value (not a number) + 5. Replace a vlue with a valid value + :expectedresults: + 1. Value is rejected + 2. Value is rejected + 3. Value is rejected + 4. Value is rejected + 5. Value is allowed + """ + + # Value too small + replica_setup(topo) + try: + topo.standalone.modify_s(REPLICA_DN, [(ldap.MOD_REPLACE, attr, too_small)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, too_small)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, too_small)) + + # Value too big + replica_setup(topo) + try: + topo.standalone.modify_s(REPLICA_DN, [(ldap.MOD_REPLACE, attr, too_big)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, too_big)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, too_big)) + + # Value overflow + replica_setup(topo) + try: + topo.standalone.modify_s(REPLICA_DN, [(ldap.MOD_REPLACE, attr, overflow)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, overflow)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, overflow)) + + # Value not a number + replica_setup(topo) + try: + topo.standalone.modify_s(REPLICA_DN, [(ldap.MOD_REPLACE, attr, notnum)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, notnum)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, notnum)) + + # Value is valid + replica_setup(topo) + try: + topo.standalone.modify_s(REPLICA_DN, [(ldap.MOD_REPLACE, attr, valid)]) + log.info('Correctly added valid agreement attribute value: {}:{}'.format(attr, valid)) + except ldap.LDAPError as e: + log.fatal('Valid value for {}:{} was incorrectly rejected. Error {}'.format(attr, valid, str(e))) + assert False + + +@pytest.mark.parametrize("attr, too_small, too_big, overflow, notnum, valid", agmt_attrs) +def test_agmt_num_add(topo, attr, too_small, too_big, overflow, notnum, valid): + """Test all the number values you can set for a replica config entry + + :id: a8b47d4a-a089-4d70-8070-e6181209bf94 + :setup: standalone instance + :steps: + 1. Use a value that is too small + 2. Use a value that is too big + 3. Use a value that overflows the int + 4. Use a value with character value (not a number) + 5. Use a valid value + :expectedresults: + 1. Add is rejected + 2. Add is rejected + 3. Add is rejected + 4. Add is rejected + 5. Add is allowed + """ + agmt_reset(topo) + + # Test too small + my_agmt = copy.deepcopy(agmt_dict) + my_agmt[attr] = too_small + try: + topo.standalone.add_s(Entry((AGMT_DN, my_agmt))) + log.fatal("Incorrectly allowed to add agreement entry with {}:{}".format(attr, too_small)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add agreement entry with {}:{} error: {}".format(attr, too_small, str(e))) + + # Test too big + my_agmt = copy.deepcopy(agmt_dict) + my_agmt[attr] = too_big + try: + topo.standalone.add_s(Entry((AGMT_DN, my_agmt))) + log.fatal("Incorrectly allowed to add agreement entry with {}:{}".format(attr, too_big)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add agreement entry with {}:{} error: {}".format(attr, too_big, str(e))) + + # Test overflow + my_agmt = copy.deepcopy(agmt_dict) + my_agmt[attr] = overflow + try: + topo.standalone.add_s(Entry((AGMT_DN, my_agmt))) + log.fatal("Incorrectly allowed to add agreement entry with {}:{}".format(attr, overflow)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add agreement entry with {}:{} error: {}".format(attr, overflow, str(e))) + + # test not a number + my_agmt = copy.deepcopy(agmt_dict) + my_agmt[attr] = notnum + try: + topo.standalone.add_s(Entry((AGMT_DN, my_agmt))) + log.fatal("Incorrectly allowed to add agreement entry with {}:{}".format(attr, notnum)) + assert False + except ldap.LDAPError as e: + log.info("Correctly failed to add agreement entry with {}:{} error: {}".format(attr, notnum, str(e))) + + # Test valid value + my_agmt = copy.deepcopy(agmt_dict) + my_agmt[attr] = valid + try: + topo.standalone.add_s(Entry((AGMT_DN, my_agmt))) + log.info("Correctly allowed to add agreement entry with {}: {}".format(attr, valid)) + except ldap.LDAPError as e: + log.fatal("Incorrectly failed to add agreement entry with {}: {} error: {}".format(attr, valid, str(e))) + assert False + + +@pytest.mark.parametrize("attr, too_small, too_big, overflow, notnum, valid", agmt_attrs) +def test_agmt_num_modify(topo, attr, too_small, too_big, overflow, notnum, valid): + """Test all the number values you can set for a replica config entry + + :id: a8b47d4a-a089-4d70-8070-e6181209bf95 + :setup: standalone instance + :steps: + 1. Replace a value that is too small + 2. Replace a value that is too big + 3. Replace a value that overflows the int + 4. Replace a value with character value (not a number) + 5. Replace a vlue with a valid value + :expectedresults: + 1. Value is rejected + 2. Value is rejected + 3. Value is rejected + 4. Value is rejected + 5. Value is allowed + """ + + # Value too small + agmt_setup(topo) + try: + topo.standalone.modify_s(AGMT_DN, [(ldap.MOD_REPLACE, attr, too_small)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, too_small)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, too_small)) + + # Value too big + agmt_setup(topo) + try: + topo.standalone.modify_s(AGMT_DN, [(ldap.MOD_REPLACE, attr, too_big)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, too_big)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, too_big)) + + # Value overflow + agmt_setup(topo) + try: + topo.standalone.modify_s(AGMT_DN, [(ldap.MOD_REPLACE, attr, overflow)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, overflow)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, overflow)) + + # Value not a number + agmt_setup(topo) + try: + topo.standalone.modify_s(AGMT_DN, [(ldap.MOD_REPLACE, attr, notnum)]) + log.fatal('Invalid value for {}:{} was incorrectly allowed'.format(attr, notnum)) + assert False + except: + log.info('Invalid value for {}:{} was correctly rejected'.format(attr, notnum)) + + # Value is valid + agmt_setup(topo) + try: + topo.standalone.modify_s(AGMT_DN, [(ldap.MOD_REPLACE, attr, valid)]) + except ldap.LDAPError as e: + log.fatal('Valid value for {}:{} was incorrectly rejected. Error {}'.format(attr, valid, 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/schema/01core389.ldif b/ldap/schema/01core389.ldif index 246495214..2bdc740ce 100644 --- a/ldap/schema/01core389.ldif +++ b/ldap/schema/01core389.ldif @@ -303,6 +303,7 @@ attributeTypes: ( 2.16.840.1.113730.3.1.2331 NAME 'nsslapd-logging-hr-timestamps attributeTypes: ( 2.16.840.1.113730.3.1.2332 NAME 'allowWeakDHParam' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2333 NAME 'nsds5ReplicaReleaseTimeout' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) attributeTypes: ( 2.16.840.1.113730.3.1.2335 NAME 'nsds5ReplicaIgnoreMissingChange' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) +attributeTypes: ( 2.16.840.1.113730.3.1.2336 NAME 'nsDS5ReplicaBindDnGroupCheckInterval' DESC 'Netscape defined attribute type' SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'Netscape Directory Server' ) # # objectclasses # @@ -312,7 +313,7 @@ objectClasses: ( 2.16.840.1.113730.3.2.44 NAME 'nsIndex' DESC 'Netscape defined objectClasses: ( 2.16.840.1.113730.3.2.109 NAME 'nsBackendInstance' DESC 'Netscape defined objectclass' SUP top MUST ( CN ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.110 NAME 'nsMappingTree' DESC 'Netscape defined objectclass' SUP top MUST ( CN ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.104 NAME 'nsContainer' DESC 'Netscape defined objectclass' SUP top MUST ( CN ) X-ORIGIN 'Netscape Directory Server' ) -objectClasses: ( 2.16.840.1.113730.3.2.108 NAME 'nsDS5Replica' DESC 'Netscape defined objectclass' SUP top MUST ( nsDS5ReplicaRoot $ nsDS5ReplicaId ) MAY (cn $ nsds5ReplicaPreciseTombstonePurging $ nsds5ReplicaCleanRUV $ nsds5ReplicaAbortCleanRUV $ nsDS5ReplicaType $ nsDS5ReplicaBindDN $ nsState $ nsDS5ReplicaName $ nsDS5Flags $ nsDS5Task $ nsDS5ReplicaReferral $ nsDS5ReplicaAutoReferral $ nsds5ReplicaPurgeDelay $ nsds5ReplicaTombstonePurgeInterval $ nsds5ReplicaChangeCount $ nsds5ReplicaLegacyConsumer $ nsds5ReplicaProtocolTimeout $ nsds5ReplicaBackoffMin $ nsds5ReplicaBackoffMax $ nsds5ReplicaReleaseTimeout ) X-ORIGIN 'Netscape Directory Server' ) +objectClasses: ( 2.16.840.1.113730.3.2.108 NAME 'nsDS5Replica' DESC 'Netscape defined objectclass' SUP top MUST ( nsDS5ReplicaRoot $ nsDS5ReplicaId ) MAY (cn $ nsds5ReplicaPreciseTombstonePurging $ nsds5ReplicaCleanRUV $ nsds5ReplicaAbortCleanRUV $ nsDS5ReplicaType $ nsDS5ReplicaBindDN $ nsState $ nsDS5ReplicaName $ nsDS5Flags $ nsDS5Task $ nsDS5ReplicaReferral $ nsDS5ReplicaAutoReferral $ nsds5ReplicaPurgeDelay $ nsds5ReplicaTombstonePurgeInterval $ nsds5ReplicaChangeCount $ nsds5ReplicaLegacyConsumer $ nsds5ReplicaProtocolTimeout $ nsds5ReplicaBackoffMin $ nsds5ReplicaBackoffMax $ nsds5ReplicaReleaseTimeout $ nsDS5ReplicaBindDnGroupCheckInterval ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.113 NAME 'nsTombstone' DESC 'Netscape defined objectclass' SUP top MAY ( nstombstonecsn $ nsParentUniqueId $ nscpEntryDN ) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.103 NAME 'nsDS5ReplicationAgreement' DESC 'Netscape defined objectclass' SUP top MUST ( cn ) MAY ( nsds5ReplicaCleanRUVNotified $ nsDS5ReplicaHost $ nsDS5ReplicaPort $ nsDS5ReplicaTransportInfo $ nsDS5ReplicaBindDN $ nsDS5ReplicaCredentials $ nsDS5ReplicaBindMethod $ nsDS5ReplicaRoot $ nsDS5ReplicatedAttributeList $ nsDS5ReplicatedAttributeListTotal $ nsDS5ReplicaUpdateSchedule $ nsds5BeginReplicaRefresh $ description $ nsds50ruv $ nsruvReplicaLastModified $ nsds5ReplicaTimeout $ nsds5replicaChangesSentSinceStartup $ nsds5replicaLastUpdateEnd $ nsds5replicaLastUpdateStart $ nsds5replicaLastUpdateStatus $ nsds5replicaUpdateInProgress $ nsds5replicaLastInitEnd $ nsds5ReplicaEnabled $ nsds5replicaLastInitStart $ nsds5replicaLastInitStatus $ nsds5debugreplicatimeout $ nsds5replicaBusyWaitTime $ nsds5ReplicaStripAttrs $ nsds5replicaSessionPauseTime $ nsds5ReplicaProtocolTimeout $ nsds5ReplicaFlowControlWindow $ nsds5ReplicaFlowControlPause $ nsDS5ReplicaWaitForAsyncResults $ nsds5ReplicaIgnoreMissingChange) X-ORIGIN 'Netscape Directory Server' ) objectClasses: ( 2.16.840.1.113730.3.2.39 NAME 'nsslapdConfig' DESC 'Netscape defined objectclass' SUP top MAY ( cn ) X-ORIGIN 'Netscape Directory Server' ) diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h index 3bd878d4d..717fd1bb4 100644 --- a/ldap/servers/plugins/replication/repl5.h +++ b/ldap/servers/plugins/replication/repl5.h @@ -685,8 +685,10 @@ PRBool replica_is_updatedn(Replica *r, const Slapi_DN *sdn); void replica_set_updatedn(Replica *r, const Slapi_ValueSet *vs, int mod_op); void replica_set_groupdn(Replica *r, const Slapi_ValueSet *vs, int mod_op); char *replica_get_generation(const Replica *r); + /* currently supported flags */ #define REPLICA_LOG_CHANGES 0x1 /* enable change logging */ + PRBool replica_is_flag_set(const Replica *r, PRUint32 flag); void replica_set_flag(Replica *r, PRUint32 flag, PRBool clear); void replica_replace_flags(Replica *r, PRUint32 flags); @@ -836,6 +838,8 @@ LDAPControl *create_managedsait_control(void); LDAPControl *create_backend_control(Slapi_DN *sdn); void repl_set_mtn_state_and_referrals(const Slapi_DN *sdn, const char *mtn_state, const RUV *ruv, char **ruv_referrals, char **other_referrals); void repl_set_repl_plugin_path(const char *path); +int repl_config_valid_num(const char *config_attr, char *config_attr_value, int min, int max, + int *returncode, char *errortext, int *retval); /* repl5_updatedn_list.c */ typedef void *ReplicaUpdateDNList; diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c index e2ab320e4..e4c68ced5 100644 --- a/ldap/servers/plugins/replication/repl5_agmt.c +++ b/ldap/servers/plugins/replication/repl5_agmt.c @@ -241,10 +241,14 @@ agmt_new_from_entry(Slapi_Entry *e) { Repl_Agmt *ra; Slapi_Attr *sattr; + char errormsg[SLAPI_DSE_RETURNTEXT_SIZE]; char *tmpstr; char **denied_attrs = NULL; char *auto_initialize = NULL; char *val_nsds5BeginReplicaRefresh = "start"; + char *val = NULL; + int ptimeout = 0; + int rc = 0; ra = (Repl_Agmt *)slapi_ch_calloc(1, sizeof(repl5agmt)); if ((ra->lock = PR_NewLock()) == NULL) { @@ -313,29 +317,35 @@ agmt_new_from_entry(Slapi_Entry *e) /* timeout. */ ra->timeout = DEFAULT_TIMEOUT; - if (slapi_entry_attr_find(e, type_nsds5ReplicaTimeout, &sattr) == 0) { - Slapi_Value *sval; - if (slapi_attr_first_value(sattr, &sval) == 0) { - ra->timeout = slapi_value_get_long(sval); + if ((val = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaTimeout))){ + int timeout; + if (repl_config_valid_num(type_nsds5ReplicaTimeout, val, 0, INT_MAX, &rc, errormsg, &timeout) != 0) { + goto loser; } + slapi_ch_free_string(&val); + ra->timeout = timeout; } /* flow control update window. */ ra->flowControlWindow = DEFAULT_FLOWCONTROL_WINDOW; - if (slapi_entry_attr_find(e, type_nsds5ReplicaFlowControlWindow, &sattr) == 0) { - Slapi_Value *sval; - if (slapi_attr_first_value(sattr, &sval) == 0) { - ra->flowControlWindow = slapi_value_get_long(sval); + if ((val = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaFlowControlWindow))){ + int flow; + if (repl_config_valid_num(type_nsds5ReplicaTimeout, val, 0, INT_MAX, &rc, errormsg, &flow) != 0) { + goto loser; } + slapi_ch_free_string(&val); + ra->flowControlWindow = flow; } /* flow control update pause. */ ra->flowControlPause = DEFAULT_FLOWCONTROL_PAUSE; - if (slapi_entry_attr_find(e, type_nsds5ReplicaFlowControlPause, &sattr) == 0) { - Slapi_Value *sval; - if (slapi_attr_first_value(sattr, &sval) == 0) { - ra->flowControlPause = slapi_value_get_long(sval); + if ((val = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaFlowControlPause))){ + int pause; + if (repl_config_valid_num(type_nsds5ReplicaFlowControlPause, val, 0, INT_MAX, &rc, errormsg, &pause) != 0) { + goto loser; } + slapi_ch_free_string(&val); + ra->flowControlPause = pause; } /* continue on missing change ? */ @@ -357,7 +367,6 @@ agmt_new_from_entry(Slapi_Entry *e) if (NULL != tmpstr) { Object *repl_obj; Replica *replica; - PRUint64 ptimeout = 0; ra->replarea = slapi_sdn_new_dn_passin(tmpstr); @@ -367,14 +376,18 @@ agmt_new_from_entry(Slapi_Entry *e) replica_incr_agmt_count(replica); } } + } - /* If this agmt has its own timeout, grab it, otherwise use the replica's protocol timeout */ - ptimeout = slapi_entry_attr_get_int(e, type_replicaProtocolTimeout); - if (ptimeout) { - slapi_counter_set_value(ra->protocol_timeout, ptimeout); + /* If this agmt has its own timeout, grab it, otherwise use the replica's protocol timeout */ + if ((val = slapi_entry_attr_get_charptr(e, type_replicaProtocolTimeout))){ + if (repl_config_valid_num(type_replicaProtocolTimeout, val, 0, INT_MAX, &rc, errormsg, &ptimeout) != 0) { + goto loser; } + slapi_ch_free_string(&val); + slapi_counter_set_value(ra->protocol_timeout, ptimeout); } + /* Replica enabled */ tmpstr = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaEnabled); if (NULL != tmpstr) { @@ -384,9 +397,8 @@ agmt_new_from_entry(Slapi_Entry *e) ra->is_enabled = PR_TRUE; } else { slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "agmt_new_from_entry - " - "Warning invalid value for nsds5ReplicaEnabled (%s), value must be \"on\" or \"off\". " - "Ignoring this repl agreement.\n", - tmpstr); + "Warning invalid value for nsds5ReplicaEnabled (%s), value must be \"on\" or \"off\". " + "Ignoring this repl agreement.\n", tmpstr); slapi_ch_free_string(&tmpstr); goto loser; } @@ -402,11 +414,24 @@ agmt_new_from_entry(Slapi_Entry *e) } /* busy wait time - time to wait after getting REPLICA BUSY from consumer */ - ra->busywaittime = slapi_entry_attr_get_long(e, type_nsds5ReplicaBusyWaitTime); + if ((val = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaBusyWaitTime))){ + int busytime = 0; + if (repl_config_valid_num(type_nsds5ReplicaBusyWaitTime, val, 0, INT_MAX, &rc, errormsg, &busytime) != 0) { + goto loser; + } + slapi_ch_free_string(&val); + ra->busywaittime = busytime; + } /* pause time - time to pause after a session has ended */ - ra->pausetime = slapi_entry_attr_get_long(e, type_nsds5ReplicaSessionPauseTime); - + if ((val = slapi_entry_attr_get_charptr(e, type_nsds5ReplicaSessionPauseTime))){ + int pausetime = 0; + if (repl_config_valid_num(type_nsds5ReplicaSessionPauseTime, val, 0, INT_MAX, &rc, errormsg, &pausetime) != 0) { + goto loser; + } + slapi_ch_free_string(&val); + ra->pausetime = pausetime; + } /* consumer's RUV */ if (slapi_entry_attr_find(e, type_ruvElement, &sattr) == 0) { RUV *ruv; @@ -534,6 +559,9 @@ agmt_new_from_entry(Slapi_Entry *e) return ra; loser: + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, + "agmt_new_from_entry - Failed to parse agreement, skipping.\n"); + slapi_ch_free_string(&val); agmt_delete((void **)&ra); return NULL; } diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c index 92f847f24..6c25ced7e 100644 --- a/ldap/servers/plugins/replication/repl5_replica.c +++ b/ldap/servers/plugins/replication/repl5_replica.c @@ -53,10 +53,10 @@ struct replica Slapi_Eq_Context repl_eqcxt_tr; /* context to cancel event that reaps tombstones */ Object *repl_csngen; /* CSN generator for this replica */ PRBool repl_csn_assigned; /* Flag set when new csn is assigned. */ - PRUint32 repl_purge_delay; /* When purgeable, CSNs are held on to for this many extra seconds */ + int64_t repl_purge_delay; /* When purgeable, CSNs are held on to for this many extra seconds */ PRBool tombstone_reap_stop; /* TRUE when the tombstone reaper should stop */ PRBool tombstone_reap_active; /* TRUE when the tombstone reaper is running */ - long tombstone_reap_interval; /* Time in seconds between tombstone reaping */ + int64_t tombstone_reap_interval; /* Time in seconds between tombstone reaping */ Slapi_ValueSet *repl_referral; /* A list of administrator provided referral URLs */ PRBool state_update_inprogress; /* replica state is being updated */ PRLock *agmt_lock; /* protects agreement creation, start and stop */ @@ -1829,7 +1829,6 @@ _replica_check_validity(const Replica *r) static int _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) { - Slapi_Attr *a = NULL; Slapi_Attr *attr; CSNGen *gen; char *precise_purging = NULL; @@ -1840,6 +1839,8 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) int backoff_max; int ptimeout = 0; int release_timeout = 0; + int interval = 0; + int rtype = 0; int rc; PR_ASSERT(r && e); @@ -1847,7 +1848,7 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) /* get replica root */ val = slapi_entry_attr_get_charptr(e, attr_replicaRoot); if (val == NULL) { - PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, "Failed to retrieve %s attribute from (%s)\n", + PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, "Failed to retrieve %s attribute from (%s)", attr_replicaRoot, (char *)slapi_entry_get_dn((Slapi_Entry *)e)); slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "_replica_init_from_config - %s\n", @@ -1858,66 +1859,94 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) r->repl_root = slapi_sdn_new_dn_passin(val); /* get replica type */ - val = slapi_entry_attr_get_charptr(e, attr_replicaType); - if (val) { - r->repl_type = atoi(val); - slapi_ch_free((void **)&val); + if (slapi_entry_attr_exists(e, attr_replicaType)) { + if ((val = slapi_entry_attr_get_charptr(e, attr_replicaType))) { + if (repl_config_valid_num(attr_replicaType, val, 0, REPLICA_TYPE_UPDATABLE, &rc, errormsg, &rtype) != 0) { + slapi_ch_free_string(&val); + return -1; + } + r->repl_type = rtype; + slapi_ch_free_string(&val); + } else { + r->repl_type = REPLICA_TYPE_READONLY; + } } else { r->repl_type = REPLICA_TYPE_READONLY; } - /* grab and validate the backoff retry settings */ + /* grab and validate the backoff min retry settings */ if (slapi_entry_attr_exists(e, type_replicaBackoffMin)) { - backoff_min = slapi_entry_attr_get_int(e, type_replicaBackoffMin); - if (backoff_min <= 0) { - slapi_log_err(SLAPI_LOG_WARNING, repl_plugin_name, "_replica_init_from_config - " - "Invalid value for %s: %d Using default value (%d)\n", - type_replicaBackoffMin, backoff_min, PROTOCOL_BACKOFF_MINIMUM); + if ((val = slapi_entry_attr_get_charptr(e, type_replicaBackoffMin))) { + if (repl_config_valid_num(type_replicaBackoffMin, val, 1, INT_MAX, &rc, errormsg, &backoff_min) != 0) { + slapi_ch_free_string(&val); + return -1; + } + slapi_ch_free_string(&val); + } else { backoff_min = PROTOCOL_BACKOFF_MINIMUM; } } else { backoff_min = PROTOCOL_BACKOFF_MINIMUM; } + /* grab and validate the backoff max retry settings */ if (slapi_entry_attr_exists(e, type_replicaBackoffMax)) { - backoff_max = slapi_entry_attr_get_int(e, type_replicaBackoffMax); - if (backoff_max <= 0) { - slapi_log_err(SLAPI_LOG_WARNING, repl_plugin_name, "_replica_init_from_config - " - "Invalid value for %s: %d Using default value (%d)\n", - type_replicaBackoffMax, backoff_max, PROTOCOL_BACKOFF_MAXIMUM); + if ((val = slapi_entry_attr_get_charptr(e, type_replicaBackoffMax))) { + if (repl_config_valid_num(type_replicaBackoffMax, val, 1, INT_MAX, &rc, errormsg, &backoff_max) != 0) { + slapi_ch_free_string(&val); + return -1; + } + slapi_ch_free_string(&val); + } else { backoff_max = PROTOCOL_BACKOFF_MAXIMUM; } } else { backoff_max = PROTOCOL_BACKOFF_MAXIMUM; } + /* Verify backoff min and max work together */ if (backoff_min > backoff_max) { - /* Ok these values are invalid, reset back the defaults */ - slapi_log_err(SLAPI_LOG_WARNING, repl_plugin_name, "_replica_init_from_config - " - "Backoff minimum (%d) can not be greater than " - "the backoff maximum (%d). Using default values: min (%d) max (%d)\n", - backoff_min, backoff_max, PROTOCOL_BACKOFF_MINIMUM, PROTOCOL_BACKOFF_MAXIMUM); - slapi_counter_set_value(r->backoff_min, PROTOCOL_BACKOFF_MINIMUM); - slapi_counter_set_value(r->backoff_max, PROTOCOL_BACKOFF_MAXIMUM); + PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, + "Backoff minimum (%d) can not be greater than the backoff maximum (%d).", + backoff_min, backoff_max); + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "_replica_init_from_config - " + "%s\n", errormsg); + return -1; } else { slapi_counter_set_value(r->backoff_min, backoff_min); slapi_counter_set_value(r->backoff_max, backoff_max); } /* get the protocol timeout */ - ptimeout = slapi_entry_attr_get_int(e, type_replicaProtocolTimeout); - if (ptimeout <= 0) { - slapi_counter_set_value(r->protocol_timeout, DEFAULT_PROTOCOL_TIMEOUT); + if (slapi_entry_attr_exists(e, type_replicaProtocolTimeout)) { + if ((val = slapi_entry_attr_get_charptr(e, type_replicaProtocolTimeout))) { + if (repl_config_valid_num(type_replicaProtocolTimeout, val, 0, INT_MAX, &rc, errormsg, &ptimeout) != 0) { + slapi_ch_free_string(&val); + return -1; + } + slapi_ch_free_string(&val); + slapi_counter_set_value(r->protocol_timeout, ptimeout); + } else { + slapi_counter_set_value(r->protocol_timeout, DEFAULT_PROTOCOL_TIMEOUT); + } } else { - slapi_counter_set_value(r->protocol_timeout, ptimeout); + slapi_counter_set_value(r->protocol_timeout, DEFAULT_PROTOCOL_TIMEOUT); } /* Get the release timeout */ - release_timeout = slapi_entry_attr_get_int(e, type_replicaReleaseTimeout); - if (release_timeout <= 0) { - slapi_counter_set_value(r->release_timeout, 0); + if (slapi_entry_attr_exists(e, type_replicaReleaseTimeout)) { + if ((val = slapi_entry_attr_get_charptr(e, type_replicaReleaseTimeout))) { + if (repl_config_valid_num(type_replicaReleaseTimeout, val, 0, INT_MAX, &rc, errortext, &release_timeout) != 0) { + slapi_ch_free_string(&val); + return -1; + } + slapi_counter_set_value(r->release_timeout, release_timeout); + slapi_ch_free_string(&val); + } else { + slapi_counter_set_value(r->release_timeout, 0); + } } else { - slapi_counter_set_value(r->release_timeout, release_timeout); + slapi_counter_set_value(r->release_timeout, 0); } /* check for precise tombstone purging */ @@ -1929,10 +1958,11 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) slapi_counter_set_value(r->precise_purging, 0); } else { /* Invalid value */ - slapi_log_err(SLAPI_LOG_WARNING, repl_plugin_name, "_replica_init_from_config - " - "Invalid value for %s: %s Using default value (off)\n", - type_replicaPrecisePurge, precise_purging); - slapi_counter_set_value(r->precise_purging, 0); + PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, "Invalid value for %s: %s", + type_replicaPrecisePurge, precise_purging); + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "_replica_init_from_config - " + "%s\n", errormsg); + return -1; } slapi_ch_free_string(&precise_purging); } else { @@ -1940,7 +1970,19 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) } /* get replica flags */ - r->repl_flags = slapi_entry_attr_get_ulong(e, attr_flags); + if (slapi_entry_attr_exists(e, attr_flags)) { + int rflags; + if((val = slapi_entry_attr_get_charptr(e, attr_flags))) { + if (repl_config_valid_num(attr_flags, val, 0, 1, &rc, errortext, &rflags) != 0) { + return -1; + } + r->repl_flags = (PRUint32)rflags; + } else { + r->repl_flags = 0; + } + } else { + r->repl_flags = 0; + } /* * Get replicaid @@ -1955,20 +1997,13 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) else if (r->repl_type == REPLICA_TYPE_UPDATABLE || r->repl_type == REPLICA_TYPE_PRIMARY) { if ((val = slapi_entry_attr_get_charptr(e, attr_replicaId))) { - int temprid = atoi(val); - slapi_ch_free((void **)&val); - if (temprid <= 0 || temprid >= READ_ONLY_REPLICA_ID) { - PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s must have a value greater than 0 " - "and less than %d: entry %s", - attr_replicaId, READ_ONLY_REPLICA_ID, - (char *)slapi_entry_get_dn((Slapi_Entry *)e)); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, - "_replica_init_from_config - %s\n", errormsg); + int rid; + if (repl_config_valid_num(attr_replicaId, val, 1, 65535, &rc, errormsg, &rid) != 0) { + slapi_ch_free_string(&val); return -1; - } else { - r->repl_rid = (ReplicaId)temprid; } + r->repl_rid = (ReplicaId)rid; + slapi_ch_free_string(&val); } else { PR_snprintf(errormsg, SLAPI_DSE_RETURNTEXT_SIZE, "Failed to retrieve required %s attribute from %s", @@ -2003,10 +2038,13 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) r->groupdn_list = replica_groupdn_list_new(r->updatedn_groups); r->updatedn_group_last_check = time(NULL); /* get groupdn check interval */ - val = slapi_entry_attr_get_charptr(e, attr_replicaBindDnGroupCheckInterval); - if (val) { - r->updatedn_group_check_interval = atoi(val); - slapi_ch_free((void **)&val); + if ((val = slapi_entry_attr_get_charptr(e, attr_replicaBindDnGroupCheckInterval))) { + if (repl_config_valid_num(attr_replicaBindDnGroupCheckInterval, val, -1, INT_MAX, &rc, errormsg, &interval) != 0) { + slapi_ch_free_string(&val); + return -1; + } + r->updatedn_group_check_interval = interval; + slapi_ch_free_string(&val); } else { r->updatedn_group_check_interval = -1; } @@ -2041,18 +2079,26 @@ _replica_init_from_config(Replica *r, Slapi_Entry *e, char *errortext) * since we don't know about LCUP replicas, and they can just * turn up whenever they want to. */ - if (slapi_entry_attr_find(e, type_replicaPurgeDelay, &a) == -1) { - /* No purge delay provided, so use default */ - r->repl_purge_delay = 60 * 60 * 24 * 7; /* One week, in seconds */ + if ((val = slapi_entry_attr_get_charptr(e, type_replicaPurgeDelay))) { + if (repl_config_valid_num(type_replicaPurgeDelay, val, -1, INT_MAX, &rc, errormsg, &interval) != 0) { + slapi_ch_free_string(&val); + return -1; + } + r->repl_purge_delay = interval; + slapi_ch_free_string(&val); } else { - r->repl_purge_delay = slapi_entry_attr_get_uint(e, type_replicaPurgeDelay); + r->repl_purge_delay = 60 * 60 * 24 * 7; /* One week, in seconds */ } - if (slapi_entry_attr_find(e, type_replicaTombstonePurgeInterval, &a) == -1) { - /* No reap interval provided, so use default */ - r->tombstone_reap_interval = 3600 * 24; /* One day */ + if ((val = slapi_entry_attr_get_charptr(e, type_replicaTombstonePurgeInterval))) { + if (repl_config_valid_num(type_replicaTombstonePurgeInterval, val, -1, INT_MAX, &rc, errormsg, &interval) != 0) { + slapi_ch_free_string(&val); + return -1; + } + r->tombstone_reap_interval = interval; + slapi_ch_free_string(&val); } else { - r->tombstone_reap_interval = slapi_entry_attr_get_int(e, type_replicaTombstonePurgeInterval); + r->tombstone_reap_interval = 3600 * 24; /* One week, in seconds */ } r->tombstone_reap_stop = r->tombstone_reap_active = PR_FALSE; diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c index 7477a292c..529d9b560 100644 --- a/ldap/servers/plugins/replication/repl5_replica_config.c +++ b/ldap/servers/plugins/replication/repl5_replica_config.c @@ -405,28 +405,35 @@ replica_config_modify(Slapi_PBlock *pb, } else if (strcasecmp(config_attr, attr_replicaBindDnGroup) == 0) { *returncode = replica_config_change_updatedngroup(r, mods[i], errortext, apply_mods); } else if (strcasecmp(config_attr, attr_replicaBindDnGroupCheckInterval) == 0) { - int interval = atoi(config_attr_value); - replica_set_groupdn_checkinterval(r, interval); + int interval = 0; + if (repl_config_valid_num(config_attr, config_attr_value, -1, INT_MAX, returncode, errortext, &interval) == 0) { + replica_set_groupdn_checkinterval(r, interval); + } else { + break; + } } else if (strcasecmp(config_attr, attr_replicaType) == 0) { + int rtype; slapi_ch_free_string(&new_repl_type); - new_repl_type = slapi_ch_strdup(config_attr_value); + if (repl_config_valid_num(config_attr, config_attr_value, 1, 3, returncode, errortext, &rtype) == 0) { + new_repl_type = slapi_ch_strdup(config_attr_value); + } else { + break; + } } else if (strcasecmp(config_attr, attr_replicaId) == 0) { - char *endp = NULL; - int64_t rid = 0; - errno = 0; - rid = strtoll(config_attr_value, &endp, 10); - if (*endp != '\0' || rid > 65535 || rid < 1 || errno == ERANGE) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number between 1 and 65535.\n", - config_attr, config_attr_value); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + int rid = 0; + if (repl_config_valid_num(config_attr, config_attr_value, 1, 65535, returncode, errortext, &rid) == 0) { + slapi_ch_free_string(&new_repl_id); + new_repl_id = slapi_ch_strdup(config_attr_value); + } else { break; } - slapi_ch_free_string(&new_repl_id); - new_repl_id = slapi_ch_strdup(config_attr_value); } else if (strcasecmp(config_attr, attr_flags) == 0) { - *returncode = replica_config_change_flags(r, config_attr_value, errortext, apply_mods); + int rflags = 0; + if (repl_config_valid_num(config_attr, config_attr_value, 0, 1, returncode, errortext, &rflags) == 0) { + *returncode = replica_config_change_flags(r, config_attr_value, errortext, apply_mods); + } else { + break; + } } else if (strcasecmp(config_attr, TASK_ATTR) == 0) { *returncode = replica_execute_task(mtnode_ext->replica, config_attr_value, errortext, apply_mods); } else if (strcasecmp(config_attr, attr_replicaReferral) == 0) { @@ -442,18 +449,21 @@ replica_config_modify(Slapi_PBlock *pb, } } else if (strcasecmp(config_attr, type_replicaPurgeDelay) == 0) { if (apply_mods && config_attr_value[0]) { - PRUint32 delay; - if (isdigit(config_attr_value[0])) { - delay = (unsigned int)atoi(config_attr_value); + int delay = 0; + if (repl_config_valid_num(config_attr, config_attr_value, -1, INT_MAX, returncode, errortext, &delay) == 0) { replica_set_purge_delay(r, delay); - } else - *returncode = LDAP_OPERATIONS_ERROR; + } else { + break; + } } } else if (strcasecmp(config_attr, type_replicaTombstonePurgeInterval) == 0) { if (apply_mods && config_attr_value[0]) { - long interval; - interval = atol(config_attr_value); - replica_set_tombstone_reap_interval(r, interval); + int interval; + if (repl_config_valid_num(config_attr, config_attr_value, -1, INT_MAX, returncode, errortext, &interval) == 0) { + replica_set_tombstone_reap_interval(r, interval); + } else { + break; + } } } /* ignore modifiers attributes added by the server */ @@ -461,68 +471,50 @@ replica_config_modify(Slapi_PBlock *pb, *returncode = LDAP_SUCCESS; } else if (strcasecmp(config_attr, type_replicaProtocolTimeout) == 0) { if (apply_mods) { - PRUint64 ptimeout = 0; - - ptimeout = atoll(config_attr_value); - - if (ptimeout <= 0) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number greater than zero.\n", - config_attr, config_attr_value); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + int ptimeout = 0; + if (repl_config_valid_num(config_attr, config_attr_value, 1, INT_MAX, returncode, errortext, &ptimeout) == 0) { + replica_set_protocol_timeout(r, ptimeout); + } else { break; } - replica_set_protocol_timeout(r, ptimeout); } } else if (strcasecmp(config_attr, type_replicaBackoffMin) == 0) { if (apply_mods) { - uint64_t val = atoll(config_attr_value); - uint64_t max; - - if (val <= 0) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number greater than zero.\n", - config_attr, config_attr_value); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); - break; - } - max = replica_get_backoff_max(r); - if (val > max){ - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number less than the max backoff time (%d).\n", - config_attr, config_attr_value, (int)max); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + int val = 0; + int max; + if (repl_config_valid_num(config_attr, config_attr_value, 1, INT_MAX, returncode, errortext, &val) == 0) { + max = replica_get_backoff_max(r); + if (val > max){ + *returncode = LDAP_UNWILLING_TO_PERFORM; + PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, + "Attribute %s value (%s) is invalid, must be a number less than the max backoff time (%d).\n", + config_attr, config_attr_value, (int)max); + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + break; + } + replica_set_backoff_min(r, val); + } else { break; } - replica_set_backoff_min(r, val); } } else if (strcasecmp(config_attr, type_replicaBackoffMax) == 0) { if (apply_mods) { - uint64_t val = atoll(config_attr_value); - uint64_t min; - - if (val <= 0) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number greater than zero.\n", - config_attr, config_attr_value); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", - errortext); - break; - } - min = replica_get_backoff_min(r); - if (val < min) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number more than the min backoff time (%d).\n", - config_attr, config_attr_value, (int)min); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + int val = 0; + int min; + if (repl_config_valid_num(config_attr, config_attr_value, 1, INT_MAX, returncode, errortext, &val) == 0) { + min = replica_get_backoff_min(r); + if (val < min) { + *returncode = LDAP_UNWILLING_TO_PERFORM; + PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, + "Attribute %s value (%s) is invalid, must be a number more than the min backoff time (%d).\n", + config_attr, config_attr_value, (int)min); + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_modify - %s\n", errortext); + break; + } + replica_set_backoff_max(r, val); + } else { break; } - replica_set_backoff_max(r, val); } } else if (strcasecmp(config_attr, type_replicaPrecisePurge) == 0) { if (apply_mods) { @@ -550,19 +542,11 @@ replica_config_modify(Slapi_PBlock *pb, } } else if (strcasecmp(config_attr, type_replicaReleaseTimeout) == 0) { if (apply_mods) { - long val = atol(config_attr_value); - - if (val < 0) { - *returncode = LDAP_UNWILLING_TO_PERFORM; - PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, - "Attribute %s value (%s) is invalid, must be a number zero or greater.\n", - config_attr, config_attr_value); - slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, - "replica_config_modify - %s\n", errortext); - break; - } else { - /* Set the timeout */ + int val; + if (repl_config_valid_num(config_attr, config_attr_value, 1, INT_MAX, returncode, errortext, &val) == 0) { replica_set_release_timeout(r, val); + } else { + break; } } } else { diff --git a/ldap/servers/plugins/replication/replutil.c b/ldap/servers/plugins/replication/replutil.c index 1b0446788..5f0302a24 100644 --- a/ldap/servers/plugins/replication/replutil.c +++ b/ldap/servers/plugins/replication/replutil.c @@ -1061,3 +1061,29 @@ repl_set_repl_plugin_path(const char *path) { replpluginpath = slapi_ch_strdup(path); } + +int +repl_config_valid_num(const char *config_attr, char *config_attr_value, int min, int max, + int *returncode, char *errortext, int *retval) +{ + int rc = 0; + char *endp = NULL; + int64_t val; + errno = 0; + + val = strtol(config_attr_value, &endp, 10); + if (*endp != '\0' || val < min || val > max || errno == ERANGE) { + *returncode = LDAP_UNWILLING_TO_PERFORM; + if (errortext){ + PR_snprintf(errortext, SLAPI_DSE_RETURNTEXT_SIZE, + "Attribute %s value (%s) is invalid, must be a number between %d and %d.", + config_attr, config_attr_value, min, max); + slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name, "replica_config_valid_num - %s\n", + errortext); + } + rc = -1; + } else { + *retval = val; + } + return rc; +} -- 2.13.6