User passwords logged when LDAP misconfigured

[Craig Miskell]

When LDAP is misconfigured, for example pointing to a non-existent LDAP server, the stack trace in the webserver log reports the users password (redacted log snippet to be attached).

It is not a major bug, in that the information is only available to the server administrator under normal circumstances (unless log files are not locked down, which does happen sometimes), but it's still bad form and should be avoided if possible.

Mahara 1.6.0dev 2012051500 (according to lib/version.php). Running on Ubuntu 10.04 and Apache2.

[Richard Mansfield (richard-mansfield)]

Also discussed publicly at https://mahara.org/interaction/forum/topic.php?id=1727 - in that thread Nigel seemed to think it wasn't worth the hassle to filter them out, but maybe we should have another look.

[Craig Miskell (3-crjig-7)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/06/12 12:55, Richard Mansfield wrote:
> Also discussed publicly at
> https://mahara.org/interaction/forum/topic.php?id=1727 - in that
> thread Nigel seemed to think it wasn't worth the hassle to filter
> them out, but maybe we should have another look.
Good to see it's been noticed before. To add to the discussion, the
particular trouble with the LDAP case is that the password is almost
invariably going to mean something elsewhere than just Mahara (e.g. in
the Catalyst case, a lot of other systems). If it was just the mahara
password, it wouldn't be quite so bad.

Although I suppose an evil administrator could just modify the PHP and
log the passwords elsewhere.

In this specific case, it looks like the password is passed as a
string argument which gets automatically dumped in the stacktrace.
Perhaps a simple solution would be to embed it in an object which gets
passed around, thus hiding it from exposure in stack traces.

- --
Craig Miskell
Systems Administrator, Catalyst IT
DDI: [phone]
==
Some of us here are sysadmins, and network admins, and even Windows
admins. Clubbing baby harp seals would a socially acceptable step
*up*. -- butting on ARK
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPzrETAAoJENezkH+p+mMX3YYP/0FLoInTS2OjcfXMj5GY6I5Q
Q3vWS1TIy1ZdYmZhSIQ0T8o/ZxlaGfi2s8+MzczS8eXZc6Pk21eYE0BHBWIS3Z4J
ptVMF9mO1LowxEUyWbrycEYf6dFbuSid8jFqb/cy6x1ygNbI9XRuzUYn14Nm5sGa
EMO2u84XdirX49CBik41mJvFL6EmFsL0MF5Nrzh7wWXngGxT5vJ7qYXz/BOrRFaa
HJf9DxqWvJevure0KChXCQG7a7OnRTv6YvRdYX7jJjiA2SjZAOcuSytVN4XrcE69
eW7UjyjJf4Wa8pdwBzxdPQ713g9eZtFNeCt+nSwXkLFLGNa9jAirbmRLz2ktDy6e
SSB73TsPrJw2pbrY6UpgBpX442Ye1BUIK4cqQFNUfqvCcwpGLHo9w70yodOFTSM9
n4P3jjKuRXjWXDtwPH4sOqjkK/kCNVFG8gbwv0t0Ff9HT8CMVlvgUQ4tgAn0Qi3z
tOR0gsEz6xjFH+Z38+hj59G8F08AEAjMbSwW22B5b6FzBb7NtRR4GTYoccgloq9g
yoCB+Q3006IrGeUCysWPLD0nakop1HmteSCOW5GZtbXXAABGUIvyBFkTMb8s9Opb
eE3OuLmTYeeIOqwQ7ZAMzEEkulNnQF/E860dQc0lLC8NuLg12U/bmSTnYwNzwUQ7
Hyhf/LxSG253+z8OWg7I
=GB8+
-----END PGP SIGNATURE-----

[Richard Mansfield (richard-mansfield)]

That authenticate_user_account function has to stick around for a while, because third party auth plugins rely on it. But it doesn't need to be called on any of the auth methods in core, we could add some new function that takes an object, as Craig suggested, and only call authenticate_user_account for those auth methods that don't implement the new function (or something like that). Might be a good thing to do anyway, because some auth methods don't want to see a password.

[Aaron Wells (u-aaronw)]

Adding Taylor Judd to this issue, because he reported the same problem: https://bugs.launchpad.net/mahara/+bug/1194616

[Aaron Wells (u-aaronw)]

It appears this issue has been around for a while. As mentioned in earlier comments, it's a tricky one to solve. We need to move the password around in plaintext because eventually it has to be passed as a plaintext string to the PHP core function ldap_bind().

As mentioned above, we could reimplement things so that the password is an object property instead of a string, and that would make it less likely to be printed out in a stack trace. We could even take things a step further and obfuscate the password, scrambling it in some reversible way using a random string as a key.

[Son Nguyen (ngson2000)]

Test case:

1. Add LDAP authentication to an institution. Set a wrong Host URL to it.
2. Logout and try to login as an user
3. The user's password must not be shown in the screen or error log

[Son Nguyen (ngson2000)]

I think this is the bug inside the LDAP auth lib.

The method ldap_find_userdn() should return false if there is no result from LDAP queries.

Please my patch in the attached file

[Son Nguyen (ngson2000)]

https://reviews.mahara.org/#/c/2767