Bug 9105 - check_password_quality does not properly handle non-ASCII characters
Summary: check_password_quality does not properly handle non-ASCII characters
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: Other (show other bugs)
Version: 4.0 beta4
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: samba4-qa@samba.org
URL:
Keywords:
: 9631 (view as bug list)
Depends on:
Blocks:
 
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
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

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!