userPassword is sent to the browser

[AlexandreTunstall]

Although the UI hides a user's existing password, it is still sent to the browser and can be seen in the network logs. Arguably, this is more an issue with the LDAP access configuration (more on this later), but fixing it is still a good idea for better defense in depth.

The only fathomable reason for this is to verify the "old password" field on the change password prompt, yet that verification process works with only =wx access (i.e. unreadable userPassword), so I think this could easily be fixed without compromising on functionality.

Using =wx access to try and work around this problem allows the user to change their own password (the userPassword attribute has to be manually added since it's not visible and the "old password" field has to be manually enabled), but then ldap-ui reports an error because it tries to read the attribute, making the user think the operation failed.

=> slap_access_allowed: read access denied by =wx

In summary, here's what I think should be changed for better security:

• Fix the issues modifying userPassword when it's not readable (disabled yet required "old password" field and misleading error after making the change). I can't think of any way of soundly determining whether an object has a password without adding other permissions, which may be less secure [1].
• When the userPassword attribute is readable, don't send its value to the browser (even better if it's possible to avoid reading it from LDAP in the first place).

[1] With OpenLDAP 2.4, =wsx allows use of a (userPassword=<value>) filter to find users with specific passwords. This might be fine with salted hashes (like the default SSHA), but certainly isn't with unsalted or unhashed passwords. If this risk is acceptable, it could be used to determine whether an object has a password with the (userPassword=*) filter.

[dnknth]

Hello @AlexandreTunstall,

Thanks for the thorough analysis. I agree that the userPassword should not be sent to the client and have masked it before transmission as a stopgap measure.

The password verification uses a server-side bind operation, so this should still work as before.

There might be two issues when changing passwords:

1. If the password is not present, the frontend should assume that it does not exist and should not require a confirmation.
2. If it is not readable, but its presence can be detected (by userPassword=* or similar), then the server should mask it such that the client can prompt for confirmation.

Will come back to it, but need to do some refactoring first, so this might take a moment.

[dnknth]

Hello @AlexandreTunstall,

Updates the password logic: The backend never sends passwords to the client, but *****. It also ignores userPassword changes on entry updates.

Exception: For new entries, clear text passwords are allowed, it is up to the directory to hash them. When this happens, the client gets the hash.

Hope this helps, please close this issue after testing.

[dnknth]

Please review, and let me know whether the issue can be closed.

[AlexandreTunstall]

Sorry, I haven't had much time to test in the past month, but I've had another try with the latest commit.

The userPassword is no longer sent to the browser, so password hashes are no longer leaked to users.

It also ignores userPassword changes on entry updates.

This still doesn't seem to work on the latest version. When I try to add a userPassword field to a user with an existing userPassword field (which is now hidden in the UI), the change still works and the UI still gives a misleading error.

I think the easiest way of reproducing this is to try setting a password for the user you're signed in as and then refresh the page. Basic authentication should fail after refreshing.