Bug 203043 - Password modify extended operation should generate new passwords
Password modify extended operation should generate new passwords
Status: CLOSED CURRENTRELEASE
Product: 389
Classification: Community
Component: Directory Server (Show other bugs)
1.0.2
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nathan Kinder
Viktor Ashirov
:
Depends On:
Blocks: 152373 fds103trackingbug 240316
  Show dependency treegraph
 
Reported: 2006-08-17 18:56 EDT by Nathan Kinder
Modified: 2015-12-07 12:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-12-07 12:11:41 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
CVS Diffs (1.84 KB, patch)
2006-08-17 19:01 EDT, Nathan Kinder
no flags Details | Diff
Revised Diffs (10.86 KB, patch)
2006-08-18 17:34 EDT, Nathan Kinder
no flags Details | Diff
Revised Diffs (10.84 KB, patch)
2006-08-18 18:52 EDT, Nathan Kinder
no flags Details | Diff
Additional code clean-up (3.34 KB, patch)
2006-08-21 11:48 EDT, Nathan Kinder
no flags Details | Diff

  None (edit)
Description Nathan Kinder 2006-08-17 18:56:15 EDT
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 19:01:48 EDT
Created attachment 134417 [details]
CVS Diffs
Comment 2 Noriko Hosoi 2006-08-17 19:08:24 EDT
Looks good to me.
Comment 3 Rich Megginson 2006-08-17 20:25:57 EDT
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-17 21:41:13 EDT
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 17:34:53 EDT
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 18:35:04 EDT
Looks good.
Comment 7 Nathan Kinder 2006-08-18 18:52:41 EDT
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 18:54:30 EDT
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 11:48:45 EDT
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 15:47:43 EDT
Looks good.
Comment 11 Nathan Kinder 2006-08-21 17:16:58 EDT
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 17:16:50 EDT
Can we mark this bug as MODIFIED?
Comment 13 Nathan Kinder 2006-10-04 17:54:32 EDT
Yes, we can.
Comment 14 Michael Gregg 2008-01-03 20:10:31 EST
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 

Note You need to log in before you can comment on or make changes to this bug.