From 6e588d795e6631c3c9d84d85fd3884a159b45849 Mon Sep 17 00:00:00 2001
From: Valentin Rusu <kde@rusu.info>
Date: Sun, 4 Jan 2015 23:36:15 +0100
Subject: [PATCH 1/2] Fix the CBC encryption algorith
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.
---
src/runtime/kwalletd/backend/cbc.cc | 95 ++++++++++++++++-----------
src/runtime/kwalletd/backend/cbc.h | 2 +
src/runtime/kwalletd/backend/tests/testbf.cpp | 8 +--
3 files changed, 62 insertions(+), 43 deletions(-)
diff --git a/src/runtime/kwalletd/backend/cbc.cc b/src/runtime/kwalletd/backend/cbc.cc
index a578bca..3e45729 100644
--- a/src/runtime/kwalletd/backend/cbc.cc
+++ b/src/runtime/kwalletd/backend/cbc.cc
@@ -19,11 +19,12 @@
#include "cbc.h"
#include <string.h>
+#include <QDebug>
CipherBlockChain::CipherBlockChain(BlockCipher *cipher) : _cipher(cipher)
{
- _next = 0L;
- _register = 0L;
+ _next = nullptr;
+ _register = nullptr;
_len = -1;
_reader = _writer = 0L;
if (cipher) {
@@ -34,9 +35,9 @@ CipherBlockChain::CipherBlockChain(BlockCipher *cipher) : _cipher(cipher)
CipherBlockChain::~CipherBlockChain()
{
delete[](char *)_register;
- _register = 0L;
+ _register = nullptr;
delete[](char *)_next;
- _next = 0L;
+ _next = nullptr;
}
bool CipherBlockChain::setKey(void *key, int bitlength)
@@ -71,6 +72,15 @@ bool CipherBlockChain::readyToGo() const
return false;
}
+void CipherBlockChain::initRegister() {
+ if (_register == nullptr) {
+ 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) {
@@ -78,24 +88,28 @@ int CipherBlockChain::encrypt(void *block, int len)
_writer |= 1;
- if (!_register) {
- _register = new unsigned char[len];
- _len = len;
- memset(_register, 0, len);
- } else if (len > _len) {
+ initRegister();
+
+ if ((len % _len) >0) {
+ qDebug() << "Block length given encrypt (" << len << ") is not a multiple of " << _len;
return -1;
}
- // This might be optimizable
- char *tb = (char *)block;
- for (int i = 0; i < len; i++) {
- tb[i] ^= ((char *)_register)[i];
- }
+ char *elemBlock = static_cast<char*>(block);
+ for (int b = 0; b < len/_len; b++) {
- rc = _cipher->encrypt(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) {
- memcpy(_register, block, len);
+ rc = _cipher->encrypt(elemBlock, _len);
+
+ if (rc != -1) {
+ memcpy(_register, elemBlock, _len);
+ }
+ elemBlock += _len;
}
return rc;
@@ -106,37 +120,42 @@ int CipherBlockChain::encrypt(void *block, int len)
int CipherBlockChain::decrypt(void *block, int len)
{
if (_cipher && !_writer) {
- int rc;
+ int rc = 0;
_reader |= 1;
- if (!_register) {
- _register = new unsigned char[len];
- _len = len;
- memset(_register, 0, len);
- } else if (len > _len) {
+ initRegister();
+
+ if ((len % _len) >0) {
+ qDebug() << "Block length given for decrypt (" << 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++) {
+ if (_next == nullptr) {
+ _next = new unsigned char[_len];
+ }
+ memcpy(_next, elemBlock, _len);
- rc = _cipher->decrypt(block, len);
+ int bytesDecrypted = _cipher->decrypt(elemBlock, _len);
- if (rc != -1) {
- // This might be optimizable
- char *tb = (char *)block;
- for (int i = 0; i < len; i++) {
- tb[i] ^= ((char *)_register)[i];
+ 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;
+ void *temp;
+ temp = _next;
+ _next = _register;
+ _register = temp;
+
+ elemBlock += _len;
+ }
return rc;
}
diff --git a/src/runtime/kwalletd/backend/cbc.h b/src/runtime/kwalletd/backend/cbc.h
index 0984907..f6c93bf 100644
--- a/src/runtime/kwalletd/backend/cbc.h
+++ b/src/runtime/kwalletd/backend/cbc.h
@@ -50,6 +50,8 @@ public:
virtual int decrypt(void *block, int len);
private:
+ void initRegister();
+
BlockCipher *_cipher;
void *_register;
void *_next;
diff --git a/src/runtime/kwalletd/backend/tests/testbf.cpp b/src/runtime/kwalletd/backend/tests/testbf.cpp
index 03c3c3f..b3d554a 100644
--- a/src/runtime/kwalletd/backend/tests/testbf.cpp
+++ b/src/runtime/kwalletd/backend/tests/testbf.cpp
@@ -8,7 +8,7 @@ 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 expect[] = "\x3f\x3c\x2d\xae\x8c\x7\x84\xf2\xa7\x6d\x28\xbd\xd\xb\xb8\x79";
char key[] = "testkey";
unsigned long et[] = {0x11223344};
@@ -25,19 +25,17 @@ int main()
}
printf("About to encrypt...\n"); fflush(stdout);
- if (-1 == bf->encrypt((void *)data, 8)) {
+ if (-1 == bf->encrypt((void *)data, 16)) {
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);
- return -1;
+ break;
}
}
printf("\n");
--
2.2.1
