Fix endianness issues once and for all in Blowfish algorithm
Needs ReviewPublic

Authored by awilcox on Jan 29 2017, 6:26 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Attempting to package KWallet 5.29.0 for PowerPC, I ran in to a test failure with the Blowfish algorithm. This led me to bug 362805. I have implemented the suggested fix from comment 17 there, with a small change; it is better to simply include <QtGlobal> than <QtCore/qglobal.h>.

Test Plan
  • Built on x86_64 computer with patch applied; opened existing wallet.
  • Built on PowerPC computer with patch applied; opened existing wallet.
  • Created new wallet on x86_64 computer, and opened it on PowerPC computer.

Diff Detail

Repository
R311 KWallet
Lint
Lint Skipped
Unit
Unit Tests Skipped
awilcox updated this revision to Diff 10678.Jan 29 2017, 6:26 PM
awilcox retitled this revision from to Fix endianness issues once and for all in Blowfish algorithm.
awilcox updated this object.
awilcox edited the test plan for this revision. (Show Details)
awilcox set the repository for this revision to R311 KWallet.
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 29 2017, 6:26 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
mpyne added a subscriber: mpyne.Jan 31 2017, 2:22 AM
mpyne added a comment.Jan 31 2017, 2:32 AM

When reviewing this, please see also the review chain at https://git.reviewboard.kde.org/r/127833/#review95717 , which had proposed removing the then-needless KWallet includes.

In essence the sticking point we have currently is that fixing the Blowfish implementation on big-endian machines will break the ability of users to open their wallets if their wallet was generated by the broken Blowfish implementation. That is the only reason the broken code remains -- the testcase was added so that we would catch breakages going into the future and prevent that bug from being introduced.

The best fix is to find a way to read files generated with the proper cipher and, on big-endian only, also if encoded with the broken cipher.

Poking around through the code this seems like it might be most easily possible in src/runtime/kwalletd/backend/backendpersisthandler.cpp, where it might be possible to retry the decryption with a byte-order-reversed key and see if it then succeeds (again, only on big-endian).