From 605de6e5b37ef4f281ce55db5bc488f13df89d0d Mon Sep 17 00:00:00 2001 From: neuronori Date: Tue, 30 Jun 2026 11:32:38 +0000 Subject: [PATCH] fix: resolve lint issues in pion socket protection - add package and exported-method doc comments - use errors.New for sentinel, drop unused nolint directives - wrap AcceptTCP error - fix errcheck/noctx in tests - extract newSettingEngine to keep negotiatePC under complexity limit --- internal/engine/jitsi/jitsi.go | 33 ++++++++++++++++++++------------ internal/protect/pionnet.go | 21 +++++++++++++++----- internal/protect/pionnet_test.go | 18 +++++------------ 3 files changed, 42 insertions(+), 30 deletions(-) diff --git a/internal/engine/jitsi/jitsi.go b/internal/engine/jitsi/jitsi.go index 636b738..c4824d2 100644 --- a/internal/engine/jitsi/jitsi.go +++ b/internal/engine/jitsi/jitsi.go @@ -470,6 +470,24 @@ func (s *Session) videoTrackHandler() func(*webrtc.TrackRemote, *webrtc.RTPRecei return s.onVideoTrack } +// newSettingEngine builds the pion SettingEngine for a conference PC. When a +// socket protector is set, it routes Pion sockets through ProtectedNet and +// disables mDNS. It fails closed instead of falling back to the default path. +func newSettingEngine() (webrtc.SettingEngine, error) { + settings := webrtc.SettingEngine{} + settings.LoggerFactory = logger.NewPionLoggerFactory() + if protect.Protector == nil { + return settings, nil + } + pnet, err := protect.NewProtectedNet() + if err != nil { + return settings, fmt.Errorf("protected net: %w", err) + } + settings.SetNet(pnet) + settings.SetICEMulticastDNSMode(ice.MulticastDNSModeDisabled) + return settings, nil +} + // negotiatePC builds the pion PeerConnection, applies Jicofo's offer, // answers it and registers all the per-side wiring (DTLS state, ICE // callbacks, transceiver direction). It's branchy on purpose - Jingle @@ -479,18 +497,9 @@ func (s *Session) videoTrackHandler() func(*webrtc.TrackRemote, *webrtc.RTPRecei // //nolint:cyclop // sequential Jingle negotiation steps; refactoring would hide ordering func (s *Session) negotiatePC(ctx context.Context, jSess *j.Session, sctpBridge bool) error { - settings := webrtc.SettingEngine{} - settings.LoggerFactory = logger.NewPionLoggerFactory() - - // When a socket protector is set, route Pion sockets through ProtectedNet. - // Do not fall back to the default network path if setup fails. - if protect.Protector != nil { - pnet, perr := protect.NewProtectedNet() - if perr != nil { - return fmt.Errorf("protected net: %w", perr) - } - settings.SetNet(pnet) - settings.SetICEMulticastDNSMode(ice.MulticastDNSModeDisabled) + settings, err := newSettingEngine() + if err != nil { + return err } // pion auto-registers a default interceptor chain (sender reports, diff --git a/internal/protect/pionnet.go b/internal/protect/pionnet.go index 2ede93e..c4768fa 100644 --- a/internal/protect/pionnet.go +++ b/internal/protect/pionnet.go @@ -1,13 +1,15 @@ // SPDX-License-Identifier: WTFPL -// + // ProtectedNet wraps Pion's network adapter. It applies Protector to each // socket fd and hides tunnel-style interfaces from candidate gathering. Callers // install it only when Protector is set, so default builds keep Pion's standard // network stack. + package protect import ( "context" + "errors" "fmt" "net" "strings" @@ -26,9 +28,7 @@ var tunInterfacePrefixes = []string{"tun", "ppp", "pptp"} // ErrUnexpectedConnType is returned when a protected listen/dial yields an // unexpected concrete type. The caller closes that connection instead of using // an unprotected fallback. -// -//nolint:gochecknoglobals // sentinel error -var ErrUnexpectedConnType = fmt.Errorf("protect: unexpected connection type") +var ErrUnexpectedConnType = errors.New("protect: unexpected connection type") // ProtectedNet wraps Pion's standard net. type ProtectedNet struct { @@ -80,6 +80,7 @@ func isTunInterface(name string) bool { return false } +// ListenPacket listens for packets on a protected socket. func (n *ProtectedNet) ListenPacket(network, address string) (net.PacketConn, error) { lc := net.ListenConfig{Control: controlFunc} conn, err := lc.ListenPacket(context.Background(), network, address) @@ -89,6 +90,7 @@ func (n *ProtectedNet) ListenPacket(network, address string) (net.PacketConn, er return conn, nil } +// ListenUDP listens for UDP packets on a protected socket. func (n *ProtectedNet) ListenUDP(network string, locAddr *net.UDPAddr) (transport.UDPConn, error) { lc := net.ListenConfig{Control: controlFunc} address := udpAddrString(locAddr) @@ -104,6 +106,7 @@ func (n *ProtectedNet) ListenUDP(network string, locAddr *net.UDPAddr) (transpor return uc, nil } +// Dial connects to the address on a protected socket. func (n *ProtectedNet) Dial(network, address string) (net.Conn, error) { d := net.Dialer{Control: controlFunc} conn, err := d.Dial(network, address) @@ -113,6 +116,7 @@ func (n *ProtectedNet) Dial(network, address string) (net.Conn, error) { return conn, nil } +// DialUDP connects to a UDP address on a protected socket. func (n *ProtectedNet) DialUDP(network string, laddr, raddr *net.UDPAddr) (transport.UDPConn, error) { d := net.Dialer{Control: controlFunc} if laddr != nil { @@ -131,6 +135,7 @@ func (n *ProtectedNet) DialUDP(network string, laddr, raddr *net.UDPAddr) (trans return uc, nil } +// DialTCP connects to a TCP address on a protected socket. func (n *ProtectedNet) DialTCP(network string, laddr, raddr *net.TCPAddr) (transport.TCPConn, error) { d := net.Dialer{Control: controlFunc} if laddr != nil { @@ -149,6 +154,7 @@ func (n *ProtectedNet) DialTCP(network string, laddr, raddr *net.TCPAddr) (trans return tc, nil } +// ListenTCP listens for TCP connections on a protected socket. func (n *ProtectedNet) ListenTCP(network string, laddr *net.TCPAddr) (transport.TCPListener, error) { lc := net.ListenConfig{Control: controlFunc} address := tcpAddrString(laddr) @@ -225,8 +231,13 @@ type protectedTCPListener struct { *net.TCPListener } +// AcceptTCP accepts the next TCP connection on the protected listener. func (l protectedTCPListener) AcceptTCP() (transport.TCPConn, error) { - return l.TCPListener.AcceptTCP() + conn, err := l.TCPListener.AcceptTCP() + if err != nil { + return nil, fmt.Errorf("accept tcp: %w", err) + } + return conn, nil } func udpAddrString(a *net.UDPAddr) string { diff --git a/internal/protect/pionnet_test.go b/internal/protect/pionnet_test.go index 1d7dcb4..8d6a034 100644 --- a/internal/protect/pionnet_test.go +++ b/internal/protect/pionnet_test.go @@ -64,8 +64,6 @@ func TestInterfaceByNameRejectsTun(t *testing.T) { } // TestControlFuncFailClosed verifies that Protector can reject a socket. -// -//nolint:tparallel // mutates package-level Protector func TestControlFuncFailClosed(t *testing.T) { old := Protector t.Cleanup(func() { Protector = old }) @@ -80,8 +78,6 @@ func TestControlFuncFailClosed(t *testing.T) { } // TestControlFuncProtects verifies that Protector receives a real fd. -// -//nolint:tparallel // mutates package-level Protector func TestControlFuncProtects(t *testing.T) { old := Protector t.Cleanup(func() { Protector = old }) @@ -99,7 +95,7 @@ func TestControlFuncProtects(t *testing.T) { if err != nil { t.Fatalf("ListenPacket: %v", err) } - defer pc.Close() + defer func() { _ = pc.Close() }() if calls == 0 { t.Error("protector was not invoked") } @@ -107,8 +103,6 @@ func TestControlFuncProtects(t *testing.T) { // TestCreateDialerProtectsAndChains verifies that CreateDialer copies the // caller's Dialer and keeps the caller's Control hook. -// -//nolint:tparallel // mutates package-level Protector func TestCreateDialerProtectsAndChains(t *testing.T) { old := Protector t.Cleanup(func() { Protector = old }) @@ -122,11 +116,11 @@ func TestCreateDialerProtectsAndChains(t *testing.T) { } // Dial a local TCP listener. - ln, err := net.Listen("tcp", "127.0.0.1:0") + ln, err := (&net.ListenConfig{}).Listen(context.Background(), "tcp", "127.0.0.1:0") if err != nil { t.Fatalf("listen: %v", err) } - defer ln.Close() + defer func() { _ = ln.Close() }() go func() { if c, aerr := ln.Accept(); aerr == nil { _ = c.Close() @@ -165,8 +159,6 @@ func TestCreateDialerProtectsAndChains(t *testing.T) { // TestCreateDialerProtectsAndChainsControlContext verifies that CreateDialer // keeps the caller's ControlContext hook. -// -//nolint:tparallel // mutates package-level Protector func TestCreateDialerProtectsAndChainsControlContext(t *testing.T) { old := Protector t.Cleanup(func() { Protector = old }) @@ -179,11 +171,11 @@ func TestCreateDialerProtectsAndChainsControlContext(t *testing.T) { t.Fatalf("NewProtectedNet: %v", err) } - ln, err := net.Listen("tcp", "127.0.0.1:0") + ln, err := (&net.ListenConfig{}).Listen(context.Background(), "tcp", "127.0.0.1:0") if err != nil { t.Fatalf("listen: %v", err) } - defer ln.Close() + defer func() { _ = ln.Close() }() go func() { if c, aerr := ln.Accept(); aerr == nil { _ = c.Close()