diff --git a/src/CryptState.cpp b/src/CryptState.cpp index 8517f8bcdb9..fbbeb3e0db1 100644 --- a/src/CryptState.cpp +++ b/src/CryptState.cpp @@ -218,7 +218,8 @@ static void inline ZERO(keyblock &block) { #define AESencrypt(src,dst,key) AES_encrypt(reinterpret_cast(src),reinterpret_cast(dst), key); #define AESdecrypt(src,dst,key) AES_decrypt(reinterpret_cast(src),reinterpret_cast(dst), key); -bool 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, bool modifyPlainOnXEXStarAttack) { keyblock checksum, delta, tmp, pad; bool success = true; @@ -227,21 +228,39 @@ bool CryptState::ocb_encrypt(const unsigned char *plain, unsigned char *encrypte ZERO(checksum); while (len > AES_BLOCK_SIZE) { - S2(delta); - XOR(tmp, delta, reinterpret_cast(plain)); - AESencrypt(tmp, tmp, &encrypt_key); - XOR(reinterpret_cast(encrypted), delta, tmp); - XOR(checksum, checksum, reinterpret_cast(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). + bool flipABit = false; // *plain is const, so we can't directly modify it 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; + if (sum == 0) { + if (modifyPlainOnXEXStarAttack) { + // The assumption that critical packets do not turn up by pure chance turned out to be incorrect + // since digital silence appears to produce them in mass. + // So instead we now modify the packet in a way which should not affect the audio but will + // prevent the attack. + flipABit = true; + } else { + // This option still exists but only to allow us to test ocb_decrypt's detection. + success = false; + } + } + } + + S2(delta); + XOR(tmp, delta, reinterpret_cast< const subblock * >(plain)); + if (flipABit) { + *reinterpret_cast< unsigned char * >(tmp) ^= 1; + } + AESencrypt(tmp, tmp, &encrypt_key); + XOR(reinterpret_cast< subblock * >(encrypted), delta, tmp); + XOR(checksum, checksum, reinterpret_cast< const subblock * >(plain)); + if (flipABit) { + *reinterpret_cast< unsigned char * >(checksum) ^= 1; } len -= AES_BLOCK_SIZE; diff --git a/src/CryptState.h b/src/CryptState.h index bf5576e6abf..8005cc4c883 100644 --- a/src/CryptState.h +++ b/src/CryptState.h @@ -44,7 +44,8 @@ class CryptState { void setKey(const unsigned char *rkey, const unsigned char *eiv, const unsigned char *div); void setDecryptIV(const unsigned char *iv); - bool ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, 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 modifyPlainOnXEXStarAttack = true); 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); diff --git a/src/tests/TestCrypt/TestCrypt.cpp b/src/tests/TestCrypt/TestCrypt.cpp index 33cd915994a..918f0617a3e 100644 --- a/src/tests/TestCrypt/TestCrypt.cpp +++ b/src/tests/TestCrypt/TestCrypt.cpp @@ -218,9 +218,9 @@ void TestCrypt::xexstarAttack() { 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); + STACKVAR(unsigned char, decrypted, 2 * AES_BLOCK_SIZE); - const bool failed_encrypt = !cs.ocb_encrypt(src, encrypted, 2 * AES_BLOCK_SIZE, nonce, enctag); + const bool failed_encrypt = !cs.ocb_encrypt(src, encrypted, 2 * AES_BLOCK_SIZE, nonce, enctag, false); // Perform the attack encrypted[AES_BLOCK_SIZE - 1] ^= AES_BLOCK_SIZE * 8; @@ -237,6 +237,22 @@ void TestCrypt::xexstarAttack() { // Make sure we detected the attack QVERIFY(failed_encrypt); QVERIFY(failed_decrypt); + + // The assumption that critical packets do not turn up by pure chance turned out to be incorrect + // since digital silence appears to produce them in mass. + // So instead we now modify the packet in a way which should not affect the audio but will + // prevent the attack. + QVERIFY(cs.ocb_encrypt(src, encrypted, 2 * AES_BLOCK_SIZE, nonce, enctag)); + QVERIFY(cs.ocb_decrypt(encrypted, decrypted, 2 * AES_BLOCK_SIZE, nonce, dectag)); + + // Tags should match + for (int i = 0; i < AES_BLOCK_SIZE; ++i) { + QCOMPARE(enctag[i], dectag[i]); + } + + // Actual content should have been changed such that the critical block is no longer all 0. + QCOMPARE(src[0], static_cast(0)); + QCOMPARE(decrypted[0], static_cast(1)); } void TestCrypt::tamper() {