Bug 315954

Summary:	LDAP password stored in clear text
Product:	[Applications] kmail2	Reporter:	Michael Reiher <redm>
Component:	general	Assignee:	kdepim bugs <kdepim-bugs>
Status:	RESOLVED FIXED
Severity:	major	CC:	montel
Priority:	NOR
Version:	4.9.5
Target Milestone:	---
Platform:	Ubuntu
OS:	Linux
Latest Commit:	http://commits.kde.org/kdepim/e13aa1412d72a0c317a5d859b9b520c31c9a40a0	Version Fixed In:	4.11
Sentry Crash Report:

Description Michael Reiher 2013-03-01 13:57:29 UTC

I configured an LDAP server to retrieve addressbook entries from a directory. I added the LDAP server via Composer -> Mail address field -> "Select" -> "Search Directory Service" -> "Configure LDAP Servers".

In .kde/share/config/kabldaprc I found my password in clear text! This is a grave security problem and the password should be stored in kwallet.



Reproducible: Always

Comment 1 Laurent Montel 2013-03-01 15:45:31 UTC

I confirm that ldap doesn't have wallet support.
Will look at how to fix it.
Regards.

Comment 2 Michael Reiher 2013-03-01 16:50:07 UTC

When searched through the bug list if this bug already exists, I found references which suggest that there is something else LDAP related with kwallet support in Akonadi? Are there two LDAP implementations in Akonadi, or kio_ldap and something else? Shouldn't they be unified?

Comment 3 Laurent Montel 2013-03-01 18:52:44 UTC

Git commit 0ec9c66a535a64bbec07e56cfd1098086c45a4c7 by Montel Laurent.
Committed on 01/03/2013 at 19:47.
Pushed by mlaurent into branch 'master'.

Start to implement store ldap password in kwallet

M  +1    -0    libkdepim/CMakeLists.txt
A  +193  -0    libkdepim/ldap/ldapclientsearchconfig.cpp     [License: LGPL (v2+)]
A  +64   -0    libkdepim/ldap/ldapclientsearchconfig.h     [License: LGPL (v2+)]

http://commits.kde.org/kdepim/0ec9c66a535a64bbec07e56cfd1098086c45a4c7

Comment 4 Laurent Montel 2013-03-02 10:37:49 UTC

Git commit 74a42eec4861b673ec599a6aaac4a1c6c4173774 by Montel Laurent.
Committed on 02/03/2013 at 11:36.
Pushed by mlaurent into branch 'master'.

Continue to implement save ldap password in kwallet

M  +7    -5    libkdepim/ldap/kcmldap.cpp
M  +5    -0    libkdepim/ldap/kcmldap_p.h
M  +14   -2    libkdepim/ldap/ldapclientsearch.cpp
M  +2    -0    libkdepim/ldap/ldapclientsearchconfig.cpp

http://commits.kde.org/kdepim/74a42eec4861b673ec599a6aaac4a1c6c4173774

Comment 5 Laurent Montel 2013-03-02 10:52:19 UTC

Git commit e13aa1412d72a0c317a5d859b9b520c31c9a40a0 by Montel Laurent.
Committed on 02/03/2013 at 11:51.
Pushed by mlaurent into branch 'master'.

Fix Bug 315954 - LDAP password stored in clear text

FIXED-IN: 4.11

M  +2    -2    libkdepim/ldap/ldapclientsearch.cpp
M  +2    -2    libkdepim/ldap/ldapclientsearch.h
M  +4    -2    libkdepim/ldap/ldapsearchdialog.cpp

http://commits.kde.org/kdepim/e13aa1412d72a0c317a5d859b9b520c31c9a40a0

Comment 6 Michael Reiher 2013-03-02 13:20:22 UTC

That was quick:) Great, thank you!

Comment 7 Laurent Montel 2013-03-04 07:43:35 UTC

now I added a messagebox to ask if we want to migrate password in kwallet.

Comment 8 Michael Reiher 2013-03-18 11:26:22 UTC

I just wonder, why ask at all? What if the user chooses "no"? Do you continue to support the cleartext password? 

In any case an existing password in kabldaprc should be removed (maybe you do this already, and this is stating the obvious, but I thought I just mention it, just in case...)

Comment 9 Laurent Montel 2013-03-18 11:42:33 UTC

because some people doesn't want to store it in kwallet.
If they want to keep in config file I keep it in config file.
it's his choose so sorry I will not remove all by default.
Regards

Comment 10 Michael Reiher 2013-03-18 14:33:13 UTC

Ok yes, maybe allow saving to file, but you should never ever store it in clear text on disk. Keep in mind that not everybody understands the technical details of saving passwords, not necessarily even what clear text means. And actually nobody should need to care. There will be people not fully understanding the question, and then ending up with keeping a cleartext password. At least put a big fat warning, but still...  So, IMHO, don't keep it or hash and salt it. Just my 2ct on this.

Oh, and with remove I meant remove after migration to KWallet.

Comment 11 Laurent Montel 2013-03-18 15:08:52 UTC

it's removed after migration to kwallet of course.