dirsrvtests/tests/tickets/ticket48896_test.py | 181 ++++++++++++++++++++++++++ ldap/servers/slapd/modify.c | 3 ldap/servers/slapd/pw.c | 43 ++++-- ldap/servers/slapd/slapi-plugin.h | 4 ldap/servers/slapd/utf8.c | 46 ++++++ 5 files changed, 266 insertions(+), 11 deletions(-)
New commits: commit 19e75b97d2d54f082d518843e3d09f2da2deb1a6 Author: Noriko Hosoi nhosoi@redhat.com Date: Wed Jun 22 18:17:15 2016 -0700
Ticket #48896 - CI test: test case for ticket 48896
Description: Default Setting for passwordMinTokenLength does not work
diff --git a/dirsrvtests/tests/tickets/ticket48896_test.py b/dirsrvtests/tests/tickets/ticket48896_test.py new file mode 100644 index 0000000..b2675ec --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket48896_test.py @@ -0,0 +1,181 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2016 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +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 + +CONFIG_DN = 'cn=config' +UID = 'buser123' +TESTDN = 'uid=%s,' % UID + DEFAULT_SUFFIX + +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 + + +@pytest.fixture(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() + + # Delete each instance in the end + def fin(): + standalone.delete() + request.addfinalizer(fin) + + # Clear out the tmp dir + standalone.clearTmpDir(__file__) + + return TopologyStandalone(standalone) + +def check_attr_val(topology, dn, attr, expected): + try: + centry = topology.standalone.search_s(dn, ldap.SCOPE_BASE, 'cn=*') + if centry: + val = centry[0].getValue(attr) + if val == expected: + log.info('Default value of %s is %s' % (attr, expected)) + else: + log.info('Default value of %s is not %s, but %s' % (attr, expected, val)) + assert False + else: + log.fatal('Failed to get %s' % dn) + assert False + except ldap.LDAPError as e: + log.fatal('Failed to search ' + dn + ': ' + e.message['desc']) + assert False + +def replace_pw(server, curpw, newpw, expstr, rc): + log.info('Binding as {%s, %s}' % (TESTDN, curpw)) + server.simple_bind_s(TESTDN, curpw) + + hit = 0 + log.info('Replacing password: %s -> %s, which should %s' % (curpw, newpw, expstr)) + try: + server.modify_s(TESTDN, [(ldap.MOD_REPLACE, 'userPassword', newpw)]) + except Exception as e: + log.info("Exception (expected): %s" % type(e).__name__) + hit = 1 + assert isinstance(e, rc) + + if (0 != rc) and (0 == hit): + log.info('Expected to fail with %d, but passed' % rc) + assert False + + log.info('PASSED') + +def test_ticket48896(topology): + """ + """ + log.info('Testing Ticket 48896 - Default Setting for passwordMinTokenLength does not work') + + log.info("Setting global password policy with password syntax.") + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + topology.standalone.modify_s(CONFIG_DN, [(ldap.MOD_REPLACE, 'passwordCheckSyntax', 'on'), + (ldap.MOD_REPLACE, 'nsslapd-pwpolicy-local', 'on')]) + + config = topology.standalone.search_s(CONFIG_DN, ldap.SCOPE_BASE, 'cn=*') + mintokenlen = config[0].getValue('passwordMinTokenLength') + history = config[0].getValue('passwordInHistory') + + log.info('Default passwordMinTokenLength == %s' % mintokenlen) + log.info('Default passwordInHistory == %s' % history) + + log.info('Adding a user.') + curpw = 'password' + topology.standalone.add_s(Entry((TESTDN, + {'objectclass': "top person organizationalPerson inetOrgPerson".split(), + 'cn': 'test user', + 'sn': 'user', + 'userPassword': curpw}))) + + newpw = 'Abcd012+' + exp = 'be ok' + rc = 0 + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = 'user' + exp = 'fail' + rc = ldap.CONSTRAINT_VIOLATION + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = UID + exp = 'fail' + rc = ldap.CONSTRAINT_VIOLATION + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = 'Tuse!1234' + exp = 'fail' + rc = ldap.CONSTRAINT_VIOLATION + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = 'Tuse!0987' + exp = 'fail' + rc = ldap.CONSTRAINT_VIOLATION + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = 'Tabc!1234' + exp = 'fail' + rc = ldap.CONSTRAINT_VIOLATION + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + curpw = 'Abcd012+' + newpw = 'Direc+ory389' + exp = 'be ok' + rc = 0 + replace_pw(topology.standalone, curpw, newpw, exp, rc) + + log.info('SUCCESS') + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + + CURRENT_FILE = os.path.realpath(__file__) + pytest.main("-s %s" % CURRENT_FILE)
commit 054f3ce507650935a54582141abac235fd1b0c00 Author: Noriko Hosoi nhosoi@redhat.com Date: Wed Jun 22 17:38:08 2016 -0700
Ticket #48896 - Default Setting for passwordMinTokenLength does not work
Description: passwordMinTokenLength is supposed to be used for the length of comparison between the substring of obvious strings and a new password. But it was not used to generate substrings. This patch implements it.
Also, old_pw was leaked in modify if password history was not enabled.
https://fedorahosted.org/389/ticket/48896
Reviewed by mreynolds@redhat.com (Thank you, Mark!)
diff --git a/ldap/servers/slapd/modify.c b/ldap/servers/slapd/modify.c index 72f2db4..2be6930 100644 --- a/ldap/servers/slapd/modify.c +++ b/ldap/servers/slapd/modify.c @@ -390,7 +390,8 @@ do_modify( Slapi_PBlock *pb ) ldap_mods_free (normalized_mods, 1 /* Free the Array and the Elements */);
free_and_return:; - slapi_ch_free ((void**)&rawdn); + slapi_ch_free_string(&old_pw); + slapi_ch_free_string(&rawdn); slapi_mods_done(&smods); }
diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index 7658064..ed83ded 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -621,7 +621,6 @@ update_pw_info ( Slapi_PBlock *pb , char *old_pw) /* update passwordHistory */ if ( old_pw != NULL && pwpolicy->pw_history == 1 ) { (void)update_pw_history(pb, sdn, old_pw); - slapi_ch_free ( (void**)&old_pw ); }
/* Update the "pwdUpdateTime" attribute */ @@ -1046,9 +1045,13 @@ retry: * This is because password policy assumes that there's only one * password in the userpassword attribute. */ - *old_pw = slapi_ch_strdup(slapi_value_get_string(va[0])); + if (old_pw) { + *old_pw = slapi_ch_strdup(slapi_value_get_string(va[0])); + } } else { - *old_pw = NULL; + if (old_pw) { + *old_pw = NULL; + } } } } @@ -1472,13 +1475,13 @@ check_trivial_words (Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Value **vals, char { /* Add new value to valueset */ valp = slapi_value_new_berval( bvp ); - slapi_valueset_add_value_ext( vs, valp, SLAPI_VALUE_FLAG_PASSIN ); + slapi_valueset_add_value_ext( vs, valp, SLAPI_VALUE_FLAG_PASSIN ); valp = NULL; } } } /* Free smod */ - slapi_mod_free(&smod); + slapi_mod_free(&smod); smod = NULL; smodp = NULL; } @@ -1490,17 +1493,37 @@ check_trivial_words (Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Value **vals, char (i != -1) && (valp != NULL); i = slapi_valueset_next_value( vs, i, &valp) ) { + char *sp, *ep, *wp; + int found = 0; /* If the value is smaller than the max token length, * we don't need to check the password */ if ( (int)ldap_utf8characters(slapi_value_get_string( valp )) < toklen ) continue;
+ sp = slapi_ch_strdup(slapi_value_get_string(valp)); + ep = sp + strlen(sp); + ep = ldap_utf8prevn(sp, ep, toklen); + if (!ep || (sp >= ep)) { + continue; + } /* See if the password contains the value */ - if ( PL_strcasestr( slapi_value_get_string( vals[0] ), - slapi_value_get_string( valp ) ) ) - { - if ( pwresponse_req == 1 ) - { + for (wp = sp; wp && (wp <= ep); wp = ldap_utf8next(wp)) { + char *tp = ldap_utf8nextn(wp, toklen); + char c; + if (tp) { + c = *tp; + *tp = '\0'; + } else { + break; + } + if (PL_strcasestr(slapi_value_get_string(vals[0]), wp)) { + found = 1; + } + *tp = c; + } + slapi_ch_free_string(&sp); + if (found) { + if ( pwresponse_req == 1 ) { slapi_pwpolicy_make_response_control ( pb, -1, -1, LDAP_PWPOLICY_INVALIDPWDSYNTAX ); } diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 89853c0..7022e59 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -7455,6 +7455,10 @@ int ldap_utf8len( const char* ); char *ldap_utf8next( char* ); /* find previous character */ char *ldap_utf8prev( char* ); +/* find n-th character */ +char *ldap_utf8nextn (char* s, int n); +/* find n-th previous character from "from" */ +char *ldap_utf8prevn (char *s, char *from, int n); /* copy one character */ int ldap_utf8copy( char* dst, const char* src ); /* total number of characters */ diff --git a/ldap/servers/slapd/utf8.c b/ldap/servers/slapd/utf8.c index a1330ee..27843c0 100644 --- a/ldap/servers/slapd/utf8.c +++ b/ldap/servers/slapd/utf8.c @@ -93,6 +93,52 @@ ldap_utf8prev (char* s) return (char*) prev; }
+/* + * Return a pointer to the n-th character following *s. + * Handle any valid UTF-8 character, including '\0' and ASCII. + * Try to handle a misaligned pointer or a malformed character. + * If the n-th character is beyond the string, it returns NULL. + */ +char* +ldap_utf8nextn (char* s, int n) +{ + char *endp; + char *next = s; + if (!s) { + return NULL; + } + endp = s + strlen(s); + for ( ;n > 0; --n) { + next = ldap_utf8next(next); + if ((next > endp) && (n > 0)) { + return NULL; + } + } + return next; +} + +/* + * Return a pointer to the n-th character preceding *from. + * Handle any valid UTF-8 character, including '\0' and ASCII. + * Try to handle a misaligned pointer or a malformed character. + * If the n-th previous character is beyond the start address, it returns NULL. + */ +char* +ldap_utf8prevn (char *s, char *from, int n) +{ + char *prev = from; + if (!s || !from || (s > from)) { + return NULL; + } + for ( ;n > 0; --n) { + prev = ldap_utf8prev(prev); + if ((prev <= s) && (n > 0)) { + return NULL; + } + } + return prev; +} + int ldap_utf8copy (char* dst, const char* src) /* Copy a character from src to dst; return the number of char's copied.
389-commits@lists.fedoraproject.org
-
Noriko Hosoi