diff --git a/net/netns/netns.go b/net/netns/netns.go index 840b90fecdc9e..901f5b6a94311 100644 --- a/net/netns/netns.go +++ b/net/netns/netns.go @@ -15,12 +15,14 @@ package netns import ( "context" + "fmt" "net" "net/netip" "sync/atomic" "tailscale.com/net/netknob" "tailscale.com/net/netmon" + "tailscale.com/net/tsaddr" "tailscale.com/types/logger" ) @@ -160,3 +162,66 @@ func isLocalhost(addr string) bool { ip, _ := netip.ParseAddr(host) return ip.IsLoopback() } + +// shouldBindToDefaultInterface determines whether a socket should be bound to +// the default interface based on the destination address and soft isolation settings. +func shouldBindToDefaultInterface(logf logger.Logf, address string) bool { + if isLocalhost(address) { + // Don't bind to an interface for localhost connections. + return false + } + + if coderSoftIsolation.Load() { + addr, err := getAddr(address) + if err != nil { + logf("[unexpected] netns: Coder soft isolation: error getting addr for %q, binding to default: %v", address, err) + return true + } + if !addr.IsValid() || addr.IsUnspecified() { + // Invalid or unspecified addresses should not be bound to any + // interface. + return false + } + if tsaddr.IsCoderIP(addr) { + logf("[unexpected] netns: Coder soft isolation: detected socket destined for Coder interface, binding to default") + return true + } + + // It doesn't look like our own interface, so we don't need to bind the + // socket to the default interface. + return false + } + + // The default isolation behavior is to always bind to the default + // interface. + return true +} + +// getAddr returns the netip.Addr for the given address, or an invalid address +// if the address is not specified. Use addr.IsValid() to check for this. +func getAddr(address string) (netip.Addr, error) { + host, _, err := net.SplitHostPort(address) + if err != nil { + return netip.Addr{}, fmt.Errorf("invalid address %q: %w", address, err) + } + if host == "" { + // netip.ParseAddr("") will fail + return netip.Addr{}, nil + } + + addr, err := netip.ParseAddr(host) + if err != nil { + return netip.Addr{}, fmt.Errorf("invalid address %q: %w", address, err) + } + if addr.Zone() != "" { + // Addresses with zones *can* be represented as a Sockaddr with extra + // effort, but we don't use or support them currently. + return netip.Addr{}, fmt.Errorf("invalid address %q, has zone: %w", address, err) + } + if addr.IsUnspecified() { + // This covers the cases of 0.0.0.0 and [::]. + return netip.Addr{}, nil + } + + return addr, nil +} diff --git a/net/netns/netns_darwin.go b/net/netns/netns_darwin.go index 0bcd84a21c0a3..6f45e8e2f7eae 100644 --- a/net/netns/netns_darwin.go +++ b/net/netns/netns_darwin.go @@ -38,8 +38,7 @@ var errInterfaceStateInvalid = errors.New("interface state invalid") // It's intentionally the same signature as net.Dialer.Control // and net.ListenConfig.Control. func controlLogf(logf logger.Logf, netMon *netmon.Monitor, network, address string, c syscall.RawConn) error { - if isLocalhost(address) { - // Don't bind to an interface for localhost connections. + if !shouldBindToDefaultInterface(logf, address) { return nil } diff --git a/net/netns/netns_test.go b/net/netns/netns_test.go index 82f919b946d4a..d62f8e141f651 100644 --- a/net/netns/netns_test.go +++ b/net/netns/netns_test.go @@ -15,7 +15,10 @@ package netns import ( "flag" + "fmt" "testing" + + "tailscale.com/net/tsaddr" ) var extNetwork = flag.Bool("use-external-network", false, "use the external network in tests") @@ -76,3 +79,74 @@ func TestIsLocalhost(t *testing.T) { } } } + +func TestShouldBindToDefaultInterface(t *testing.T) { + t.Run("Normal", func(t *testing.T) { + tests := []struct { + address string + want bool + }{ + {"127.0.0.1:0", false}, + {"127.0.0.1:1234", false}, + {"1.2.3.4:0", true}, + {"1.2.3.4:1234", true}, + } + + for _, test := range tests { + t.Run(test.address, func(t *testing.T) { + got := shouldBindToDefaultInterface(t.Logf, test.address) + if got != test.want { + t.Errorf("want %v, got %v", test.want, got) + } + }) + } + }) + + t.Run("CoderSoftIsolation", func(t *testing.T) { + SetCoderSoftIsolation(true) + t.Cleanup(func() { + SetCoderSoftIsolation(false) + }) + + tests := []struct { + address string + want bool + }{ + // localhost should still not bind to any interface. + {"127.0.0.1:0", false}, + {"127.0.0.1:0", false}, + {"127.0.0.1:1234", false}, + {"127.0.0.1:1234", false}, + + // Unspecified addresses should not be bound to any interface. + {":1234", false}, + {":1234", false}, + {"0.0.0.0:1234", false}, + {"0.0.0.0:1234", false}, + {"[::]:1234", false}, + {"[::]:1234", false}, + + // Special cases should always bind to default: + {"[::%eth0]:1234", true}, // zones are not supported + {"a:1234", true}, // not an IP + + // Coder IPs should bind to default. + {fmt.Sprintf("[%s]:8080", tsaddr.CoderServiceIPv6()), true}, + {fmt.Sprintf("[%s]:8080", tsaddr.CoderV6Range().Addr().Next()), true}, + + // Non-Coder IPs should not bind to default. + {fmt.Sprintf("[%s]:8080", tsaddr.TailscaleServiceIPv6()), false}, + {fmt.Sprintf("%s:8080", tsaddr.TailscaleServiceIP()), false}, + {"1.2.3.4:8080", false}, + } + + for _, test := range tests { + t.Run(test.address, func(t *testing.T) { + got := shouldBindToDefaultInterface(t.Logf, test.address) + if got != test.want { + t.Errorf("want %v, got %v", test.want, got) + } + }) + } + }) +} diff --git a/net/netns/netns_windows.go b/net/netns/netns_windows.go index f7554e222ed6a..72bb29ce343b2 100644 --- a/net/netns/netns_windows.go +++ b/net/netns/netns_windows.go @@ -4,11 +4,7 @@ package netns import ( - "fmt" "math/bits" - "net" - "net/netip" - "strings" "syscall" "golang.org/x/sys/cpu" @@ -16,7 +12,6 @@ import ( "golang.zx2c4.com/wireguard/windows/tunnel/winipcfg" "tailscale.com/net/interfaces" "tailscale.com/net/netmon" - "tailscale.com/net/tsaddr" "tailscale.com/types/logger" ) @@ -77,40 +72,6 @@ func controlLogf(logf logger.Logf, _ *netmon.Monitor, network, address string, c return nil } -func shouldBindToDefaultInterface(logf logger.Logf, address string) bool { - if strings.HasPrefix(address, "127.") { - // Don't bind to an interface for localhost connections, - // otherwise we get: - // connectex: The requested address is not valid in its context - // (The derphttp tests were failing) - return false - } - - if coderSoftIsolation.Load() { - addr, err := getAddr(address) - if err != nil { - logf("[unexpected] netns: Coder soft isolation: error getting addr for %q, binding to default: %v", address, err) - return true - } - if !addr.IsValid() || addr.IsUnspecified() { - // Invalid or unspecified addresses should not be bound to any - // interface. - return false - } - if tsaddr.IsCoderIP(addr) { - logf("[unexpected] netns: Coder soft isolation: detected socket destined for Coder interface, binding to default") - return true - } - - // It doesn't look like our own interface, so we don't need to bind the - // socket to the default interface. - return false - } - - // The default isolation behavior is to always bind to the default - // interface. - return true -} // sockoptBoundInterface is the value of IP_UNICAST_IF and IPV6_UNICAST_IF. // @@ -163,31 +124,3 @@ func nativeToBigEndian(i uint32) uint32 { return bits.ReverseBytes32(i) } -// getAddr returns the netip.Addr for the given address, or an invalid address -// if the address is not specified. Use addr.IsValid() to check for this. -func getAddr(address string) (netip.Addr, error) { - host, _, err := net.SplitHostPort(address) - if err != nil { - return netip.Addr{}, fmt.Errorf("invalid address %q: %w", address, err) - } - if host == "" { - // netip.ParseAddr("") will fail - return netip.Addr{}, nil - } - - addr, err := netip.ParseAddr(host) - if err != nil { - return netip.Addr{}, fmt.Errorf("invalid address %q: %w", address, err) - } - if addr.Zone() != "" { - // Addresses with zones *can* be represented as a Sockaddr with extra - // effort, but we don't use or support them currently. - return netip.Addr{}, fmt.Errorf("invalid address %q, has zone: %w", address, err) - } - if addr.IsUnspecified() { - // This covers the cases of 0.0.0.0 and [::]. - return netip.Addr{}, nil - } - - return addr, nil -} diff --git a/net/netns/netns_windows_test.go b/net/netns/netns_windows_test.go deleted file mode 100644 index ab4eb110e9270..0000000000000 --- a/net/netns/netns_windows_test.go +++ /dev/null @@ -1,82 +0,0 @@ -// Copyright (c) Tailscale Inc & AUTHORS -// SPDX-License-Identifier: BSD-3-Clause - -package netns - -import ( - "fmt" - "testing" - - "tailscale.com/net/tsaddr" -) - -func TestShouldBindToDefaultInterface(t *testing.T) { - t.Run("Normal", func(t *testing.T) { - tests := []struct { - address string - want bool - }{ - {"127.0.0.1:0", false}, - {"127.0.0.1:1234", false}, - {"1.2.3.4:0", true}, - {"1.2.3.4:1234", true}, - } - - for _, test := range tests { - t.Run(test.address, func(t *testing.T) { - got := shouldBindToDefaultInterface(t.Logf, test.address) - if got != test.want { - t.Errorf("want %v, got %v", test.want, got) - } - }) - } - }) - - t.Run("CoderSoftIsolation", func(t *testing.T) { - SetCoderSoftIsolation(true) - t.Cleanup(func() { - SetCoderSoftIsolation(false) - }) - - tests := []struct { - address string - want bool - }{ - // localhost should still not bind to any interface. - {"127.0.0.1:0", false}, - {"127.0.0.1:0", false}, - {"127.0.0.1:1234", false}, - {"127.0.0.1:1234", false}, - - // Unspecified addresses should not be bound to any interface. - {":1234", false}, - {":1234", false}, - {"0.0.0.0:1234", false}, - {"0.0.0.0:1234", false}, - {"[::]:1234", false}, - {"[::]:1234", false}, - - // Special cases should always bind to default: - {"[::%eth0]:1234", true}, // zones are not supported - {"a:1234", true}, // not an IP - - // Coder IPs should bind to default. - {fmt.Sprintf("[%s]:8080", tsaddr.CoderServiceIPv6()), true}, - {fmt.Sprintf("[%s]:8080", tsaddr.CoderV6Range().Addr().Next()), true}, - - // Non-Coder IPs should not bind to default. - {fmt.Sprintf("[%s]:8080", tsaddr.TailscaleServiceIPv6()), false}, - {fmt.Sprintf("%s:8080", tsaddr.TailscaleServiceIP()), false}, - {"1.2.3.4:8080", false}, - } - - for _, test := range tests { - t.Run(test.address, func(t *testing.T) { - got := shouldBindToDefaultInterface(t.Logf, test.address) - if got != test.want { - t.Errorf("want %v, got %v", test.want, got) - } - }) - } - }) -}