From 5a1e156b35835d37bbc732803bf2619b0a9c3867 Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Dec 05 2018 14:56:52 +0000 Subject: Ticket 50070 - new option to store unhashed password only in replication changelog Bug Description: The option 'nsslapd-unhashed-pw-switch: nolog' prevents to log unhashed#user#password in the logs (replication changelog and retroCL). It could be a security concern to, releasing unhashed password to a ldap client. A new option is to prevent logging of unhashed password in the retroCL. Fix Description: The fix is to not log in retroCL the unhashed password when the nsslapd-unhashed-pw-switch is 'nolog' or 'on_only_repl' https://pagure.io/389-ds-base/issue/50070 Reviewed by: ? Platforms tested: F27 Flag Day: no Doc impact: no --- diff --git a/dirsrvtests/tests/tickets/ticket50070_test.py b/dirsrvtests/tests/tickets/ticket50070_test.py new file mode 100644 index 0000000..1d8c09d --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket50070_test.py @@ -0,0 +1,160 @@ +import logging +import pytest +import os +import ldap +import re +import base64 +from lib389 import Entry +from lib389._constants import * +from lib389.topologies import topology_m2 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__) + +PASSWORD_ADD = "password_during_add" +CLEAR_TXT_PASSWORD = b'modifiedpassword' +def add_user(server, uid, testbase): + dn = 'uid=%s,%s' % (uid, testbase) + log.fatal('Adding user (%s): ' % dn) + server.add_s(Entry((dn, {'objectclass': ['top', 'person', 'organizationalPerson', 'inetOrgPerson'], + 'cn': 'user_%s' % uid, + 'sn': 'user_%s' % uid, + 'uid': uid, + 'userpassword': PASSWORD_ADD}))) + return dn + +def test_ticket50070(topo): + """Checks that with nsslapd-unhashed-pw-switch: on_only_repl the + unhashed#user#password is not recorded in the retroCL + + :id: 1cc8d77f-c0d9-46e4-b626-942577729b8a + :setup: 2 Master Instances + :steps: + 1. Enable retroCL with unhashed passwd + 2. Add a user (and check its userpassword value, just for fun) + 3. update the password of the user password and check unhashed#user#password is in the retroCL + 4. Disable retroCL in unhashed passwd + 5. Add a second user + 6. update the password of the second user and check unhashed#user#password is not in the retroCL + :expectedresults: + 1. Should succeeds + 2. Should succeeds + 3. Should succeeds + 4. Should succeeds + 5. Should succeeds + 6. Should succeeds + """ + + M1 = topo.ms["master1"] + M2 = topo.ms["master2"] + + # Enable retroCL to register unhashed password + M1.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-unhashed-pw-switch', b'on')]) + M1.plugins.enable(name=PLUGIN_RETRO_CHANGELOG) + M1.restart() + + M2.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-unhashed-pw-switch', b'on')]) + M2.plugins.enable(name=PLUGIN_RETRO_CHANGELOG) + M2.restart() + + # Create a testuser1 user + # Checking its password is not strictly related to the purpose of the ticket but just for recording + testbase = "ou=people,%s" % SUFFIX + testuser = add_user(M1, 'testuser1', testbase) + ents = M1.search_s("cn=changelog", ldap.SCOPE_SUBTREE,"(&(targetDN=%s)(changeType=add))" % testuser,["changes"]) + assert len(ents) == 1 + assert ents[0].hasAttr("changes") + value = ents[0].getValue("changes") + assert value + password_started = False + userpassword =b'' + for line in value.split(b'\n'): + log.debug("ADD line: %s" % line) + if line.lower().startswith(b'userpassword'): + # This is the beginning of the userpassword + + x = line[len('userpassword:'):] + if x.startswith(b':'): + x = x[1:] + x = x[1:] + log.debug("x: %s" % x) + password_started = True + userpassword = userpassword + x + elif password_started: + if line.startswith(b' '): + # this is the continuation of userpassword + x = line[1:] + log.debug("x: %s" % x) + userpassword = userpassword + x + #else: + # this is a new attribute + #break + log.debug("resultat ====> %s" % base64.b64decode(userpassword)) + + # Check unhashed#user#password is registered in the FIRST MOD(testuser1) in the retroCL + M1.modify_s(testuser, [(ldap.MOD_REPLACE, 'userpassword', CLEAR_TXT_PASSWORD)]) + ents = M1.search_s("cn=changelog", ldap.SCOPE_SUBTREE,"(&(targetDN=%s)(changeType=modify))" % testuser,["changes"]) + assert ents[0].hasAttr("changes") + value = ents[0].getValue("changes") + assert value + found = False + for line in value.split(b'\n'): + log.debug("MOD line: %s" % line) + if line.lower().startswith(b'unhashed#user#password'): + # should contain clear test password + # and anyway should not contain unhashed#user#password in the retroCL + found = True + assert CLEAR_TXT_PASSWORD in line + assert found + + # Disable to register unhashed password in retroCL + M1.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-unhashed-pw-switch', b'on_only_repl')]) + M1.plugins.enable(name=PLUGIN_RETRO_CHANGELOG) + M1.restart() + + M2.modify_s(DN_CONFIG, [(ldap.MOD_REPLACE, 'nsslapd-unhashed-pw-switch', b'on_only_repl')]) + M2.plugins.enable(name=PLUGIN_RETRO_CHANGELOG) + M2.restart() + + # Create a testuser2 + testuser = add_user(M1, 'testuser2', testbase) + + # Check unhashed#user#password is not registered in the FIRST MOD(testuser1) in the retroCL + M1.modify_s(testuser, [(ldap.MOD_REPLACE, 'userpassword', CLEAR_TXT_PASSWORD)]) + ents = M1.search_s("cn=changelog", ldap.SCOPE_SUBTREE,"(&(targetDN=%s)(changeType=modify))" % testuser,["changes"]) + assert ents[0].hasAttr("changes") + value = ents[0].getValue("changes") + assert value + found = False + for line in value.split(b'\n'): + log.debug("MOD line: %s" % line) + if line.lower().startswith(b'unhashed#user#password'): + # should contain clear test password + # and anyway should not contain unhashed#user#password in the retroCL + assert CLEAR_TXT_PASSWORD not in line + found = True + break + assert not found + + # If you need any test suite initialization, + # please, write additional fixture for that (including finalizer). + # Topology for suites are predefined in lib389/topologies.py. + + # If you need host, port or any other data about instance, + # Please, use the instance object attributes for that (for example, topo.ms["master1"].serverid) + + if DEBUGGING: + # Add debugging steps(if any)... + pass + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main(["-s", CURRENT_FILE]) + diff --git a/ldap/servers/plugins/retrocl/retrocl_po.c b/ldap/servers/plugins/retrocl/retrocl_po.c index d2af79b..9eef3ab 100644 --- a/ldap/servers/plugins/retrocl/retrocl_po.c +++ b/ldap/servers/plugins/retrocl/retrocl_po.c @@ -72,7 +72,8 @@ make_changes_string(LDAPMod **ldm, const char **includeattrs) continue; } } - if (SLAPD_UNHASHED_PW_NOLOG == slapi_config_get_unhashed_pw_switch()) { + if ((SLAPD_UNHASHED_PW_NOLOG == slapi_config_get_unhashed_pw_switch()) || + ((SLAPD_UNHASHED_PW_ON_ONLY_REPL == slapi_config_get_unhashed_pw_switch()))) { if (0 == strcasecmp(ldm[i]->mod_type, PSEUDO_ATTR_UNHASHEDUSERPASSWORD)) { /* If nsslapd-unhashed-pw-switch == nolog, skip writing it to cl. */ continue; diff --git a/ldap/servers/slapd/libglobs.c b/ldap/servers/slapd/libglobs.c index a31a340..f41af74 100644 --- a/ldap/servers/slapd/libglobs.c +++ b/ldap/servers/slapd/libglobs.c @@ -7353,7 +7353,7 @@ config_set_unhashed_pw_switch(const char *attrname, char *value, char *errorbuf, } if ((strcasecmp(value, "on") != 0) && (strcasecmp(value, "off") != 0) && - (strcasecmp(value, "nolog") != 0)) { + (strcasecmp(value, "nolog") != 0) && (strcasecmp(value, "on_only_repl") != 0)) { slapi_create_errormsg(errorbuf, SLAPI_DSE_RETURNTEXT_SIZE, "%s: invalid value \"%s\". Valid values are \"on\", \"off\", or \"nolog\".", attrname, value); retVal = LDAP_OPERATIONS_ERROR; @@ -7372,6 +7372,8 @@ config_set_unhashed_pw_switch(const char *attrname, char *value, char *errorbuf, slapdFrontendConfig->unhashed_pw_switch = SLAPD_UNHASHED_PW_OFF; } else if (strcasecmp(value, "nolog") == 0) { slapdFrontendConfig->unhashed_pw_switch = SLAPD_UNHASHED_PW_NOLOG; + } else if (strcasecmp(value, "on_only_repl") == 0) { + slapdFrontendConfig->unhashed_pw_switch = SLAPD_UNHASHED_PW_ON_ONLY_REPL; } CFG_UNLOCK_WRITE(slapdFrontendConfig); diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 467e783..526aff7 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -7662,6 +7662,7 @@ char **slapi_str2charray_ext(char *str, char *brkstr, int allow_dups); #define SLAPD_UNHASHED_PW_OFF 0 #define SLAPD_UNHASHED_PW_ON 1 #define SLAPD_UNHASHED_PW_NOLOG 2 +#define SLAPD_UNHASHED_PW_ON_ONLY_REPL 3 /** * Set given "type: value" to the plugin default config entry