| Summary: | Padding for blowfish encrytion is not done correctly | ||
|---|---|---|---|
| Product: | [Applications] konversation | Reporter: | lubyou <lgoodboi> |
| Component: | general | Assignee: | Travis McHenry <wordsizzle> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | wordsizzle |
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Ubuntu | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Attachments: |
proposed patch, note that i am not a c++ developer, so things might not confirm to your coding guidelines
append \0's for padding |
||
Created attachment 34792 [details]
proposed patch, note that i am not a c++ developer, so things might not confirm to your coding guidelines
Have you seen any actual problems with encryption for this? I looked into the Qt code and it adds \0 as far as I can tell. Please give me a reproducible situation where encryption problems arise. (discussed on irc) try this patch Created attachment 34793 [details]
append \0's for padding
Try this patch
SVN commit 988801 by tjmchenry: Implement DH1080 key exchange /keyx Add a combobox in the settings dialog to choose CBC or ECB (disabled when qca not present) Rework Cipher object to have one in every query and channel (needed for key exchange) ECB is now default, it's more commonly used and should help usability. Fix padding to add only \0 to the end and not random bytes BUG:197769 M +123 -23 cipher.cpp M +12 -2 cipher.h M +118 -72 config/connectionbehavior_config.ui M +5 -0 config/konversation.kcfg M +4 -0 config/settingsdialog.cpp M +5 -8 irc/channel.cpp M +13 -1 irc/channel.h M +34 -13 irc/inputfilter.cpp M +20 -1 irc/outputfilter.cpp M +1 -0 irc/outputfilter.h M +4 -0 irc/query.cpp M +14 -0 irc/query.h M +111 -12 irc/server.cpp M +10 -0 irc/server.h WebSVN link: http://websvn.kde.org/?view=rev&revision=988801 |
Version: (using KDE 4.2.90) OS: Linux Installed from: Ubuntu Packages Blowfish requires strings to be a multiple of eight. While this is done in konversation, the padding is not done correctly. QByteArray::resize is used to resize the array to a length which is multiple of eight, but it is padded with random values ("(..) byte array is extended to make it size bytes with the extra bytes added to the end. The new bytes are uninitialized.") This leads to different encrypted strings for the same plain text in ECB mode and it also makes it impossible for implementations from other clients to know where the plain text string actually ends. Padding should be done with \0, which is what the other implementations i have looked at do.