Bug 95615

CVS commit by vkrause: 

Fix password storage in configuration file.
The user will now be asked once per account if [s]he would like to store the password in the (unsafe) configuration file instead of KWallet (no more message box spam) and - most important - the password is now really stored.

Reviewed by Ingo Kloecker.

BUG: 92932, 93789
CCBUG: 93183, 95615


  M +22 -2     accountdialog.cpp   1.159
  M +52 -22    kmtransport.cpp   1.47
  M +1 -1      kmtransport.h   1.13
  M +41 -12    networkaccount.cpp   1.15
  M +1 -1      networkaccount.h   1.9

Could you please test with current CVS HEAD (3.4beta1 is too old) if the problem still occurs?

Problem is still occurring, but is slightly diffrent. 

1. It requests a password for kWallet, which I cancel
2. Informs me it can't access KWallet probably because of a bad password press ok.
3. Asks whether to store password in configuration, I press store password.

This process, is repeated evertime I quit KMail, I also logged out, and back in just in case, and still I get these requests.

So you have KWallet enabled globally but still want to store your mail 
passwords in the configuration file?
This is currently not supported, KMail tries to migrate your passwords from 
the configuration to the wallet if it is enabled. This happens on startup, 
thus it is repeated every time.

I do intend to use KWallet myself, but this is going to cause problems..

The fact that KMail works as normal, its only when you quit that it ask, instead of at the beginning, which would mark more sense. Maybe a welcome to a new version of KMail, you have KWallet enabled, so we're moving the password.

The situation now seems silly, especially as KMail works as people expect, It then asks for a password AFTER you already used it, the gives you an option that is meaningless, since it doesn't work. If theres really no option than don't ask the question?

As I already stated, I do intend to use KWallet, but when this goes live, normal users are going to be complaining that KMail is broken. If KWallet is global then make it mandatory for KMail?

On Wednesday 12 January 2005 06:57, Leon Pennington wrote:
> The situation now seems silly, especially as KMail works as people expect,
> It then asks for a password AFTER you already used it, the gives you an
> option that is meaningless, since it doesn't work. If theres really no
> option than don't ask the question?

KMail tries to prevent unnecessary wallet openings and thus defers it as long 
as possible. In case of the migration the wallet is only needed to save the 
password (which happens during shutdown). We could of course move the saving 
to the startup, but I'm not sure if this really matters.
During normal operation (ie. after the initial migration) the wallet will of 
course be opened on the beginning.

Just to clarify the problem: Is this just about an unexpected/unintuitive 
behavior during the migration to KWallet or is there really something not 
working functionality-wise?

Well, something is messed up. I have CVS as of 10 jan 2005, kmail asks my IMAP/SSL password on startup and asks for the wallet password on shutdown.

This does not seem migration related.

>Just to clarify the problem: Is this just about an unexpected/unintuitive 
>behavior during the migration to KWallet or is there really something not 
>working functionality-wise? 

unexpected/unintuitive it is, but its also giving you options that don't work, and continues to ask the questions everytime, I'd call that a functional bug. 

to #7: this is unrelated to the migration, KMail is either unable to write the 
password to the wallet or unable to read it back again. Could you have a look 
into the wallet and see if it has been written there? What versions of KMail 
and kdelibs are you using?

to #8: Sorry, but there is still some misunderstanding on my side: What 
'options that don't work' do you mean?
In comment #3 you say that you cancel the wallet password dialog (which would 
of course explain the behaviour you are seeing), but does it work as expected 
if you open the wallet?

Do I understand this right, it is no longer possible to tell kmail to store the password, everytime I want to check for mail, I'm asked for the password of kwallet (when kwallet is installed)?
Is this the intended behaviour?
If this is true then I'll have to hack kmail, because this is absolutly anoying.

> to #8: Sorry, but there is still some misunderstanding on my side: What
>'options that don't work' do you mean?

It asks do you want to store the password in the configuration. This doesn't work. If your offering options they have to work, or its broken.

> In comment #3 you say that you cancel the wallet password dialog (which
> would of course explain the behaviour you are seeing), but does it work as
> expected  if you open the wallet? 

It worked as expected the last time I did it.

OK, so here's what's happening:
. KDE boots
. Kontact starts
. It doesn't open the wallet, and asks me for my dIMAP password.
. I open the wallet, and check the existance of my password. Not there.
. I quit kontact, it opens the wallet (asking me for my password). The password is stored (I checked).
. I start again (the wallet is open now), and it doesn't ask me for my password.
. I restart.
. GOTO 1.

Um yeah, in the above: kontact doesn't actually ask me for the wallet password in this particular scenario, since it's already open.

Also: lib/base from yesterday, pim from the 12th.

Additional to my last comment. Now KMail is supposed to use KWallet, I had left KMail in the systray. Once I've restarted, I get a request not for KWallet but for the server. So this is also broken. 

Should I report as a separate bug?

to #10: No, KMail caches the passwords, thus you are only asked once for the 
wallet password.

to #11: Ah, I think I finally understand the problem. The password is sotred 
in the configuration until the next start where KMail tries to migrate it to 
KWallet. Since this does not work when the wallet password prompt is 
cancelled, you are asked again...
This only happens in case KWallet is enabled globally but the user tries to 
store KMail passwords in the configuration file. I'm not really sure what we 
should do in this case, IMHO a config option for this is unnecessary. Maybe 
adding a 'Do not ask again' option to the 'store in config file' message box 
would be a compromise.

to #12-#14: This seems to be a similar problem as #97324. This seems to only 
happen if KMail is not closed normally (File->Quit), but by session 
management, crash, etc.. I'll see what I can do about this.

I am still seeing this problem in 3.4.0-final, but only once (after first startup of Kontact).

> "So you have KWallet enabled globally but still want to store your mail 
passwords in the configuration file?" 

I would like to - yes, for one reason: I don't want to be asked for some password when I do something with my mail program. But I want to use kwallet to store other passwords for internet sites. When it is absolutely necessary for kmail to store the passwords in a wallet, maybe it can use a separate wallet without a master password...? When I try to use two different wallets (for local and internet passwords) kmail writes to the wrong (IMHO should write to local, not to internet) and (!) cannot find the account passwords it wrote to the wallet so asks every time I quit kmail to store in the config file which does not work.

Please see Bug 97925. I have added a patch to kdepim-3.4.1 that may solve the problem of repeated asking when saving to config file.

Because this is still present in KDE 3.5.7, I would like to make a couple suggestions. As far as I can tell, there are currently five possible ways to use KMail:

1) No KWallet, no KMail password storage
This scenario forces the user to enter a password for each account every time they open KMail.  Not exactly user-friendly, especially if they have 3-5 email accounts (as I do).

2) KMail password storage, KWallet disabled
In this scenario, the user stores the passwords locally within KMail itself.  However, the user must disable the entire KWallet system in order for this to work without constant nag screens. Again, not very user-friendly, since KWallet has a lot more to offer than simply storing email account passwords.

3) KMail passwords stored in KWallet with a master password
The KMail account passwords are stored within KWallet, forcing the user to enter the master Kwallet password each time they open KMail, assuming the wallet was not already opened.  Again, not very user-friendly. 

4) KMail passwords stored in KWallet with no master password
Sadly, this seems to be the advice of many forum posts.  In this scenario, not only are the KMail passwords completely vulnerable, but so are all the other passwords stored in the wallet.

5) Avoid using KMail entirely until this 2 year old bug is finally addressed
I am afraid that many who encounter the above annoyances will likely choose this option.  Having been a Thunderbird user for almost 3 years, I decided to migrate to KMail (Kontact actually) because of the excellent calendar integration.  However, I am finding this constant prompting of passwords too much.

I propose any one of the following solutions:

1) Make it possible for KMail to store passwords locally with and without KWallet system integration enabled.

2) Make it possible for KMail to reference a separate wallet that may be set with a blank password.  Although the KMail passwords would be insecure, the rest of the KWallet system integration would remain in tact.

3) Make an option for Kwallet to integrate into the user's logon password.  This way, the default wallet is authenticated automatically when a user logs into the KDE environment.  Say for example, if someone created a wallet with the same password as their logon credentials, KDE could pass that information onto KWallet.  This solution is perfect for folks who use KWallet for lesser items, such as wireless network passwords and email passwords.  It provides a transparent encryption as long as the user is logged in.  Then, for items requiring further security, the user can create a separate wallet with a stronger password or better yet, use a standalone program like PwManager.

This of course is all just speculation.  I'm no programmer so I have no idea how feasible it would be to implement any of these, but that's the way I see it.

KMail is a great email client and I want to stick with it, but if I can't find a sensible workaround for such a simple feature, I'll have to leave it behind.

Re #19: In Bug 97925 you'll find a patch that, once applied, will implement your sollution 1
You'll have to patch kdepim sources and rebuild, though.

SVN commit 674135 by tmcguire:

More password storing fixes:
- Don't try to migrate to the wallet once the user has decided not to use it.
  Now KMail doesn't ask to migrate the password when exiting.
- If the user chooses not to store the password at all, honor that
  and don't ask him again to store it.


BUG: 95615
CCBUG: 131516

 M  +51 -31    networkaccount.cpp  


--- trunk/KDE/kdepim/kmail/networkaccount.cpp #674134:674135
@@ -120,6 +120,8 @@
     if( mStorePasswd != store && store )
       mPasswdDirty = true;
     mStorePasswd = store;
+    if ( !store )
+      mStorePasswdInConfig = false;
   }
 
   void NetworkAccount::setHost( const QString & host ) {
@@ -163,16 +165,8 @@
 
       if ( !encpasswd.isEmpty() ) {
         setPasswd( KStringHandler::obscure( encpasswd ), true );
-        // migrate to KWallet if available
-        if ( Wallet::isEnabled() ) {
-          config.deleteEntry( "pass" );
-          config.deleteEntry( "passwd" );
-          mPasswdDirty = true;
-          mStorePasswdInConfig = false;
-        } else {
-          mPasswdDirty = false; // set by setPasswd() on first read
-          mStorePasswdInConfig = true;
-        }
+        mPasswdDirty = false; // set by setPasswd() on first read
+        mStorePasswdInConfig = true;
       } else {
         // read password if wallet is already open, otherwise defer to on-demand loading
         if ( Wallet::isOpen( Wallet::NetworkWallet() ) )
@@ -199,37 +193,56 @@
   void NetworkAccount::writeConfig( KConfigGroup & config ) {
     KMAccount::writeConfig( config );
 
-    config.writeEntry( "login", login() );
-    config.writeEntry( "store-passwd", storePasswd() );
-
     if ( storePasswd() ) {
       // write password to the wallet if possible and necessary
       bool passwdStored = false;
-      Wallet *wallet = kmkernel->wallet();
-      if ( mPasswdDirty ) {
-        if ( wallet && wallet->writePassword( "account-" + QString::number(mId), passwd() ) == 0 ) {
+
+      //If the password should be written to the wallet, do that
+      if ( !mStorePasswdInConfig ) {
+        Wallet *wallet = kmkernel->wallet();
+
+        //If the password is dirty, try to store it in the wallet
+        if ( mPasswdDirty ) {
+          if ( wallet && wallet->writePassword( "account-" + QString::number(mId), passwd() ) == 0 )
+            passwdStored = true;
+        }
+
+        //If the password isn't dirty, it is already stored in the wallet.
+        else if ( wallet )
           passwdStored = true;
+
+        //If the password is stored in the wallet, it isn't dirty or stored in the config
+        if ( passwdStored ) {
           mPasswdDirty = false;
           mStorePasswdInConfig = false;
         }
-      } else {
-        passwdStored = wallet ? !mStorePasswdInConfig /*already in the wallet*/ : config.hasKey("pass");
       }
+      else
+        passwdStored = config.hasKey("pass");
+
       // if wallet is not available, write to config file, since the account
       // manager deletes this group, we need to write it always
-      if ( !passwdStored && ( mStorePasswdInConfig || KMessageBox::warningYesNo( 0,
-           i18n("KWallet is not available. It is strongly recommended to use "
-                "KWallet for managing your passwords.\n"
-                "However, KMail can store the password in its configuration "
-                "file instead. The password is stored in an obfuscated format, "
-                "but should not be considered secure from decryption efforts "
-                "if access to the configuration file is obtained.\n"
-                "Do you want to store the password for account '%1' in the "
-                "configuration file?", name() ),
-           i18n("KWallet Not Available"),
-           KGuiItem( i18n("Store Password") ),
-           KGuiItem( i18n("Do Not Store Password") ) )
-           == KMessageBox::Yes ) ) {
+      bool writeInConfigNow = !passwdStored && mStorePasswdInConfig;
+      if ( !passwdStored && !mStorePasswdInConfig ) {
+        int answer = KMessageBox::warningYesNo( 0,
+            i18n("KWallet is not available. It is strongly recommended to use "
+                 "KWallet for managing your passwords.\n"
+                 "However, KMail can store the password in its configuration "
+                 "file instead. The password is stored in an obfuscated format, "
+                 "but should not be considered secure from decryption efforts "
+                 "if access to the configuration file is obtained.\n"
+                 "Do you want to store the password for account '%1' in the "
+                 "configuration file?", name() ),
+            i18n("KWallet Not Available"),
+            KGuiItem( i18n("Store Password") ),
+            KGuiItem( i18n("Do Not Store Password") ) );
+        if (answer == KMessageBox::Yes)
+          writeInConfigNow = true;
+        if (answer == KMessageBox::No)
+          mStorePasswd = false;
+      }
+
+      if ( writeInConfigNow ) {
         config.writeEntry( "pass", KStringHandler::obscure( passwd() ) );
         mStorePasswdInConfig = true;
       }
@@ -243,6 +256,13 @@
         wallet->removeEntry( "account-" + QString::number(mId) );
     }
 
+    // delete password from config file if it is stored in the wallet or
+    // not stored at all
+    if ( !mStorePasswdInConfig )
+      config.deleteEntry( "pass" );
+
+    config.writeEntry( "store-passwd", storePasswd() );
+    config.writeEntry( "login", login() );
     config.writeEntry( "host", host() );
     config.writeEntry( "port", static_cast<unsigned int>( port() ) );
     config.writeEntry( "auth", auth() );

SVN commit 675974 by tmcguire:

Always try the wallet again when the user changes his password and has it stored in the config.
See the comment for the reasons.
CCBUGS: 95615,131516


 M  +11 -0     networkaccount.cpp  
 M  +1 -1      networkaccount.h  


--- trunk/KDE/kdepim/kmail/networkaccount.cpp #675973:675974
@@ -165,6 +165,7 @@
 
       if ( !encpasswd.isEmpty() ) {
         setPasswd( KStringHandler::obscure( encpasswd ), true );
+        mOldPassKey = encpasswd;
         mPasswdDirty = false; // set by setPasswd() on first read
         mStorePasswdInConfig = true;
       } else {
@@ -197,6 +198,15 @@
       // write password to the wallet if possible and necessary
       bool passwdStored = false;
 
+      //If the password is different from the one stored in the config,
+      //try to store the new password in the wallet again.
+      //This ensures a malicious user can't just write a dummy pass key in the
+      //config, which would get overwritten by the real password and therefore
+      //leak out of the more secure wallet.
+      if ( mStorePasswdInConfig &&
+           KStringHandler::obscure( mOldPassKey ) != passwd() )
+        mStorePasswdInConfig = false;
+
       //If the password should be written to the wallet, do that
       if ( !mStorePasswdInConfig ) {
         Wallet *wallet = kmkernel->wallet();
@@ -244,6 +254,7 @@
 
       if ( writeInConfigNow ) {
         config.writeEntry( "pass", KStringHandler::obscure( passwd() ) );
+        mOldPassKey = KStringHandler::obscure( passwd() );
         mStorePasswdInConfig = true;
       }
     }
--- trunk/KDE/kdepim/kmail/networkaccount.h #675973:675974
@@ -129,7 +129,7 @@
   protected:
     KMail::SieveConfig mSieveConfig;
     KIO::Slave * mSlave;
-    QString mLogin, mPasswd, mAuth, mHost;
+    QString mLogin, mPasswd, mAuth, mHost, mOldPassKey;
     unsigned short int mPort;
     bool mStorePasswd : 1;
     bool mUseSSL : 1;

In version kdepim3-3.5.9-57.1 Kmail still wants to open KWallet even if I choose  not to use KWallet any more and even if I don't change any passwords. The only solution to avoid using KWallet is to use Miguel Angel's patch and recompile it whenever a new version is out. Am I the only one who has this very annoying problem?