feat(sip): M6 CP3 — external_media_address FQDN-vs-literal in SDP, fail-closed on DNS failure - #151
Merged
Merged
Conversation
added 2 commits
July 17, 2026 03:43
…ail closed on DNS failure (M6 CP3) external_media_address was emitted verbatim into c=/o= — an FQDN produced a malformed 'c=IN IP4 myhost.example.com'. CP3 interprets it: an IP literal is emitted as-is; an FQDN is resolved to an IP (preferring the peer's address family). On DNS failure it FAILS CLOSED — advertised_media_ip now returns Option<String> and every caller rejects the call setup rather than advertise an unresolved FQDN or fall back to a leaky internal address: - outbound offer (channel_driver::request) -> Err, dial fails - inbound INVITE answer -> 488 Not Acceptable Here - UPDATE renegotiation answer -> 488 - re-INVITE answer -> 488 Receiver-side proof (e2e_media_fail_closed.rs): a peer sends an INVITE; an IP-literal external is ANSWERED 200 with that c= (positive control), an unresolvable FQDN external is REJECTED with a 4xx and NEVER 200. RED: fall open (emit the unresolved FQDN) -> the FQDN case is accepted/answered -> assertion RED. Plus unit tests: IP-literal passthrough, FQDN resolves, unresolvable fails-closed (None), and end-to-end advertised_media_ip_with fail-closed.
…port selection, bounded DNS - F1 (MAJOR): resolve the SDP answer BEFORE mutating the media plane in the UPDATE and re-INVITE renegotiation paths, so a fail-closed rejection leaves the established call's media/hold/next-hop untouched instead of repointing the pump and then 488-ing the peer. - F2 (MAJOR): select the media transport that COVERS local_addr first, then read its external_media_address (same fix as CP2 F1 for signaling). Prevents a same-ip/wildcard transport's unresolvable external FQDN from rejecting a normal call on a transport that has no external address. Unit test added. - F3 (MAJOR): bound the blocking FQDN resolution with a 3s timeout on a scratch thread — a slow NSS/DNS lookup times out to None (fail closed) rather than stalling the serialized SIP control path. Full async+TTL DNS remains M1/M-l. - F4 (test): filter the fail-closed e2e response collection by Call-ID so an earlier call's retransmit cannot contaminate the assertion.
Owner
Author
Codex review disposition (gpt-5.5, xhigh)Codex verified all four callers handle None, IPv4/IPv6 literal + family preference, empty-resolution None, initial-INVITE cleanup parity, gating, non-trivial positive control, and the captured RED. Four findings, all FIXED (0b8715b):
CI green; workspace test + clippy green locally. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
M6 CP3 — FQDN-vs-literal in SDP, fail-closed on DNS failure
FINDING.
external_media_addresswas an uninterpreted string emitted verbatim into SDPc=/o=. A configured FQDN produced a malformedc=IN IP4 myhost.example.com; there was noresolution and no fail-closed posture.
FIX. Interpret
external_media_addressfor a non-local (external) peer:IPv6 media address), else any resolved address.
advertised_media_ipnow returnsOption<String>(None= failclosed), and every caller rejects the call setup rather than advertise an unresolved FQDN or fall
back to a leaky internal address:
channel_driver::request) →Err, the dial fails488 Not Acceptable Here488488The resolution is blocking, matching the existing blocking route-selection in the same function;
async resolution + TTL caching is a separate M1/M-l hardening, out of this fail-closed scope.
RECEIVER-SIDE PROOF (
e2e_media_fail_closed.rs). A SIP peer sends an INVITE and observesrustisk's response:
203.0.113.99→ ANSWERED200withc=203.0.113.99.no-such-host.invalid(RFC 6761 reserved) → theINVITE is NOT accepted, rustisk sends a 4xx and NEVER a
200(no bogus/internalc=reaches thepeer).
CAPTURED RED. Make the resolution fall open (emit the unresolved FQDN instead of
None):Plus unit tests: IP-literal passthrough (v4/v6), FQDN resolves (localhost → loopback IP), and
advertised_media_ip_withend-to-end fail-closed (must not fall back to the internal bind).GATES.
cargo test --workspace --exclude pjsip-shimgreen;cargo clippy --workspace --exclude pjsip-shim -- -D warningsclean. Rust 1.97.0.