diff options
| -rw-r--r-- | src/crypto/CryptStateOCB2.cpp | 34 | ||||
| -rw-r--r-- | src/crypto/CryptStateOCB2.h | 2 | ||||
| -rw-r--r-- | src/tests/TestCrypt/TestCrypt.cpp | 20 |
3 files changed, 45 insertions, 11 deletions
diff --git a/src/crypto/CryptStateOCB2.cpp b/src/crypto/CryptStateOCB2.cpp index fb1b892e7..1e6a99109 100644 --- a/src/crypto/CryptStateOCB2.cpp +++ b/src/crypto/CryptStateOCB2.cpp @@ -261,7 +261,7 @@ static void inline ZERO(keyblock &block) { AES_decrypt(reinterpret_cast< const unsigned char * >(src), reinterpret_cast< unsigned char * >(dst), key); bool CryptStateOCB2::ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, - const unsigned char *nonce, unsigned char *tag) { + const unsigned char *nonce, unsigned char *tag, bool modifyPlainOnXEXStarAttack) { keyblock checksum, delta, tmp, pad; bool success = true; @@ -270,21 +270,39 @@ bool CryptStateOCB2::ocb_encrypt(const unsigned char *plain, unsigned char *encr ZERO(checksum); while (len > AES_BLOCK_SIZE) { - S2(delta); - XOR(tmp, delta, reinterpret_cast< const subblock * >(plain)); - 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). + 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/crypto/CryptStateOCB2.h b/src/crypto/CryptStateOCB2.h index 033fbdf87..8c34fd49a 100644 --- a/src/crypto/CryptStateOCB2.h +++ b/src/crypto/CryptStateOCB2.h @@ -33,7 +33,7 @@ public: virtual bool encrypt(const unsigned char *source, unsigned char *dst, unsigned int plain_length) Q_DECL_OVERRIDE; bool ocb_encrypt(const unsigned char *plain, unsigned char *encrypted, unsigned int len, const unsigned char *nonce, - unsigned char *tag); + 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); diff --git a/src/tests/TestCrypt/TestCrypt.cpp b/src/tests/TestCrypt/TestCrypt.cpp index abc1ec370..6954e0ce2 100644 --- a/src/tests/TestCrypt/TestCrypt.cpp +++ b/src/tests/TestCrypt/TestCrypt.cpp @@ -232,9 +232,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; @@ -251,6 +251,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<unsigned char>(0)); + QCOMPARE(decrypted[0], static_cast<unsigned char>(1)); } void TestCrypt::tamper() { |