🐛: LDAP with bind auth still requires a secret to be present

[mpldr]

What happened?

While setting up LDAP auth, I get the following error:

authentication error: listenerId = "https", localPort = 443, remoteIp = 172.24.0.1, remotePort = 53848, details = Authentication error (auth.error) { details = Account does not contain secrets, causedBy = crates/common/src/auth/oauth/token.rs:239, causedBy = crates/common/src/auth/oauth/token.rs:53 }, causedBy = crates/jmap/src/auth/oauth/token.rs:129

That is indeed correct, as my identity provider includes an OTP, so a static hash is not available

How can we reproduce the problem?

I can reproduce the problem by doing the following steps:

• Setup LDAP with Bind auth and remove the secret field

Version

v0.11.x

What database are you using?

PostgreSQL

What blob storage are you using?

S3-compatible

Where is your directory located?

LDAP

What operating system are you using?

Docker

Relevant log output

Code of Conduct

• I agree to follow this project's Code of Conduct

[mpldr]

While it is possible to bypass this requirement by setting any key that actually exists, this should probably not be the way, right?

[eyJhb]

I just encountered this error as well. Setting secret to any key does not aid :)

Weird part, it knows when the username/password is incorrect, as when it's correct it will throw "unauthorized" back instead of the "invalid username/password".

[eyJhb]

This seems like it's a know limitation, based on what I can read here https://stalw.art/docs/auth/backend/ldap#limitations-regarding-password-hashes and here #491

EDIT: if I create a secret user attribute, with a bogus {clear}SuperSecret!, then I can authenticate. But you can't do much in the self-service portal, as updating/inserting is not supported for LDAP. So unsure how useful the portal is. I had assumed it would store such things in a different local database instead.

[mdecimus]

That is correct, Stalwart needs to have access to the secret in order to be able to invalidate OAuth tokens.

[CEbbinghaus]

This is not quite correct, a lot of LDAP's have a pwdChangedTime which would allow for trivially easy token invalidation. All that needs to be checked is if the token was issued before the last password changed time, and it immediately becomes invalid. It is a much more secure alternative to requiring the password hash which itself is privileged information and can be used for a data breach.

It would be nice if the userPassword attribute would be removed from the required attributes, and instead use pwdChangedTime (or similar) for determining the time. It is also silly to require these attributes if all they do is enable additional functionality (OAuth & SCRAM), which isn't required in a majority of cases. But requiring all queries to have a attribute to set the user password is a nuisance given all the ldap servers that don't support that attribute. (and therefore make every query fail)

[Firstyear]

Hi there,

A better approach to invalidate oauth tokens would be to read an attribute that describes when the userPassword attribute changed.

We see this issue come up in Kanidm quite a bit, and the issue is that we will never expose a userPassword to any application for any reason. Even if we did, we store our passwords with argon2id (and in future with hmac from a HSM) so the password hash is useless to any external application.

It would be better to use something like https://ldapwiki.com/wiki/Wiki.jsp?page=PwdChangedTime for this purpose instead, since that is not a security sensitive attribute and still allows the invalidation of sessions.

[mdecimus]

In this case use pwdChangedTime (or a property that indicates that the password has been changed) as your secret. It is crucial that you do this only when using Bind Auth, otherwise it will be used as the secret.

[mdecimus]

Yes, the password attribute was always optional in LDAP for basic authentication. At least one secret was required in order to create and revoke the OAuth bearer token, but now you can use the password change time (or similar) attribute for that.

[CEbbinghaus]

Pardon if I am misunderstanding something but it seems to be required here:

[CEbbinghaus]

I'd rather avoid having it set altogether given the possibility for accidentally setting something insecure (e.g someone disables bind auth and it reverts to whatever random value was in Secret)

[mdecimus]

Sorry about that, the password attribute is optional when the server reads the LDAP config but the webadmin was requiring it. It has now been fixed.

[CEbbinghaus]

Thank you so much. ❤️