diff --git a/pkg/rtcconfig/config.go b/pkg/rtcconfig/config.go index 9502428..b0d39d3 100644 --- a/pkg/rtcconfig/config.go +++ b/pkg/rtcconfig/config.go @@ -51,6 +51,31 @@ type RTCConfig struct { NodeIPAutoGenerated bool `yaml:"-"` STUNServers []string `yaml:"stun_servers,omitempty"` UseExternalIP bool `yaml:"use_external_ip"` + // AdvertisePrivateIPs lets LiveKit publish private host candidates alongside + // the STUN-derived server-reflexive candidates (instead of replacing them). + // Useful for hybrid topologies where some peers reach LiveKit via cluster- + // internal routing on the private IP while others reach it via the public + // IP. + // + // Has effect only when UseExternalIP is true and ExternalIPOnly is false. + // Validate() rejects two configuration combinations that would silently + // break the feature: + // - ForceTCP: incompatible because Pion's gatherCandidatesSrflxMapped + // skips TCP network types, so the public IP would never be advertised. + // - Anything other than full port-range mode: Pion's srflx mapped gather + // opens its own UDP listener via listenUDPInPortRange and falls back + // to an OS-ephemeral port whenever ICEPortRangeStart/End are not both + // non-zero. That port isn't covered by firewall rules, so the + // published srflx candidate is unreachable from external clients. + // This rejects single-port udp_port mode, partial port ranges (only + // one of start/end set), and unbounded UDP gather. Use both + // port_range_start and port_range_end together. + // + // Operator note: external clients on networks that block UDP but allow + // outbound TCP/7881 lose the public TCP host candidate they had under the + // legacy (Replace+Host) mode. Such clients require TCP TURN at 443 to + // reach the SFU; ensure TURN/TLS is provisioned before opting in. + AdvertisePrivateIPs bool `yaml:"advertise_private_ips,omitempty"` UseICELite bool `yaml:"use_ice_lite,omitempty"` Interfaces InterfacesConfig `yaml:"interfaces,omitempty"` IPs IPsConfig `yaml:"ips,omitempty"` @@ -84,7 +109,48 @@ type BatchIOConfig struct { MaxFlushInterval time.Duration `yaml:"max_flush_interval,omitempty"` } +// validateAdvertisePrivateIPs checks that AdvertisePrivateIPs is not combined +// with configurations that would silently break the patch path. The gates +// only fire when AdvertisePrivateIPs would actually take effect — that is, +// when UseExternalIP is true and ExternalIPOnly is false — to avoid erroring +// on a flag that is otherwise a no-op. See the AdvertisePrivateIPs docstring +// for the reasoning behind each gate. +func (conf *RTCConfig) validateAdvertisePrivateIPs() error { + if !conf.AdvertisePrivateIPs || !conf.UseExternalIP || conf.ExternalIPOnly { + return nil + } + if conf.ForceTCP { + return fmt.Errorf("advertise_private_ips is incompatible with force_tcp: " + + "pion's gatherCandidatesSrflxMapped skips TCP network types, so the " + + "public IP would never be advertised") + } + // Mirror NewWebRTCConfig's port-mode selection: full port range mode + // requires BOTH port_range_start and port_range_end to be non-zero. Any + // other UDP configuration (single-port udp_port, partial port range with + // only one bound set, or no UDP configuration at all) falls back to + // pion's listenUDPInPortRange with portMin=portMax=0, which binds an + // OS-ephemeral port that isn't covered by firewall rules. + if conf.ICEPortRangeStart == 0 || conf.ICEPortRangeEnd == 0 { + return fmt.Errorf("advertise_private_ips requires both port_range_start " + + "and port_range_end to be set: any other UDP configuration " + + "(single-port udp_port, partial port range, or unbounded ephemeral) " + + "leaves pion's srflx mapped gather binding to an OS-ephemeral port " + + "not covered by firewall rules") + } + return nil +} + func (conf *RTCConfig) Validate(development bool) error { + // Validate advertise_private_ips against the raw config before applying + // port defaults. The defaulting logic below fills missing port range + // fields based on whether ICEPortRangeStart is zero (without checking + // ICEPortRangeEnd), which would silently mask end-only partial ranges + // and let the operator's typo run with overwritten defaults. Validating + // first preserves the operator's actual intent for the gate. + if err := conf.validateAdvertisePrivateIPs(); err != nil { + return err + } + // set defaults for ports if none are set if !conf.UDPPort.Valid() && conf.ICEPortRangeStart == 0 { // to make it easier to run in dev mode/docker, default to single port diff --git a/pkg/rtcconfig/webrtc_config.go b/pkg/rtcconfig/webrtc_config.go index 188e74c..3ff3132 100644 --- a/pkg/rtcconfig/webrtc_config.go +++ b/pkg/rtcconfig/webrtc_config.go @@ -103,8 +103,15 @@ func NewWebRTCConfig(rtcConf *RTCConfig, development bool) (*WebRTCConfig, error } } else { logger.Infow("using external IPs", "ips", ips) - if err := SetNAT1To1AddressRewriteRules(&s, ips, webrtc.ICECandidateTypeHost); err != nil { - return nil, err + mode, candidateType := chooseNAT1To1Mode(rtcConf.AdvertisePrivateIPs, rtcConf.ExternalIPOnly, len(ips)) + if mode == webrtc.ICEAddressRewriteAppend && candidateType == webrtc.ICECandidateTypeSrflx { + if err := SetNAT1To1AddressRewriteRulesAppendSrflx(&s, ips); err != nil { + return nil, err + } + } else { + if err := SetNAT1To1AddressRewriteRules(&s, ips, webrtc.ICECandidateTypeHost); err != nil { + return nil, err + } } } nat1to1IPs = ips @@ -237,15 +244,61 @@ func NewWebRTCConfig(rtcConf *RTCConfig, development bool) (*WebRTCConfig, error } func SetNAT1To1AddressRewriteRules(s *webrtc.SettingEngine, ips []string, candidateType webrtc.ICECandidateType) error { + return s.SetICEAddressRewriteRules(buildNAT1To1Rules(ips, candidateType, defaultNAT1To1Mode(candidateType))...) +} + +// SetNAT1To1AddressRewriteRulesAppendSrflx publishes ips as server-reflexive +// candidates via Pion's append-mode rewrite rules, leaving any host candidates +// (typically the private IP) intact. This is the wiring used when +// AdvertisePrivateIPs is enabled. +func SetNAT1To1AddressRewriteRulesAppendSrflx(s *webrtc.SettingEngine, ips []string) error { + return s.SetICEAddressRewriteRules(buildNAT1To1Rules(ips, webrtc.ICECandidateTypeSrflx, webrtc.ICEAddressRewriteAppend)...) +} + +// chooseNAT1To1Mode picks the rewrite mode + candidate type for the discovered +// external IPs. AdvertisePrivateIPs only takes effect when the operator did +// not opt into ExternalIPOnly and STUN actually returned mappings. +func chooseNAT1To1Mode(advertisePrivateIPs, externalIPOnly bool, externalMappingsCount int) (webrtc.ICEAddressRewriteMode, webrtc.ICECandidateType) { + if advertisePrivateIPs && !externalIPOnly && externalMappingsCount > 0 { + return webrtc.ICEAddressRewriteAppend, webrtc.ICECandidateTypeSrflx + } + return webrtc.ICEAddressRewriteReplace, webrtc.ICECandidateTypeHost +} + +// defaultNAT1To1Mode preserves the legacy behavior of SetNAT1To1AddressRewriteRules +// for callers that don't pass an explicit mode: replace for host candidates, +// append for everything else. +func defaultNAT1To1Mode(candidateType webrtc.ICECandidateType) webrtc.ICEAddressRewriteMode { + if candidateType == webrtc.ICECandidateTypeUnknown || candidateType == webrtc.ICECandidateTypeHost { + return webrtc.ICEAddressRewriteReplace + } + return webrtc.ICEAddressRewriteAppend +} + +// buildNAT1To1Rules produces the rewrite-rule slice fed to Pion. The catch-all +// rule (one entry per external IP, no Local) is always emitted regardless of +// mode: pion's gatherCandidatesSrflxMapped opens its UDP socket on the +// wildcard address (0.0.0.0), and rule lookup keyed on the wildcard local IP +// must hit the catch-all to publish the right external IP. Without it, srflx +// candidates fall back to publishing 0.0.0.0 as base address. +// +// Self-mappings (External == Local) are skipped in append mode because +// publishing " -> " as srflx is meaningless and only +// inflates ICE check overhead. +func buildNAT1To1Rules(ips []string, candidateType webrtc.ICECandidateType, mode webrtc.ICEAddressRewriteMode) []webrtc.ICEAddressRewriteRule { rules := make([]webrtc.ICEAddressRewriteRule, 0, len(ips)+1) catchAll := make([]string, 0, len(ips)) for _, ip := range ips { if parts := strings.Split(ip, "/"); len(parts) == 2 { + if mode == webrtc.ICEAddressRewriteAppend && parts[0] == parts[1] { + continue + } rules = append(rules, webrtc.ICEAddressRewriteRule{ External: []string{parts[0]}, Local: parts[1], AsCandidateType: candidateType, + Mode: mode, }) catchAll = append(catchAll, parts[0]) } else { @@ -256,10 +309,11 @@ func SetNAT1To1AddressRewriteRules(s *webrtc.SettingEngine, ips []string, candid rules = append(rules, webrtc.ICEAddressRewriteRule{ External: catchAll, AsCandidateType: candidateType, + Mode: mode, }) } - return s.SetICEAddressRewriteRules(rules...) + return rules } func iceServerForStunServers(servers []string) webrtc.ICEServer { diff --git a/pkg/rtcconfig/webrtc_config_test.go b/pkg/rtcconfig/webrtc_config_test.go index ae91c15..e3996c8 100644 --- a/pkg/rtcconfig/webrtc_config_test.go +++ b/pkg/rtcconfig/webrtc_config_test.go @@ -18,9 +18,301 @@ import ( "net" "testing" + "github.com/pion/webrtc/v4" "github.com/stretchr/testify/require" ) +func TestBuildNAT1To1Rules(t *testing.T) { + tests := []struct { + name string + ips []string + candidateType webrtc.ICECandidateType + mode webrtc.ICEAddressRewriteMode + want []webrtc.ICEAddressRewriteRule + }{ + { + name: "replace host keeps legacy mapping shape", + ips: []string{"203.0.113.10/10.0.0.10", "198.51.100.20"}, + candidateType: webrtc.ICECandidateTypeHost, + mode: webrtc.ICEAddressRewriteReplace, + want: []webrtc.ICEAddressRewriteRule{ + { + External: []string{"203.0.113.10"}, + Local: "10.0.0.10", + AsCandidateType: webrtc.ICECandidateTypeHost, + Mode: webrtc.ICEAddressRewriteReplace, + }, + { + External: []string{"203.0.113.10", "198.51.100.20"}, + AsCandidateType: webrtc.ICECandidateTypeHost, + Mode: webrtc.ICEAddressRewriteReplace, + }, + }, + }, + { + name: "append srflx emits explicit and catch-all", + ips: []string{"203.0.113.10/10.0.0.10"}, + candidateType: webrtc.ICECandidateTypeSrflx, + mode: webrtc.ICEAddressRewriteAppend, + want: []webrtc.ICEAddressRewriteRule{ + { + External: []string{"203.0.113.10"}, + Local: "10.0.0.10", + AsCandidateType: webrtc.ICECandidateTypeSrflx, + Mode: webrtc.ICEAddressRewriteAppend, + }, + { + External: []string{"203.0.113.10"}, + AsCandidateType: webrtc.ICECandidateTypeSrflx, + Mode: webrtc.ICEAddressRewriteAppend, + }, + }, + }, + { + name: "append srflx skips self mappings but keeps real external in catch-all", + ips: []string{"203.0.113.10/10.0.0.10", "10.0.0.20/10.0.0.20"}, + candidateType: webrtc.ICECandidateTypeSrflx, + mode: webrtc.ICEAddressRewriteAppend, + want: []webrtc.ICEAddressRewriteRule{ + { + External: []string{"203.0.113.10"}, + Local: "10.0.0.10", + AsCandidateType: webrtc.ICECandidateTypeSrflx, + Mode: webrtc.ICEAddressRewriteAppend, + }, + { + External: []string{"203.0.113.10"}, + AsCandidateType: webrtc.ICECandidateTypeSrflx, + Mode: webrtc.ICEAddressRewriteAppend, + }, + }, + }, + { + name: "append srflx keeps unmapped external ips as catch-all", + ips: []string{"198.51.100.20"}, + candidateType: webrtc.ICECandidateTypeSrflx, + mode: webrtc.ICEAddressRewriteAppend, + want: []webrtc.ICEAddressRewriteRule{ + { + External: []string{"198.51.100.20"}, + AsCandidateType: webrtc.ICECandidateTypeSrflx, + Mode: webrtc.ICEAddressRewriteAppend, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := buildNAT1To1Rules(tt.ips, tt.candidateType, tt.mode) + require.Equal(t, tt.want, got) + for _, rule := range got { + require.Nil(t, rule.Networks) + } + }) + } +} + +func TestChooseNAT1To1Mode(t *testing.T) { + tests := []struct { + name string + advertisePrivateIPs bool + externalIPOnly bool + externalMappingCount int + wantMode webrtc.ICEAddressRewriteMode + wantCandidateType webrtc.ICECandidateType + }{ + { + name: "advertise private ips disabled", + wantMode: webrtc.ICEAddressRewriteReplace, + wantCandidateType: webrtc.ICECandidateTypeHost, + }, + { + name: "external only wins", + advertisePrivateIPs: true, + externalIPOnly: true, + externalMappingCount: 1, + wantMode: webrtc.ICEAddressRewriteReplace, + wantCandidateType: webrtc.ICECandidateTypeHost, + }, + { + name: "no mappings falls back to replace", + advertisePrivateIPs: true, + wantMode: webrtc.ICEAddressRewriteReplace, + wantCandidateType: webrtc.ICECandidateTypeHost, + }, + { + name: "advertise private ips appends srflx", + advertisePrivateIPs: true, + externalMappingCount: 1, + wantMode: webrtc.ICEAddressRewriteAppend, + wantCandidateType: webrtc.ICECandidateTypeSrflx, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotMode, gotCandidateType := chooseNAT1To1Mode(tt.advertisePrivateIPs, tt.externalIPOnly, tt.externalMappingCount) + require.Equal(t, tt.wantMode, gotMode) + require.Equal(t, tt.wantCandidateType, gotCandidateType) + }) + } +} + +func TestRTCConfig_ValidateAdvertisePrivateIPs(t *testing.T) { + // Tests target validateAdvertisePrivateIPs directly so they don't need to + // go through Validate's full flow (which calls determineIP and reaches + // the network when UseExternalIP is true). + tests := []struct { + name string + conf RTCConfig + wantErr bool + errSubstr string + }{ + { + name: "advertise_private_ips disabled is always allowed", + conf: RTCConfig{ + AdvertisePrivateIPs: false, + ForceTCP: true, + UDPPort: PortRange{Start: 7882}, + }, + }, + { + name: "advertise_private_ips with use_external_ip=false is allowed (flag has no effect)", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: false, + ForceTCP: true, + UDPPort: PortRange{Start: 7882}, + }, + }, + { + name: "advertise_private_ips with external_ip_only=true is allowed (flag has no effect)", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ExternalIPOnly: true, + ForceTCP: true, + UDPPort: PortRange{Start: 7882}, + }, + }, + { + name: "advertise_private_ips with force_tcp is rejected", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ForceTCP: true, + ICEPortRangeStart: 50000, + ICEPortRangeEnd: 60000, + }, + wantErr: true, + errSubstr: "force_tcp", + }, + { + name: "advertise_private_ips with single-port udp_port (no port range) is rejected", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + UDPPort: PortRange{Start: 7882}, + }, + wantErr: true, + errSubstr: "port_range_start", + }, + { + name: "advertise_private_ips with partial port range (only start) plus udp_port is rejected", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ICEPortRangeStart: 50000, + // ICEPortRangeEnd intentionally left zero — operator typo + UDPPort: PortRange{Start: 7882}, + }, + wantErr: true, + errSubstr: "port_range_start", + }, + { + name: "advertise_private_ips with partial port range (only end) plus udp_port is rejected", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + // ICEPortRangeStart intentionally left zero + ICEPortRangeEnd: 60000, + UDPPort: PortRange{Start: 7882}, + }, + wantErr: true, + errSubstr: "port_range_start", + }, + { + name: "advertise_private_ips with partial port range and no udp_port is rejected", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ICEPortRangeStart: 50000, + // ICEPortRangeEnd intentionally left zero, no udp_port either + }, + wantErr: true, + errSubstr: "port_range_start", + }, + { + // Operators enabling advertise_private_ips must explicitly configure + // the port range. Relying on Validate's defaulting logic would mask + // end-only partial-range typos (defaulting only checks start==0 and + // then overwrites end), so the gate must run before defaulting and + // reject configurations that don't have an explicit port range. + name: "advertise_private_ips with no port config at all is rejected (must be explicit)", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + // no port_range_*, no udp_port — pre-defaulting state + }, + wantErr: true, + errSubstr: "port_range_start", + }, + { + name: "advertise_private_ips with port range only is allowed", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ICEPortRangeStart: 50000, + ICEPortRangeEnd: 60000, + }, + }, + { + name: "advertise_private_ips with port range and udp_port (port range wins) is allowed", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ICEPortRangeStart: 50000, + ICEPortRangeEnd: 60000, + UDPPort: PortRange{Start: 7882}, + }, + }, + { + name: "force_tcp gate fires before port_range gate when both invalid", + conf: RTCConfig{ + AdvertisePrivateIPs: true, + UseExternalIP: true, + ForceTCP: true, + UDPPort: PortRange{Start: 7882}, + }, + wantErr: true, + errSubstr: "force_tcp", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.conf.validateAdvertisePrivateIPs() + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.errSubstr) + } else { + require.NoError(t, err) + } + }) + } +} + func Test_IPFilterFromConf(t *testing.T) { testData := IPsConfig{ Includes: []string{"10.0.0.0/19"},