KWallet git version from 20160502T210733 commit 15e6feb works fine and opens kdewallet (only wallet). KWallet git version from 20160507T105330 commit b3a95ba can't open the same wallet using same (and surely correct) password with error code -9. Reproducible: Always
Same here, kwalletmanager ( 4x work perfect ) my kwallet has no password kwalletmanager5 fail with error -9 related system log May 08 14:02:50 dbus[2111]: [system] Activating service name='org.kde.kcontrol.kcmkwallet5' (using servicehelper) May 08 14:02:50 org.kde.kcontrol.kcmkwallet5[5266]: QDBusArgument: read from a write-only object May 08 14:02:50 org.kde.kcontrol.kcmkwallet5[5266]: QDBusArgument: read from a write-only object May 08 14:02:50 org.kde.kcontrol.kcmkwallet5[5266]: QDBusArgument: read from a write-only object May 08 14:02:50 dbus[2111]: [system] Successfully activated service 'org.kde.kcontrol.kcmkwallet5' May 08 14:03:05 polkitd[2257]: Operator of unix-session:1 successfully authenticated as unix-user:bruno to gain TEMPORARY authorization for action org.kde.kcontrol.kcmkwallet5.save for system-bus-name::1.104 [/usr/bin/kcmshell5 kwalletconfig5] (owned by unix-user:bruno) May 08 14:03:35 dbus[2111]: [system] Activating service name='org.kde.kcontrol.kcmkwallet5' (using servicehelper) May 08 14:03:35 org.kde.kcontrol.kcmkwallet5[5317]: QDBusArgument: read from a write-only object May 08 14:03:35 org.kde.kcontrol.kcmkwallet5[5317]: QDBusArgument: read from a write-only object May 08 14:03:35 org.kde.kcontrol.kcmkwallet5[5317]: QDBusArgument: read from a write-only object May 08 14:03:35 dbus[2111]: [system] Successfully activated service 'org.kde.kcontrol.kcmkwallet5' May 08 14:03:39 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:03:39 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:03:39 org.kde.kwalletd5[2919]: fsize: 145769968 encrypted.size(): 141760 blksz: 8 May 08 14:03:46 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:03:46 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:03:46 org.kde.kwalletd5[2919]: fsize: 1568022951 encrypted.size(): 141760 blksz: 8 May 08 14:03:51 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:03:51 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:03:51 org.kde.kwalletd5[2919]: fsize: 3415025526 encrypted.size(): 141760 blksz: 8 May 08 14:03:52 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:03:52 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:03:52 org.kde.kwalletd5[2919]: fsize: 145769968 encrypted.size(): 141760 blksz: 8 May 08 14:03:54 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:03:54 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:03:54 org.kde.kwalletd5[2919]: fsize: 145769968 encrypted.size(): 141760 blksz: 8 May 08 14:03:54 org.kde.kwalletd5[2919]: QXcbConnection: XCB error: 3 (BadWindow), sequence: 3128, resource id: 35651792, major code: 40 (TranslateCoords), minor code: 0 May 08 14:05:40 org.kde.kwalletd5[2919]: Setting useNewHash to true May 08 14:05:40 org.kde.kwalletd5[2919]: Wallet new enough, using new hash May 08 14:05:40 org.kde.kwalletd5[2919]: fsize: 145769968 encrypted.size(): 141760 blksz: 8
Valentin, if you are like me using kua/kuf repository you can still downgrade all wallet stuf to what oss propose. It will allow you at least be able to work.
Allen, can you check what happened with your last commit? Note that it also ended up in 5.22.0 tar.
Please compare also bug 344162, we had similar situation there.
are the problems happening on big endian or little endian systems? you guys are using blowfish? interestingly, if I remove the include <qglobal.h> #if Q_BYTE_ORDER == Q_BIG_ENDIAN gives a warning but evaluates to true, so on a little endian system like mine the big endian code will be compiled. if I put back the include <qglobal.h> #if Q_BYTE_ORDER == Q_BIG_ENDIAN evaluates to false on my system, as it should. so... seems to be me we need to include <qglobal.h> but reverse the conditional Q_LITTLE_ENDIAN I wish we had a make test in kwallet
yep. if you look at Bruce Schneier's blowfish.c you'll see code like this: #ifdef little_endian data = ((data & 0xFF000000) >> 24) | ((data & 0x00FF0000) >> 8) | ((data & 0x0000FF00) << 8) | ((data & 0x000000FF) << 24); #endif which is what we do with the shuffle macro.
Those are normal x86_64 systems, so little endian, I'd say.
Created attachment 98845 [details] Patch to fix blowfish algorithm
I just attached a patch. Can anyone test it?
if you do test the patch, please remember to apply the patch to the un-reverted code. in other works, make sure you have #include <QtCore/qglobal.h>
Patch works for me. Thanks!
Git commit 87e774825b779ba846315a8b2ffe6479dd9f9814 by Allen Winter. Committed on 08/05/2016 at 21:24. Pushed by winterz into branch 'master'. kwalletd/backend/blowfish.cc -fix Q_BYTE_ORDER for little endian M +5 -5 src/runtime/kwalletd/backend/blowfish.cc http://commits.kde.org/kwallet-framework/87e774825b779ba846315a8b2ffe6479dd9f9814
Hello Allen , well I cannot see how this can work .. from my tests this breaks kwallet <=5.21.0. meaning 5.21.0 -> master is broken always gives you a -9 error master setup and back to 5.21.0 is broken gives always -9 error The Q_BYTE_ORDER == .. in code was right just br0ken by missing header files on <=5.21.0 so I cannot see how this can be fixed to keep compatibility..
Allen , this commit : https://quickgit.kde.org/?p=kwallet.git&a=commitdiff&h=0d56c68d7a2204a987a5255096d004d5a696c0e5&hp=87e774825b779ba846315a8b2ffe6479dd9f9814 is not really right.. It does not matter how close one will look at it right now you get BIG ENDIAN algo on LITTLE ENDIAN becuase you are missing to include the header file.. the build has now again lots : warning: "Q_BIG_ENDIAN" is not defined [-Wundef] #if Q_BYTE_ORDER == Q_BIG_ENDIAN ^
that's exactly what I was trying to fix that got us into this mess in the first place. The code is now rolled-back to what it was before I messed with it. well, except for a new comment.
yes , now is again broken in the way it was broken before your first fix. ( and this one was right ) I don't think there is a way to fix it and remain compatible with <=5.21.0 .. however you guys have to decide the right 'point' to break older installs .. Best Regards
With the addtion of the test case in: https://quickgit.kde.org/?p=kwallet.git&a=commitdiff&h=7e7644b608e22a13e110284ed6c5426c1b493b43 It's clear now that the blowfish backend doesn't work in big endian, as shown in the buildds logs for mips and powerpc in Debian: https://buildd.debian.org/status/package.php?p=kwallet-kf5 Adding back the include: #include <QtCore/qglobal.h> and changing the checks for #if Q_BYTE_ORDER == Q_LITTLE_ENDIAN (effectively reverting the blowfish backend to the state in https://quickgit.kde.org/?p=kwallet.git&a=blob&h=e09d5422dfb404f2f15e74d693b24bb6cd8e1689&hb=87e774825b779ba846315a8b2ffe6479dd9f9814&f=src%2Fruntime%2Fkwalletd%2Fbackend%2Fblowfish.cc) makes the test pass in little and big endian machines. This changes nothing for little endian machines. And in big endian machines it should fix the blowfish backend. Happy hacking,
Maximiliano Curia: I'm confused. Are you saying that you have a fix that unbreaks big endian systems without breaking little endian systems? That sounds good. Can you attach an actual patch? Tests passing is good, but being able to open existing wallets is even more important. Would the change still support existing wallets created on big endian systems and on little endian systems?
Michael Pyne's comment https://phabricator.kde.org/D4335 says otherwise. I'm afraid we have to keep the current mess for compat reasons. Some distros (debian, neon, probably opensuse?) have applied the patch that tests for little endian [1]. Others (arch, upstream KDE) are doing 0==0 in that #if (or 1==1 with my recent commit to silence -Wundef). In both cases we go into those #if, on little endian machines. I believe the original intent (see the old code at [2]) was to NOT go into those #if (which were for big endian) but that ship has sailed. Changing that would break all wallets. On big endian, though, IIUC we have the choice between broken-unittest (without the patch) and broken-for-existing-users (with the patch). No good solution :( [1] https://packaging.neon.kde.org/kde/kwallet.git/tree/debian/patches/blowfish_endianess.diff?h=Neon/unstable&id=572447bcb13e5f36247b3872a60d2959a3f82e12 [2] https://cgit.kde.org/kde-runtime.git/tree/kwalletd/backend/blowfish.cc?id=a6c3e399e8674914ccde5d80710de866cbb700da Please correct me if I'm wrong in any of this. What a mess!