From 00fce81f1398be041f409e71cfde3169fa78841b Mon Sep 17 00:00:00 2001 From: Repin Agent Date: Fri, 17 Jul 2026 08:20:44 -0600 Subject: [PATCH] =?UTF-8?q?fix(sip):=20M7=20follow-up=20batch=20=E2=80=94?= =?UTF-8?q?=20re-INVITE=20From=20identity=20+=20CP3=20early-media=20teeth?= =?UTF-8?q?=20+=20CP1=20ACK=20labels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small items from the M7 independent reviews: 1. CMD-MINOR (CP2 From consistency): build_reinvite hardcoded sip:asterisk@ in From, so a re-INVITE on a carrier dialog with from_user configured reverted to the internal identity mid-dialog (ConfBridge SFU-video path), contradicting the "From identity stays stable for the dialog's lifetime" invariant. build_reinvite and build_reinvite_ack now use from_uri() like the INVITE/BYE. Red-capable regression: reinvite_and_its_ack_from_match_configured_identity. 2. WIRE-MINOR-1 (CP3 test teeth): the NO-RTP-before-answer guard could only ever fail as an orphaned-200 — no code path emitted app media before answer. The harness now (a) runs an early-media scenario where the carrier sends 183 Session Progress WITH SDP before its delayed 200 (same To tag, one early dialog), so a pre-answer media path EXISTS, and (b) plays a real generated G.711u file via Playback() and REQUIRES post-answer RTP > 0, proving the app media pump delivers. Together: if the Originate wait were defeated so the app ran on the 183, the app's Playback RTP arrives pre-answer and the RTP phase=pre == 0 guard REDs on its own (captured: 52 pre-answer datagrams under a 183-treated-as-answer defeat). 3. WIRE-MINOR-2 (CP1 label clarity): carrier.py labeled ANY non-challenge ACK as ACK-2XX-AT-CORE by a coarse branch match, mislabeling a 403 retry's non-2xx ACK (which correctly returns to the core with the rejected INVITE's branch per RFC 3261 §17.1.1.3) as a route-set violation. ACKs are now labeled per-transaction: ACK-CHALLENGE / ACK-CHALLENGE-RETX / ACK-NON2XX-AT-CORE / ACK-2XX-AT-CORE. Existing run.sh assertions unchanged. TEST values only; isolated offline Docker harnesses; live stack untouched. --- crates/asterisk-sip/src/session/mod.rs | 88 ++++++++++++++++++- tests/cp1-origination-challenge/carrier.py | 26 +++++- tests/cp3-originate-wait/carrier_delay.py | 27 +++++- .../{extensions.conf => extensions.conf.tmpl} | 1 + tests/cp3-originate-wait/run.sh | 75 +++++++++++++--- 5 files changed, 197 insertions(+), 20 deletions(-) rename tests/cp3-originate-wait/config/{extensions.conf => extensions.conf.tmpl} (74%) diff --git a/crates/asterisk-sip/src/session/mod.rs b/crates/asterisk-sip/src/session/mod.rs index de5dd9b..1709e01 100644 --- a/crates/asterisk-sip/src/session/mod.rs +++ b/crates/asterisk-sip/src/session/mod.rs @@ -711,6 +711,10 @@ impl SipSession { /// Used by the SFU ConfBridge to add/remove video streams for participants. pub fn build_reinvite(&mut self, sdp: &SessionDescription) -> Option { let sig = self.signaling_hostport(); + // From identity must match the INVITE's (same user@domain + local tag) + // for the dialog's lifetime — a re-INVITE that reverts to the internal + // identity breaks carriers that key in-dialog requests on From. + let from_base = self.from_uri(); let dialog = self.dialog.as_mut()?; let cseq = dialog.next_cseq(); @@ -727,8 +731,8 @@ impl SipSession { let branch = format!("z9hG4bK{}", &Uuid::new_v4().to_string().replace('-', "")[..16]); - // For UAS (inbound call), From = our local tag, To = remote tag. - let from_value = format!(";tag={}", sig, dialog.local_tag); + // From = our identity + local tag, To = remote URI + remote tag. + let from_value = format!("<{}>;tag={}", from_base, dialog.local_tag); let to_value = format!("<{}>;tag={}", dialog.remote_uri, dialog.remote_tag); let body = sdp.to_string(); @@ -776,7 +780,8 @@ impl SipSession { let branch = format!("z9hG4bK{}", &Uuid::new_v4().to_string().replace('-', "")[..16]); let sig = self.signaling_hostport(); - let from_value = format!(";tag={}", sig, dialog.local_tag); + // Same From identity as the re-INVITE this ACKs (dialog-stable). + let from_value = format!("<{}>;tag={}", self.from_uri(), dialog.local_tag); let to_value = format!("<{}>;tag={}", dialog.remote_uri, dialog.remote_tag); // CSeq from the response we're ACKing. @@ -1054,4 +1059,81 @@ mod cp2_tests { assert!(from.contains("sip:+19995551234@carrier.example.net"), "BYE From wrong: {from}"); assert!(!from.contains("asterisk@"), "BYE From still hardcodes asterisk: {from}"); } + + #[test] + fn reinvite_and_its_ack_from_match_configured_identity() { + // M7 follow-up (CMD-MINOR): the in-dialog re-INVITE (ConfBridge SFU + // video renegotiation) and its ACK must carry the SAME From + // user@domain as the INVITE/BYE — the From identity is stable for the + // dialog's lifetime. A build_reinvite that reverts to the hardcoded + // internal `asterisk@` identity fails this. + let mut s = SipSession::new_outbound(addr("10.0.0.1:5060"), addr("10.0.0.2:5060")); + s.from_user = Some("+19995551234".to_string()); + s.from_domain = Some("carrier.example.net".to_string()); + let invite = s.build_invite_with_uri("sip:+15550001111@10.0.0.2:5060", "sip:+15550001111@10.0.0.2"); + let ok = SipMessage::parse( + format!( + "SIP/2.0 200 OK\r\n\ + Via: {via}\r\n\ + From: {from}\r\n\ + To: ;tag=c200\r\n\ + Call-ID: {cid}\r\n\ + CSeq: 1 INVITE\r\n\ + Contact: \r\n\ + Content-Length: 0\r\n\r\n", + via = invite.get_header(header_names::VIA).unwrap(), + from = invite.from_header().unwrap(), + cid = s.call_id, + ) + .as_bytes(), + ) + .unwrap(); + s.on_response(&ok); + + let sdp = SessionDescription::parse( + "v=0\r\n\ + o=rustisk 0 1 IN IP4 10.0.0.1\r\n\ + s=-\r\n\ + c=IN IP4 10.0.0.1\r\n\ + t=0 0\r\n\ + m=audio 30000 RTP/AVP 0\r\n\ + a=rtpmap:0 PCMU/8000\r\n", + ) + .unwrap(); + let reinvite = s.build_reinvite(&sdp).unwrap(); + let from = reinvite.from_header().unwrap().to_string(); + assert!( + from.contains("sip:+19995551234@carrier.example.net"), + "re-INVITE From must keep the configured identity, got: {from}" + ); + assert!(!from.contains("asterisk@"), "re-INVITE From reverts to the internal identity: {from}"); + + // The ACK for the re-INVITE's 200 carries the same From identity. + let reinvite_cseq = reinvite.cseq().unwrap().to_string(); + let reinvite_200 = SipMessage::parse( + format!( + "SIP/2.0 200 OK\r\n\ + Via: {via}\r\n\ + From: {from}\r\n\ + To: ;tag=c200\r\n\ + Call-ID: {cid}\r\n\ + CSeq: {cseq}\r\n\ + Contact: \r\n\ + Content-Length: 0\r\n\r\n", + via = reinvite.get_header(header_names::VIA).unwrap(), + from = reinvite.from_header().unwrap(), + cid = s.call_id, + cseq = reinvite_cseq, + ) + .as_bytes(), + ) + .unwrap(); + let ack = s.build_reinvite_ack(&reinvite_200).unwrap(); + let from = ack.from_header().unwrap().to_string(); + assert!( + from.contains("sip:+19995551234@carrier.example.net"), + "re-INVITE ACK From must keep the configured identity, got: {from}" + ); + assert!(!from.contains("asterisk@"), "re-INVITE ACK From reverts to the internal identity: {from}"); + } } diff --git a/tests/cp1-origination-challenge/carrier.py b/tests/cp1-origination-challenge/carrier.py index 551e09a..9501654 100755 --- a/tests/cp1-origination-challenge/carrier.py +++ b/tests/cp1-origination-challenge/carrier.py @@ -181,6 +181,7 @@ def serve_core(sock, own, capture, route_target, realm, password, answer_delay): # Per-Call-ID state: challenged INVITE branch + retransmit bookkeeping. challenged = {} # call_id -> {"branch", "retx", "deadline", "acked"} answered = set() # call_id already 200'd + rejected = {} # call_id -> set of branches given a non-2xx final (403) RETX_INTERVAL = 1.0 MAX_RETX = 3 while True: @@ -234,6 +235,10 @@ def serve_core(sock, own, capture, route_target, realm, password, answer_delay): own, src[0], src[1], branch, cseq, "yes" if valid else "no", prev.get("branch"), call_id)) if not valid: sock.sendto(build_response(text, 403, "Forbidden", to_tag="cp1core403"), src) + # Remember this branch: its non-2xx ACK (same branch, RFC + # 3261 §17.1.1.3) correctly comes back to the core and must + # not be mislabeled as a 2xx-ACK route-set violation. + rejected.setdefault(call_id, set()).add(branch) continue if call_id in answered: continue @@ -253,10 +258,23 @@ def serve_core(sock, own, capture, route_target, realm, password, answer_delay): branch = branch_of(text) cseq = cseq_of(text) st = challenged.get(call_id) - if st and branch == st["branch"] and not st.get("marked_ack"): - st["acked"] = True - st["marked_ack"] = True - append_capture(capture, "ACK-CHALLENGE own=%s src=%s:%d branch=%s cseq=%s callid=%s" % ( + # Label by the SPECIFIC transaction the ACK belongs to (branch), + # not a coarse challenged-or-not match — a non-2xx ACK reuses its + # INVITE's branch (§17.1.1.3), while a 2xx ACK is a NEW transaction + # with a fresh branch. Only the latter at the core is a route-set + # violation (RED). + if st and branch == st["branch"]: + if not st.get("marked_ack"): + st["acked"] = True + st["marked_ack"] = True + append_capture(capture, "ACK-CHALLENGE own=%s src=%s:%d branch=%s cseq=%s callid=%s" % ( + own, src[0], src[1], branch, cseq, call_id)) + else: + append_capture(capture, "ACK-CHALLENGE-RETX own=%s src=%s:%d branch=%s cseq=%s callid=%s" % ( + own, src[0], src[1], branch, cseq, call_id)) + elif branch in rejected.get(call_id, set()): + # The 403'd retry's ACK — hop-by-hop, correctly at the core. + append_capture(capture, "ACK-NON2XX-AT-CORE own=%s src=%s:%d branch=%s cseq=%s callid=%s" % ( own, src[0], src[1], branch, cseq, call_id)) else: # A 2xx ACK reaching CORE means the route set was IGNORED (RED). diff --git a/tests/cp3-originate-wait/carrier_delay.py b/tests/cp3-originate-wait/carrier_delay.py index db0df86..5446437 100755 --- a/tests/cp3-originate-wait/carrier_delay.py +++ b/tests/cp3-originate-wait/carrier_delay.py @@ -12,6 +12,15 @@ leg down before the delayed 200; the 200 is never ACKed and no post-answer BYE appears. +--early-media (M7 follow-up, WIRE-MINOR-1): the carrier additionally sends a +183 Session Progress WITH SDP immediately (before its delayed 200), so a +pre-answer media path EXISTS — the caller knows the carrier's RTP address from +the 183. This gives the `RTP phase=pre == 0` guard independent teeth: if the +Originate wait were defeated so the app ran on the 183, the app's DTMF RTP +would arrive pre-answer and that guard alone REDs (instead of the defeat only +ever surfacing as an orphaned-200). The 183 and the 200 carry the SAME To tag +(one early dialog, confirmed by the 200). + Captures SIP on :5060 and RTP on :40000 (the SDP-answered media port). """ @@ -100,11 +109,19 @@ def main(): ap.add_argument("--caller", required=True) ap.add_argument("--capture", required=True) ap.add_argument("--answer-delay", type=float, default=3.0) + ap.add_argument("--early-media", action="store_true", + help="send 183 Session Progress WITH SDP before the delayed 200") args = ap.parse_args() own = own_ip_toward(args.caller) t0 = time.time() + # In early-media mode the 183 opens an early dialog; the 200 (and a 487 on + # CANCEL) must carry the SAME To tag or the caller rightly treats the final + # as a stray from a different dialog. + tag_200 = "cp3del183" if args.early_media else "cp3del200" + tag_487 = "cp3del183" if args.early_media else "cp3del487" + def cap(line): with open(args.capture, "a") as f: f.write(line + "\n") @@ -131,7 +148,7 @@ def phase(): if pending_200 is not None and time.time() >= pending_200[0]: send_at, inv, src = pending_200 pending_200 = None - ok = build_response(inv, 200, "OK", own, to_tag="cp3del200", sdp=carrier_sdp(own)) + ok = build_response(inv, 200, "OK", own, to_tag=tag_200, sdp=carrier_sdp(own)) sip.sendto(ok, src) answered = True cap("SENT-200 own=%s rel=%.3f" % (own, time.time() - t0)) @@ -157,6 +174,12 @@ def phase(): phase(), own, src[0], src[1], cseq_of(text), rel)) # 100 Trying immediately; 200 OK is DELAYED. sip.sendto(build_response(text, 100, "Trying", own), src) + if args.early_media and not answered: + # Early media: 183 WITH SDP before the delayed 200 — the + # pre-answer media path now exists (RTP-guard teeth). + sip.sendto(build_response(text, 183, "Session Progress", own, + to_tag=tag_200, sdp=carrier_sdp(own)), src) + cap("SENT-183 own=%s rel=%.3f" % (own, time.time() - t0)) if pending_200 is None and not answered: pending_200 = (time.time() + args.answer_delay, text, src) elif first.startswith("ACK "): @@ -174,7 +197,7 @@ def phase(): if pending_200 is not None: _, inv, isrc = pending_200 pending_200 = None - sip.sendto(build_response(inv, 487, "Request Terminated", own, to_tag="cp3del487"), isrc) + sip.sendto(build_response(inv, 487, "Request Terminated", own, to_tag=tag_487), isrc) if __name__ == "__main__": diff --git a/tests/cp3-originate-wait/config/extensions.conf b/tests/cp3-originate-wait/config/extensions.conf.tmpl similarity index 74% rename from tests/cp3-originate-wait/config/extensions.conf rename to tests/cp3-originate-wait/config/extensions.conf.tmpl index 0b8250a..ecd7d8d 100644 --- a/tests/cp3-originate-wait/config/extensions.conf +++ b/tests/cp3-originate-wait/config/extensions.conf.tmpl @@ -1,5 +1,6 @@ [default] exten => s,1,Answer() + same => n,Playback(@MEDIA_FILE@) same => n,SendDTMF(9876) same => n,Wait(1) same => n,Hangup() diff --git a/tests/cp3-originate-wait/run.sh b/tests/cp3-originate-wait/run.sh index 08ce4d7..23d15ea 100755 --- a/tests/cp3-originate-wait/run.sh +++ b/tests/cp3-originate-wait/run.sh @@ -9,19 +9,47 @@ # PRE-ANSWER SILENCE: in the pre-answer window the carrier receives ONLY the # INVITE — no ACK/BYE/CANCEL and no RTP (the app has not run). # POST-ANSWER RUN: after the 200 the answer is ACKed and the app runs — the -# BYE (and the app's DTMF RTP) appear phase=post. +# BYE and the app's Playback RTP appear phase=post. The app plays a real +# (generated, TEST-only) audio file, so post-answer RTP > 0 is REQUIRED: +# it proves the media pump delivers, making pre-answer RTP == 0 a real +# no-media-before-answer guarantee instead of a vacuous one. # # RED (revert CP3 -> app runs immediately): the app runs and tears the unanswered # leg down before the delayed 200; the 200 is orphaned (no post-answer ACK, no # BYE). Captured below by reverting the wait. # +# Scenarios (both run by default; select one with CP3_SCENARIO=baseline|early-media): +# baseline — carrier sends 100 then the DELAYED 200 (the original CP3 run). +# early-media — carrier ALSO sends a 183 Session Progress WITH SDP before the +# delayed 200 (M7 follow-up, WIRE-MINOR-1). A pre-answer media path now +# EXISTS, giving the `RTP phase=pre == 0` guard independent teeth: if the +# Originate wait were defeated so the app ran on the 183 (the classic +# "early media treated as answer" defect), the app's Playback RTP would +# arrive pre-answer and that guard alone REDs (in the baseline a defeated +# wait can only surface as an orphaned 200 — the app has no remote media +# address before the 200 there). +# # Isolated Docker only; never touches the live voice stack / carrier / real PIN. # tests/cp3-originate-wait/run.sh set -euo pipefail HARNESS_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" REPO_DIR="$(cd -- "$HARNESS_DIR/../.." && pwd)" -RUNTIME_DIR="$REPO_DIR/target/cp3-originate-wait" + +# With no CP3_SCENARIO set, run BOTH scenarios sequentially. +if [[ -z "${CP3_SCENARIO:-}" ]]; then + CP3_SCENARIO=baseline "${BASH_SOURCE[0]}" + CP3_SCENARIO=early-media "${BASH_SOURCE[0]}" + printf '\nPASS: both CP3 scenarios (baseline + early-media) passed.\n' + exit 0 +fi +case "$CP3_SCENARIO" in + baseline) EARLY_MEDIA=0 ;; + early-media) EARLY_MEDIA=1 ;; + *) printf 'FAIL: unknown CP3_SCENARIO=%s (baseline|early-media)\n' "$CP3_SCENARIO" >&2; exit 2 ;; +esac + +RUNTIME_DIR="$REPO_DIR/target/cp3-originate-wait/$CP3_SCENARIO" CONFIG_DIR="$RUNTIME_DIR/config" RUN_DIR="$RUNTIME_DIR/run" RUSTISK_LOG="$RUNTIME_DIR/rustisk.log" @@ -137,20 +165,29 @@ say "Building rustisk (Rust 1.97.0, CARGO_BUILD_JOBS=${CARGO_BUILD_JOBS:-6})..." sed -e "s|@CONFIG_DIR@|$CONFIG_DIR|g" -e "s|@RUN_DIR@|$RUN_DIR|g" \ "$HARNESS_DIR/config/asterisk.conf.tmpl" >"$CONFIG_DIR/asterisk.conf" cp "$HARNESS_DIR/config/manager.conf" "$CONFIG_DIR/manager.conf" -cp "$HARNESS_DIR/config/extensions.conf" "$CONFIG_DIR/extensions.conf" +# The app plays a real (generated, TEST-only) 1s G.711u file so app media +# genuinely flows: post-answer RTP > 0 proves the media pump works, which is +# what gives the pre-answer RTP == 0 guard its teeth (see verdict). +MEDIA_FILE="$RUNTIME_DIR/media/app-tone.ulaw" +mkdir -p "$RUNTIME_DIR/media" +python3 -c "import sys; sys.stdout.buffer.write(bytes(range(256)) * 32)" >"$MEDIA_FILE" +sed -e "s|@MEDIA_FILE@|$MEDIA_FILE|g" \ + "$HARNESS_DIR/config/extensions.conf.tmpl" >"$CONFIG_DIR/extensions.conf" cp "$HARNESS_DIR/config/rtp.conf" "$CONFIG_DIR/rtp.conf" printf '[general]\nsecret_file = /run/secrets/rustisk/pin\n' >"$CONFIG_DIR/pin_gate.conf" say "Creating isolated --internal network $NET..." docker network create --internal --subnet "$SUBNET" --ip-range "$IP_RANGE" "$NET" >/dev/null -say "Starting offline carrier (delays 200 by ${ANSWER_DELAY}s; captures SIP + RTP by phase)..." +EM_FLAG="" +(( EARLY_MEDIA )) && EM_FLAG="--early-media" +say "Starting offline carrier (scenario=$CP3_SCENARIO; delays 200 by ${ANSWER_DELAY}s; captures SIP + RTP by phase)..." docker run -d --rm --name "$CARRIER_CONTAINER" \ --network "$NET" --user "$(id -u):$(id -g)" \ --mount "type=bind,src=$HARNESS_DIR/carrier_delay.py,dst=/carrier_delay.py,readonly" \ --mount "type=bind,src=$RUNTIME_DIR,dst=/runtime" \ "$RUSTISK_IMAGE" python3 /carrier_delay.py --caller "$RUSTISK_IP" \ - --capture /runtime/carrier.log --answer-delay "$ANSWER_DELAY" >/dev/null + --capture /runtime/carrier.log --answer-delay "$ANSWER_DELAY" $EM_FLAG >/dev/null S="" for _ in $(seq 1 40); do S="$(container_ip "$CARRIER_CONTAINER")"; [[ -n "$S" ]] && break; sleep 0.25; done [[ -n "$S" ]] || fail "could not read carrier container IP" @@ -197,25 +234,41 @@ else POST="FAIL" fi -# Informational: app's DTMF media arrived (only) after answer. +# App media (the app's Playback RTP) must have arrived — and only after answer. +# RTP_POST > 0 proves the app's media pump actually delivers to the carrier, so +# RTP_PRE == 0 is a real no-media-before-answer guarantee, not a vacuous one. RTP_PRE="$(grep -c 'RTP phase=pre' "$CAPTURE" || true)" RTP_POST="$(grep -c 'RTP phase=post' "$CAPTURE" || true)" +# Early-media scenario: the pre-answer media path must have EXISTED (183 with +# SDP sent BEFORE the 200) — otherwise the RTP phase=pre check has no teeth. +EARLY="n/a" +if (( EARLY_MEDIA )); then + L183="$(grep -n -m1 'SENT-183 ' "$CAPTURE" | cut -d: -f1 || true)" + L200="$(grep -n -m1 'SENT-200 ' "$CAPTURE" | cut -d: -f1 || true)" + if [[ -n "$L183" && -n "$L200" ]] && (( L183 < L200 )); then EARLY="PASS"; else EARLY="FAIL"; fi +fi + VERDICT_OK=1 [[ "$SILENCE" == "PASS" ]] || VERDICT_OK=0 [[ "$POST" == "PASS" ]] || VERDICT_OK=0 [[ "${RTP_PRE:-0}" == "0" ]] || VERDICT_OK=0 +[[ "${RTP_POST:-0}" != "0" ]] || VERDICT_OK=0 +[[ "$EARLY" == "FAIL" ]] && VERDICT_OK=0 { - echo "CP3 wait-for-answer harness — PROOF" + echo "CP3 wait-for-answer harness — PROOF (scenario=$CP3_SCENARIO)" echo "generated: $(date -u +%FT%TZ)" echo "rustisk HEAD: $(cd "$REPO_DIR" && git rev-parse --short HEAD 2>/dev/null || echo unknown)" echo "answer delay: ${ANSWER_DELAY}s" echo echo "PRE-ANSWER SILENCE (only INVITE before the 200, no ACK/BYE/CANCEL/RTP): $SILENCE" echo "POST-ANSWER RUN (answer ACKed + app BYE after the 200): $POST" - echo "RTP datagrams pre-answer (must be 0): ${RTP_PRE:-0}" - echo "RTP datagrams post-answer (app media): ${RTP_POST:-0}" + if (( EARLY_MEDIA )); then + echo "EARLY-MEDIA PATH (183 with SDP sent before the 200): $EARLY" + fi + echo "RTP datagrams pre-answer (must be 0): ${RTP_PRE:-0}" + echo "RTP datagrams post-answer (app media, must be >0): ${RTP_POST:-0}" echo echo "--- carrier capture (receiver-side, phase-tagged, rel = seconds since start) ---" cat "$CAPTURE" 2>/dev/null || true @@ -227,8 +280,8 @@ cat "$PROOF" if (( VERDICT_OK == 1 )); then say '' - say "PASS: Originate stayed SILENT before answer and ran the app only after the delayed 200 (receiver-side)." + say "PASS ($CP3_SCENARIO): Originate stayed SILENT before answer and ran the app only after the delayed 200 (receiver-side)." exit 0 else - fail "CP3 harness verdict FAILED (see verdict above)" + fail "CP3 harness verdict FAILED (scenario=$CP3_SCENARIO; see verdict above)" fi