Issue #49524: password policy: minimum token length fails when the token length is equal to attribute length. - 389-ds-base

389-ds-base

#49524 password policy: minimum token length fails when the token length is equal to attribute length.

Closed: wontfix Jan 8, 2018 Opened Jan 5, 2018 by mreynolds.

Ticket was cloned from Red Hat Bugzilla (product Red Hat Enterprise Linux 7): Bug 1517788

Description of problem:
the function check_trivial_words is failing to recongnize a token that is
exactly same size as the token length.
Example:
dn: uid=Dord,ou=People,dc=parente,dc=local
sn: David
objectClass: top
objectClass: account
objectClass: posixaccount
objectClass: inetOrgPerson
objectClass: organizationalPerson
objectClass: person
uid: Dord
cn: David
loginShell: /bin/bash
homeDirectory: /home/dorda
uidNumber: 501003
gidNumber: 510003
passwordMinTokenLength: 4
ldapmodify -D "uid=Dord,ou=People,dc=parente,dc=local" -w secret12
dn: uid=Dord,ou=People,dc=parente,dc=local
changetype: modify
replace: userpassword
userPassword: dord
modifying entry "uid=Dord,ou=People,dc=parente,dc=local"
Actual results
the password including a token of 4 length is accepted (it's the exact length
of the attribute).
Version-Release number of selected component (if applicable):
389-ds-base-1.3.6.1-21
How reproducible: always
Actual results:
password is not refused.
Expected results:
password refused when the token length is same length than the attribute.
Additional info:
After some debugging, the function:
check_trivial_words calls
ldap_utf8prevn(sp, ep, toklen);
This function returns NULL when the token length is same length than the
attribute value in "sp".
IMHO, this should be the fix:
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)) { <============= should be (prev < s)
return NULL;
}
}
return prev;
}
For instance, in the previous example,
ldap_utf8prevn("dord", char *from,4)
will loop to find the postfix d, rd, ord, and will return NULL for dord.
So check_trivial_words will fail to refuse the password.

Metadata Update from @mreynolds:
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1517788

Jan 5, 2018

Metadata Update from @mreynolds:
- Issue assigned to mreynolds

Jan 5, 2018

Metadata Update from @mreynolds:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None
- Issue set to the milestone: 1.3.6.0 (was: 0.0 NEEDS_TRIAGE)

Jan 5, 2018

mreynolds commented Jan 5, 2018

Metadata Update from @mreynolds:
- Custom field reviewstatus adjusted to review (was: None)

Jan 5, 2018

spichugi commented Jan 5, 2018

A couple of small issue with the test:

It fails on Python 3 because 'passwd' is not byte type.

topo.standalone.modify_s(USER_DN, [(ldap.MOD_REPLACE, 'userpassword', passwd)])

I think you can use the user returned by the fixture and user.replace method.

Also, please, change the first line in the docstring and the :setup: step (to Standalone instance)

68 +    """Specify a test case purpose or name here
69 +
70 +    :id: dae9d916-2a03-4707-b454-9e901d295b13
71 +    :setup: Fill in set up configuration here

firstyear commented Jan 8, 2018

To follow on from @spichugi

If you make:

return users.create(....)

In def pwd_setup, then in your test case you can do:

user = pwd_setup

Then do:

user.replace('userPassword', "value")

Instead.

From now on direct calls to modify_s/add_s are generally going to break python 3, so we really really need to stop using them,a nd use the lib389 wrappers that actually take care of bytesafety.

Additionally, we shouldn't be calling Entry directly, because I want to eventually get rid of this type in favour of just using DSLdapObject.

Hope that helps,

For the C code:

+        if ((prev < s) && (n > 0)) {

You need to check n> 0 first, else you may over-read prev/s.

Thanks!

mreynolds commented Jan 8, 2018

From now on direct calls to modify_s/add_s are generally going to break python 3, so we really really need to stop using them,a nd use the lib389 wrappers that actually take care of bytesafety.

Wow, okay I am really behind on the changes you are guys are doing to lib389. Do we have updated docs on how to do add/modifies/deletes/etc? Where can I find the new "wrappers"? There is nothing about this on Simon's Lib389 doc page.

The attached patch fixes the test case and the C code. On the C code, it has always had that order without issue, but I still changed it :-)

New patch:

spichugi commented Jan 8, 2018

From now on direct calls to modify_s/add_s are generally going to break python 3, so we really really need to stop using them,a nd use the lib389 wrappers that actually take care of bytesafety.

Wow, okay I am really behind on the changes you are guys are doing to lib389. Do we have updated docs on how to do add/modifies/deletes/etc? Where can I find the new "wrappers"? There is nothing about this on Simon's Lib389 doc page.

But you've done everything right... You've used the wrappers. :) This is the wrappers - https://fedorapeople.org/~spichugi/html/user.html

You have my ack for Python code.
Though, please, change the first docstring line to the test related one

67 +    """Specify a test case purpose or name here

mreynolds commented Jan 8, 2018

From now on direct calls to modify_s/add_s are generally going to break python 3, so we really really need to stop using them,a nd use the lib389 wrappers that actually take care of bytesafety.
Wow, okay I am really behind on the changes you are guys are doing to lib389. Do we have updated docs on how to do add/modifies/deletes/etc? Where can I find the new "wrappers"? There is nothing about this on Simon's Lib389 doc page.

But you've done everything right... You've used the wrappers. :) This is the wrappers - https://fedorapeople.org/~spichugi/html/user.html

Oh so we are doing wrappers for each type of object. One set of wrappers for users, one for plugins, config, schema, indexes, etc? I know this was somewhat implemented with the mappedDSobject, but I didn't realize how far its come. Nice! But that's a lot of testcases to rewrite :scream:

You have my ack for Python code.
Though, please, change the first docstring line to the test related one
67 + """Specify a test case purpose or name here

Will do, thanks!

mreynolds commented Jan 8, 2018

d79102f6b..790be09fc master -> master

b8f1b190e..288e7313b 389-ds-base-1.3.7 -> 389-ds-base-1.3.7

572f97871..689b38861 389-ds-base-1.3.6 -> 389-ds-base-1.3.6

Metadata Update from @mreynolds:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Jan 8, 2018

firstyear commented Jan 8, 2018

Great, much happier with the new patch :)

spichugi commented Sep 13, 2020

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/2583

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: fixed)

Sep 13, 2020

Metadata

Assignee

mreynolds

Tags

None

Blocking

None

Depending on

None

Priority

None

reviewstatus

review

origin

None

389-ds-base

#49524 password policy: minimum token length fails when the token length is equal to attribute length. Closed: wontfix Jan 8, 2018 Opened Jan 5, 2018 by mreynolds.

Metadata

#49524 password policy: minimum token length fails when the token length is equal to attribute length.

Closed: wontfix Jan 8, 2018 Opened Jan 5, 2018 by mreynolds.