fix: ignore an ACK whose CSeq does not match the server INVITE

[tgeorge06]

Assessment

• Verdict: genuine library defect, confidence HIGH
• Spec: RFC 3261 §17.1.1.3 and §13.2.2.4 (an ACK carries the CSeq number of the INVITE it acknowledges); §13.3.1.4 (2xx retransmission stops only on the ACK for that response).
• Codex tiebreak (library defect vs. application usage): LIBRARY-DEFECT, high: reordering an ACK behind the next re-INVITE is ordinary UDP behaviour (§14.2 allows the next re-INVITE once the final response is sent), and the stack confirms and stops retransmission before the TU sees the ACK, so an application cannot repair it.
• Red on main (5958064): cargo test --lib test_server_invite_ack → 1 failed: an ACK with CSeq 1 must not be delivered by the CSeq 2 INVITE transaction.
• Green on this branch: cargo test → 331 lib + 65 doc passed; cargo fmt --all -- --check clean; clippy warning set identical to main.
• Upstream: upstream-ready, queued (first in the trickle; coordinate with open feat(transaction): RFC 6026 Accepted state + Timer L/M for INVITE 200 OK retransmission absorption restsend/rsipstack#128).
• Codex adversarial review: 1 finding kept out of scope: after this fix, a delayed ACK of re-INVITE N is ignored rather than routed back to a still-live transaction N, because waiting_ack holds one entry per dialog (the N+1 entry overwrote N before this change too, so this is not a regression; proper routing needs a CSeq-aware key). 1 Minor (test naming follows the existing Timer G implementation rather than RFC 6026 Accepted) waived to stay consistent with the current code.

The bug

A server INVITE transaction in Completed or Confirmed moves to Confirmed on any ACK (transaction.rs, the Completed | Confirmed if req.method == Method::Ack arm). It never checks the ACK's CSeq.

An ACK for a 2xx starts a new transaction (new branch), so the endpoint routes it per dialog through waiting_ack (endpoint.rs, on_received_message). That means a delayed ACK of an earlier re-INVITE on the same dialog reaches the transaction of the current re-INVITE. That transaction then:

• moves to Confirmed and stops Timer G, so a lost 2xx is never retransmitted and the UAC never gets the answer;
• removes its waiting_ack entry (the Confirmed transition does this since the waiting_ack leak fix), so the real ACK can no longer be routed and is dropped;
• hands the stale ACK to the TU as if it acknowledged this INVITE.

We hit this in production with back-to-back re-INVITEs over UDP.

RFC references

• RFC 3261 §17.1.1.3: the ACK for a non-2xx response has the same CSeq number as the INVITE.
• RFC 3261 §13.2.2.4: the CSeq number of the ACK for a 2xx must equal the INVITE's.

So an ACK with any other CSeq number does not acknowledge this INVITE.

Reproduction

New test src/transaction/tests/test_server_invite_ack.rs::test_server_invite_ignores_ack_with_other_cseq uses real UDP sockets and the endpoint serve loop. It follows the same pattern as test_server_invite_drop.rs.

1. The client sends a re-INVITE with CSeq: 2 on an established dialog, and the server replies 200 (state Completed, Timer G armed).
2. The client sends an ACK with CSeq: 1 ACK on the same dialog: a delayed ACK of the previous re-INVITE.
3. The test asserts the stale ACK is not delivered, the state stays Completed, Timer G is still armed, the waiting_ack route is still there, and the 200 is retransmitted.
4. The client sends the ACK with CSeq: 2 ACK. The test asserts it is delivered, the state is Confirmed, Timer G is stopped and waiting_ack is empty.

On current main the test fails at step 3:

an ACK with CSeq 1 must not be delivered by the CSeq 2 INVITE transaction, got Some("ACK sip:bob@127.0.0.1 SIP/2.0\r\n...CSeq: 1 ACK\r\n...")

The fix

In that arm, compare the ACK's CSeq number with self.original's. On a mismatch, log at debug level and return None before touching any state. There is no transition, Timer G keeps running and nothing goes to the TU. This is a 16-line change in src/transaction/transaction.rs, plus the test.

Compatibility / risk

• ACKs that follow the RFC are unaffected. A non-2xx ACK (hop-by-hop, same branch) and a 2xx ACK both carry the INVITE's CSeq number, so both still confirm.
• A request with a missing or unparseable CSeq never reaches this point, because TransactionKey::from_request rejects it first.
• No public API change.
• CI gates pass: cargo build, cargo test (331 lib + 65 doc; main has 330 lib) and cargo fmt --all -- --check. cargo clippy --all-targets shows no new warnings. Its one error (never_loop in tests/test_endpoint.rs) already fails the same way on main.

Out of scope, and left unchanged to keep this minimal:

• The ACK method in CSeq is not validated: an ACK carrying CSeq: 2 INVITE is still accepted, as it is today.
• on_received_message's finished_transactions ACK path (for a transaction the TU already dropped) removes waiting_ack without a CSeq check. That path absorbs the ACK silently either way, so it has no effect on retransmission.

Happy to follow up on either.

Relation to restsend#128 (RFC 6026 Accepted state)

restsend#128 does not address this. It keeps the same unchecked Completed | Confirmed ACK arm, and its new Accepted arm also forwards any ACK routed to it to the TU.

The two changes overlap textually in on_received_request, so whichever lands second needs a small rebase. They are compatible: with restsend#128, the same CSeq-number guard should also go in the Accepted arm, since Accepted is where the 2xx ACKs land. In that arm a stale ACK can't stop retransmissions, but it would still reach the TU and be taken for the real ACK. I'm glad to rebase either way.

🤖 Generated with Claude Code

https://claude.ai/code/session_01L1Gu5CifqgBjASbxYmJ6mr