Talon/voice-cat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(media): stop permanent voice loss after bad-network blip (protocol v2)

Browse Source 
A bad UDP packet on a flaky link could permanently wedge the voice path,
unrecoverable even across app restarts. Three defects:

1. Anti-replay window was advanced from the UNAUTHENTICATED header seq
   before the AEAD tag was checked, and not rolled back on failure. One
   corrupted/forged frame shoved recv_highest_ far ahead, after which every
   legitimate frame was rejected as "too old" forever. Reorder to
   replay-check -> authenticate -> update (RFC 3711 3.3); the window now
   moves only after a successful tag check.

2. The wire seq was only the low 16 bits of the nonce counter (zero-extended
   on receive). After 65,536 frames the nonce desynced and all frames failed
   auth. Widen the voice frame seq u16 -> u64 (header 14 -> 20 bytes). The
   core owns all UDP framing, so Swift/C# clients need only a rebuild. This
   is a versioned wire change: VOICECAT_PROTOCOL_VERSION 1 -> 2, handshake
   rejects on mismatch.

3. Server leaked per-session UDP state on disconnect; unregister_session now
   frees udp_endpoints_/udp_tokens_/ssrc_to_session_.

Also add rate-limited dropped-frame logging to MediaRelay so a wedged media
path is observable. New regression tests in test_media_aead.cpp cover the
poison (fails on old code) and the 16-bit wrap. ctest --preset dev
-E external_pcm: 22/22 pass (external_pcm aborts on a pre-existing CoreAudio
shutdown race, unrelated).
This commit is contained in:
Talon
2026-06-21 17:45:28 +02:00
parent 5be6d8430d
commit 6071c8e238
17 changed files with 299 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

99
tests/test_media_aead.cpp
View File

@@ -24,8 +24,8 @@ static int g_failures = 0;
++g_failures; \
}} while (0)
// Build a synthetic 14-byte AAD (voice frame header).
static std::vector<uint8_t> make_aad(uint16_t seq) {
// Build a synthetic voice-frame-header AAD.
static std::vector<uint8_t> make_aad(uint64_t seq) {
VoiceFrame f;
f.ssrc = 0xCAFEBABE;
f.seq = seq;
@@ -132,7 +132,7 @@ static void test_multiple_packets() {
std::vector<uint8_t> plain(60, 0x99);
for (uint16_t seq = 0; seq < 10; ++seq) {
for (uint64_t seq = 0; seq < 10; ++seq) {
auto aad = make_aad(seq);
std::vector<uint8_t> cipher(plain.size() + crypto_aead_chacha20poly1305_ietf_ABYTES);
long sealed_len = sender.seal(plain.data(), plain.size(),
@@ -149,8 +149,8 @@ static void test_multiple_packets() {
}
}
// Build a 14-byte AAD (voice frame header) with a given ssrc + seq.
static std::vector<uint8_t> make_aad_ssrc(uint32_t ssrc, uint16_t seq) {
// Build a voice-frame-header AAD with a given ssrc + seq.
static std::vector<uint8_t> make_aad_ssrc(uint32_t ssrc, uint64_t seq) {
VoiceFrame f;
f.ssrc = ssrc;
f.seq = seq;
@@ -159,6 +159,12 @@ static std::vector<uint8_t> make_aad_ssrc(uint32_t ssrc, uint16_t seq) {
return aad;
}
// Overwrite the 8-byte big-endian seq field (header bytes [8..15]) in an AAD buffer.
static void set_aad_seq(std::vector<uint8_t>& aad, uint64_t seq) {
for (int i = 0; i < 8; ++i)
aad[8 + i] = static_cast<uint8_t>((seq >> (56 - 8 * i)) & 0xFF);
}
// Simulate one server relay hop for a single frame, sender → recipient R.
// - sender seals with its send key, setting header seq = its own send counter (client contract).
// - server opens with the sender's key, then re-seals with R's send key.
@@ -169,7 +175,7 @@ static bool relay_one(SodiumMediaCrypto& sender_send, SodiumMediaCrypto& server_
SodiumMediaCrypto& r_send, SodiumMediaCrypto& r_recv,
uint32_t ssrc, const std::vector<uint8_t>& plain, bool rewrite_seq) {
// Client A→server: seq carries the sender's send counter.
auto in_aad = make_aad_ssrc(ssrc, static_cast<uint16_t>(sender_send.peek_send_counter()));
auto in_aad = make_aad_ssrc(ssrc, sender_send.peek_send_counter());
std::vector<uint8_t> cipher(plain.size() + crypto_aead_chacha20poly1305_ietf_ABYTES);
long sealed = sender_send.seal(plain.data(), plain.size(), in_aad.data(), in_aad.size(),
cipher.data(), cipher.size());
@@ -185,11 +191,7 @@ static bool relay_one(SodiumMediaCrypto& sender_send, SodiumMediaCrypto& server_
// Server re-seals to R. Header passes through except seq, which (when fixed) is set to R's
// own send counter so R's open() reconstructs the matching nonce.
std::vector<uint8_t> out_aad = in_aad; // copy header verbatim
if (rewrite_seq) {
uint64_t ctr = r_send.peek_send_counter();
out_aad[8] = static_cast<uint8_t>((ctr >> 8) & 0xFF);
out_aad[9] = static_cast<uint8_t>(ctr & 0xFF);
}
if (rewrite_seq) set_aad_seq(out_aad, r_send.peek_send_counter());
std::vector<uint8_t> relay_cipher(recovered.size() + crypto_aead_chacha20poly1305_ietf_ABYTES);
long resealed = r_send.seal(recovered.data(), static_cast<size_t>(opened),
out_aad.data(), out_aad.size(),
@@ -251,6 +253,79 @@ static void test_relay_interleaved_reseal() {
}
}
// Regression for the bad-wifi wedge: the anti-replay window must NOT be advanced by a
// packet that fails authentication. A single corrupted/forged frame carrying a huge seq
// used to shove recv_highest_ far ahead (before the AEAD tag was checked), after which
// every legitimate frame was rejected as "too old" — permanent silence. open() now
// advances the window only after a successful tag check (RFC 3711 §3.3).
static void test_corrupted_seq_does_not_poison_window() {
uint8_t key[crypto_aead_chacha20poly1305_ietf_KEYBYTES];
crypto_generichash(key, sizeof(key),
reinterpret_cast<const uint8_t*>("poison-key"), 10, nullptr, 0);
SodiumMediaCrypto sender(key);
SodiumMediaCrypto receiver(key);
std::vector<uint8_t> plain(64, 0x5A);
std::vector<uint8_t> recovered(plain.size());
auto seal_at_current = [&](std::vector<uint8_t>& aad_out, std::vector<uint8_t>& cipher_out) {
aad_out = make_aad_ssrc(0xABCD, sender.peek_send_counter());
cipher_out.assign(plain.size() + crypto_aead_chacha20poly1305_ietf_ABYTES, 0);
long s = sender.seal(plain.data(), plain.size(), aad_out.data(), aad_out.size(),
cipher_out.data(), cipher_out.size());
CHECK(s > 0);
};
// 1. A normal frame (counter 0) decrypts. recv_highest_ = 0.
std::vector<uint8_t> aad0, cipher0;
seal_at_current(aad0, cipher0); // sender counter 0 → 1
CHECK(receiver.open(cipher0.data(), cipher0.size(), aad0.data(), aad0.size(),
recovered.data(), recovered.size()) == static_cast<long>(plain.size()));
// 2. A frame whose header seq has been corrupted to a huge value: it fails auth
// (the AAD no longer matches what was sealed) and must NOT move the window.
std::vector<uint8_t> aad1, cipher1;
seal_at_current(aad1, cipher1); // sender counter 1 → 2
std::vector<uint8_t> forged_aad = aad1;
set_aad_seq(forged_aad, 0x0000FFFFFFFFFFFFULL); // bit-flip-style corruption
CHECK(receiver.open(cipher1.data(), cipher1.size(), forged_aad.data(), forged_aad.size(),
recovered.data(), recovered.size()) < 0);
// 3. The next legitimate frame (counter 2) must still decrypt. On the old code this
// returned "too old" because step 2 had poisoned recv_highest_.
std::vector<uint8_t> aad2, cipher2;
seal_at_current(aad2, cipher2); // sender counter 2 → 3
CHECK(receiver.open(cipher2.data(), cipher2.size(), aad2.data(), aad2.size(),
recovered.data(), recovered.size()) == static_cast<long>(plain.size()));
}
// Regression for the 16-bit seq wrap: with a full 64-bit wire counter, sealing/opening
// across the old u16 boundary (65,535 → 65,536) must keep decrypting. On the old code the
// nonce desynced at the wrap and every frame failed auth permanently.
static void test_seq_past_16bit_boundary() {
uint8_t key[crypto_aead_chacha20poly1305_ietf_KEYBYTES];
crypto_generichash(key, sizeof(key),
reinterpret_cast<const uint8_t*>("wrap-key"), 8, nullptr, 0);
SodiumMediaCrypto sender(key);
SodiumMediaCrypto receiver(key);
std::vector<uint8_t> plain(48, 0x6B);
std::vector<uint8_t> recovered(plain.size());
std::vector<uint8_t> cipher(plain.size() + crypto_aead_chacha20poly1305_ietf_ABYTES);
bool all_ok = true;
for (uint64_t i = 0; i < 70000; ++i) { // crosses 65,536
auto aad = make_aad_ssrc(0x1234, sender.peek_send_counter());
long s = sender.seal(plain.data(), plain.size(), aad.data(), aad.size(),
cipher.data(), cipher.size());
if (s < 0) { all_ok = false; break; }
long o = receiver.open(cipher.data(), static_cast<size_t>(s), aad.data(), aad.size(),
recovered.data(), recovered.size());
if (o != static_cast<long>(plain.size())) { all_ok = false; break; }
}
CHECK(all_ok);
}
int main() {
if (sodium_init() < 0) {
std::printf("FAIL: sodium_init failed\n");
@@ -262,6 +337,8 @@ int main() {
test_tamper_detection();
test_multiple_packets();
test_relay_interleaved_reseal();
test_corrupted_seq_does_not_poison_window();
test_seq_past_16bit_boundary();
if (g_failures == 0) {
std::printf("media_aead: all tests passed\n");