igriffiths / pwm

Automatically exported from code.google.com/p/pwm
0 stars 0 forks source link

Microsoft AD Password Complexity logic is incorrect #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

---
Attempt to change an AD password via PWM, specific AD passwords may be flagged 
as invalid (when they actually would be valid) 

or 

in the case of an invalid password (for example one that incorporates the 
samAccountName) is flagged as OK by PWM but an error is generated by AD when 
attempting to change the password.

What version of the product are you using? On what operating system?
PWM 1.5.2 - Windows 2003.
AD domain mixed Windows 2003/Windows 2008

Please provide any additional information below.

---
The most detailed/accurate description of AD Password Complexity I can find on 
microsoft.com "Microsoft AD Password Complexity" implements the following rules 
http://technet.microsoft.com/en-us/library/cc786468%28WS.10%29.aspx

PWM checks AD Password Complexity via function checkPasswordForADComplexity in 
Validator.java

Currently, PWM does not perform a proper check according to the referenced 
Microsoft document. It appears that the check was coded against an earlier 
published reference.

The following inconsistencies exist with the PWM checkPasswordForADComplexity 
feature:

- Does not check the attribute samAccountName at all.
- Display name check is totally wrong, but is actually stricter than it needs 
to be.
- Check of CN and full name are not necessary (according to MS document CN is 
not used in AD Password complexity check). Also full name is not an LDAP 
attribute in AD.
- A 5th category is referenced (Any Unicode character that is categorised as an 
alphabetic character but is not upper-case or lower-case. This includes Unicode 
characters from Asian languages.) but this category is not coded in PWM.

Original issue reported on code.google.com by oner...@gmail.com on 3 Feb 2011 at 1:59

GoogleCodeExporter commented 9 years ago
Attached is a patch that implements more correct checking.

Two additional items appear in this patch - they were useful to us but might 
not be implemented in an ideal manner.

1. Try to retrieve minimum password length value from the AD domain attribute 
minPwdLength instead of using the value explicitly defined PWM.
implemented in UserStatusHelper.java and Validator.java 

2. An added check in PasswordUtility.java that probably should be in LDAPChai 
related to testing for a password violation return code 19 from AD and 
returning a more user friendly error message.

Original comment by oner...@gmail.com on 3 Feb 2011 at 2:42

Attachments:

GoogleCodeExporter commented 9 years ago
Nice patches!  Thanks for contributing!

For #1, I have added your domain checking logic to the password policy reader 
in chai:

http://code.google.com/p/ldapchai/source/browse/trunk/src/com/novell/ldapchai/im
pl/ad/entry/UserImpl.java#60

For #2, I have added the error code translation to the AD error map in chai:

http://code.google.com/p/ldapchai/source/browse/trunk/src/com/novell/ldapchai/im
pl/ad/ADErrorMap.java#75

I've also done some other chai error handler refactoring that should get things 
a littler closer to being flexible enough to handle non-edirectory errors 
correctly.  Both of these methods are pretty sparse though, contributions 
welcome.

The remainder of the patch around Validator.java should be added.  Please test 
the patches and confirm they are working as expected here.

Thanks again for supplying a patch!

Original comment by jrivard on 7 Feb 2011 at 5:17

GoogleCodeExporter commented 9 years ago
Forgot to mention the changes are in svn revision 105.

Original comment by jrivard on 7 Feb 2011 at 5:20

GoogleCodeExporter commented 9 years ago

Original comment by jrivard on 21 Mar 2011 at 3:56