diff options
author | Jonas Herzig <me@johni0702.de> | 2021-01-26 16:40:45 +0300 |
---|---|---|
committer | Jonas Herzig <me@johni0702.de> | 2021-02-05 01:48:03 +0300 |
commit | 8990f5a57b00fa68c821bdb0462df3cea293d368 (patch) | |
tree | 6da6a0f91a878bf8e86de2f6e7ec97d3f5dd097f /src/tests | |
parent | bc6b6a3a75285be41d4b60310e60154a4038a861 (diff) |
FIX(ocb2): Work around packet loss due to OCB2 XEX* mitigation
The mitigation for vulnerabilities discovered in
OCB2 (https://eprint.iacr.org/2019/311, called XEX* attack, or XEXStarAttack, in code)
introduced in be97594 (#4227) willingly allowed for some packets with specific
characteristics to be dropped during encryption to prevent the vulnerability
from being exploited.
It was assumed that the chance of such packets was sufficiently small (given
we are dealing with compressed audio) that such loss was acceptable.
It was however discovered that digital silence (as produced by e.g. a noise
gate) will cause Opus to emit almost exclusively such packets, leading to strong
artifacts on the receiving end. See #4385.
This commit tries to work around the issue by modifying such packets in a way
which will no longer require them to be dropped, and yet produce the expected
output on the receiver side.
As far as I understand [Opus] (specifically section 4.1, 4.3.0 and 4.3.3), the
0s are simply unused bits and are only there because we running Opus in constant
bitrate mode. So, flipping one of them should have no effect on the resulting
audio.
[Opus]: https://tools.ietf.org/html/rfc6716
Fixes #4719
Diffstat (limited to 'src/tests')
-rw-r--r-- | src/tests/TestCrypt/TestCrypt.cpp | 20 |
1 files changed, 18 insertions, 2 deletions
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() { |