Bug 203043

Summary: Password modify extended operation should generate new passwords
Product: [Retired] 389 Reporter: Nathan Kinder <nkinder>
Component: Directory ServerAssignee: Nathan Kinder <nkinder>
Status: CLOSED CURRENTRELEASE QA Contact: Viktor Ashirov <vashirov>
Severity: medium Docs Contact:
Priority: medium    
Version: 1.0.2   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-12-07 17:11:41 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 152373, 208654, 240316    
Attachments:
Description Flags
CVS Diffs
none
Revised Diffs
none
Revised Diffs
none
Additional code clean-up none

Description Nathan Kinder 2006-08-17 22:56:15 UTC
When using the password modify extended operation, the password modify request
is allowed to have all three of it's fields (userIdentity, oldPassword,
newPassword) empty.  When this happens, our server returns LDAP_PROTOCOL_ERROR
along with an error message of "ber_scanf failed".

Even though all three of the password modify request fields are optional, our
server requires a new password to be supplied since we don't support server-side
password generation.  Because of this, we can safely return an error message
stating that the user must supply a new password.

Comment 1 Nathan Kinder 2006-08-17 23:01:48 UTC
Created attachment 134417 [details]
CVS Diffs

Comment 2 Noriko Hosoi 2006-08-17 23:08:24 UTC
Looks good to me.

Comment 3 Rich Megginson 2006-08-18 00:25:57 UTC
I think the error code should be LDAP_UNWILLING_TO_PERFORM - LDAP_PARAM_ERROR is
only intended as an API error code e.g. an invalid parameter to ldap_search_s
detected by the client side API library.
The RFC doesn't say what error code to use, other than it must be !=
LDAP_SUCCESS - http://www.rfc-archive.org/getrfc.php?rfc=3062 - and openldap
actually does generate a random password.  They just use a random string of 8
bytes converted to base64 - we could do the same - we do the same thing in uuid
generation.

Comment 4 Nathan Kinder 2006-08-18 01:41:13 UTC
I think that it would be good to support generating a random password.  I'll
look into doing that.

Comment 5 Nathan Kinder 2006-08-18 21:34:53 UTC
Created attachment 134486 [details]
Revised Diffs

This new set of diffs adds password generation support to ns-slapd.  If a user
doesn't supply a new password in  the password modify extop request, we will
generate a random 8 character password, store it as the users password, and
return it to the user in the password modify extop response.

I also removed a block of code that was setting the extop return OID in the pb
to the password modify OID.  RFC3062 explicitly states that the password modify
response should have the responseName field absent.

Comment 6 Rich Megginson 2006-08-18 22:35:04 UTC
Looks good.

Comment 7 Nathan Kinder 2006-08-18 22:52:41 UTC
Created attachment 134493 [details]
Revised Diffs

I made two minor changes due to feedback from Noriko.  One is simply adding a
bit more detail in one of the comments in the password generation function. 
The other change is removing a check to see if slapi_ch_malloc() failed.  If
slapi_ch_malloc() can't allocate memory, the server exits, so there's no need
for a check after calling it.

Comment 8 Nathan Kinder 2006-08-18 22:54:30 UTC
Checked changes in to ldapserver (HEAD).  Thanks to Rich and Noriko for reviews!

Checking in passwd_extop.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/passwd_extop.c,v  <--  passwd_extop.c
new revision: 1.10; previous revision: 1.9
done

Comment 9 Nathan Kinder 2006-08-21 15:48:45 UTC
Created attachment 134569 [details]
Additional code clean-up

I made a few optimizations to the previous fix.

We only need to generate 6 random bytes from NSS to end up with an 8 character
password after base64 encoding.  We were previously generating 8 bytes.  The
new fix generates 6 bytes instead.

I also cleaned up the calls to ber_printf().  We really only need to make one
call to ber_printf() instead of three.	This looks cleaner, and should be a
little bit more efficient.

Comment 10 Noriko Hosoi 2006-08-21 19:47:43 UTC
Looks good.

Comment 11 Nathan Kinder 2006-08-21 21:16:58 UTC
Checked in the additional changes from comment 9.  Thanks for the review Noriko!

Checking in passwd_extop.c;
/cvs/dirsec/ldapserver/ldap/servers/slapd/passwd_extop.c,v  <--  passwd_extop.c
new revision: 1.11; previous revision: 1.10
done

Comment 12 Rich Megginson 2006-10-04 21:16:50 UTC
Can we mark this bug as MODIFIED?

Comment 13 Nathan Kinder 2006-10-04 21:54:32 UTC
Yes, we can.

Comment 14 Michael Gregg 2008-01-04 01:10:31 UTC
Verified with the following command:
ldappasswd -w *pw* -D"cn=directory manager" -h localhost -p 1111 -ZZ -x
"uid=mgreggtest,ou=people,o=redhat"

The return was:
New password: 28Ht0Pon
Result: Success (0)

This worked. No unwilling preform, or LDAP_PROTOCOL_ERROR

Verified aginst:
1199305764 idm-console-framework-1.1.0-7.el4idm Wed Jan 02 2008 
1199305765 redhat-ds-console-8.0.0-9.el4dsrv Wed Jan 02 2008 
1199305766 redhat-admin-console-8.0.0-9.el4dsrv Wed Jan 02 2008 
1199305768 redhat-idm-console-1.0.0-21.el4idm Wed Jan 02 2008