Add patch for CVE-2013-7252 (CBC encryption handling bug).

MFH:		2015Q1
Security:	7a8a74d1-9c34-11e4-a40b-5453ed2e2b49
This commit is contained in:
Raphael Kubo da Costa 2015-01-14 21:57:39 +00:00
parent a1819b117a
commit 627f9b8db8
Notes: svn2git 2021-03-31 03:12:20 +00:00
svn path=/head/; revision=377054
2 changed files with 759 additions and 1 deletions

View file

@ -2,7 +2,7 @@
PORTNAME= kde-runtime
PORTVERSION= ${KDE4_VERSION}
PORTREVISION= 2
PORTREVISION= 3
CATEGORIES= x11 kde
MASTER_SITES= KDE/${KDE4_BRANCH}/${PORTVERSION}/src
DIST_SUBDIR= KDE/${PORTVERSION}

View file

@ -0,0 +1,758 @@
commit 466859302095a4ee4a9214ab24144eab7697224f
Author: Valentin Rusu <kde@rusu.info>
Date: Mon Jan 5 23:00:58 2015 +0100
Backporting CBC algorithm fix from frameworks/kwallet
The CBC algorithm was not enrcrypting in CBC but in ECB, despite it being
named CBC. This problem was found by Itay Duvdevani who let us know it on the
security mailing list. His mail eventually got forwarded to me and here is the
fix. I also fixed the test program which was incorrectly checking the
expected bytes.
This commit corresponds to the following commits from:
frameworks/kwallet
88bc1ff01e5fdf59a13fe012aa03e31e9eb8a3b1
6e588d795e6631c3c9d84d85fd3884a159b45849
--- kwalletd/backend/backendpersisthandler.cpp
+++ kwalletd/backend/backendpersisthandler.cpp
@@ -40,14 +40,15 @@
#include "sha1.h"
#include "cbc.h"
-#ifdef Q_OS_WIN
+#ifdef Q_OS_WIN
#include <windows.h>
#include <wincrypt.h>
#endif
-#define KWALLET_CIPHER_BLOWFISH_CBC 0
+#define KWALLET_CIPHER_BLOWFISH_ECB 0 // this was the old KWALLET_CIPHER_BLOWFISH_CBC
#define KWALLET_CIPHER_3DES_CBC 1 // unsupported
#define KWALLET_CIPHER_GPG 2
+#define KWALLET_CIPHER_BLOWFISH_CBC 3
#define KWALLET_HASH_SHA1 0
#define KWALLET_HASH_MD5 1 // unsupported
@@ -164,13 +165,18 @@ BackendPersistHandler *BackendPersistHandler::getPersistHandler(BackendCipherTyp
return 0;
}
}
-
+
BackendPersistHandler *BackendPersistHandler::getPersistHandler(char magicBuf[12])
{
- if (magicBuf[2] == KWALLET_CIPHER_BLOWFISH_CBC &&
+ if ((magicBuf[2] == KWALLET_CIPHER_BLOWFISH_ECB || magicBuf[2] == KWALLET_CIPHER_BLOWFISH_CBC) &&
(magicBuf[3] == KWALLET_HASH_SHA1 || magicBuf[3] == KWALLET_HASH_PBKDF2_SHA512)) {
- if (0 == blowfishHandler)
- blowfishHandler = new BlowfishPersistHandler;
+ if (0 == blowfishHandler) {
+ bool useECBforReading = magicBuf[2] == KWALLET_CIPHER_BLOWFISH_ECB;
+ if (useECBforReading) {
+ qDebug() << "this wallet uses ECB encryption. It'll be converted to CBC on next save.";
+ }
+ blowfishHandler = new BlowfishPersistHandler(useECBforReading);
+ }
return blowfishHandler;
}
#ifdef HAVE_QGPGME
@@ -183,11 +189,16 @@ BackendPersistHandler *BackendPersistHandler::getPersistHandler(char magicBuf[12
#endif // HAVE_QGPGME
return 0; // unknown cipher or hash
}
-
+
int BlowfishPersistHandler::write(Backend* wb, KSaveFile& sf, QByteArray& version, WId)
{
assert(wb->_cipherType == BACKEND_CIPHER_BLOWFISH);
+ if (_useECBforReading) {
+ qDebug() << "This wallet used ECB and is now saved using CBC";
+ _useECBforReading = false;
+ }
+
version[2] = KWALLET_CIPHER_BLOWFISH_CBC;
if(!wb->_useNewHash) {
version[3] = KWALLET_HASH_SHA1;
@@ -358,7 +369,7 @@ int BlowfishPersistHandler::read(Backend* wb, QFile& db, WId)
assert(encrypted.size() < db.size());
BlowFish _bf;
- CipherBlockChain bf(&_bf);
+ CipherBlockChain bf(&_bf, _useECBforReading);
int blksz = bf.blockSize();
if ((encrypted.size() % blksz) != 0) {
return -5; // invalid file structure
@@ -502,7 +513,7 @@ int GpgPersistHandler::write(Backend* wb, KSaveFile& sf, QByteArray& version, WI
sf.abort();
return -5;
}
-
+
boost::shared_ptr< GpgME::Context > ctx( GpgME::Context::createForProtocol(GpgME::OpenPGP) );
if (0 == ctx) {
kDebug() << "Cannot setup OpenPGP context!";
@@ -511,7 +522,7 @@ int GpgPersistHandler::write(Backend* wb, KSaveFile& sf, QByteArray& version, WI
}
assert(wb->_cipherType == BACKEND_CIPHER_GPG);
-
+
QByteArray hashes;
QDataStream hashStream(&hashes, QIODevice::WriteOnly);
KMD5 md5;
@@ -549,7 +560,7 @@ int GpgPersistHandler::write(Backend* wb, KSaveFile& sf, QByteArray& version, WI
dataStream << keyID;
dataStream << hashes;
dataStream << values;
-
+
GpgME::Data decryptedData(dataBuffer.data(), dataBuffer.size(), false);
GpgME::Data encryptedData;
std::vector< GpgME::Key > keys;
@@ -574,7 +585,7 @@ int GpgPersistHandler::write(Backend* wb, KSaveFile& sf, QByteArray& version, WI
return -4; // write error
}
}
-
+
return 0;
}
@@ -596,7 +607,7 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
while (bytes = sf.read(buffer, sizeof(buffer)/sizeof(buffer[0]))){
encryptedData.write(buffer, bytes);
}
-
+
retry_label:
boost::shared_ptr< GpgME::Context > ctx( GpgME::Context::createForProtocol(GpgME::OpenPGP) );
if (0 == ctx) {
@@ -620,13 +631,13 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
}
return -1;
}
-
+
decryptedData.seek(0, SEEK_SET);
QByteArray dataBuffer;
while (bytes = decryptedData.read(buffer, sizeof(buffer)/sizeof(buffer[0]))){
dataBuffer.append(buffer, bytes);
}
-
+
// load the wallet from the decrypted data
QDataStream dataStream(dataBuffer);
QString keyID;
@@ -661,10 +672,10 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
return -1;
}
-
+
QDataStream hashStream(hashes);
QDataStream valueStream(values);
-
+
quint32 hashCount;
hashStream >> hashCount;
if (hashCount > 0xFFFF) {
@@ -675,10 +686,10 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
while (hashCount--){
KMD5::Digest d;
hashStream.readRawData(reinterpret_cast<char *>(d), 16);
-
+
quint32 folderSize;
hashStream >> folderSize;
-
+
MD5Digest ba = MD5Digest(reinterpret_cast<char *>(d));
QMap<MD5Digest, QList<MD5Digest> >::iterator it = wb->_hashes.insert(ba, QList<MD5Digest>());
while (folderSize--){
@@ -688,27 +699,27 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
(*it).append(ba);
}
}
-
+
while (folderCount--){
QString folder;
valueStream >> folder;
-
+
quint32 entryCount;
valueStream >> entryCount;
-
+
wb->_entries[folder].clear();
-
+
while (entryCount--){
KWallet::Wallet::EntryType et = KWallet::Wallet::Unknown;
Entry *e = new Entry;
-
+
QString key;
valueStream >> key;
-
+
qint32 x =0; // necessary to read properly
valueStream >> x;
et = static_cast<KWallet::Wallet::EntryType>(x);
-
+
switch (et) {
case KWallet::Wallet::Password:
case KWallet::Wallet::Stream:
@@ -718,7 +729,7 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
delete e;
continue;
}
-
+
QByteArray a;
valueStream >> a;
e->setValue(a);
@@ -727,7 +738,7 @@ int GpgPersistHandler::read(Backend* wb, QFile& sf, WId w)
wb->_entries[folder][key] = e;
}
}
-
+
wb->_open = true;
return 0;
--- kwalletd/backend/backendpersisthandler.h
+++ kwalletd/backend/backendpersisthandler.h
@@ -60,11 +60,13 @@ public:
class BlowfishPersistHandler : public BackendPersistHandler {
public:
- BlowfishPersistHandler() {}
+ explicit BlowfishPersistHandler(bool useECBforReading =false) : _useECBforReading(useECBforReading) {}
virtual ~BlowfishPersistHandler() {}
virtual int write(Backend* wb, KSaveFile& sf, QByteArray& version, WId w);
virtual int read(Backend* wb, QFile& sf, WId w);
+private:
+ bool _useECBforReading;
};
#ifdef HAVE_QGPGME
--- kwalletd/backend/cbc.cc
+++ kwalletd/backend/cbc.cc
@@ -1,149 +1,212 @@
/* This file is part of the KDE project
Copyright (C) 2001 George Staikos <staikos@kde.org>
-
+
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
-
+
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
-
+
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
-
#include "cbc.h"
#include <string.h>
-
-
-
-CipherBlockChain::CipherBlockChain(BlockCipher *cipher) : _cipher(cipher) {
- _next = 0L;
- _register = 0L;
- _len = -1;
- _reader = _writer = 0L;
- if (cipher) {
- _blksz = cipher->blockSize();
- }
+#include <kdebug.h>
+
+CipherBlockChain::CipherBlockChain(BlockCipher *cipher, bool useECBforReading) :
+ _cipher(cipher)
+ , _useECBforReading(useECBforReading)
+{
+ _next = 0L;
+ _register = 0L;
+ _len = -1;
+ _reader = _writer = 0L;
+ if (cipher) {
+ _blksz = cipher->blockSize();
+ }
}
-
-CipherBlockChain::~CipherBlockChain() {
- delete[] (char *)_register;
- _register = 0L;
- delete[] (char *)_next;
- _next = 0L;
+CipherBlockChain::~CipherBlockChain()
+{
+ delete[](char *)_register;
+ _register = 0L;
+ delete[](char *)_next;
+ _next = 0L;
}
-
-bool CipherBlockChain::setKey(void *key, int bitlength) {
- if (_cipher) {
- return _cipher->setKey(key, bitlength);
- }
- return false;
+bool CipherBlockChain::setKey(void *key, int bitlength)
+{
+ if (_cipher) {
+ return _cipher->setKey(key, bitlength);
+ }
+ return false;
}
-
-int CipherBlockChain::keyLen() const {
- if (_cipher) {
- return _cipher->keyLen();
- }
- return -1;
+int CipherBlockChain::keyLen() const
+{
+ if (_cipher) {
+ return _cipher->keyLen();
+ }
+ return -1;
}
-
-bool CipherBlockChain::variableKeyLen() const {
- if (_cipher) {
- return _cipher->variableKeyLen();
- }
- return false;
+bool CipherBlockChain::variableKeyLen() const
+{
+ if (_cipher) {
+ return _cipher->variableKeyLen();
+ }
+ return false;
}
-
-bool CipherBlockChain::readyToGo() const {
- if (_cipher) {
- return _cipher->readyToGo();
- }
- return false;
+bool CipherBlockChain::readyToGo() const
+{
+ if (_cipher) {
+ return _cipher->readyToGo();
+ }
+ return false;
}
-
-int CipherBlockChain::encrypt(void *block, int len) {
- if (_cipher && !_reader) {
- int rc;
-
- _writer |= 1;
-
- if (!_register) {
- _register = new unsigned char[len];
- _len = len;
- memset(_register, 0, len);
- } else if (len > _len) {
- return -1;
- }
-
- // This might be optimizable
- char *tb = (char *)block;
- for (int i = 0; i < len; i++) {
- tb[i] ^= ((char *)_register)[i];
- }
-
- rc = _cipher->encrypt(block, len);
-
- if (rc != -1) {
- memcpy(_register, block, len);
- }
-
- return rc;
- }
- return -1;
+void CipherBlockChain::initRegister() {
+ if (_register == 0L) {
+ size_t registerLen = _cipher->blockSize();
+ _register = new unsigned char[registerLen];
+ _len = registerLen;
+ }
+ memset(_register, 0, _len);
}
+int CipherBlockChain::encrypt(void *block, int len)
+{
+ if (_cipher && !_reader) {
+ int rc;
-int CipherBlockChain::decrypt(void *block, int len) {
- if (_cipher && !_writer) {
- int rc;
+ _writer |= 1;
- _reader |= 1;
+ initRegister();
- if (!_register) {
- _register = new unsigned char[len];
- _len = len;
- memset(_register, 0, len);
- } else if (len > _len) {
- return -1;
- }
+ if ((len % _len) >0) {
+ kDebug() << "Block length given encrypt (" << len << ") is not a multiple of " << _len;
+ return -1;
+ }
- if (!_next)
- _next = new unsigned char[_len];
- memcpy(_next, block, _len);
+ char *elemBlock = static_cast<char*>(block);
+ for (int b = 0; b < len/_len; b++) {
- rc = _cipher->decrypt(block, len);
+ // This might be optimizable
+ char *tb = static_cast<char*>(elemBlock);
+ for (int i = 0; i < _len; i++) {
+ *tb++ ^= ((char *)_register)[i];
+ }
- if (rc != -1) {
- // This might be optimizable
- char *tb = (char *)block;
- for (int i = 0; i < len; i++) {
- tb[i] ^= ((char *)_register)[i];
- }
- }
+ rc = _cipher->encrypt(elemBlock, _len);
- void *temp;
- temp = _next;
- _next = _register;
- _register = temp;
+ if (rc != -1) {
+ memcpy(_register, elemBlock, _len);
+ }
+ elemBlock += _len;
+ }
- return rc;
- }
- return -1;
+ return rc;
+ }
+ return -1;
}
+// This is the old decrypt method, that was decrypting using ECB
+// instead of CBC
+int CipherBlockChain::decryptECB(void *block, int len) {
+ if (_cipher && !_writer) {
+ int rc;
+
+ _reader |= 1;
+
+ if (!_register) {
+ _register = new unsigned char[len];
+ _len = len;
+ memset(_register, 0, len);
+ } else if (len > _len) {
+ return -1;
+ }
+
+ if (!_next) {
+ _next = new unsigned char[_len];
+ }
+ memcpy(_next, block, _len);
+
+ rc = _cipher->decrypt(block, len);
+
+ if (rc != -1) {
+ // This might be optimizable
+ char *tb = (char *)block;
+ for (int i = 0; i < len; i++) {
+ tb[i] ^= ((char *)_register)[i];
+ }
+ }
+
+ void *temp;
+ temp = _next;
+ _next = _register;
+ _register = temp;
+
+ return rc;
+ }
+ return -1;
+}
-
+int CipherBlockChain::decrypt(void *block, int len)
+{
+ if (_useECBforReading) {
+ kDebug() << "decrypting using ECB!";
+ return decryptECB(block, len);
+ }
+
+ if (_cipher && !_writer) {
+ int rc = 0;
+
+ _reader |= 1;
+
+ initRegister();
+
+ if ((len % _len) >0) {
+ kDebug() << "Block length given for decrypt (" << len << ") is not a multiple of " << _len;
+ return -1;
+ }
+
+ char *elemBlock = static_cast<char*>(block);
+ for (int b = 0; b < len/_len; b++) {
+ if (_next == 0L) {
+ _next = new unsigned char[_len];
+ }
+ memcpy(_next, elemBlock, _len);
+
+ int bytesDecrypted = _cipher->decrypt(elemBlock, _len);
+
+ if (bytesDecrypted != -1) {
+ rc += bytesDecrypted;
+ // This might be optimizable
+ char *tb = (char *)elemBlock;
+ for (int i = 0; i < _len; i++) {
+ *tb++ ^= ((char *)_register)[i];
+ }
+ }
+
+ void *temp;
+ temp = _next;
+ _next = _register;
+ _register = temp;
+
+ elemBlock += _len;
+ }
+
+ return rc;
+ }
+ return -1;
+}
--- kwalletd/backend/cbc.h
+++ kwalletd/backend/cbc.h
@@ -1,24 +1,22 @@
/* This file is part of the KDE project
Copyright (C) 2001 George Staikos <staikos@kde.org>
-
+
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
-
+
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
-
+
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
-
-
#ifndef __CBC__KO__H
#define __CBC__KO__H
@@ -33,30 +31,34 @@
* calls to the other will fail in this instance.
*/
-class CipherBlockChain : public BlockCipher {
- public:
- CipherBlockChain(BlockCipher *cipher);
- virtual ~CipherBlockChain();
+class CipherBlockChain : public BlockCipher
+{
+public:
+ CipherBlockChain(BlockCipher *cipher, bool useECBforReading =false);
+ virtual ~CipherBlockChain();
- virtual bool setKey(void *key, int bitlength);
+ virtual bool setKey(void *key, int bitlength);
- virtual int keyLen() const;
+ virtual int keyLen() const;
- virtual bool variableKeyLen() const;
+ virtual bool variableKeyLen() const;
- virtual bool readyToGo() const;
+ virtual bool readyToGo() const;
- virtual int encrypt(void *block, int len);
+ virtual int encrypt(void *block, int len);
- virtual int decrypt(void *block, int len);
+ virtual int decrypt(void *block, int len);
- private:
- BlockCipher *_cipher;
- void *_register;
- void *_next;
- int _len;
- int _reader, _writer;
+private:
+ void initRegister();
+ int decryptECB(void *block, int len);
+ BlockCipher *_cipher;
+ void *_register;
+ void *_next;
+ int _len;
+ int _reader, _writer;
+ bool _useECBforReading;
};
#endif
--- kwalletd/backend/tests/testbf.cpp
+++ kwalletd/backend/tests/testbf.cpp
@@ -4,64 +4,60 @@
#include "blowfish.h"
#include "cbc.h"
-
-int main() {
-BlockCipher *bf;
-char data[] = "This is a test.";
-char expect[] = "\x22\x30\x7e\x2f\x42\x28\x44\x01\xda\xdf\x5a\x81\xd7\xe5\x7c\xd0";
-char key[] = "testkey";
-unsigned long et[] = {0x11223344};
-
- printf("%d: 0x11 == %d and 0x44 == %d\n", ((unsigned char *)et)[0],
- 0x11, 0x44);
- bf = new BlowFish();
+int main()
+{
+ BlockCipher *bf;
+ char data[] = "This is a test.";
+ char expect[] = "\x3f\x3c\x2d\xae\x8c\x7\x84\xf2\xa7\x6d\x28\xbd\xd\xb\xb8\x79";
+ char key[] = "testkey";
+ unsigned long et[] = {0x11223344};
+
+ printf("%d: 0x11 == %d and 0x44 == %d\n", ((unsigned char *)et)[0],
+ 0x11, 0x44);
+ bf = new BlowFish();
// bf = new CipherBlockChain(new BlowFish());
- bf->setKey((void *)key, 7*8);
-
- if (!bf->readyToGo()) {
- printf("Error: not ready to go!\n");
- return -1;
- }
+ bf->setKey((void *)key, 7 * 8);
- printf("About to encrypt...\n"); fflush(stdout);
- if (-1 == bf->encrypt((void *)data, 8)) {
- printf("Error: encrypt failed!\n");
- return -1;
- }
- printf("About to encrypt part 2...\n"); fflush(stdout);
- bf->encrypt((void *)(data+8), 8);
-
- printf("Encryption done. data[] is now: ");
- for (int i = 0; i < 16; i++) {
- printf("0x%x ", data[i]&0xff);
- if ((data[i]&0xff) != (expect[i]&0xff)) {
- printf("Error. This byte failed the comparison. It should have been 0x%x.\n", expect[i]&0xff);
+ if (!bf->readyToGo()) {
+ printf("Error: not ready to go!\n");
return -1;
- }
- }
- printf("\n");
+ }
- delete bf;
- bf = new BlowFish();
+ printf("About to encrypt...\n"); fflush(stdout);
+ if (-1 == bf->encrypt((void *)data, 16)) {
+ printf("Error: encrypt failed!\n");
+ return -1;
+ }
+
+ printf("Encryption done. data[] is now: ");
+ for (int i = 0; i < 16; i++) {
+ printf("0x%x ", data[i] & 0xff);
+ if ((data[i] & 0xff) != (expect[i] & 0xff)) {
+ printf("Error. This byte failed the comparison. It should have been 0x%x.\n", expect[i] & 0xff);
+ break;
+ }
+ }
+ printf("\n");
+
+ delete bf;
+ bf = new BlowFish();
// bf = new CipherBlockChain(new BlowFish());
- bf->setKey((void *)key, 7*8);
+ bf->setKey((void *)key, 7 * 8);
- printf("About to decrypt...\n"); fflush(stdout);
- if (-1 == bf->decrypt((void *)data, 16)) {
- printf("Error: decrypt failed!\n");
- return -1;
- }
- //bf->decrypt((void *)(data+8), 8);
+ printf("About to decrypt...\n"); fflush(stdout);
+ if (-1 == bf->decrypt((void *)data, 16)) {
+ printf("Error: decrypt failed!\n");
+ return -1;
+ }
+ //bf->decrypt((void *)(data+8), 8);
- printf("All done! Result... data[] = \"%s\"\n", data);
- if (strcmp(data, "This is a test.")) {
- printf("ERROR. Decryption failed.\n");
- return -1;
- }
+ printf("All done! Result... data[] = \"%s\"\n", data);
+ if (strcmp(data, "This is a test.")) {
+ printf("ERROR. Decryption failed.\n");
+ return -1;
+ }
- delete bf;
+ delete bf;
}
-
-