Bug 9105 - check_password_quality does not properly handle non-ASCII characters
check_password_quality does not properly handle non-ASCII characters
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: Other
4.0 beta4
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
samba4-qa@samba.org
:
: 9631 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-21 17:21 UTC by Arvid Requate
Modified: 2013-02-05 13:26 UTC (History)
6 users (show)

See Also:


Attachments
This patch seems to fix the bug. (1.37 KB, patch)
2012-08-21 17:22 UTC, Arvid Requate
no flags Details
Make password complexity rule closer to AD default (741 bytes, patch)
2012-08-22 09:01 UTC, Arvid Requate
no flags Details
Adjusted test cases for password complexity (1.33 KB, patch)
2012-08-22 09:03 UTC, Arvid Requate
no flags Details
Patches for v4-0-test (19.30 KB, patch)
2013-02-04 20:12 UTC, Stefan Metzmacher
obnox: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate 2012-08-21 17:21:36 UTC
The function check_password_quality in lib/util/genrand.c does not properly handle non-ASCII characters:

root@samba4:~# samba-tool user setpassword user1 --newpassword='Ütf8pass'
ERROR: Failed to set password for user 'user1': (19, '0000052D: Constraint violation - check_password_restrictions: the password does not meet the complexity criteria!'

root@samba4:~# samba-tool user setpassword user1 --newpassword='üTf8pass'
Changed password OK

root@samba4:~# samba-tool user setpassword user1 --newpassword='ÜÜÜ8p'
Changed password OK

This also affects password changes initiated on Windows clients.
Comment 1 Arvid Requate 2012-08-21 17:22:27 UTC
Created attachment 7798 [details]
This patch seems to fix the bug.
Comment 2 Andrew Bartlett 2012-08-21 23:24:13 UTC
I tried to autobuild this, but it fails this test.  We either need to fix the test or the code (perhaps compare with MS).

UNEXPECTED(failure): samba4.local.genrand.check_password_quality(none)
REASON: _StringException: _StringException: ../lib/util/tests/genrand.c:44: Expression `check_password_quality("abcdééàçè")' failed: valid

FAILED (1 failures, 0 errors and 0 unexpected successes in 0 testsuites)
Comment 3 Arvid Requate 2012-08-22 09:01:39 UTC
Created attachment 7804 [details]
Make password complexity rule closer to AD default

The standard AD complexity rules differ a bit, and require checking for chracters from the username, which probably would require a change in the function arguments, requiring changes e.g. in the semantic of generate_random_password generate_random_string.
Comment 4 Arvid Requate 2012-08-22 09:03:37 UTC
Created attachment 7805 [details]
Adjusted test cases for password complexity

The test case "abcdééàçè" used to work, because every two-byte charcater counted as two "high" characters (non-uppercase/-lowercase/-digit/-special) and check_password_quality permitted a password if
 number("high" characters)>number(total)/2                                       
which IMHO does not seem to be a useful criterion. The previous commit adjusted the rule, this one adjusts the test cases.
Comment 6 Andrew Bartlett 2013-02-04 09:51:08 UTC
*** Bug 9631 has been marked as a duplicate of this bug. ***
Comment 7 Stefan Metzmacher 2013-02-04 20:12:04 UTC
Created attachment 8530 [details]
Patches for v4-0-test
Comment 8 Michael Adam 2013-02-04 21:08:03 UTC
==> Karolin for 4.0.X
Comment 9 Karolin Seeger 2013-02-05 08:53:52 UTC
Pushed to autobuild-v4-0-test.
Comment 10 Karolin Seeger 2013-02-05 13:26:52 UTC
Pushed to v4-0-test.
Included in 4.0.3.
Closing out bug report.

Thanks!