Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Safeguard from potential attacks against OCB2 #4227

Merged
merged 1 commit into from
Jun 6, 2020
Merged

Safeguard from potential attacks against OCB2 #4227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 36 additions & 6 deletions src/CryptState.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,23 @@ void CryptState::setDecryptIV(const unsigned char *iv) {
memcpy(decrypt_iv, iv, AES_BLOCK_SIZE);
}

void CryptState::encrypt(const unsigned char *source, unsigned char *dst, unsigned int plain_length) {
bool CryptState::encrypt(const unsigned char *source, unsigned char *dst, unsigned int plain_length) {
unsigned char tag[AES_BLOCK_SIZE];

// First, increase our IV.
for (int i=0;i<AES_BLOCK_SIZE;i++)
if (++encrypt_iv[i])
break;

ocb_encrypt(source, dst+4, plain_length, encrypt_iv, tag);
if (!ocb_encrypt(source, dst+4, plain_length, encrypt_iv, tag)) {
return false;
}

dst[0] = encrypt_iv[0];
dst[1] = tag[0];
dst[2] = tag[1];
dst[3] = tag[2];
return true;
}

bool CryptState::decrypt(const unsigned char *source, unsigned char *dst, unsigned int crypted_length) {
Expand Down Expand Up @@ -157,9 +160,9 @@ bool CryptState::decrypt(const unsigned char *source, unsigned char *dst, unsign
}
}

ocb_decrypt(source+4, dst, plain_length, decrypt_iv, tag);
bool ocb_success = ocb_decrypt(source+4, dst, plain_length, decrypt_iv, tag);

if (memcmp(tag, source+1, 3) != 0) {
if (!ocb_success || memcmp(tag, source+1, 3) != 0) {
memcpy(decrypt_iv, saveiv, AES_BLOCK_SIZE);
return false;
}
Expand Down Expand Up @@ -224,8 +227,9 @@ static void inline ZERO(keyblock &block) {
#define AESencrypt(src,dst,key) AES_encrypt(reinterpret_cast<const unsigned char *>(src),reinterpret_cast<unsigned char *>(dst), key);
#define AESdecrypt(src,dst,key) AES_decrypt(reinterpret_cast<const unsigned char *>(src),reinterpret_cast<unsigned char *>(dst), key);

void CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, unsigned char *tag) {
bool CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, unsigned char *tag) {
keyblock checksum, delta, tmp, pad;
bool success = true;

// Initialize
AESencrypt(nonce, delta, &encrypt_key);
Expand All @@ -237,6 +241,18 @@ void CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypte
AESencrypt(tmp, tmp, &encrypt_key);
XOR(reinterpret_cast<subblock *>(encrypted), delta, tmp);
XOR(checksum, checksum, reinterpret_cast<const subblock *>(plain));

// Counter-cryptanalysis described in section 9 of https://eprint.iacr.org/2019/311
// For an attack, the second to last block (i.e. the last iteration of this loop)
// must be all 0 except for the last byte (which may be 0 - 128).
if (len - AES_BLOCK_SIZE <= AES_BLOCK_SIZE) {
unsigned char sum = 0;
for (int i = 0; i < AES_BLOCK_SIZE - 1; ++i) {
sum |= plain[i];
}
success &= sum != 0;
}

len -= AES_BLOCK_SIZE;
plain += AES_BLOCK_SIZE;
encrypted += AES_BLOCK_SIZE;
Expand All @@ -256,10 +272,13 @@ void CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypte
S3(delta);
XOR(tmp, delta, checksum);
AESencrypt(tmp, tag, &encrypt_key);

return success;
}

void CryptState::ocb_decrypt(const unsigned char *encrypted, unsigned char *plain, unsigned int len, const unsigned char *nonce, unsigned char *tag) {
bool CryptState::ocb_decrypt(const unsigned char *encrypted, unsigned char *plain, unsigned int len, const unsigned char *nonce, unsigned char *tag) {
keyblock checksum, delta, tmp, pad;
bool success = true;

// Initialize
AESencrypt(nonce, delta, &encrypt_key);
Expand Down Expand Up @@ -287,7 +306,18 @@ void CryptState::ocb_decrypt(const unsigned char *encrypted, unsigned char *plai
XOR(checksum, checksum, tmp);
memcpy(plain, tmp, len);

// Counter-cryptanalysis described in section 9 of https://eprint.iacr.org/2019/311
// In an attack, the decrypted last block would need to equal `delta ^ len(128)`.
// With a bit of luck (or many packets), smaller values than 128 (i.e. non-full blocks) are also
// feasible, so we check `tmp` instead of `plain`.
// Since our `len` only ever modifies the last byte, we simply check all remaining ones.
if (memcmp(tmp, delta, AES_BLOCK_SIZE - 1) == 0) {
success = false;
}

S3(delta);
XOR(tmp, delta, checksum);
AESencrypt(tmp, tag, &encrypt_key);

return success;
}
6 changes: 3 additions & 3 deletions src/CryptState.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,11 @@ class CryptState {
void setKey(const unsigned char *rkey, const unsigned char *eiv, const unsigned char *div);
void setDecryptIV(const unsigned char *iv);

void ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, unsigned char *tag);
void ocb_decrypt(const unsigned char *encrypted, unsigned char *plain, unsigned int len, const unsigned char *nonce, unsigned char *tag);
bool ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, unsigned char *tag);
bool ocb_decrypt(const unsigned char *encrypted, unsigned char *plain, unsigned int len, const unsigned char *nonce, unsigned char *tag);

bool decrypt(const unsigned char *source, unsigned char *dst, unsigned int crypted_length);
void encrypt(const unsigned char *source, unsigned char *dst, unsigned int plain_length);
bool encrypt(const unsigned char *source, unsigned char *dst, unsigned int plain_length);
};

#endif
5 changes: 3 additions & 2 deletions src/mumble/ServerHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,9 @@ void ServerHandler::sendMessage(const char *data, int len, bool force) {

QApplication::postEvent(this, new ServerHandlerMessageEvent(qba, MessageHandler::UDPTunnel, true));
} else {
connection->csCrypt.encrypt(reinterpret_cast<const unsigned char *>(data), crypto, len);
if (!connection->csCrypt.encrypt(reinterpret_cast<const unsigned char *>(data), crypto, len)) {
return;
}
qusUdp->writeDatagram(reinterpret_cast<const char *>(crypto), len + 4, qhaRemote, usResolvedPort);
}
}
Expand Down Expand Up @@ -1065,4 +1067,3 @@ QUrl ServerHandler::getServerURL(bool withPassword) const {

return url;
}

5 changes: 3 additions & 2 deletions src/murmur/Server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,8 +952,9 @@ void Server::sendMessage(ServerUser *u, const char *data, int len, QByteArray &c
return;
}

u->csCrypt.encrypt(reinterpret_cast<const unsigned char *>(data), reinterpret_cast<unsigned char *>(buffer),
len);
if (!u->csCrypt.encrypt(reinterpret_cast<const unsigned char *>(data), reinterpret_cast<unsigned char *>(buffer), len)) {
return;
}
}
#ifdef Q_OS_WIN
DWORD dwFlow = 0;
Expand Down
47 changes: 43 additions & 4 deletions src/tests/TestCrypt/TestCrypt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class TestCrypt : public QObject {
void cleanupTestCase();
void testvectors();
void authcrypt();
void xexstarAttack();
void ivrecovery();
void reverserecovery();
void tamper();
Expand Down Expand Up @@ -148,7 +149,7 @@ void TestCrypt::testvectors() {
cs.setKey(rawkey, rawkey, rawkey);

unsigned char tag[16];
cs.ocb_encrypt(NULL, NULL, 0, rawkey, tag);
QVERIFY(cs.ocb_encrypt(NULL, NULL, 0, rawkey, tag));

const unsigned char blanktag[AES_BLOCK_SIZE] = {0xBF,0x31,0x08,0x13,0x07,0x73,0xAD,0x5E,0xC7,0x0E,0xC6,0x9E,0x78,0x75,0xA7,0xB0};
for (int i=0;i<AES_BLOCK_SIZE;i++)
Expand All @@ -158,7 +159,7 @@ void TestCrypt::testvectors() {
unsigned char crypt[40];
for (int i=0;i<40;i++)
source[i]=i;
cs.ocb_encrypt(source, crypt, 40, rawkey, tag);
QVERIFY(cs.ocb_encrypt(source, crypt, 40, rawkey, tag));
const unsigned char longtag[AES_BLOCK_SIZE] = {0x9D,0xB0,0xCD,0xF8,0x80,0xF7,0x3E,0x3E,0x10,0xD4,0xEB,0x32,0x17,0x76,0x66,0x88};
const unsigned char crypted[40] = {0xF7,0x5D,0x6B,0xC8,0xB4,0xDC,0x8D,0x66,0xB8,0x36,0xA2,0xB0,0x8B,0x32,0xA6,0x36,0x9F,0x1C,0xD3,0xC5,0x22,0x8D,0x79,0xFD,
0x6C,0x26,0x7F,0x5F,0x6A,0xA7,0xB2,0x31,0xC7,0xDF,0xB9,0xD5,0x99,0x51,0xAE,0x9C
Expand Down Expand Up @@ -187,8 +188,8 @@ void TestCrypt::authcrypt() {
STACKVAR(unsigned char, encrypted, len);
STACKVAR(unsigned char, decrypted, len);

cs.ocb_encrypt(src, encrypted, len, nonce, enctag);
cs.ocb_decrypt(encrypted, decrypted, len, nonce, dectag);
QVERIFY(cs.ocb_encrypt(src, encrypted, len, nonce, enctag));
QVERIFY(cs.ocb_decrypt(encrypted, decrypted, len, nonce, dectag));

for (int i=0;i<AES_BLOCK_SIZE;i++)
QCOMPARE(enctag[i], dectag[i]);
Expand All @@ -198,6 +199,44 @@ void TestCrypt::authcrypt() {
}
}

// Test prevention of the attack described in section 4.1 of https://eprint.iacr.org/2019/311
void TestCrypt::xexstarAttack() {
const unsigned char rawkey[AES_BLOCK_SIZE] = {0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f};
const unsigned char nonce[AES_BLOCK_SIZE] = {0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa, 0x99, 0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11, 0x00};
CryptState cs;
cs.setKey(rawkey, nonce, nonce);

STACKVAR(unsigned char, src, 2 * AES_BLOCK_SIZE);
// Set first block to `len(secondBlock)`
memset(src, 0, AES_BLOCK_SIZE);
src[AES_BLOCK_SIZE - 1] = AES_BLOCK_SIZE * 8;
// Set second block to arbitrary value
memset(src + AES_BLOCK_SIZE, 42, AES_BLOCK_SIZE);

unsigned char enctag[AES_BLOCK_SIZE];
unsigned char dectag[AES_BLOCK_SIZE];
STACKVAR(unsigned char, encrypted, 2 * AES_BLOCK_SIZE);
STACKVAR(unsigned char, decrypted, 1 * AES_BLOCK_SIZE);

const bool failed_encrypt = !cs.ocb_encrypt(src, encrypted, 2 * AES_BLOCK_SIZE, nonce, enctag);

// Perform the attack
encrypted[AES_BLOCK_SIZE - 1] ^= AES_BLOCK_SIZE * 8;
for (int i = 0; i < AES_BLOCK_SIZE; ++i)
enctag[i] = src[AES_BLOCK_SIZE + i] ^ encrypted[AES_BLOCK_SIZE + i];

const bool failed_decrypt = !cs.ocb_decrypt(encrypted, decrypted, 1 * AES_BLOCK_SIZE, nonce, dectag);

// Verify forged tag (should match if attack is properly implemented)
for (int i = 0; i < AES_BLOCK_SIZE; ++i) {
QCOMPARE(enctag[i], dectag[i]);
}

// Make sure we detected the attack
QVERIFY(failed_encrypt);
QVERIFY(failed_decrypt);
}

void TestCrypt::tamper() {
const unsigned char rawkey[AES_BLOCK_SIZE] = {0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0x0c,0x0d,0x0e,0x0f};
const unsigned char nonce[AES_BLOCK_SIZE] = {0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa, 0x99, 0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11, 0x00};
Expand Down