Bug 131516

There's another problem related to this bug:

When I close KMail, it wants to store the password.

That's ok, I told KMail to do so.

But ... *everytime* when I close KMail, it prompts me again
if I want to install KWallet. I then cancel the KWallet
installation because I don't want to.

After that KMail *always* asks, if I really want KMail to store
the password in its own configuration file. I have to confirm that
after every single KMail session.

Christian

If someone doesn't want to use kwallet at all, there's a workaround:

Security & Privacy -> KDE Wallet -> *DIS*able the KDE wallet subsystem.

The original bug is fixed now, not by removing the checkbox, but by setting the correct value to the checkbox.

Fixed by:

>SVN commit 673666 by tmcguire:

>Fix 2 issues with the password dialog:
>- The 'keep password' checkbox was always hidden
>- The 'keep password' checkbox wasn't initalized with the correct value

>Now KMail can save passwords again.

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;