[Feature Request]: Please add some configuration setting so that LDAP passwords are not kept in Snipe-IT DB

[phep]

Is your feature request related to a problem? Please describe.

I noticed that LDAP users passwords (once users have authenticated once) are copied over in Snipe-IT DB which:

1. increases the attack surface of our infrastructure, considered a a whole.
2. is not coherent with the purpose of using an LDAP authentication.

Describe the solution you'd like

I understand (but did not test, to be honest) that this might be useful should the LDAP authentication be broken somehow, but I'd really prefer to be given the choice between:

• a broken application,
• our user passwords being kept in more than one place,

Besides it looks to me that having those passwords copied into the application DB plainly defeats the whole purpose of a central authentication mechanism such as LDAP that let passwords being kept in one single place.

I think some configuration setting to let this password copy being en/disabled would be a great feature.

Thanks in advance

Describe alternatives you've considered

No response

Additional context

No response

[uberbrady]

If you turn off the "LDAP Password Sync" setting, does that do what you want? We are supposed to no longer sync passwords into the Snipe-IT User record when that setting is turned off. If not, then that's a bug.

[phep]

Hi,

Thank you for your prompt reply. It turns out that the translation for the help text of this field is wrong in our language: it just says the contrary of what it should ;-). I'll try to help on this later this month.

Yet, un-ticking the box did not change the behaviour. The passwords are still going to the password column :-/. Besides, those already present are not erased when we delete a user from the web UI.

Hope this helps.

[snipe]

@uberbrady we generate a completely random (very long) password to initially create the user, as password is required on all users. However the password does not get synced from LDAP if the "LDAP Password Sync" option is unchecked. There is a password, just not their password, and because we implement brute force prevention/login throttling, it would be statistically impossible to brute force using the generated password.

[phep]

OK, I just checked this and you are right @snipe , the value does not match the bcrypt-ed value of the passwords.
This might deserved to be documented somehow yet ;-).
On my side I'll do my best to provide a fixed translation next week for the help text that go along the check box, I still have not check the "Contributing" part of the documentation ;-).
Thank you all!

[phep]

Oh, just one more thing: it looks like once a (ldap) user has authenticated with the "sync" setting enabled, when the setting is disabled and the user re-authenticates, the user password in the DB is not reset to the arbitrary value you mention.

[uberbrady]

The trick we do on SCIM is we put *NO PASSWORD* as the password - which will always fail hashing, as it doesn't even have the correct format. But it still passes validation. Of course, the randomized password is very unlikely (probably impossible) to match a hash as well.

But that's a long-winded way of saying "I think if you uncheck that setting it should do what you want." :)

[snipe]

Oh, just one more thing: it looks like once a (ldap) user has authenticated with the "sync" setting enabled, when the setting is disabled and the user re-authenticates, the user password in the DB is not reset to the arbitrary value you mention.

Let me test this and see if I can reproduce.

[snipe]

the user password in the DB is not reset to the arbitrary value you mention.

Since the arbitrary password value is, well, arbitrary, I'm not sure how we could reset it back to what it was, but we should at least be giving it a different arbitrary value.

@uberbrady If you'd like to standardize using `NO PASSWORD as an unhashed password (stored in cleartext, since the hash comparison will never, ever match and therefore they cannot login using that value, we could probably do that, but I'd think we'd want it to be consistent across the boards. We just need to make sure we're NOT hashing it anywhere we're doing an LDAP import/sync/etc. It might speed things up a little (since hashing has a cost), but I'm on the fence since if we miss removing the hash, we've just made a standard password for LDAP users. 😱

That can also be part of a separate discussion though. For now, I can just fix this one bit.

[uberbrady]

I did double-check an instance by doing a SELECT password FROM users and it all comes back as, unhashed, *NO PASSWORD* - which will never match a hash (it's not even in the right format). Well, unless the user-password-hashing format defaults to MD5 - which it might - but, then again, there is no way to make an MD5 that has a * in it since it's in hex. So I'm pretty sure it's safe.

[snipe]

@uberbrady that's not what I meant. We specifically use bcrypt when we sync users, so md5 wouldn't matter.

snipe-it/app/Console/Commands/LdapSync.php

Lines 183 to 185 in 74a5bcd

	/* Create user account entries in Snipe-IT */
	$tmp_pass = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 20);
	$pass = bcrypt($tmp_pass);

I just mean that if we were going to insert *NO PASSWORD* as a string when creating users, we'd want to make sure we triple-check that we're NOT bcrypting *NO PASSWORD* anywhere that we're syncing users, since if we DID do that, the users password would be encrypted and *NO PASSWORD* would actually work.

Lemme get this small change out for now at least, we can chat more about the *NO PASSWORD* thing later.

[uberbrady]

It also doesn't start with the $2y$10$ parameter, either.

[phep]

the user password in the DB is not reset to the arbitrary value you mention.

Since the arbitrary password value is, well, arbitrary, I'm not sure how we could reset it back to what it was, but we should at least be giving it a different arbitrary value.

Sorry, I'm not a native speaker. I must have be unclear. Please consider this sequence of actions:

1. the admin ticks the check box to activate password sync
2. some ldap user authenticates herself
3. her bcrypt-ed password is set in the DB
4. the admin untick the check box to prevent password sync
5. the same user authenticates again, later

Now, from what I saw earlier today, the bcrypted password is kept unchanged in the DB. I mean the hash is still the actual password hash generated at step 3 above.

I'd suggest that on such occasions the actual password hash be replaced by the usual random generated password hash. I understand that since there is no way for the application to know if the current hash in the DB is from an actual or arbitrary password, then the hash should be replaced every time according to the current setting. I don't think this would be costly as problematic as long as bcrypt is used. This might become so if argon were used with a long ARGON_TIME setting, yet.

[snipe]

I understood what you meant and was already working on a fix :) My internet connection was crappy so I couldn't push the branch yesterday.