Skip to content

feat(sip): M6 CP2 — external signaling address + port on all Via/Contact/From (New-3) - #150

Merged
ryanmurf merged 2 commits into
masterfrom
feat/m6-cp2-external-signaling-address-port
Jul 17, 2026
Merged

feat(sip): M6 CP2 — external signaling address + port on all Via/Contact/From (New-3)#150
ryanmurf merged 2 commits into
masterfrom
feat/m6-cp2-external-signaling-address-port

Conversation

@ryanmurf

Copy link
Copy Markdown
Owner

M6 CP2 — external_signaling_address on all Via/Contact/From sites + external signaling PORT (New-3)

FINDING. external_signaling_address was parsed into TransportConfig but applied to NO
Via/Contact/From builder — every signaling header interpolated the raw bind local_addr. A NAT
deployment leaked the internal address in Via/Contact/From, and any topology where the advertised
port differs from the bind port (external port forward) died at the first in-dialog request (New-3).

FIX.

  • New advertised_signaling_hostport(local, remote) in sdp/mod.rs, mirroring
    advertised_media_ip's transport-coverage lookup + local_net scoping: a peer OUTSIDE
    local_net gets external_signaling_address (with the new external_signaling_port override,
    else the bind port); a peer INSIDE gets the internal bind host:port. IPv6 literals are
    bracketed; FQDNs pass through (legal in a SIP sent-by / URI host).
  • New external_signaling_port: Option<u16> on TransportConfig (New-3) + config parse.
  • SipSession::signaling_hostport() is the per-session entry point (scoped by the session's
    remote peer). Applied to every signaling builder: build_invite_with_uri (Via/From/Contact),
    build_200_ok (Contact), build_ack (Via), build_bye (Via/From), build_reinvite
    (Via/From/Contact), build_reinvite_ack (Via/From), and the event_handler UPDATE / re-INVITE
    200 OK Contacts. No behavior change when no external is configured or the peer is local — the
    bind host:port is returned unchanged, so non-NAT deployments are unaffected.

RECEIVER-SIDE PROOF (e2e_signaling_address.rs). A SIP peer establishes a call and CAPTURES
rustisk's real datagrams: the INVITE 200 OK Contact and the rtptimeout BYE Via sent-by +
From URI. It asserts EXTERNAL 203.0.113.99:6666 for a peer outside local_net, and the INTERNAL
bind 127.0.0.1:<bind-port> (explicitly != 6666) for a peer inside local_net. The BYE is still
physically delivered to the peer's real transport address, so the advertised external address is
proven at the receiver independent of the send target. Plus 7 unit tests on the scoping core (port
override applies; local_net bypass; IPv6 bracketing; FQDN; foreign-transport no-donate).

CAPTURED RED. Force signaling_hostport to always return the internal bind (defeat scoping):

assertion `left == right` failed: external peer must see the external signaling ADDRESS in Contact
  left: "127.0.0.1"
 right: "203.0.113.99"

GATES. cargo test --workspace --exclude pjsip-shim green; cargo clippy --workspace --exclude pjsip-shim -- -D warnings clean. Rust 1.97.0.

Repin Agent added 2 commits July 17, 2026 03:19
…t/From (M6 CP2, New-3)

external_signaling_address was parsed but never applied to any Via/Contact/From
builder. Apply it — plus a new external_signaling_port override (New-3) — to
every signaling builder (build_invite/build_200_ok/build_ack/build_bye/
build_reinvite/build_reinvite_ack, and the event_handler UPDATE/re-INVITE 200 OK
Contacts), transport-scoped by local_net exactly like advertised_media_ip does
for SDP: a peer outside local_net sees the external address AND external port; a
peer inside local_net sees the internal bind. This lets a NAT/forward map an
external port to a different internal bind port without breaking the first
in-dialog request (which targets the advertised Contact).

New advertised_signaling_hostport(local, remote) in sdp/mod.rs mirrors
advertised_media_ip's transport-coverage + local_net logic and brackets IPv6
literals. SipSession::signaling_hostport() is the per-session entry point.

Receiver-side proof (e2e_signaling_address.rs): a peer establishes a call and
CAPTURES rustisk's 200 OK Contact and rtptimeout BYE Via+From datagrams,
asserting external addr:port for a non-local peer and internal bind for a
local_net peer. RED: force signaling_hostport to the internal bind -> the
external peer sees 127.0.0.1 not 203.0.113.99. Plus 7 unit tests on the scoping
core (port override, local_net bypass, IPv6 bracketing, FQDN, foreign-transport
no-donate).
…onger e2e

- F1 (High): select the transport that COVERS local_addr first, then read its
  NAT config, instead of filtering for transports that HAVE an external address.
  Stops a same-ip/wildcard transport from donating its external addr/port to a
  covering transport that deliberately set none. Unit test added.
- F6 (Low): reject external_signaling_port=0 and malformed values (warn, fall
  back to the bind port) rather than silently advertising an unusable port 0.
- Strengthen e2e: assert the internal-peer BYE From host:port (was logged but
  not asserted) and that the external port override never reaches it.
@ryanmurf

Copy link
Copy Markdown
Owner Author

Codex review disposition (gpt-5.5, xhigh)

Codex verified the happy path, the port override / bind-port fallback, format_hostport (IPv6/bracketed/IPv4/FQDN), the borrow ordering, the no-external passthrough, the receiver-side capture, and the RED. Six findings; disposition:

  • F1 (High) — FIXED (05afed7): transport selection filtered for external_signaling_address.is_some() before applying bind-coverage precedence, so a same-ip/wildcard transport could donate its external to a covering transport that set none. Now select the COVERING transport first (exact ip+port → exact ip → wildcard), then read its NAT config. Unit test test_signaling_covering_transport_without_external_wins added. (The pre-existing advertised_media_ip shares this pattern; flagged for a parity hardening — not changed here to keep CP2 signaling-scoped.)
  • F6 (Low) — FIXED (05afed7): external_signaling_port=0 and malformed values now warn and fall back to the bind port instead of advertising an unusable port 0.
  • F2 (Medium, accuracy) — wording corrected + assessed: the other builders are not live-leaking today. session_ext::build_reinvite has NO production caller (dead/alternate builder). OutboundRegistration is only re-exported, never instantiated — the outbound REGISTER Contact/Via NAT-scoping lands with CP5 (dynamic bridge REGISTER). Only adhoc-NOTIFY (notify_service) is live and peripheral (not core call signaling); noted as a follow-up. CP2 covers the canonical call-signaling builders in session/mod.rs (INVITE/200/ACK/BYE/re-INVITE/re-INVITE-ACK) + the event_handler UPDATE/re-INVITE 200 OK Contacts.
  • F3 (Medium) — noted: after a mid-dialog target refresh, signaling_hostport scopes by the original remote_addr not the refreshed next_hop; correcting this needs threading the destination into the builders (a signature change beyond CP2). Follow-up.
  • F4 (Medium) — noted: outbound multi-candidate failover builds the INVITE before selecting the working candidate; only matters if candidates straddle local_net. Follow-up.
  • F5 (Medium, pre-existing) — flagged: IPv4-mapped-IPv6 peers bypass IPv4 local_net because the shared acl.rs compares address families. This exposure is IDENTICAL in the existing media advertised_media_ip path; it is a pre-existing cross-cutting ACL limitation (not introduced by CP2). Flagged for a dedicated ACL-normalization hardening.

CI green; workspace test + clippy green locally.

@ryanmurf
ryanmurf merged commit c54dea1 into master Jul 17, 2026
3 checks passed
@ryanmurf
ryanmurf deleted the feat/m6-cp2-external-signaling-address-port branch July 17, 2026 09:36
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.

1 participant