Skip to content

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

Open
tgeorge06 wants to merge 1 commit into
restsend:mainfrom
tgeorge06:fix/ack-cseq-mismatch
Open

tgeorge06 wants to merge 1 commit into
restsend:mainfrom
tgeorge06:fix/ack-cseq-mismatch

Conversation

@tgeorge06

Copy link
Copy Markdown

Fixes #154.

Problem

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 of on_received_request); it never checks the ACK's CSeq.

An ACK for a 2xx has its own branch, so EndpointInner::on_received_message routes it per dialog through waiting_ack. A delayed ACK of an earlier INVITE or re-INVITE on the same dialog therefore reaches the transaction of the current re-INVITE. That transaction then stops Timer G (a lost 2xx is never retransmitted), removes its waiting_ack entry (the real ACK loses its route and is dropped), and hands the stale ACK to the TU as this re-INVITE's ACK.

Spec

  • RFC 3261 §17.1.1.3: the ACK for a non-2xx has the same CSeq number as the INVITE.
  • RFC 3261 §13.2.2.4: the CSeq number of the ACK for a 2xx equals the INVITE's.
  • RFC 3261 §13.3.1.4: the 2xx is retransmitted until the ACK for it arrives.

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: no transition, Timer G and the waiting_ack route stay as they are, nothing goes to the TU. 16 lines in src/transaction/transaction.rs, plus tests. No public API change.

Contract / coverage

Left out to keep this minimal (happy to follow up):

  • The CSeq method is not checked: an ACK carrying CSeq: 2 INVITE is still accepted, as today.
  • The finished_transactions ACK path in on_received_message (transaction already dropped by the TU) removes the waiting_ack entry without a CSeq check. The ACK is absorbed silently there either way and nothing is retransmitted any more, so there is no observable effect.
  • waiting_ack holds one entry per dialog, so a delayed ACK of re-INVITE N is now ignored rather than routed back to a still-live transaction N (the N+1 entry already overwrote N before this change).

Tests

New src/transaction/tests/test_server_invite_ack.rs (real sockets and the endpoint serve loop, same pattern as test_server_invite_drop.rs):

  • test_server_invite_ignores_ack_with_other_cseq (UDP): re-INVITE CSeq: 2 answered 200; a delayed CSeq: 1 ACK is not delivered, the state stays Completed, Timer G keeps running, the waiting_ack route survives and the 200 is retransmitted; the CSeq: 2 ACK then confirms, stops Timer G and clears waiting_ack.
  • test_server_invite_ignores_ack_with_other_cseq_over_tcp: same dialog over TCP; the CSeq: 1 and CSeq: 2 ACKs are written back to back on the ordered stream, and the first ACK the transaction delivers is the CSeq: 2 one, which confirms it and clears waiting_ack.

Checks (on main @ 3ea35cd)

  • Red: with the src/ change reverted and the tests kept, cargo test --lib test_server_invite_ack → 2 failed (an ACK with CSeq 1 must not be delivered by the CSeq 2 INVITE transaction, got Some("ACK … CSeq: 1 ACK …") over UDP; left: 1, right: 2 over TCP).
  • Green: cargo test → 334 lib + 65 doc passed, 0 failed (main: 332 lib). rustfmt --check clean on the changed files.

ACKs for 2xx are routed to the server INVITE transaction per dialog
(waiting_ack), so a delayed ACK of an earlier re-INVITE on the same
dialog reaches the transaction of the current re-INVITE. Any ACK used
to move it to Confirmed: Timer G stopped retransmitting the 2xx, the
waiting_ack entry was removed so the real ACK lost its route, and the
stale ACK was handed to the TU as if it acknowledged this INVITE.

An ACK carries the CSeq number of the INVITE it acknowledges (RFC 3261
§17.1.1.3, §13.2.2.4). Ignore an ACK with any other CSeq number before
touching the transaction state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server INVITE transaction is confirmed by an ACK with a different CSeq (delayed ACK of an earlier re-INVITE)

1 participant