Skip to content

fix(sip): accept proxy-rewritten 2xx ACK routes #169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryanmurf merged 1 commit into from
Jul 19, 2026
Merged

fix(sip): accept proxy-rewritten 2xx ACK routes #169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 111 additions & 41 deletions crates/asterisk-sip/src/stack.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,6 @@ fn request_identity_matches(original: &SipMessage, retransmission: &SipMessage)
== retransmission.from_header().and_then(extract_tag)
}

fn ack_route_set_matches(invite: &SipMessage, ack: &SipMessage) -> bool {
let actual = ack.get_headers(header_names::ROUTE);
if actual.is_empty() {
return true;
}
let expected = invite.get_headers(header_names::RECORD_ROUTE);
actual.len() == expected.len()
&& actual.iter().zip(expected).all(|(actual, expected)| {
actual.split_whitespace().collect::<String>().eq_ignore_ascii_case(
&expected.split_whitespace().collect::<String>(),
)
})
}

/// Build the transaction ACK for a 300-699 INVITE response. It reuses the
/// INVITE branch and Request-URI, unlike the new-branch dialog ACK for 2xx.
fn build_non_2xx_ack(invite: &SipMessage, response: &SipMessage) -> Option<SipMessage> {
Expand Down Expand Up @@ -335,28 +321,78 @@ impl TransactionLayer {
/// its packet source identifies the INVITE server transaction. Stateful
/// SIP proxy pools may deliver the ACK from a different tuple than the
/// INVITE. Match the dialog identity and INVITE CSeq instead (RFC 3261
/// Sections 12.2.1.1 and 13.2.2.4). The TU separately performs full source
/// tuple and dialog validation before accepting any application effects.
/// Sections 12.2.1.1 and 13.2.2.4). Route headers are deliberately not a
/// transaction identity predicate: proxies consume and may rewrite them
/// while forwarding an in-dialog request. The TU separately performs full
/// source tuple and dialog validation before accepting application effects.
fn accepted_ack_branch(&self, ack: &SipMessage, _src: SocketAddr) -> Option<String> {
let call_id = ack.call_id()?;
let from_tag = ack.from_header().and_then(extract_tag)?;
let to_tag = ack.to_header().and_then(extract_tag)?;
let ack_cseq = cseq_number(ack)?;

self.invite_server_txns.iter().find_map(|(branch, txn)| {
if txn.state != crate::transaction::InviteServerState::Accepted
|| txn.request.call_id() != Some(call_id)
|| txn.request.from_header().and_then(extract_tag).as_deref()
!= Some(from_tag.as_str())
|| cseq_number(&txn.request) != Some(ack_cseq)
|| !ack_route_set_matches(&txn.request, ack)
let Some(call_id) = ack.call_id() else {
debug!(reason = "missing Call-ID", "ACK identity was incomplete");
return None;
};
let Some(from_tag) = ack.from_header().and_then(extract_tag) else {
debug!(reason = "missing From tag", "ACK identity was incomplete");
return None;
};
let Some(to_tag) = ack.to_header().and_then(extract_tag) else {
debug!(reason = "missing To tag", "ACK identity was incomplete");
return None;
};
let Some(ack_cseq) = cseq_number(ack) else {
debug!(reason = "missing CSeq", "ACK identity was incomplete");
return None;
};

let mut accepted_candidates = 0usize;
let mut call_id_matches = 0usize;
let mut from_tag_matches = 0usize;
let mut cseq_matches = 0usize;
let mut response_to_tags = 0usize;
let mut to_tag_mismatches = 0usize;

for (branch, txn) in &self.invite_server_txns {
if txn.state != crate::transaction::InviteServerState::Accepted {
continue;
}
accepted_candidates += 1;
if txn.request.call_id() != Some(call_id) {
continue;
}
call_id_matches += 1;
if txn.request.from_header().and_then(extract_tag).as_deref()
!= Some(from_tag.as_str())
{
return None;
continue;
}
let response_to_tag = txn.last_response.as_ref()?
.to_header().and_then(extract_tag)?;
(response_to_tag == to_tag).then(|| branch.clone())
})
from_tag_matches += 1;
if cseq_number(&txn.request) != Some(ack_cseq) {
continue;
}
cseq_matches += 1;
let Some(response_to_tag) = txn.last_response.as_ref()
.and_then(SipMessage::to_header)
.and_then(extract_tag)
else {
continue;
};
response_to_tags += 1;
if response_to_tag != to_tag {
to_tag_mismatches += 1;
continue;
}
return Some(branch.clone());
}

debug!(
accepted_candidates,
call_id_matches,
from_tag_matches,
cseq_matches,
response_to_tags,
to_tag_mismatches,
"ACK did not match an accepted INVITE dialog identity"
);
None
}

/// Collect branches that need retransmission for INVITE client transactions.
Expand Down Expand Up @@ -1713,7 +1749,8 @@ mod tests {

/// The TU retains and retransmits a successful final until its dialog ACK
/// arrives. A production proxy pool may change the ACK's Via stack and
/// source tuple; dialog identity, not transport tuple, matches the 2xx ACK.
/// source tuple and may consume or rewrite Route hops; dialog identity
/// matches the 2xx ACK.
#[tokio::test]
async fn test_2xx_retransmits_until_new_branch_ack_reaches_tu() {
let addr: SocketAddr = "127.0.0.1:0".parse().unwrap();
Expand All @@ -1723,7 +1760,14 @@ mod tests {
let peer_addr = peer.local_addr().unwrap();

let mut invite = build_invite_request("accepted-call", "z9hG4bKaccepted");
invite.add_header("Record-Route", "<sip:proxy.example.com;lr>");
invite.add_header(
"Record-Route",
"<sip:edge.example.invalid:5060;r2=on;lr;ftag=c55;did=fixture;nat=yes>",
);
invite.add_header(
"Record-Route",
"<sip:edge.example.invalid;transport=tcp;r2=on;lr;ftag=c55;did=fixture;nat=yes>",
);
stack.handle_request(invite.clone(), peer_addr).await;
let _ = rx.recv().await;
let mut ok = invite.create_response(200, "OK").unwrap();
Expand All @@ -1739,11 +1783,31 @@ mod tests {
assert_eq!(recv_peer(&peer).await.to_string(), ok.to_string(),
"the cached 2xx must be byte-equivalent");

let mut forged_ack =
build_2xx_ack("accepted-call", "z9hG4bKforged-ack-branch", "server-tag");
forged_ack.add_header("Route", "<sip:attacker.example.com;lr>");
stack.handle_request(forged_ack, peer_addr).await;
assert!(rx.try_recv().is_err(), "forged route-set ACK must not reach TU");
let forged_to_tag =
build_2xx_ack("accepted-call", "z9hG4bKforged-to-branch", "wrong-server-tag");
stack.handle_request(forged_to_tag, peer_addr).await;
assert!(rx.try_recv().is_err(), "wrong To-tag ACK must not reach TU");

let mut forged_from_tag =
build_2xx_ack("accepted-call", "z9hG4bKforged-from-branch", "server-tag");
forged_from_tag.headers.iter_mut()
.find(|header| header.name.eq_ignore_ascii_case(header_names::FROM))
.unwrap().value = "<sip:caller@127.0.0.1>;tag=wrong-caller-tag".to_string();
stack.handle_request(forged_from_tag, peer_addr).await;
assert!(rx.try_recv().is_err(), "wrong From-tag ACK must not reach TU");

let forged_call_id =
build_2xx_ack("wrong-call", "z9hG4bKforged-call-branch", "server-tag");
stack.handle_request(forged_call_id, peer_addr).await;
assert!(rx.try_recv().is_err(), "wrong Call-ID ACK must not reach TU");

let mut forged_cseq =
build_2xx_ack("accepted-call", "z9hG4bKforged-cseq-branch", "server-tag");
forged_cseq.headers.iter_mut()
.find(|header| header.name.eq_ignore_ascii_case(header_names::CSEQ))
.unwrap().value = "2 ACK".to_string();
stack.handle_request(forged_cseq, peer_addr).await;
assert!(rx.try_recv().is_err(), "wrong CSeq ACK must not reach TU");
assert_eq!(
stack
.transaction_layer
Expand All @@ -1753,7 +1817,7 @@ mod tests {
.unwrap()
.state,
crate::transaction::InviteServerState::Accepted,
"forged ACK must not stop 2xx retransmission"
"forged dialog ACKs must not stop 2xx retransmission"
);

let ack_peer = tokio::net::UdpSocket::bind("127.0.0.2:0").await.unwrap();
Expand All @@ -1763,6 +1827,12 @@ mod tests {
"Via",
"SIP/2.0/UDP proxy.invalid:5060;branch=z9hG4bKproxy-hop;rport",
);
// Production-shaped double Record-Route topologies may consume one
// hop and rewrite the surviving Route before the ACK reaches the UAS.
ack.add_header(
"Route",
"<sip:rewritten-edge.example.invalid:5080;transport=udp;lr;nat=yes>",
);
let mut expected_ack = ack.clone();
expected_ack.stamp_via_received_rport(ack_peer_addr);
stack.handle_request(ack.clone(), ack_peer_addr).await;
Expand Down
12 changes: 11 additions & 1 deletion tests/e2e-trunk/chime_caller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,10 +318,20 @@ def main():
if not rip or rip == "0.0.0.0":
rip = args.dst_ip
log(f"answered: PCMU_PT={pcmu_pt} TEV_PT={tev_pt} remote_media={rip}:{rport}")
# A production proxy may consume one hop from the captured double
# Record-Route topology and rewrite the surviving Route before the ACK
# reaches rustisk. Exercise that shape end-to-end without retaining any
# production carrier address or identifier.
ack_route = ""
if args.invite_fixture:
ack_route = (
"Route: <sip:rewritten-edge.example.invalid:5080;"
"transport=udp;lr;nat=yes>\r\n"
)
ack = (
f"ACK {ruri} SIP/2.0\r\n"
f"Via: SIP/2.0/UDP {args.src_ip}:{args.sip_port};branch=z9hG4bK{random.randint(0, 1 << 32):08x};rport\r\n"
f"Max-Forwards: 70\r\nFrom: <sip:chime@{args.src_ip}>;tag={fromtag}\r\n"
f"Max-Forwards: 70\r\n{ack_route}From: <sip:chime@{args.src_ip}>;tag={fromtag}\r\n"
f"To: <{ruri}>;tag={totag}\r\nCall-ID: {callid}\r\nCSeq: {cseq} ACK\r\nContent-Length: 0\r\n\r\n"
)
sipsock.sendto(ack.encode(), (args.dst_ip, args.dst_port))
Expand Down
Loading