From af66372b655f05f4fc8e778ec58902e15ce25531 Mon Sep 17 00:00:00 2001 From: deepashreekedia <56016920+deepashreeKedia@users.noreply.github.com> Date: Thu, 21 May 2026 07:28:43 +0530 Subject: [PATCH] fix(webRTC): LAN IP filtering (#333) * fix webRTC voice call for LAN setup * log re-add * refactor: extract ICE candidate filtering policy * fix: decouple relay-only diagnostics from LAN TURN setup * fix: fix remote_up script --------- Co-authored-by: deepashreeKedia Co-authored-by: Abhishek Kumar --- api/routes/webrtc_signaling.py | 123 +++++++++++++++------- api/tests/test_is_private_ip_candidate.py | 84 ++++++++++++++- remote_up.sh | 11 +- scripts/setup_local.ps1 | 5 + scripts/setup_local.sh | 17 +-- scripts/setup_remote.sh | 12 +-- 6 files changed, 190 insertions(+), 62 deletions(-) diff --git a/api/routes/webrtc_signaling.py b/api/routes/webrtc_signaling.py index e39b13d..c2ea683 100644 --- a/api/routes/webrtc_signaling.py +++ b/api/routes/webrtc_signaling.py @@ -18,6 +18,7 @@ import asyncio import ipaddress import os from datetime import UTC, datetime +from enum import Enum from typing import Dict, List, Optional from aiortc import RTCIceServer @@ -49,6 +50,63 @@ from api.services.quota_service import check_dograh_quota router = APIRouter(prefix="/ws") +class NonRelayFilterPolicy(Enum): + """What to filter from non-relay ICE candidates. Relay candidates always pass.""" + + NONE = "none" # filter nothing — pass all candidates + PRIVATE = "private" # filter non-relay candidates with private/CGNAT IPs + ALL = "all" # filter all non-relay candidates (relay-only mode) + + +def is_local_or_cgnat_ip(ip_str: str) -> bool: + """Return True for RFC1918, loopback, link-local, and CGNAT addresses.""" + + try: + ip = ipaddress.ip_address(ip_str) + except ValueError: + return False + + is_cgnat = ip.version == 4 and ip in ipaddress.ip_network("100.64.0.0/10") + return ip.is_private or ip.is_loopback or ip.is_link_local or is_cgnat + + +def resolve_ice_filter_policies( + environment: str, + force_turn_relay: bool, + server_ip: str, +) -> tuple[NonRelayFilterPolicy, NonRelayFilterPolicy]: + """Resolve outbound and inbound non-relay filtering for this deployment.""" + + private_lan_deployment = ( + environment != Environment.LOCAL.value and is_local_or_cgnat_ip(server_ip) + ) + + if force_turn_relay: + # Relay-only diagnostics stay explicit. On private LAN deployments we + # must still accept inbound private candidates for relay<->host pairs. + outbound_policy = NonRelayFilterPolicy.ALL + inbound_policy = ( + NonRelayFilterPolicy.NONE + if private_lan_deployment + else NonRelayFilterPolicy.PRIVATE + ) + return outbound_policy, inbound_policy + + if environment == Environment.LOCAL.value or private_lan_deployment: + return NonRelayFilterPolicy.NONE, NonRelayFilterPolicy.NONE + + # Public remote deployment: drop private-IP host candidates to avoid + # coturn denied-peer-ip errors against Docker bridge and LAN interfaces. + return NonRelayFilterPolicy.PRIVATE, NonRelayFilterPolicy.PRIVATE + + +ICE_OUTBOUND_POLICY, ICE_INBOUND_POLICY = resolve_ice_filter_policies( + ENVIRONMENT, + FORCE_TURN_RELAY, + os.getenv("SERVER_IP", ""), +) + + def is_private_ip_candidate(candidate_str: str) -> bool: """Check if ICE candidate contains a private IP address or CGNAT IP Address. @@ -69,61 +127,58 @@ def is_private_ip_candidate(candidate_str: str) -> bool: if "typ" in parts: typ_index = parts.index("typ") ip_str = parts[typ_index - 2] - ip = ipaddress.ip_address(ip_str) - is_cgnat = ip in ipaddress.ip_network("100.64.0.0/10") - return ip.is_private or is_cgnat + return is_local_or_cgnat_ip(ip_str) except (ValueError, IndexError): pass return False -def filter_outbound_sdp(sdp: str) -> str: - """Strip ICE candidates from an outbound answer SDP based on env config. +def _keep_candidate(candidate_str: str, policy: NonRelayFilterPolicy) -> bool: + """Return True if this ICE candidate should be kept under the given policy. - Two filters apply: - - 1. In non-LOCAL environments, drop host candidates with private/CGNAT IPs. - aiortc gathers host candidates from every interface on the box, including - Docker bridges (172.17.0.1, 172.18.0.1). Advertising those to the browser - causes coturn "peer IP X denied" errors when the browser asks TURN to - permit them. - - 2. When FORCE_TURN_RELAY is set, drop every non-relay candidate so the - only path the browser can use is via TURN. Lets you verify TURN - connectivity end-to-end — if TURN is broken, the call simply fails. + Relay candidates always pass — a relay with a private IP (LAN TURN server) + must never be dropped regardless of policy. """ - if ENVIRONMENT == Environment.LOCAL.value and not FORCE_TURN_RELAY: + if " typ relay" in candidate_str: + return True + if policy == NonRelayFilterPolicy.NONE: + return True + if policy == NonRelayFilterPolicy.ALL: + return False + # PRIVATE: drop non-relay candidates with private/CGNAT IPs + return not is_private_ip_candidate(candidate_str) + + +def filter_outbound_sdp(sdp: str) -> str: + """Strip ICE candidates from an outbound answer SDP based on ICE_OUTBOUND_POLICY.""" + if ICE_OUTBOUND_POLICY == NonRelayFilterPolicy.NONE: return sdp lines = sdp.split("\r\n") filtered: List[str] = [] - dropped_non_relay = 0 + dropped = 0 kept_relay = 0 for line in lines: if line.startswith("a=candidate:"): candidate_str = line[2:] - if FORCE_TURN_RELAY and " typ relay" not in candidate_str: - dropped_non_relay += 1 + if not _keep_candidate(candidate_str, ICE_OUTBOUND_POLICY): + dropped += 1 continue - if ENVIRONMENT != Environment.LOCAL.value and is_private_ip_candidate( - candidate_str - ): - continue - if FORCE_TURN_RELAY: + if " typ relay" in candidate_str: kept_relay += 1 filtered.append(line) - if FORCE_TURN_RELAY: + if ICE_OUTBOUND_POLICY == NonRelayFilterPolicy.ALL: if kept_relay == 0: logger.warning( "FORCE_TURN_RELAY is on but the answer SDP has no relay candidates " - f"(dropped {dropped_non_relay} non-relay). TURN may be unreachable; " + f"(dropped {dropped} non-relay). TURN may be unreachable; " "the connection will fail." ) else: logger.info( f"FORCE_TURN_RELAY: kept {kept_relay} relay candidates, " - f"dropped {dropped_non_relay} non-relay" + f"dropped {dropped} non-relay" ) return "\r\n".join(filtered) @@ -370,9 +425,7 @@ class SignalingManager: Uses SmallWebRTC's native ICE trickling support via add_ice_candidate(). Candidates are parsed using aiortc's candidate_from_sdp() for proper formatting, consistent with SmallWebRTCRequestHandler.handle_patch_request(). - - In non-local environments, private IP candidates are filtered out to prevent - TURN relay errors when coturn blocks private IP ranges (denied-peer-ip). + Candidates are filtered according to ICE_INBOUND_POLICY before being added. """ pc_id = payload.get("pc_id") candidate_data = payload.get("candidate") @@ -389,13 +442,9 @@ class SignalingManager: if candidate_data: candidate_str = candidate_data.get("candidate", "") - # Filter out private IP candidates in non-local environments - # This prevents TURN relay errors when coturn blocks private IP ranges - if ENVIRONMENT != Environment.LOCAL.value and is_private_ip_candidate( - candidate_str - ): + if not _keep_candidate(candidate_str, ICE_INBOUND_POLICY): logger.debug( - f"Skipping private IP candidate in {ENVIRONMENT}: {candidate_str[:50]}..." + f"Dropping inbound candidate per policy ({ICE_INBOUND_POLICY.value}): {candidate_str[:50]}..." ) return diff --git a/api/tests/test_is_private_ip_candidate.py b/api/tests/test_is_private_ip_candidate.py index 25feb5a..1ea6834 100644 --- a/api/tests/test_is_private_ip_candidate.py +++ b/api/tests/test_is_private_ip_candidate.py @@ -1,4 +1,11 @@ -from api.routes.webrtc_signaling import is_private_ip_candidate +from api.enums import Environment +from api.routes.webrtc_signaling import ( + NonRelayFilterPolicy, + _keep_candidate, + is_local_or_cgnat_ip, + is_private_ip_candidate, + resolve_ice_filter_policies, +) class TestIsPrivateIpCandidate: @@ -142,3 +149,78 @@ class TestIsPrivateIpCandidate: "candidate:999 1 tcp 1518280447 192.168.1.100 9 typ host tcptype active" ) assert is_private_ip_candidate(candidate) is True + + +class TestIsLocalOrCgnatIp: + def test_loopback_is_local(self): + assert is_local_or_cgnat_ip("127.0.0.1") is True + + def test_link_local_is_local(self): + assert is_local_or_cgnat_ip("169.254.1.1") is True + + def test_cgnat_is_local(self): + assert is_local_or_cgnat_ip("100.64.0.1") is True + + def test_public_ipv4_is_not_local(self): + assert is_local_or_cgnat_ip("8.8.8.8") is False + + +class TestKeepCandidate: + def test_private_relay_candidate_survives_private_policy(self): + candidate = ( + "candidate:111 1 udp 41885439 192.168.1.50 50000 typ relay raddr 0.0.0.0 rport 0" + ) + assert _keep_candidate(candidate, NonRelayFilterPolicy.PRIVATE) is True + + def test_private_host_candidate_drops_under_private_policy(self): + candidate = ( + "candidate:123 1 udp 2122260223 192.168.50.24 63603 typ host generation 0" + ) + assert _keep_candidate(candidate, NonRelayFilterPolicy.PRIVATE) is False + + +class TestResolveIceFilterPolicies: + def test_local_deployment_keeps_all_candidates(self): + outbound, inbound = resolve_ice_filter_policies( + Environment.LOCAL.value, + False, + "", + ) + assert outbound == NonRelayFilterPolicy.NONE + assert inbound == NonRelayFilterPolicy.NONE + + def test_private_lan_remote_keeps_all_candidates(self): + outbound, inbound = resolve_ice_filter_policies( + Environment.PRODUCTION.value, + False, + "192.168.50.24", + ) + assert outbound == NonRelayFilterPolicy.NONE + assert inbound == NonRelayFilterPolicy.NONE + + def test_public_remote_filters_private_candidates(self): + outbound, inbound = resolve_ice_filter_policies( + Environment.PRODUCTION.value, + False, + "8.8.8.8", + ) + assert outbound == NonRelayFilterPolicy.PRIVATE + assert inbound == NonRelayFilterPolicy.PRIVATE + + def test_force_turn_relay_stays_relay_only_on_private_lan(self): + outbound, inbound = resolve_ice_filter_policies( + Environment.PRODUCTION.value, + True, + "192.168.50.24", + ) + assert outbound == NonRelayFilterPolicy.ALL + assert inbound == NonRelayFilterPolicy.NONE + + def test_force_turn_relay_keeps_public_remote_private_filter(self): + outbound, inbound = resolve_ice_filter_policies( + Environment.PRODUCTION.value, + True, + "8.8.8.8", + ) + assert outbound == NonRelayFilterPolicy.ALL + assert inbound == NonRelayFilterPolicy.PRIVATE diff --git a/remote_up.sh b/remote_up.sh index 1b01ef1..29dc784 100755 --- a/remote_up.sh +++ b/remote_up.sh @@ -66,7 +66,14 @@ else fi if [[ "$MODE" == "build" ]]; then - exec "${COMPOSE_CMD[@]}" --profile remote up -d --build --force-recreate "${EXTRA_ARGS[@]}" + CMD=("${COMPOSE_CMD[@]}" --profile remote up -d --build --force-recreate) else - exec "${COMPOSE_CMD[@]}" --profile remote up -d --pull always --force-recreate "${EXTRA_ARGS[@]}" + CMD=("${COMPOSE_CMD[@]}" --profile remote up -d --pull always --force-recreate) fi + +# Bash 3.2 on macOS treats "${empty_array[@]}" as unbound under `set -u`. +if (( ${#EXTRA_ARGS[@]} )); then + CMD+=("${EXTRA_ARGS[@]}") +fi + +exec "${CMD[@]}" diff --git a/scripts/setup_local.ps1 b/scripts/setup_local.ps1 index 95b4c42..2958f30 100644 --- a/scripts/setup_local.ps1 +++ b/scripts/setup_local.ps1 @@ -167,6 +167,7 @@ if ([string]::IsNullOrEmpty($env:ENABLE_COTURN)) { $UseCoturn = Test-IsEnabled $EnableCoturn $TurnHost = $env:TURN_HOST $TurnSecret = $env:TURN_SECRET +$ForceTurnRelay = if ([string]::IsNullOrEmpty($env:FORCE_TURN_RELAY)) { 'false' } else { $env:FORCE_TURN_RELAY } if ($UseCoturn) { $defaultTurnHost = Get-DefaultLanIPv4 @@ -208,6 +209,7 @@ Write-Host " Coturn: $EnableCoturn" -ForegroundColor Blue if ($UseCoturn) { Write-Host " TURN Host: $TurnHost" -ForegroundColor Blue Write-Host ' TURN Secret: ********' -ForegroundColor Blue + Write-Host " Force relay: $ForceTurnRelay" -ForegroundColor Blue } Write-Host " Telemetry: $EnableTelemetry" -ForegroundColor Blue Write-Host " Registry: $Registry" -ForegroundColor Blue @@ -251,6 +253,9 @@ $envLines = @( '' '# Telemetry (set to false to disable)' "ENABLE_TELEMETRY=$EnableTelemetry" + '' + '# Relay-only ICE candidates for explicit TURN diagnostics' + "FORCE_TURN_RELAY=$ForceTurnRelay" ) if ($UseCoturn) { diff --git a/scripts/setup_local.sh b/scripts/setup_local.sh index 590b774..674185e 100755 --- a/scripts/setup_local.sh +++ b/scripts/setup_local.sh @@ -50,8 +50,8 @@ if [[ "${ENABLE_COTURN:-false}" == "true" ]]; then # Pick a TURN_HOST that's reachable from BOTH the browser (running on the # host) and the API container (running in docker). 127.0.0.1 is tempting # but doesn't work for the api container — its own loopback isn't where - # coturn lives, so aiortc can't allocate a relay and FORCE_TURN_RELAY - # ends up with an empty answer SDP. The host's LAN IP works for both. + # coturn lives, so aiortc can't allocate a relay. The host's LAN IP works + # for both. detect_lan_ip() { local ip="" if command -v ipconfig >/dev/null 2>&1; then @@ -102,16 +102,7 @@ if [[ "${ENABLE_COTURN:-false}" == "true" ]]; then fi fi -if [[ "${ENABLE_COTURN:-false}" != "true" ]]; then - FORCE_TURN_RELAY=false -elif [[ -z "${FORCE_TURN_RELAY:-}" ]]; then - if dograh_is_local_ipv4 "$TURN_HOST"; then - FORCE_TURN_RELAY=true - echo -e "${YELLOW}Detected a local/private TURN host IP; enabling FORCE_TURN_RELAY=true.${NC}" - else - FORCE_TURN_RELAY=false - fi -fi +FORCE_TURN_RELAY="${FORCE_TURN_RELAY:-false}" # Telemetry opt-out (default: true) ENABLE_TELEMETRY="${ENABLE_TELEMETRY:-true}" @@ -170,7 +161,7 @@ OSS_JWT_SECRET=$OSS_JWT_SECRET # Telemetry (set to false to disable) ENABLE_TELEMETRY=$ENABLE_TELEMETRY -# Relay-only ICE candidates (auto-enabled for local/private TURN host IPs) +# Relay-only ICE candidates for explicit TURN diagnostics FORCE_TURN_RELAY=$FORCE_TURN_RELAY ENV_EOF diff --git a/scripts/setup_remote.sh b/scripts/setup_remote.sh index 073689f..d958b69 100755 --- a/scripts/setup_remote.sh +++ b/scripts/setup_remote.sh @@ -49,14 +49,7 @@ if ! dograh_is_ipv4 "$SERVER_IP"; then dograh_fail "Invalid IP address format" fi -if [[ -z "${FORCE_TURN_RELAY:-}" ]]; then - if dograh_is_local_ipv4 "$SERVER_IP"; then - FORCE_TURN_RELAY=true - dograh_warn "Detected a local/private server IP; enabling FORCE_TURN_RELAY=true." - else - FORCE_TURN_RELAY=false - fi -fi +FORCE_TURN_RELAY="${FORCE_TURN_RELAY:-false}" # Get the TURN secret (skip prompt if TURN_SECRET is already set) if [[ -z "${TURN_SECRET:-}" ]]; then @@ -260,7 +253,7 @@ echo -e "${BLUE}[4/$TOTAL] Creating environment file...${NC}" OSS_JWT_SECRET=$(openssl rand -hex 32) cat > .env << ENV_EOF -# Change environment from local to production so that coturn filters local IPs +# Remote deployments run with production signaling and HTTPS defaults ENVIRONMENT=production # Canonical public host/base URL for this install. @@ -277,6 +270,7 @@ MINIO_PUBLIC_ENDPOINT=https://$SERVER_IP # TURN Server Configuration (time-limited credentials via TURN REST API) TURN_HOST=$SERVER_IP TURN_SECRET=$TURN_SECRET +# Relay-only ICE candidates for explicit TURN diagnostics FORCE_TURN_RELAY=$FORCE_TURN_RELAY # JWT secret for OSS authentication