> On May 23, 2016, 11:20 a.m., David Faure wrote: > > Please note that: > > * the Q_BYTE_ORDER change was reverted, since it made the wallet storage > > incompatible with previous releases. This needs further analysis in order > > to improve the code while retaining compat. > > * kwallet is definitely usable with a home install, I'm doing that right > > now. I do have the distro KF5 packages installed though, so maybe the > > polkit stuff got installed at the right place into the system as well. > > Anyhow it means it should be fixable with a one-time copy operation as root. > > Michael Pyne wrote: > That reminds me, I made an autotest for the Blowfish cipher that would > hopefully catch such errors in the future. I've committed the autotests now > (they pass, hopefully they don't break CI!). > > With that autotest I think that Allen Winter's last Q_BYTE_ORDER change > had actually been correct (to enable the relevant code sections when > Q_BYTE_ORDER == Q_LITTLE_ENDIAN), but his first commit (to #include > qglobal.h) would have generated incorrect Wallets which would then not match > with KWallet when it was fixed. > > Either way someone's systems are being broken here, because the current > code unilaterally enables bit-shuffling always no matter if the CPU is > big-endian or little-endian. But I don't have a big-endian machine to test on > to make sure a fix would (or would not) work.
This indeed shows that the current situation (kwallet 5.23) is broken on big endian platforms; looking at the failures we got when it was uploaded to Debian few days ago, https://buildd.debian.org/status/logs.php?pkg=kwallet-kf5&ver=5.23.0-1&suite=sid, we've got failures (related to this) on mips, powerpc, and hppa (s390x is not built yet, but it will fail there too) -- all of them are big endian. I can help in testing patches fixing the `blowfishtest` unit test on those platforms. > the current code unilaterally enables bit-shuffling always no matter if the > CPU is big-endian or little-endian. A simple fix could then be use QtGlobal again, but invert the byte order conditions like: -#if Q_BYTE_ORDER == Q_BIG_ENDIAN +#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN this would preserve the compatibility with little endian platforms, although breaking wallets on big endian machines. - Pino ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127833/#review95717 ----------------------------------------------------------- On May 8, 2016, 12:10 a.m., Michael Pyne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127833/ > ----------------------------------------------------------- > > (Updated May 8, 2016, 12:10 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kwallet > > > Description > ------- > > This is a collection of minor fixes: > > An uninit variable usage was noted by Coverity (CID 1289177) for a CBC crypto > function, though it only happens for encryption lengths that would not be hit > in practice. I troubleshot this in December but forgot to make a RR, but IIRC > the lengths that would cause problems are 7 bytes or less -- but it's still > better to fix. > > The other Coverity fix is to avoid a needless dup(2) of an opened socket > since it's immediately turned into a FILE* object anyways (CID 1353007). This > avoids a minor resource leak of a file descriptor. > > Finally, some of the ciphers use Qt checks for endianness, and need to > actually include the header that does this instead of relying on other parts > of the code incidentally pulling in the needed #includes. > > > Diffs > ----- > > src/runtime/kwalletd/backend/cbc.cc 4c13466 > src/runtime/kwalletd/main.cpp 90c60d8 > > Diff: https://git.reviewboard.kde.org/r/127833/diff/ > > > Testing > ------- > > Everything still compiles -- I'm limited in my ability to test since I'm > still using KDE4's KWallet (as the KF5 stuff seems to require polkit to > actually work, which isn't possible with a homedir install like mine). > > > Thanks, > > Michael Pyne > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
