Version: 1.9.1 (using KDE KDE 3.5.2) Installed from: Gentoo Packages Compiler: gcc-3.4.6 OS: Linux (First of all: I do *not* use kwallet.) To reproduce this bug do the following: 1. Add a new POP3 account (the account doesn't have to be a real existing account - that's not needed for this test) and select "Store POP password"in the configuration dialog. Click OK. (1b. If kwallet pops up, cancel it, then confirm KMail's dialog box that it really should store the password in its own configuraton file instead of using kwallet.) 2. Check mail for the new account. A small popup window appears with an *un*checked checkbox. It should be checked since KMail already should know from the configuration dialog that it should store the password. That's annoying because if I forget to check this box KMail also turns off the configuration setting. Simple solution: Remove the checkbox from the popup window if "store password" is turned on in the configuration. If it's turned off there, I think it's no problem to keep it in the popup window. Christian
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;