KPasswordDialog

[George Staikos]

A few issues have come up recently with KPasswordDialog and, more importantly,
KPasswordEdit. I think both of these classes need to be deprecated, and
KPasswordDialog rewritten. Here's why:

1) KLineEdit provides everything that KPasswordDialog does, but better, in
that it allows selection without setting the X selection or clipboard. (it
makes editting easier, and there is a wishlist for this)
2) It doesn't work properly with UTF-8 passwords (there is a bug report for
this)
3) KPasswordDialog doesn't seem to be fixable without breaking binary
compatibility or doing messy things

I notice the careful attention to using a char* for storing the password, but
is this really necessary? I think QString is perfectly acceptable - we have
many other ways people can steal passwords from memory anyway.

The final question is, given all of this, does anyone feel like taking on this
job? It should be rather straightforward...

[Andrew Coles]

- Replace KPasswordEdit with KLineEdit and adjust KPasswordDialog
accordingly.
Cheers,
Waldo

I'll provisionally volunteer for this. I've done a bit of work last week on
KPasswordDialog so I'm quite familiar with the code.

Andrew

[George Staikos]

Post by Andrew Coles

Post by Waldo Bastian
- Replace KPasswordEdit with KLineEdit and adjust KPasswordDialog accordingly.

I'll provisionally volunteer for this. I've done a bit of work last week
on KPasswordDialog so I'm quite familiar with the code.

That sounds great! Thanks!

[Waldo Bastian]

I have added to the KDE4 TODO:
- Replace KPasswordEdit with KLineEdit and adjust KPasswordDialog accordingly.

Cheers,
Waldo

Post by George Staikos
A few issues have come up recently with KPasswordDialog and, more
importantly, KPasswordEdit. I think both of these classes need to be
1) KLineEdit provides everything that KPasswordDialog does, but better, in
that it allows selection without setting the X selection or clipboard. (it
makes editting easier, and there is a wishlist for this)
2) It doesn't work properly with UTF-8 passwords (there is a bug report
for this)
3) KPasswordDialog doesn't seem to be fixable without breaking binary
compatibility or doing messy things
I notice the careful attention to using a char* for storing the password,
but is this really necessary? I think QString is perfectly acceptable - we
have many other ways people can steal passwords from memory anyway.
The final question is, given all of this, does anyone feel like taking on
this job? It should be rather straightforward...

[Olivier Goffart]

About KPasswordDialog,
I wrote few times ago a class KPassword that make the automation of saving the
result of a KPasswordDialog in the KWallet, or retreive it from there if it
already exist.

I plan to use it in Kopete insteads of the KopetePassword
http://webcvs.kde.org/cgi-bin/cvsweb.cgi/~checkout~/kdenetwork/kopete/libkopete/kopetepassword.h
(see also the .cpp file with the same name)

I attached here how it is in my project.
I've tested it with Kopete (i've a diff over it if someone is interested)
(of course, it's not mean to be a final version. i plan to add more default
value, or other function. and some stuff need to be finished)

What do you think ?

If it goes in kdelibs, where ? as it depends of kwallet and kdeui.
(i thought about putting it in kwallet, but kwallet is not linked against
kdeui)


Another thing which is missing in KPasswordDialog is the ability to show a
default password. (if the previous was wrong)

and, another thing, kwallet documentation is not in the online api reference.
http://developer.kde.org/documentation/library/cvs-api/

[David Faure]

Post by Olivier Goffart
If it goes in kdelibs, where ? as it depends of kwallet and kdeui.
(i thought about putting it in kwallet, but kwallet is not linked against
kdeui)

That would be KIO then, since that uses both kwalletclient and kdeui, and the
plan is to make kwalletclient part of it at some point.

[George Staikos]

I would prefer that this does not go into kdelibs [yet] because it uses the
Synchronous API. This is the thing that people constantly complain about
with respect ot KWallet, and hopefully I will have KHTML moved away from this
by tomorrow morning. Kopete, KNode and KMail also need to move to the
asynchronous interface (which ironically I was asked for multiple times, I
provided, and now I'm the only user of). See KWalletManager for how it
works. KWalletManager never freezes.

Now, can this be fixed in this class? Maybe. It would need to enter a new
event loop I think. Give it a try and see how it works... Other than that,
I think it's a good idea conceptually.

Post by Olivier Goffart
About KPasswordDialog,
I wrote few times ago a class KPassword that make the automation of saving
the result of a KPasswordDialog in the KWallet, or retreive it from there
if it already exist.
I plan to use it in Kopete insteads of the KopetePassword
http://webcvs.kde.org/cgi-bin/cvsweb.cgi/~checkout~/kdenetwork/kopete/libko
pete/kopetepassword.h (see also the .cpp file with the same name)
I attached here how it is in my project.
I've tested it with Kopete (i've a diff over it if someone is interested)
(of course, it's not mean to be a final version. i plan to add more
default value, or other function. and some stuff need to be finished)
What do you think ?
If it goes in kdelibs, where ? as it depends of kwallet and kdeui.
(i thought about putting it in kwallet, but kwallet is not linked against
kdeui)
Another thing which is missing in KPasswordDialog is the ability to show a
default password. (if the previous was wrong)
and, another thing, kwallet documentation is not in the online api
reference. http://developer.kde.org/documentation/library/cvs-api/

[Olivier Goffart]

Post by George Staikos
I would prefer that this does not go into kdelibs [yet] because it uses the
Synchronous API.

there is a method to call it assync.
anyway, i see now why QTimer::singleShot() is not enough.
We have to yse the assync API of KWallet dirrectly. that could be possible to
change it.

[Richard Smith]

Post by Olivier Goffart
About KPasswordDialog,
I wrote few times ago a class KPassword that make the automation of saving
the result of a KPasswordDialog in the KWallet, or retreive it from there
if it already exist.
I plan to use it in Kopete insteads of the KopetePassword

One question: why? What's wrong with KopetePassword? It supports a lot of
functionality that Kopete needs, that your KPassword class does not provide
(such as caching the password, async password retrieval, prompting the user
for the password, caching the wallet pointer, avoiding nested event loops,
handling a 'remember password' checkbox and a 'password is wrong' flag,
dealing with the user refusing to open the wallet, providing a
password-editing widget, providing a KConfig fallback, having been used for a
while so bugs have been found, etc).

Why not instead consider KopetePassword for inclusion in kdelibs (after a
rename, of course)? Only the wallet folder name is Kopete-specific (that's
hard-coded in in kopetewalletmanager.cpp).

[George Staikos]

Post by Richard Smith

Post by Olivier Goffart
About KPasswordDialog,
I wrote few times ago a class KPassword that make the automation of
saving the result of a KPasswordDialog in the KWallet, or retreive it
from there if it already exist.
I plan to use it in Kopete insteads of the KopetePassword

One question: why? What's wrong with KopetePassword? It supports a lot of
functionality that Kopete needs, that your KPassword class does not provide
(such as caching the password, async password retrieval, prompting the user
for the password, caching the wallet pointer, avoiding nested event loops,
handling a 'remember password' checkbox and a 'password is wrong' flag,
dealing with the user refusing to open the wallet, providing a
password-editing widget, providing a KConfig fallback, having been used for
a while so bugs have been found, etc).
Why not instead consider KopetePassword for inclusion in kdelibs (after a
rename, of course)? Only the wallet folder name is Kopete-specific (that's
hard-coded in in kopetewalletmanager.cpp).

Again, please see my concerns. Why exactly does Kopete::WalletManager open
the wallet asynchronously sometimes, but synchronously others?

[Richard Smith]

Post by George Staikos

Post by Richard Smith

Post by Olivier Goffart
About KPasswordDialog,
I wrote few times ago a class KPassword that make the automation of
saving the result of a KPasswordDialog in the KWallet, or retreive it
from there if it already exist.
I plan to use it in Kopete insteads of the KopetePassword

One question: why? What's wrong with KopetePassword? It supports a lot of
functionality that Kopete needs, that your KPassword class does not
provide (such as caching the password, async password retrieval,
prompting the user for the password, caching the wallet pointer, avoiding
nested event loops, handling a 'remember password' checkbox and a
'password is wrong' flag, dealing with the user refusing to open the
wallet, providing a
password-editing widget, providing a KConfig fallback, having been used
for a while so bugs have been found, etc).
Why not instead consider KopetePassword for inclusion in kdelibs (after a
rename, of course)? Only the wallet folder name is Kopete-specific
(that's hard-coded in in kopetewalletmanager.cpp).

Again, please see my concerns. Why exactly does Kopete::WalletManager
open the wallet asynchronously sometimes, but synchronously others?

It opens it synchronously if it's asked to open it synchronously,
asynchronously if it's asked to open it asynchronously. The reason for that
was that Kopete used to depend on being able to grab the password
synchronously, and I wanted to allow the existing code to keep working in the
interim. As it happens, all but two of the protocols in Kopete have been
ported for months, but I forgot to commit my changes to the other two (and
the patches have since gone stale). I'm working on porting them now; when
I've done that I'll be ripping all the synchronous stuff out of
Kopete::Password and Kopete::WalletManager.

[Olivier Goffart]

Post by George Staikos

Post by Richard Smith
One question: why? What's wrong with KopetePassword?

nothing.

Post by George Staikos

Post by Richard Smith
It supports a lot of
functionality that Kopete needs, that your KPassword class does not
provide (such as caching the password, async password retrieval,
prompting the user for the password, caching the wallet pointer, avoiding
nested event loops, handling a 'remember password' checkbox and a
'password is wrong' flag, dealing with the user refusing to open the
wallet, providing a
password-editing widget, providing a KConfig fallback, having been used
for a while so bugs have been found, etc).

Yes, i wanted to add it alter.
I used KopetePassword as model.

Post by George Staikos

Post by Richard Smith
Why not instead consider KopetePassword for inclusion in kdelibs (after a
rename, of course)? Only the wallet folder name is Kopete-specific
(that's hard-coded in in kopetewalletmanager.cpp).

Yes, it's what i'm trying to do with that.
But KopetePassword has several things that are kopete specific. the KWallet
thing is one thing, but also the KConfig stuff.

in my KPassword i tried to make simple, without having to keep the object.
(but this seems needed if we want to cache the password, unless we want to
start with static stuff)

Post by George Staikos
Again, please see my concerns. Why exactly does Kopete::WalletManager
open the wallet asynchronously sometimes, but synchronously others?

That's kopete internal.
protocols that uses the KopetePasswordedAccount API load it assync.
the old API is deprecated, but some protocol (Oscar) still uses it. Anyway,
this is going to change.

[Richard Smith]

Post by Olivier Goffart

Post by Richard Smith
Why not instead consider KopetePassword for inclusion in kdelibs (after
a rename, of course)? Only the wallet folder name is Kopete-specific
(that's hard-coded in in kopetewalletmanager.cpp).

Yes, it's what i'm trying to do with that.
But KopetePassword has several things that are kopete specific. the
KWallet thing is one thing, but also the KConfig stuff.

It wouldn't be hard to make the wallet folder name application-defined. Or to
add a flag to say whether the KConfig fallback is allowed. Apart from the
synchronous stuff, I can't think of anything you could remove and still have
it do enough for Kopete.

[Ingo Klöcker]

Post by George Staikos
A few issues have come up recently with KPasswordDialog and, more
importantly, KPasswordEdit. I think both of these classes need to be
1) KLineEdit provides everything that KPasswordDialog does, but
better, in that it allows selection without setting the X selection
or clipboard. (it makes editting easier, and there is a wishlist for
this)
2) It doesn't work properly with UTF-8 passwords (there is a bug
report for this)
3) KPasswordDialog doesn't seem to be fixable without breaking binary
compatibility or doing messy things
I notice the careful attention to using a char* for storing the
password, but is this really necessary? I think QString is perfectly
acceptable - we have many other ways people can steal passwords from
memory anyway.

Does the operating system clean memory pages after they are freed or
before they are alloc'ed? If not, then an attacker could simply alloc
memory and search it for freed passwords. By using char* this threat
can be countered by zeroing the password before the memory is freed.

The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

Regards,
Ingo

[Brad Hards]

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

QCA 2 will have a QSecureArray, which uses mlock() if available, else uses
mmap to a file which is then unlinked, and overwritten on exit.
See
http://webcvs.kde.org/cgi-bin/cvsweb.cgi/kdesupport/qca/src/qca_tools.cpp?rev=1.12;content-type=text%2Fx-cvsweb-markup
for the implementation.

Brad

[Olivier Goffart]

Post by Brad Hards

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

QCA 2 will have a QSecureArray, which uses mlock() if available, else uses
mmap to a file which is then unlinked, and overwritten on exit.
See
http://webcvs.kde.org/cgi-bin/cvsweb.cgi/kdesupport/qca/src/qca_tools.cpp?r
ev=1.12;content-type=text%2Fx-cvsweb-markup for the implementation.

pinentry-qt has also a fork QString to create a secure QSecString

[Stefan Taferner]

Am Freitag, 5. November 2004 01:06 schrieb Ingo Klöcker:
[...]

Post by Ingo Klöcker

Post by George Staikos
I notice the careful attention to using a char* for storing the
password, but is this really necessary? I think QString is perfectly
acceptable - we have many other ways people can steal passwords from
memory anyway.

Does the operating system clean memory pages after they are freed or
before they are alloc'ed? If not, then an attacker could simply alloc
memory and search it for freed passwords. By using char* this threat
can be countered by zeroing the password before the memory is freed.

The operating system does not clear memory at all (for performance reasons).
Some memory allocation methods do this (calloc for example), but AFAIK
malloc not.

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

Thats the old discussion if you can trust root or not. Some years ago there
was some consens that you should not use these things on a computer where
you do not trust root. Simply because root can replace any program with
a version that logs your input.

--Stefan

[Waldo Bastian]

Post by Ingo Klöcker
Does the operating system clean memory pages after they are freed or
before they are alloc'ed?

The current process (malloc) can reuse non-clean memory pages, but other
processes will get clean pages.

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

Indeed. That said, before we use a password where it is needed we probably
make several copies at varies places. If you want to make sure that passwords
don't end up in swap you will need to trace their complete usage.

Cheers,
Waldo

[Stefan Taferner]

Post by Waldo Bastian

Post by Ingo Klöcker
Does the operating system clean memory pages after they are freed or
before they are alloc'ed?

The current process (malloc) can reuse non-clean memory pages, but other
processes will get clean pages.

Is this Linux specific?

--Stefan

[Oswald Buddenhagen]

Post by Stefan Taferner

Post by Waldo Bastian
The current process (malloc) can reuse non-clean memory pages, but other
processes will get clean pages.

Is this Linux specific?

it is specific to any sane system. what would all this permission crap
be good for, if every user could read his precedessor's data?

[George Staikos]

Post by Ingo Klöcker

Post by George Staikos
I notice the careful attention to using a char* for storing the
password, but is this really necessary? I think QString is perfectly
acceptable - we have many other ways people can steal passwords from
memory anyway.

Does the operating system clean memory pages after they are freed or
before they are alloc'ed? If not, then an attacker could simply alloc
memory and search it for freed passwords. By using char* this threat
can be countered by zeroing the password before the memory is freed.
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

That's cool, we can provide a mechanism to prevent people from stealing
passwords of out KPasswordDialog and instead force them to steal it from
whatever uses KPasswordDialog. :) Really..... I know the argument you're
making and I think it's rather pointless for this. If someone needs a
KSecuredButNotVeryUserFriendlyOri18nCompatiblePasswordDialog, they can use a
separate one or a fork of the existing one made more secure.

[Oswald Buddenhagen]

Post by George Staikos

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

That's cool, we can provide a mechanism to prevent people from stealing
passwords of out KPasswordDialog and instead force them to steal it from
whatever uses KPasswordDialog. :) Really..... I know the argument you're
making and I think it's rather pointless for this. If someone needs a
KSecuredButNotVeryUserFriendlyOri18nCompatiblePasswordDialog, they can use a
separate one or a fork of the existing one made more secure.

fwiw, what is your stance on
http://bugs.kde.org/show_bug.cgi?id=87580 :
KDE [KDM] does not mlock sensitive data (password)
?
imo this is a cantfix for the PAM case, and a wontfix for the other
cases.

[George Staikos]

Post by Oswald Buddenhagen

Post by George Staikos

Post by Ingo Klöcker
The other threat is that passwords are written to the swap partition.
This can only be countered by using mlock'ed char* memory. mlocking
QString is impossible (unless you or Qt writes QSecureString).

That's cool, we can provide a mechanism to prevent people from stealing
passwords of out KPasswordDialog and instead force them to steal it from
whatever uses KPasswordDialog. :) Really..... I know the argument
you're making and I think it's rather pointless for this. If someone
needs a KSecuredButNotVeryUserFriendlyOri18nCompatiblePasswordDialog,
they can use a separate one or a fork of the existing one made more
secure.

fwiw, what is your stance on
KDE [KDM] does not mlock sensitive data (password)
?
imo this is a cantfix for the PAM case, and a wontfix for the other
cases.

Funny, I read this report just yesterday and I thought the same thing as
you. It's even more funny to try to reconcile this with reports that people
want KDM to save the login password and send it through to KWallet after
login.

[Ingo Klöcker]

Post by George Staikos

Post by Ingo Klöcker
The other threat is that passwords are written to the swap
partition. This can only be countered by using mlock'ed char*
memory. mlocking QString is impossible (unless you or Qt writes
QSecureString).

That's cool, we can provide a mechanism to prevent people from
stealing passwords of out KPasswordDialog and instead force them to
steal it from whatever uses KPasswordDialog. :) Really..... I know
the argument you're making and I think it's rather pointless for
this. If someone needs a
KSecuredButNotVeryUserFriendlyOri18nCompatiblePasswordDialog, they
can use a separate one or a fork of the existing one made more
secure.

My argument is rather pointless? Well, your last sentence is completely
pointless. Why would a secure password dialog not be user friendly or
i18n compatible? Is this a law of nature or are you just making this
up? For one the user wouldn't even notice that the password dialog is
more secure (why should he notice that internally mlocked memory is
used?). Moreover there is apparently already a secure QString
replacement that could potentially be used and last time I checked
QString was i18n compatible.

Also your argument about forcing them to steal it from whatever uses
KPasswordDialog is wrong. If passwords can be stolen from
KPasswordDialog then that a single point to steal passwords from all
applications. Moreover not all applications are as careless about
passwords as KDE applications. Just take a look at the
gpg/gpg-agent/pinentry-* combo of GnuPG 2. But if you think it's okay
to make password handling in KDE even more insecure than it already is
then that's fine with me because unfortunately I don't have the time to
do anything against it. Still I'd prefer that somebody writes a
KSecureString that can/should then be used by all KDE applications if
they deal with passwords. Then all KDE apps would have secure password
handling by default. Isn't that what KDE core is about? Making life
easier for KDE developers? And you propose to leave such an important
issue to each individual developer?

Regards,
Ingo

[George Staikos]

Post by Ingo Klöcker
easier for KDE developers? And you propose to leave such an important
issue to each individual developer?

No, I leave it to the first developer with a need for it to implement it,
the second one to use it, and then for it to pass API review and move into
kdelibs in parallel or as a replacement, just as all such widgets do. For
now I don't see any security gain with the hacks the current one does, in
comparison to the i18n and functionality loss it results in.