#394 modify-delete userpassword
Closed: Fixed None Opened 7 years ago by nhosoi.

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)

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

set default ticket origin to Community

Added initial screened field value.

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

{{{
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
}}}

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.

Replying to [comment:8 mreynolds]:

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.

Replying to [comment:9 nhosoi]:

Replying to [comment:8 mreynolds]:

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.

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

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.0

3 years ago

Login to comment on this ticket.

Metadata