From 758c37b83d8fa2abd0ac461b4d6d1be41447b25b Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Thu, 18 Nov 2021 12:18:02 -0800 Subject: [PATCH] net/netns: thread logf into control functions So that darwin can log there without panicking during tests. Signed-off-by: Josh Bleecher Snyder --- control/controlclient/direct.go | 2 +- derp/derphttp/derphttp_client.go | 4 ++-- logpolicy/logpolicy.go | 2 +- net/dns/resolver/forwarder.go | 2 +- net/dnsfallback/dnsfallback.go | 2 +- net/netcheck/netcheck.go | 6 +++--- net/netns/netns.go | 13 +++++++------ net/netns/netns_android.go | 10 ++++++++-- net/netns/netns_darwin_tailscaled.go | 16 +++++++++++----- net/netns/netns_default.go | 14 +++++++++++--- net/netns/netns_linux.go | 9 +++++++-- net/netns/netns_test.go | 2 +- net/netns/netns_windows.go | 9 +++++++-- net/portmapper/portmapper.go | 2 +- net/portmapper/upnp.go | 2 +- wgengine/magicsock/magicsock.go | 2 +- 16 files changed, 64 insertions(+), 33 deletions(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index e24b34667..82b0f8229 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -160,7 +160,7 @@ func NewDirect(opts Options) (*Direct, error) { UseLastGood: true, LookupIPFallback: dnsfallback.Lookup, } - dialer := netns.NewDialer() + dialer := netns.NewDialer(opts.Logf) tr := http.DefaultTransport.(*http.Transport).Clone() tr.Proxy = tshttpproxy.ProxyFromEnvironment tshttpproxy.SetTransportGetProxyConnectHeader(tr) diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index f13ce508e..f94117a12 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -429,7 +429,7 @@ func (c *Client) dialURL(ctx context.Context) (net.Conn, error) { return c.dialer(ctx, "tcp", net.JoinHostPort(host, urlPort(c.url))) } hostOrIP := host - dialer := netns.NewDialer() + dialer := netns.NewDialer(c.logf) if c.DNSCache != nil { ip, _, _, err := c.DNSCache.LookupIP(ctx, host) @@ -519,7 +519,7 @@ func (c *Client) DialRegionTLS(ctx context.Context, reg *tailcfg.DERPRegion) (tl } func (c *Client) dialContext(ctx context.Context, proto, addr string) (net.Conn, error) { - return netns.NewDialer().DialContext(ctx, proto, addr) + return netns.NewDialer(c.logf).DialContext(ctx, proto, addr) } // shouldDialProto reports whether an explicitly provided IPv4 or IPv6 diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index 8633c26f9..fefbab25c 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -587,7 +587,7 @@ func newLogtailTransport(host string) *http.Transport { // Log whenever we dial: tr.DialContext = func(ctx context.Context, netw, addr string) (net.Conn, error) { - nd := netns.FromDialer(&net.Dialer{ + nd := netns.FromDialer(log.Printf, &net.Dialer{ Timeout: 30 * time.Second, KeepAlive: netknob.PlatformTCPKeepAlive(), }) diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 41ac11227..15ca87cdf 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -342,7 +342,7 @@ func (f *forwarder) getKnownDoHClient(ip netaddr.IP) (urlBase string, c *http.Cl if f.dohClient == nil { f.dohClient = map[string]*http.Client{} } - nsDialer := netns.NewDialer() + nsDialer := netns.NewDialer(f.logf) c = &http.Client{ Transport: &http.Transport{ IdleConnTimeout: dohTransportTimeout, diff --git a/net/dnsfallback/dnsfallback.go b/net/dnsfallback/dnsfallback.go index 5dd99fdc2..cdbb26844 100644 --- a/net/dnsfallback/dnsfallback.go +++ b/net/dnsfallback/dnsfallback.go @@ -90,7 +90,7 @@ func Lookup(ctx context.Context, host string) ([]netaddr.IP, error) { // serverName and serverIP of are, say, "derpN.tailscale.com". // queryName is the name being sought (e.g. "controlplane.tailscale.com"), passed as hint. func bootstrapDNSMap(ctx context.Context, serverName string, serverIP netaddr.IP, queryName string) (dnsMap, error) { - dialer := netns.NewDialer() + dialer := netns.NewDialer(log.Printf) tr := http.DefaultTransport.(*http.Transport).Clone() tr.Proxy = tshttpproxy.ProxyFromEnvironment tr.DialContext = func(ctx context.Context, netw, addr string) (net.Conn, error) { diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index e9fbdc381..2c0e70287 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -807,7 +807,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, } // Create a UDP4 socket used for sending to our discovered IPv4 address. - rs.pc4Hair, err = netns.Listener().ListenPacket(ctx, "udp4", ":0") + rs.pc4Hair, err = netns.Listener(c.logf).ListenPacket(ctx, "udp4", ":0") if err != nil { c.logf("udp4: %v", err) return nil, err @@ -835,7 +835,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, if f := c.GetSTUNConn4; f != nil { rs.pc4 = f() } else { - u4, err := netns.Listener().ListenPacket(ctx, "udp4", c.udpBindAddr()) + u4, err := netns.Listener(c.logf).ListenPacket(ctx, "udp4", c.udpBindAddr()) if err != nil { c.logf("udp4: %v", err) return nil, err @@ -848,7 +848,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, if f := c.GetSTUNConn6; f != nil { rs.pc6 = f() } else { - u6, err := netns.Listener().ListenPacket(ctx, "udp6", c.udpBindAddr()) + u6, err := netns.Listener(c.logf).ListenPacket(ctx, "udp6", c.udpBindAddr()) if err != nil { c.logf("udp6: %v", err) } else { diff --git a/net/netns/netns.go b/net/netns/netns.go index f3f7e0291..bdcf97089 100644 --- a/net/netns/netns.go +++ b/net/netns/netns.go @@ -21,6 +21,7 @@ import ( "inet.af/netaddr" "tailscale.com/net/netknob" "tailscale.com/syncs" + "tailscale.com/types/logger" ) var disabled syncs.AtomicBool @@ -34,19 +35,19 @@ func SetEnabled(on bool) { // Listener returns a new net.Listener with its Control hook func // initialized as necessary to run in logical network namespace that // doesn't route back into Tailscale. -func Listener() *net.ListenConfig { +func Listener(logf logger.Logf) *net.ListenConfig { if disabled.Get() { return new(net.ListenConfig) } - return &net.ListenConfig{Control: control} + return &net.ListenConfig{Control: control(logf)} } // NewDialer returns a new Dialer using a net.Dialer with its Control // hook func initialized as necessary to run in a logical network // namespace that doesn't route back into Tailscale. It also handles // using a SOCKS if configured in the environment with ALL_PROXY. -func NewDialer() Dialer { - return FromDialer(&net.Dialer{ +func NewDialer(logf logger.Logf) Dialer { + return FromDialer(logf, &net.Dialer{ KeepAlive: netknob.PlatformTCPKeepAlive(), }) } @@ -55,11 +56,11 @@ func NewDialer() Dialer { // network namespace that doesn't route back into Tailscale. It also // handles using a SOCKS if configured in the environment with // ALL_PROXY. -func FromDialer(d *net.Dialer) Dialer { +func FromDialer(logf logger.Logf, d *net.Dialer) Dialer { if disabled.Get() { return d } - d.Control = control + d.Control = control(logf) if wrapDialer != nil { return wrapDialer(d) } diff --git a/net/netns/netns_android.go b/net/netns/netns_android.go index 6fbc3e377..dad3358d8 100644 --- a/net/netns/netns_android.go +++ b/net/netns/netns_android.go @@ -11,6 +11,8 @@ import ( "fmt" "sync" "syscall" + + "tailscale.com/types/logger" ) var ( @@ -44,11 +46,15 @@ func SetAndroidProtectFunc(f func(fd int) error) { androidProtectFunc = f } -// control marks c as necessary to dial in a separate network namespace. +func control(logger.Logf) func(network, address string, c syscall.RawConn) error { + return controlC +} + +// controlC marks c as necessary to dial in a separate network namespace. // // It's intentionally the same signature as net.Dialer.Control // and net.ListenConfig.Control. -func control(network, address string, c syscall.RawConn) error { +func controlC(network, address string, c syscall.RawConn) error { var sockErr error err := c.Control(func(fd uintptr) { androidProtectFuncMu.Lock() diff --git a/net/netns/netns_darwin_tailscaled.go b/net/netns/netns_darwin_tailscaled.go index cc2b74c68..ce35aa999 100644 --- a/net/netns/netns_darwin_tailscaled.go +++ b/net/netns/netns_darwin_tailscaled.go @@ -9,26 +9,32 @@ package netns import ( "fmt" - "log" "strings" "syscall" "golang.org/x/sys/unix" "tailscale.com/net/interfaces" + "tailscale.com/types/logger" ) -// control marks c as necessary to dial in a separate network namespace. +func control(logf logger.Logf) func(network, address string, c syscall.RawConn) error { + return func(network, address string, c syscall.RawConn) error { + return controlLogf(logf, network, address, c) + } +} + +// controlLogf marks c as necessary to dial in a separate network namespace. // // It's intentionally the same signature as net.Dialer.Control // and net.ListenConfig.Control. -func control(network, address string, c syscall.RawConn) error { +func controlLogf(logf logger.Logf, network, address string, c syscall.RawConn) error { if strings.HasPrefix(address, "127.") || address == "::1" { // Don't bind to an interface for localhost connections. return nil } idx, err := interfaces.DefaultRouteInterfaceIndex() if err != nil { - log.Printf("netns: DefaultRouteInterfaceIndex: %v", err) + logf("[unexpected] netns: DefaultRouteInterfaceIndex: %v", err) return nil } v6 := strings.Contains(address, "]:") || strings.HasSuffix(network, "6") // hacky test for v6 @@ -47,7 +53,7 @@ func control(network, address string, c syscall.RawConn) error { return fmt.Errorf("RawConn.Control on %T: %w", c, err) } if sockErr != nil { - log.Printf("netns: control(%q, %q), v6=%v, index=%v: %v", network, address, v6, idx, sockErr) + logf("[unexpected] netns: control(%q, %q), v6=%v, index=%v: %v", network, address, v6, idx, sockErr) } return sockErr } diff --git a/net/netns/netns_default.go b/net/netns/netns_default.go index 825027f2f..009cd01c1 100644 --- a/net/netns/netns_default.go +++ b/net/netns/netns_default.go @@ -7,9 +7,17 @@ package netns -import "syscall" +import ( + "syscall" -// control does nothing to c. -func control(network, address string, c syscall.RawConn) error { + "tailscale.com/types/logger" +) + +func control(logger.Logf) func(network, address string, c syscall.RawConn) error { + return controlC +} + +// controlC does nothing to c. +func controlC(network, address string, c syscall.RawConn) error { return nil } diff --git a/net/netns/netns_linux.go b/net/netns/netns_linux.go index 05f9313d9..d2d5872ec 100644 --- a/net/netns/netns_linux.go +++ b/net/netns/netns_linux.go @@ -17,6 +17,7 @@ import ( "golang.org/x/sys/unix" "tailscale.com/net/interfaces" + "tailscale.com/types/logger" ) // tailscaleBypassMark is the mark indicating that packets originating @@ -82,11 +83,15 @@ func ignoreErrors() bool { return false } -// control marks c as necessary to dial in a separate network namespace. +func control(logger.Logf) func(network, address string, c syscall.RawConn) error { + return controlC +} + +// controlC marks c as necessary to dial in a separate network namespace. // // It's intentionally the same signature as net.Dialer.Control // and net.ListenConfig.Control. -func control(network, address string, c syscall.RawConn) error { +func controlC(network, address string, c syscall.RawConn) error { if isLocalhost(address) { // Don't bind to an interface for localhost connections. return nil diff --git a/net/netns/netns_test.go b/net/netns/netns_test.go index caaf78079..ce8801f0c 100644 --- a/net/netns/netns_test.go +++ b/net/netns/netns_test.go @@ -25,7 +25,7 @@ func TestDial(t *testing.T) { if !*extNetwork { t.Skip("skipping test without --use-external-network") } - d := NewDialer() + d := NewDialer(t.Logf) c, err := d.Dial("tcp", "google.com:80") if err != nil { t.Fatal(err) diff --git a/net/netns/netns_windows.go b/net/netns/netns_windows.go index 9ea475ea1..deff38936 100644 --- a/net/netns/netns_windows.go +++ b/net/netns/netns_windows.go @@ -12,6 +12,7 @@ import ( "golang.org/x/sys/windows" "golang.zx2c4.com/wireguard/windows/tunnel/winipcfg" "tailscale.com/net/interfaces" + "tailscale.com/types/logger" "tailscale.com/util/endian" ) @@ -26,9 +27,13 @@ func interfaceIndex(iface *winipcfg.IPAdapterAddresses) uint32 { return iface.IfIndex } -// control binds c to the Windows interface that holds a default +func control(logger.Logf) func(network, address string, c syscall.RawConn) error { + return controlC +} + +// controlC binds c to the Windows interface that holds a default // route, and is not the Tailscale WinTun interface. -func control(network, address string, c syscall.RawConn) error { +func controlC(network, address string, c syscall.RawConn) error { if strings.HasPrefix(address, "127.") { // Don't bind to an interface for localhost connections, // otherwise we get: diff --git a/net/portmapper/portmapper.go b/net/portmapper/portmapper.go index fdbb9ba32..cf4227b0c 100644 --- a/net/portmapper/portmapper.go +++ b/net/portmapper/portmapper.go @@ -251,7 +251,7 @@ func (c *Client) listenPacket(ctx context.Context, network, addr string) (net.Pa var lc net.ListenConfig return lc.ListenPacket(ctx, network, addr) } - return netns.Listener().ListenPacket(ctx, network, addr) + return netns.Listener(c.logf).ListenPacket(ctx, network, addr) } func (c *Client) invalidateMappingsLocked(releaseOld bool) { diff --git a/net/portmapper/upnp.go b/net/portmapper/upnp.go index de6b32026..54cb8a211 100644 --- a/net/portmapper/upnp.go +++ b/net/portmapper/upnp.go @@ -219,7 +219,7 @@ func (c *Client) upnpHTTPClientLocked() *http.Client { if c.uPnPHTTPClient == nil { c.uPnPHTTPClient = &http.Client{ Transport: &http.Transport{ - DialContext: netns.NewDialer().DialContext, + DialContext: netns.NewDialer(c.logf).DialContext, IdleConnTimeout: 2 * time.Second, // LAN is cheap }, } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 6f14f13b1..1ac79616f 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -2719,7 +2719,7 @@ func (c *Conn) listenPacket(network string, port uint16) (net.PacketConn, error) if c.testOnlyPacketListener != nil { return c.testOnlyPacketListener.ListenPacket(ctx, network, addr) } - return netns.Listener().ListenPacket(ctx, network, addr) + return netns.Listener(c.logf).ListenPacket(ctx, network, addr) } // bindSocket initializes rucPtr if necessary and binds a UDP socket to it.