Bug 233104

Summary: Changes to KDE Wallet configuration does not require password
Product: kwalletmanager Reporter: Richard Hartmann <richih-kde>
Component: generalAssignee: Valentin Rusu <valir>
Status: RESOLVED FIXED    
Severity: critical CC: bartotten, chalkerx, faure, kumaran, lemma, mitchell, MurzNN, p1037905, security, valir
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 4.12

Description Richard Hartmann 2010-04-02 20:34:04 UTC
Version:           1.0 (using 4.4.2 (KDE 4.4.2), Debian packages)
Compiler:          cc
OS:                Linux (i686) release 2.6.31-1-686

Problem:

You can disable options like "Close when last application stops using it" _without_ having to provide a password.

This provides a direct attack vector for people with local, physical access. They can simply switch off this option (or the screen saver one) and wait for the victim to leave his/her pc. Changes to these security-relevant settings should require the wallet's password.
Comment 1 Kumaran Santhanam 2010-05-14 08:31:28 UTC
I would like to ask if somebody can please update this to be a severe bug instead of a lower priority item.  Security issues can seriously compromise the usage of kwallet.
Comment 2 Richard Hartmann 2010-05-14 10:03:14 UTC
Good point; I can raise it. I will need to figure out what severity is applicable as https://bugs.kde.org/page.cgi?id=fields.html#bug_severity does not list security-related classification.
Comment 3 Richard Hartmann 2010-05-14 10:12:22 UTC
CC'ing security@kde.org as per wstephenson's suggestion.
Comment 4 Jeff Mitchell 2010-05-14 12:50:42 UTC
(In reply to comment #0)
> the victim to leave his/her pc. Changes to these security-relevant settings
> should require the wallet's password.

The problem is that these are wallet *manager* settings. The manager can be managing multiple wallets -- so if you change the settings, which wallet's password do you ask for?

Most likely this should instead require the currently-logged-in-user's password.

I've marked this as critical, and CCed the currently-listed maintainer. Michael, can you work on this?
Comment 5 Michael Leupold 2010-05-14 13:08:46 UTC
In order to make the settings more secure the following needs to be done:
- store settings for closing the wallet inside the kwl file
- store acl settings inside the kwl file

I'm not really sure if I can work on it. Unfortunately my KDE time is currently pretty limited so I have to choose carefully what I want to work on in order to get anything finished. I'm currently spending most of my time on replacing kwallet with something that supports the fd.o spec, has more flexibility and fixes many of the security concerns.

I don't think preventing attacks as raised in this bug, #171608 or #208371 was one of the goals of the original kwallet. That's why plumbing a fix for those bugs is often more work than it might initially seem.
Comment 6 Jeff Mitchell 2010-05-14 19:06:56 UTC
Michael,

Since right now settings are global, not per-wallet (unless the wallet manager tray icon is showing settings for the default wallet as opposed to all wallets) a "quick fix" would be to require the current user's password to change these settings, similar to how some of the systemsettings modules work. Is this something that would be doable?

I agree with your assessment of the problems/threats, but it does seem in this case like there may be a quick fix, unless I am mistaking something.
Comment 7 Richard Hartmann 2010-05-14 19:51:15 UTC
I agree. Having to provide the user password would be better than nothing.
Comment 8 Kumaran Santhanam 2010-05-14 20:10:17 UTC
I think that for any kwalletmanager settings, asking for the user's password is a reasonable quick fix.  Please keep in mind that root can simply change the password, but at least the intrusion would then be detectable.
Comment 9 Richard Hartmann 2010-05-14 20:17:24 UTC
It would not, as root can easily change /etc/shadow and reset atime, as well.

As per https://bugs.kde.org/show_bug.cgi?id=171608 , local root will always trump kwallet, no matter what we do.
Comment 10 Kumaran Santhanam 2010-05-14 22:37:12 UTC
LDAP or Kerberos authentication would be more difficult to circumvent, since those might be under the control of a different administrator.  However, authconfig can be changed so root would still have access through local authentication.  Ultimately, it comes down to how many steps we require root to take before the wallet is compromised.
Comment 11 Richard Hartmann 2010-05-17 09:48:39 UTC
This is a good point. The magic words here would probably be "criminal intent" and "raising of the bar".
Comment 12 BartOtten 2011-10-12 00:04:18 UTC
And now what? *bump*
Comment 13 Jeff Mitchell 2011-10-12 11:16:09 UTC
I believe we are now waiting for someone with the time, knowledge, and initiative to fix it.
Comment 14 David Faure 2011-10-16 10:43:41 UTC
I guess this requires code that uses kcheckpass, similar to krunner/lock/lockprocess.cc
Comment 15 Nikita Skovoroda 2012-01-19 16:51:18 UTC
Mm. One possible solution:

1) Store the critical settings (for example, «keep open» flag) per-wallet, in the wallet file. Not one global setting, as it is now.
2) Sign the wallet file (if not done already), using the password.
3) Do now allow to open incorrectly signed wallets with kwallet.

So changing that setting would require opening the wallet.

This method would depend on kwalletmanager not being modified by the attacker (because of #3), but if someone could modify kwalletmanager (for example, local root), he can also just store everything the user types in the password prompt. Local root can even just log all the keypresses, nothing could be done here.
Comment 16 Nikita Skovoroda 2012-01-19 17:04:40 UTC
Actually, IMHO, all the settings, except default wallet and gui settings, should be stored per-wallet, and should be signed.

Most important are «access control» and the «keep open / when to close» blocks.

Access control not being signed is also a security issue.

Am i right that as it is now, any application could change the access control settings (add itself to [Auto Allow] in ~/.kde4/share/config/kwalletrc)?
Comment 17 Jeff Mitchell 2012-01-21 18:05:37 UTC
I don't know for sure, but I think many of these issues might be moot with the introduction of KSecretService in 4.8.
Comment 18 Nikita Skovoroda 2012-01-21 19:23:17 UTC
> I don't know for sure, but I think many of these issues might be moot with the introduction of KSecretService in 4.8.

1) Does this mean that kwallet would be deprecated soon, and excluded from KDE SC? If yes — good, if no — kwallet still should be fixed.

2) Does this mean that ksecrets manages this in a right way?
Comment 19 Nikita Skovoroda 2012-08-30 10:38:48 UTC
Still no info about this?
Kde 4.8 was released long ago, kde 4.9 was also released.
Still i didn't see KSecretService in action, tbh.
And kwallet is still not fixed.
Comment 20 Jeff Mitchell 2012-08-30 12:09:02 UTC
It appears that nobody has had the time and/or desire to to so. It's an open-source project...if this is your itch to scratch, feel free to get involved...
Comment 21 Murz 2013-01-14 07:09:55 UTC
What is current status of KSecretService development? Did we have any news? Any chance that KSecretService will be added to KDE 4.11 Feature Plan?
Comment 22 Valentin Rusu 2013-09-04 12:39:40 UTC
@Murz: KSecretService will eventually be a KF5-based release feature. However, I think that kwallet is a good point to start and I'll continue to better it, so that KDE4 users continue to benefit it.

Taking over the list of kwallet issue I see two kinds of users :
- some do not want to re-enter passwords when applications are accessing wallets,
- some others request that kwallet systematically enter password when applications open wallets.

These two requirements are opposite. Leaving the wallet open for the entire session - to satisfy first user category - will make the other say that the wallet is not secure enough.

There are other cases where leaving your computer unattended may lead to abuse. Do you guys are using ssh-agent to store ssh remote keys? That would allow attackers access remote computers on your behalf.

I think that the wallet system needs PAM integration and when doing sensible changes, the current user password should be prompted. And event in that case, some users may to have the option to disable that, to prevent too many password prompts.
Comment 23 Valentin Rusu 2013-09-21 08:31:32 UTC
Git commit 717f925b77f13c54e92ecd81ea92487f933a1915 by Valentin Rusu.
Committed on 09/09/2013 at 14:17.
Pushed by vrusu into branch 'master'.

Require user's password when changing configuration

The KCM now uses KAuth in order to get user's password when attempting
configuration changes.

M  +10   -0    src/konfigurator/CMakeLists.txt
M  +27   -0    src/konfigurator/konfigurator.cpp
A  +9    -0    src/konfigurator/kwallet.actions
A  +15   -0    src/konfigurator/savehelper.cpp     [License: UNKNOWN]  *
A  +15   -0    src/konfigurator/savehelper.h     [License: UNKNOWN]  *

The files marked with a * at the end have a non valid license. Please read: http://techbase.kde.org/Policies/Licensing_Policy and use the headers which are listed at that page.


http://commits.kde.org/kwallet/717f925b77f13c54e92ecd81ea92487f933a1915
Comment 24 Murz 2013-10-21 08:09:47 UTC
*** This bug has been confirmed by popular vote. ***