[core] RCV: recover undecryptable (AEAD) packets by re-requesting them (completes #2649)

[danryu]

Summary

I hit this while testing SRT for sample-accurate audio delivery. On ordinary
consumer network paths, occasional audible dropouts persisted even with AES-GCM
enabled. They turned out not to be packet loss: the packets were arriving and
failing their auth tag — genuine payload corruption that survived the UDP
checksum, which AEAD correctly detected and then discarded without ever
re-requesting. Application-layer CRC32 stamping confirmed the corruption
independently. On the affected path this ran at a steady 2–5 events per minute,
each one a permanent gap in otherwise lossless audio.

With AEAD (AES-GCM) the receiver can detect a payload that was corrupted in
transit — the auth tag fails. Today the packet is then dropped and never
re-requested, so it becomes a permanent gap in the delivered stream. For
applications that need bit-exact delivery over paths that can produce corruption
surviving the UDP checksum (checksum-recomputing middleboxes / CGNAT), AEAD is
currently detect-but-not-recover: a silent corruption becomes a dropout.

This PR makes the receiver treat an undecryptable packet exactly like a lost
packet: the payload is erased (slot kept fillable), the sequence stays in or
goes back into the receiver loss list, a NAK is sent, and SRT's existing ARQ
retransmits the good copy into the held-open slot. Bit-exact delivery is
restored with no sender-side change and no new machinery.

I should say up front that I'm a user of the library rather than a regular
contributor here, and that this work was done with LLM assistance (guided steps
with Fable 5). The reasoning, the adversarial testing and the evidence below are
all real and reproducible, but please review it as work from an outsider to this
codebase.

Relationship to #2649 and #2626

• [FR][MAINT] Revise RCV message dropping logic #2626 tracked revising how the RCV buffer handles a failed decryption. It
  was closed by [core] Drop undecrypted packet based on sequence number. #2654, which switched to dropping by sequence number. Dropping
  marks the buffer slot as dropped, which is what makes the corrupted sequence
  unrecoverable — a later valid copy can no longer be stored.
• RCV erase undecrypted packet instead of dropping. #2649 (still open, from 2023) added CRcvBuffer::erase(int32_t seqno),
  which drops the corrupt payload while leaving the slot fillable, and left the
  TODO:

  // TODO: Erasing the packet means it has to be added back to the loss list.

This PR is #2649 rebased onto current master, plus the changes needed to make
recovery actually work. The first commit is @maxsharabayko's original work with
authorship preserved, adapted to the current CRcvBuffer API (the dropUpTo
signature, EntryState entries) and to the statistics/frequent-log block that
arrived with #2654. The second commit is mine.

I implemented the naive completion of that TODO first, and broke it under
adversarial testing. The three additional failure modes are the interesting part
and are the reason the one-liner isn't enough.

The change (4 parts, receiver-side only)

1. Undo insert()'s countBytes() accounting in erase(). Callers that
   release a unit account for it themselves (readMessage, dropMessage);
   releaseUnitInPos() does not. Without this, the retransmission landing in the
   freed slot double-counts buffer bytes and packets.
2. Re-request the erased sequence by pushing (seq, seq) into
   w_srt_loss_seqs — after the loss-detection block (see 4).
   CRcvLossList::insert() rejects entries below its largest-ever sequence, so a
   preceding jump range has to be recorded first; order matters here.
3. Do not unlose() an undecrypted packet. The arrival of a corrupt
   retransmission removed the sequence from the receiver loss list, and the
   re-add was then rejected by the m_iLargestSeq monotonic guard — leaving the
   sequence in no loss list at all. Since the ACK position
   (getFirstNoncontSequence) is derived from that list, the next full ACK
   advances past the erased hole, after which every further copy — corrupt or
   clean — is discarded as belated (seq %< m_iRcvLastAck) and the sequence is
   unrecoverable. Measured before the fix: ~5–6% chain death per corrupt
   retransmit round (≈ round latency / ACK period); clean retransmissions were
   log-captured being belated-discarded. An erased packet is still missing — keep
   it in the list.
4. Run sequence-jump loss detection for undecrypted packets too. The gap
   check is gated on adding_successful (false on decrypt failure) while
   m_iRcvCurrSeqNo advances regardless — so packets genuinely lost immediately
   before a corrupt arrival were never recorded or NAKed, and became permanent
   gaps. Measured before the fix at loss+corruption: gaps ≈ loss rate ×
   corruption rate, exactly as predicted (17 predicted ≈ 17 observed at 10% + 2%).
   This defect is reachable in the existing GCM path on its own, independently
   of anything else in this PR: a sequence jump that arrives together with an
   undecryptable packet is silently never reported. It may be worth addressing
   even if the rest is rejected.

Why it should be low-risk

• Every new branch is conditional on an AEAD decryption failure. On clean streams
  and unencrypted streams the code is inert — verified byte-identical behaviour
  versus stock: plaintext ± corruption, loss-only, and a 1 h clean soak with
  pktRcvRetrans=0.
• It reuses the existing loss list → NAK → periodic NAK → retransmit machinery.
• Sender side unchanged.
• One user-visible statistics change, inherited from RCV erase undecrypted packet instead of dropping. #2649: a decryption failure
  no longer increments pktRcvDrop, because the packet is no longer dropped.
  pktRcvUndecrypt still counts it.

Testing

Unit tests. test-srt is green on this branch: 288/288 pass (macOS
clang, C++17). I also built USE_CXX_STD=11 and ENABLE_ENCRYPTION=OFF
configurations clean, and codespell with your config passes on the changed
files.

I have not added a new unit test, and I'd like guidance on where you'd want
one. The behaviour needs a packet whose ciphertext is corrupted on the wire so
that the auth tag genuinely fails, plus a full ARQ round trip to observe the
recovery — my harness does that with a userspace UDP relay between caller and
listener, which doesn't obviously fit the existing test-srt scaffolding. If
you'd point me at the right seam (an srt-test-* case, or something at the
CRcvBuffer level asserting that an erased slot is refillable and the sequence
is re-requested) I'll write it.

Lab evidence (deterministic). Isolated caller → listener, live mode, AES-GCM,
SRTO_LATENCY=500, paced 2 ms. A userspace UDP relay flips one byte inside the
ciphertext on the wire (data packets only) so the auth tag genuinely fails.
The application payload is sequence + CRC32 stamped, so delivery is verified
bit-exact end to end.

scenario	master (unpatched)	with this change
0% corruption (regression), 10k msgs	bit-perfect	bit-perfect, pktRcvRetrans=0 (inert)
0.5% corruption	45 gaps	0 gaps, bit-perfect
2%	361 gaps	0 gaps, bit-perfect
5% / 10% / 20% iid	gaps grow	0 gaps at every rate
10% loss + 20% corruption composite	—	0 gaps (2359 corruptions + 1477 losses recovered)
same sequence corrupted k=1,2,3,5,10 consecutive times (sequence-targeted relay)	chains die (part 3)	897/897 recovered across 299 chains, 0 gaps
emulated WAN RTT 65 ms + loss + corruption	—	bit-perfect; the envelope degrades only beyond RTT ≈250 ms at 2%, where two round trips no longer fit a 500 ms latency budget

badCrc — corrupt bytes delivered to the application — was 0 in every run,
patched or not. AEAD never delivers corrupt data; this change closes the recovery
half of it.

Field evidence (real WAN). A production-shaped path: Windows sender → cloud
relay → macOS receiver, where the sender's uplink demonstrably produces
checksum-surviving corruption (independently verified by application-layer CRC32
instrumentation). With this patch deployed on the relay and receivers, real
corruption events logged at relay ingress at ~2–4/min were all recovered:
pktRcvDrop=0, zero application-level sequence gaps, zero corrupt bytes
delivered, across the acceptance runs. An unpatched same-day control turned every
detection into a dropout. A later post-deploy check recovered 9 events in a single
2-minute session, and 15-run strict batches showed zero delivered corruption.

Open questions for maintainers

• Latency budget. Recovery costs ~1 RTT per round, so bit-exact AEAD delivery
  needs the receiver's negotiated latency to be ≳2.5×RTT. Should that be
  documented as an operational requirement somewhere in the encryption docs?
• RCV-LOSS/insert ... REJECTING warning now fires benignly (the entry is
  already present) on corrupt-retransmit rounds. Worth demoting to debug?
• Buffer / message mode. erase() handles the non-TSBPD out-of-order
  bookkeeping explicitly, inherited from RCV erase undecrypted packet instead of dropping. #2649, but all of my validation is
  live mode with TSBPD — which is also what the existing SRT_ASSERT(m_bTsbPd)
  on this path restricts it to. Someone who exercises message mode should look
  at that path.
• Commit structure. I've kept the rebase of RCV erase undecrypted packet instead of dropping. #2649 as its own commit with
  original authorship, on the assumption that's most reviewable. Happy to squash,
  reorder, or split part 4 out as an independent fix if you'd prefer — it stands
  on its own.