389ds / 389-ds-base

The enterprise-class Open Source LDAP server for Linux
https://www.port389.org/
Other
210 stars 90 forks source link

modify-delete userpassword #394

Closed 389-ds-bot closed 4 years ago

389-ds-bot commented 4 years ago

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/394


Steps to duplicate this issue:

  1. A user uid=tuser0,ou=People,dc=example,dc=com has a userpassword: newpassword.
  2. delete the userpassword with the value. ldapmodify ... dn: uid=tuser0,ou=People,dc=example,dc=com changetype: modify delete: userpassword userpassword: newpassword

    ldap_modify: No such attribute

Internally, the newpassword in the mod has the value:

(gdb) p *vals[0]
$19 = {bv_len = 46,
  bv_val = 0x7f6b940055f0 "{SSHA}Uj1d+WvfAfa/hx2eCnM3WuJJzxEtprB37SbArA=="}

while the value in the entry is hashed differently.

(gdb) p *a->a_present_values.va[0]
$23 = {bv = {bv_len = 46,
    bv_val = 0x7f6b940056a0 "{SSHA}s9Jcj0zOofoEK4HgyUugR/KVOE8hOaJhftTNvw=="},
  v_csnset = 0x7f6b9400acc0, v_flags = 0}

The failed point:

(gdb) bt
0  entry_delete_present_values_wsi (e=0x7f6b94000a30,
    type=0x7f6b94007f10 "userPassword", vals=0x7f6b94007e40,
    csn=0x7f6bb9ff4960, urp=0, mod_op=129, replacevals=0x0)
    at ldap/servers/slapd/entrywsi.c:597
1  0x00007f6bc9d8cb8a in entry_apply_mod_wsi (e=0x7f6b94000a30,
    mod=0x7f6b94007f30, csn=0x7f6bb9ff4960, urp=0)
    at ldap/servers/slapd/entrywsi.c:733
2  0x00007f6bc9d8cd73 in entry_apply_mods_wsi (e=0x7f6b94000a30,
    smods=0x7f6bb9ff4a10, csn=0x7f6b9400c8c0, urp=0)
    at ldap/servers/slapd/entrywsi.c:776
3  0x00007f6bc528bf05 in ldbm_back_modify (pb=0x2488680)
    at ldap/servers/slapd/back-ldbm/ldbm_modify.c:321
4  0x00007f6bc9db7acc in op_shared_modify (pb=0x2488680, pw_change=1,
    old_pw=0x0) at ldap/servers/slapd/modify.c:936
5  0x00007f6bc9db6851 in do_modify (pb=0x2488680)
    at ldap/servers/slapd/modify.c:405
6  0x0000000000413ff7 in connection_dispatch_operation (conn=0x7f6bc02cfbf0,
    op=0x24d7c70, pb=0x2488680) at ldap/servers/slapd/connection.c:591
7  0x00000000004159de in connection_threadmain ()
    at ldap/servers/slapd/connection.c:2336
8  0x0000003d20628793 in _pt_root (arg=0x24cfd00)
    at ../../../mozilla/nsprpub/pr/src/pthreads/ptthread.c:187
9  0x0000003d16e07d90 in start_thread (arg=0x7f6bb9ffb700)
    at pthread_create.c:309
10 0x0000003d16af0f5d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:115

Next, I tried the returned value from search, then the userPassword value matches the one in the entry.

userpassword: {SSHA}s9Jcj0zOofoEK4HgyUugR/KVOE8hOaJhftTNvw==

But, now it fails to delete unhashed#user#password in the entry since it stores the clear text.

We can specially delete the unhashed password without checking the value. But if an entry has multiple userPasswords, it may cause a problem if we cannot delete just one of them... Even if we stash the clear text passwords in the extension (as I'm working on it now), we still have the same problem. Probably, the right thing to do would be letting modify-delete take a clear text password and we should match the passwords considering the hash?

 527 static int
 528 entry_delete_present_values_wsi(Slapi_Entry *e, const char *type, struct berval **vals, const CSN *csn, int urp, int mod_op, struct berval **replacevals)
        [...]
 596                 retVal= valueset_remove_valuearray(&a->a_present_values, a, valuestodelete, 0 /* Do Not I gnore Errors */,&deletedvalues);
       <== this returns error LDAP_NO_SUCH_ATTRIBUTE (16)
389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-06-25 23:56:41

Note: this problem is temporarily observed. Once the server is restarted and the unhashed password is no longer stored in the entry in memory, the userpassword could be successfully removed. $ ldapmodify -x ... dn: uid=tuser1, o=my.com changetype: modify delete: userpassword userpassword: {SSHA}zWIkZAgHamvu9fw6w26JHerN5HfEkobJ/TRN+g==

modifying entry uid=tuser1, o=my.com

389-ds-bot commented 4 years ago

Comment from rmeggins (@richm) at 2012-08-14 19:57:05

set default ticket origin to Community

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2012-08-28 04:14:55

Added initial screened field value.

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-11-15 01:37:24

I reran the test...

I see "No such attribute" (as expected) if the password storage scheme is non-clear (e.g., SSHA).

$ ldapmodify ...
dn: uid=tuser0,o=my.com
changetype: modify
delete: userpassword
userpassword: tuser0

modifying entry uid=tuser0,o=my.com
ldap_modify: No such attribute

If I set clear to password storage scheme, the deletion works fine.


$ ldapmodify ...
dn: uid=tuser1,o=my.com
changetype: modify
delete: userpassword
userpassword: tuser1

modifying entry uid=tuser1,o=my.com
}}
This bug was fixed with this ticket.
https://fedorahosted.org/389/ticket/455
389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-11-15 03:10:53

Bug Description:  Attempting to delete an existing, encoded, password using
                  the clear text password fails with an error 16 (no such attribute)

Fix Description:  If performing a delete of a specific userpassword, check the existing
                  password values for their encoding.  Then encode the clear text password
                  for comparision, and the delete the match.

May I ask what behaviours does your patch changes?

If the userpassword is hashed by, e.g., SSHA, you can now delete the specific password with the password string? (Or it still fails, but you can get a better error message?)

And I'm curious why these lines are not necessary any more?

841  /* delete pseudo password attribute if it exists in the entry */ 
842  if (!slapi_entry_attr_find(e, unhashed_pw_attr, &a)) { 
843 slapi_mods_add_mod_values(&smods, pw_mod->mod_op, 
844
389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2012-11-15 03:17:55

The patch now allows you to use the clear text password to delete the encoded one in the entry.

As for the removed code... I don't see unhashed_userpassword in the entry after adding/replacing userpassword, and you implied that was expected. I asked you to double check and you said it was ok. So I removed that code.

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2012-11-15 03:34:32

Replying to [comment:8 mreynolds389]:

The patch now allows you to use the clear text password to delete the encoded one in the entry.

As for the removed code... I don't see unhashed_userpassword in the entry after adding/replacing userpassword, and you implied that was expected. I asked you to double check and you said it was ok. So I removed that code.

Now the unhashed userpassword is stashed in the entry object extension. In case the entry in the entry cache has it and we delete a specified password, we want to delete the stashed unhashed userpassword, too. To do so, mods need to have the operation, I think.

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2012-11-15 20:05:46

Replying to [comment:9 nhosoi]:

Replying to [comment:8 mreynolds389]:

The patch now allows you to use the clear text password to delete the encoded one in the entry.

As for the removed code... I don't see unhashed_userpassword in the entry after adding/replacing userpassword, and you implied that was expected. I asked you to double check and you said it was ok. So I removed that code.

Now the unhashed userpassword is stashed in the entry object extension. In case the entry in the entry cache has it and we delete a specified password, we want to delete the stashed unhashed userpassword, too. To do so, mods need to have the operation, I think.

I still have the original code that cleaned up the unhashed userpassword, so I'll add it back in. I'll send the new patch out shortly.

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2012-11-20 03:17:59

Added the password extension changes 0001-Ticket-394-modify-delete-userpassword.patch

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2012-11-20 03:54:39

git merge ticket394 Updating 32ab01f..ecf3cc3 Fast-forward ldap/servers/slapd/modify.c | 170 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 161 insertions(+), 9 deletions(-)

[mareynol@localhost ds]$ git push origin master Counting objects: 11, done. Delta compression using up to 4 threads. Compressing objects: 100% (6/6), done. Writing objects: 100% (6/6), 2.77 KiB, done. Total 6 (delta 4), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git 32ab01f..ecf3cc3 master -> master

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2013-03-07 00:12:40

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=918684

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2017-02-11 22:56:30

Metadata Update from @mreynolds389:

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2014-01-20 21:37:59

Linked to Bugzilla bug: https://bugzilla.redhat.com/show_bug.cgi?id=1053232 (''Red Hat Enterprise Linux 6'')

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2014-01-21 01:36:59

attachment 0001-Ticket-47678-modify-delete-userpassword.patch

389-ds-bot commented 4 years ago

Comment from nhosoi (@nhosoi) at 2014-01-22 22:54:00

Noriko, can you give me the official ack in the ticket?

Sure! My pleasure.

389-ds-bot commented 4 years ago

Comment from mreynolds (@mreynolds389) at 2014-01-22 22:59:26

git merge ticket47678 Updating d128dbd..a3b6e22 Fast-forward ldap/servers/slapd/modify.c | 168 ++++++++++++++++++++++++++++++++++-- ldap/servers/slapd/slapi-plugin.h | 11 --- 2 files changed, 158 insertions(+), 21 deletions(-)

git push origin 389-ds-base-1.2.11 Counting objects: 13, done. Delta compression using up to 4 threads. Compressing objects: 100% (7/7), done. Writing objects: 100% (7/7), 2.42 KiB, done. Total 7 (delta 5), reused 0 (delta 0) To ssh://git.fedorahosted.org/git/389/ds.git d128dbd..a3b6e22 389-ds-base-1.2.11 -> 389-ds-base-1.2.11

commit a3b6e22cec1fb8cb5c55e8b848bec4a60f924849 Author: Mark Reynolds mreynolds389@redhat.com Date: Mon Jan 20 14:36:43 2014 -0500

389-ds-bot commented 4 years ago

Comment from nkinder (@nkinder) at 2017-02-11 23:07:04

Metadata Update from @nkinder: