Safeguard from potential attacks against OCB2

OCB2 is known to be broken under certain conditions:
https://eprint.iacr.org/2019/311

To execute the universal attacks described in the paper, an attacker needs
access to an encryption oracle that allows it to perform encryption queries with
attacker-chosen nonce. Luckily in Mumble the encryption nonce is a fixed counter
which is far too restrictive for the universal attacks to be feasible against
Mumble.

The basic attacks do not require an attacker-chosen nonce and as such are more
applicable to Mumble. They are however of limited use and do require an en- and
a decryption oracle which Mumble seemingly does not provide at the same time.

To be on the safe side, this commit implements the counter-cryptanalysis
measure described in the paper in section 9 for the sender and receiver side.
This way if either server of client are patched, their communication is almost
certainly (merely lacking formal proof) not susceptible to the attacks described
in the paper.
This commit is contained in:
Jonas Herzig 2020-06-01 13:07:54 +02:00 committed by Robert Adam
parent dbac1b9192
commit be97594046
5 changed files with 88 additions and 17 deletions

View File

@ -68,7 +68,7 @@ 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.
@ -76,12 +76,15 @@ void CryptState::encrypt(const unsigned char *source, unsigned char *dst, unsign
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) {
@ -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;
}
@ -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);
@ -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;
@ -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);
@ -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;
}

View File

@ -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

View File

@ -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);
}
}
@ -1065,4 +1067,3 @@ QUrl ServerHandler::getServerURL(bool withPassword) const {
return url;
}

View File

@ -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;

View File

@ -18,6 +18,7 @@ class TestCrypt : public QObject {
void cleanupTestCase();
void testvectors();
void authcrypt();
void xexstarAttack();
void ivrecovery();
void reverserecovery();
void tamper();
@ -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++)
@ -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
@ -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]);
@ -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};