Skip to content

Commit 4941977

Browse files
deansheatherethanndickson
deansheather
and
ethanndickson
authored
fix: add soft isolation mode and windows impl (#88)
Co-authored-by: Ethan Dickson <ethan@coder.com>
1 parent f14d20d commit 4941977

File tree

20 files changed

+335
-73
lines changed

20 files changed

+335
-73
lines changed

‎control/controlbase/conn_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func TestConnStd(t *testing.T) {
228228
}
229229

230230
// tests that the idle memory overhead of a Conn blocked in a read is
231-
// reasonable (under 2K). It was previously over 8KB with two 4KB
231+
// reasonable (under 2.5K). It was previously over 8KB with two 4KB
232232
// buffers for rx/tx. This make sure we don't regress. Hopefully it
233233
// doesn't turn into a flaky test. If so, const max can be adjusted,
234234
// or it can be deleted or reworked.
@@ -281,7 +281,7 @@ func TestConnMemoryOverhead(t *testing.T) {
281281
growthTotal := int64(ms.HeapAlloc) - int64(ms0.HeapAlloc)
282282
growthEach := float64(growthTotal) / float64(num)
283283
t.Logf("Alloced %v bytes, %.2f B/each", growthTotal, growthEach)
284-
const max = 2000
284+
const max = 2500
285285
if growthEach > max {
286286
t.Errorf("allocated more than expected; want max %v bytes/each", max)
287287
}

‎net/dns/nm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func (m *nmManager) trySet(ctx context.Context, config OSConfig) error {
139139
// tell it explicitly to keep it. Read out the current interface
140140
// settings and mirror them out to NetworkManager.
141141
var addrs6 []map[string]any
142-
addrs, _, err := interfaces.Tailscale()
142+
addrs, _, err := interfaces.Coder()
143143
if err == nil {
144144
for _, a := range addrs {
145145
if a.Is6() {

‎net/interfaces/interfaces.go

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,16 @@ import (
2424
// which HTTP proxy the system should use.
2525
var LoginEndpointForProxyDetermination = "https://controlplane.tailscale.com/"
2626

27-
// Tailscale returns the current machine's Tailscale interface, if any.
27+
// Coder returns the current machine's Coder interface, if any.
2828
// If none is found, all zero values are returned.
2929
// A non-nil error is only returned on a problem listing the system interfaces.
30-
func Tailscale() ([]netip.Addr, *net.Interface, error) {
30+
func Coder() ([]netip.Addr, *net.Interface, error) {
3131
ifs, err := netInterfaces()
3232
if err != nil {
3333
return nil, nil, err
3434
}
3535
for _, iface := range ifs {
36-
if !maybeTailscaleInterfaceName(iface.Name) {
36+
if !maybeCoderInterfaceName(iface.Name) {
3737
continue
3838
}
3939
addrs, err := iface.Addrs()
@@ -45,7 +45,7 @@ func Tailscale() ([]netip.Addr, *net.Interface, error) {
4545
if ipnet, ok := a.(*net.IPNet); ok {
4646
nip, ok := netip.AddrFromSlice(ipnet.IP)
4747
nip = nip.Unmap()
48-
if ok && tsaddr.IsTailscaleIP(nip) {
48+
if ok && tsaddr.IsCoderIP(nip) {
4949
tsIPs = append(tsIPs, nip)
5050
}
5151
}
@@ -57,13 +57,11 @@ func Tailscale() ([]netip.Addr, *net.Interface, error) {
5757
return nil, nil, nil
5858
}
5959

60-
// maybeTailscaleInterfaceName reports whether s is an interface
61-
// name that might be used by Tailscale.
62-
func maybeTailscaleInterfaceName(s string) bool {
63-
return s == "Tailscale" ||
64-
strings.HasPrefix(s, "wg") ||
65-
strings.HasPrefix(s, "ts") ||
66-
strings.HasPrefix(s, "tailscale") ||
60+
// maybeCoderInterfaceName reports whether s is an interface
61+
// name that might be used by Coder.
62+
func maybeCoderInterfaceName(s string) bool {
63+
return s == "Coder" ||
64+
strings.HasPrefix(s, "coder") ||
6765
strings.HasPrefix(s, "utun")
6866
}
6967

@@ -120,7 +118,7 @@ func LocalAddresses() (regular, loopback []netip.Addr, err error) {
120118
// very well be something we can route to
121119
// directly, because both nodes are
122120
// behind the same CGNAT router.
123-
if tsaddr.IsTailscaleIP(ip) {
121+
if tsaddr.IsCoderIP(ip) {
124122
continue
125123
}
126124
if ip.IsLoopback() || ifcIsLoopback {
@@ -479,7 +477,7 @@ func (s *State) AnyInterfaceUp() bool {
479477

480478
func hasTailscaleIP(pfxs []netip.Prefix) bool {
481479
for _, pfx := range pfxs {
482-
if tsaddr.IsTailscaleIP(pfx.Addr()) {
480+
if tsaddr.IsCoderIP(pfx.Addr()) {
483481
return true
484482
}
485483
}
@@ -496,8 +494,8 @@ func isTailscaleInterface(name string, ips []netip.Prefix) bool {
496494
// macOS NetworkExtensions and utun devices.
497495
return true
498496
}
499-
return name == "Tailscale" || // as it is on Windows
500-
strings.HasPrefix(name, "tailscale") // TODO: use --tun flag value, etc; see TODO in method doc
497+
return name == "Coder" || // as it is on Windows
498+
strings.HasPrefix(name, "coder") // TODO: use --tun flag value, etc; see TODO in method doc
501499
}
502500

503501
// getPAC, if non-nil, returns the current PAC file URL.

‎net/netmon/netmon_linux.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ func (c *nlConn) Receive() (message, error) {
178178
if rmsg.Table == tsTable && dst.IsSingleIP() {
179179
// Don't log. Spammy and normal to see a bunch of these on start-up,
180180
// which we make ourselves.
181-
} else if tsaddr.IsTailscaleIP(dst.Addr()) {
181+
} else if tsaddr.IsCoderIP(dst.Addr()) {
182182
// Verbose only.
183183
c.logf("%s: [v1] src=%v, dst=%v, gw=%v, outif=%v, table=%v", typeStr,
184184
condNetAddrPrefix(src), condNetAddrPrefix(dst), condNetAddrIP(gw),
@@ -271,7 +271,7 @@ type newRouteMessage struct {
271271
const tsTable = 52
272272

273273
func (m *newRouteMessage) ignore() bool {
274-
return m.Table == tsTable || tsaddr.IsTailscaleIP(m.Dst.Addr())
274+
return m.Table == tsTable || tsaddr.IsCoderIP(m.Dst.Addr())
275275
}
276276

277277
// newAddrMessage is a message for a new address being added.
@@ -282,7 +282,7 @@ type newAddrMessage struct {
282282
}
283283

284284
func (m *newAddrMessage) ignore() bool {
285-
return tsaddr.IsTailscaleIP(m.Addr)
285+
return tsaddr.IsCoderIP(m.Addr)
286286
}
287287

288288
type ignoreMessage struct{}

‎net/netmon/netmon_windows.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func (m *winMon) Receive() (message, error) {
131131
// unicastAddressChanged is the callback we register with Windows to call when unicast address changes.
132132
func (m *winMon) unicastAddressChanged(_ winipcfg.MibNotificationType, row *winipcfg.MibUnicastIPAddressRow) {
133133
what := "addr"
134-
if ip := row.Address.Addr(); ip.IsValid() && tsaddr.IsTailscaleIP(ip.Unmap()) {
134+
if ip := row.Address.Addr(); ip.IsValid() && tsaddr.IsCoderIP(ip.Unmap()) {
135135
what = "tsaddr"
136136
}
137137

@@ -143,7 +143,7 @@ func (m *winMon) unicastAddressChanged(_ winipcfg.MibNotificationType, row *wini
143143
func (m *winMon) routeChanged(_ winipcfg.MibNotificationType, row *winipcfg.MibIPforwardRow2) {
144144
what := "route"
145145
ip := row.DestinationPrefix.Prefix().Addr().Unmap()
146-
if ip.IsValid() && tsaddr.IsTailscaleIP(ip) {
146+
if ip.IsValid() && tsaddr.IsCoderIP(ip) {
147147
what = "tsroute"
148148
}
149149
// start a goroutine to finish our work, to return to Windows out of this callback

‎net/netns/netns.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,39 @@ func SetDisableBindConnToInterface(v bool) {
5353
disableBindConnToInterface.Store(v)
5454
}
5555

56+
var coderSoftIsolation atomic.Bool
57+
58+
// SetCoderSoftIsolation enables or disables Coder's soft-isolation
59+
// functionality. All other network isolation settings are ignored when this is
60+
// set.
61+
//
62+
// Soft isolation is a workaround for allowing Coder Connect to function with
63+
// corporate VPNs. Without this, Coder Connect cannot connect to Coder
64+
// deployments behind corporate VPNs.
65+
//
66+
// Soft isolation does the following:
67+
// 1. Determine the interface that will be used for a given destination IP by
68+
// consulting the OS.
69+
// 2. If that interface looks like our own, we will bind the socket to the
70+
// default interface (to match the existing behavior).
71+
// 3. If it doesn't look like our own, we will let the packet flow through
72+
// without binding the socket to the interface.
73+
//
74+
// This is considered "soft" because it doesn't force the socket to be bound to
75+
// a single interface, which causes problems with direct connections in
76+
// magicsock.
77+
//
78+
// Enabling this has the risk of potential network loops, as sockets could race
79+
// changes to the OS routing table or interface list. Coder doesn't provide
80+
// functionality similar to Tailscale's Exit Nodes, so we don't expect loops
81+
// to occur in our use case.
82+
//
83+
// This currently only has an effect on Windows and macOS, and is only used by
84+
// Coder Connect.
85+
func SetCoderSoftIsolation(v bool) {
86+
coderSoftIsolation.Store(v)
87+
}
88+
5689
// Listener returns a new net.Listener with its Control hook func
5790
// initialized as necessary to run in logical network namespace that
5891
// doesn't route back into Tailscale.

‎net/netns/netns_darwin.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ func getInterfaceIndex(logf logger.Logf, netMon *netmon.Monitor, address string)
108108
return defaultIdx()
109109
}
110110

111-
// Verify that we didn't just choose the Tailscale interface;
111+
// Verify that we didn't just choose the Coder interface;
112112
// if so, we fall back to binding from the default.
113-
_, tsif, err2 := interfaces.Tailscale()
113+
_, tsif, err2 := interfaces.Coder()
114114
if err2 == nil && tsif != nil && tsif.Index == idx {
115-
logf("[unexpected] netns: interfaceIndexFor returned Tailscale interface")
115+
logf("[unexpected] netns: interfaceIndexFor returned Coder interface")
116116
return defaultIdx()
117117
}
118118

‎net/netns/netns_darwin_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func TestGetInterfaceIndex(t *testing.T) {
5555
}
5656

5757
t.Run("NoTailscale", func(t *testing.T) {
58-
_, tsif, err := interfaces.Tailscale()
58+
_, tsif, err := interfaces.Coder()
5959
if err != nil {
6060
t.Fatal(err)
6161
}

‎net/netns/netns_windows.go

Lines changed: 115 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
package netns
55

66
import (
7+
"fmt"
78
"math/bits"
9+
"net"
10+
"net/netip"
11+
"strconv"
812
"strings"
913
"syscall"
1014

@@ -27,20 +31,30 @@ func interfaceIndex(iface *winipcfg.IPAdapterAddresses) uint32 {
2731
return iface.IfIndex
2832
}
2933

30-
func control(logger.Logf, *netmon.Monitor) func(network, address string, c syscall.RawConn) error {
31-
return controlC
34+
// getBestInterface can be swapped out in tests.
35+
var getBestInterface func(addr windows.Sockaddr, idx *uint32) error = windows.GetBestInterfaceEx
36+
37+
// isInterfaceCoderInterface can be swapped out in tests.
38+
var isInterfaceCoderInterface func(int) bool = isInterfaceCoderInterfaceDefault
39+
40+
func isInterfaceCoderInterfaceDefault(idx int) bool {
41+
_, tsif, err := interfaces.Coder()
42+
return err == nil && tsif != nil && tsif.Index == idx
43+
}
44+
45+
func control(logf logger.Logf, netMon *netmon.Monitor) func(network, address string, c syscall.RawConn) error {
46+
return func(network, address string, c syscall.RawConn) error {
47+
return controlLogf(logf, netMon, network, address, c)
48+
}
3249
}
3350

3451
// controlC binds c to the Windows interface that holds a default
3552
// route, and is not the Tailscale WinTun interface.
36-
func controlC(network, address string, c syscall.RawConn) error {
37-
if strings.HasPrefix(address, "127.") {
38-
// Don't bind to an interface for localhost connections,
39-
// otherwise we get:
40-
// connectex: The requested address is not valid in its context
41-
// (The derphttp tests were failing)
53+
func controlLogf(logf logger.Logf, _ *netmon.Monitor, network, address string, c syscall.RawConn) error {
54+
if !shouldBindToDefaultInterface(logf, address) {
4255
return nil
4356
}
57+
4458
canV4, canV6 := false, false
4559
switch network {
4660
case "tcp", "udp":
@@ -74,6 +88,54 @@ func controlC(network, address string, c syscall.RawConn) error {
7488
return nil
7589
}
7690

91+
func shouldBindToDefaultInterface(logf logger.Logf, address string) bool {
92+
if strings.HasPrefix(address, "127.") {
93+
// Don't bind to an interface for localhost connections,
94+
// otherwise we get:
95+
// connectex: The requested address is not valid in its context
96+
// (The derphttp tests were failing)
97+
return false
98+
}
99+
100+
if coderSoftIsolation.Load() {
101+
sockAddr, err := getSockAddr(address)
102+
if err != nil {
103+
logf("[unexpected] netns: Coder soft isolation: error getting sockaddr for %q, binding to default: %v", address, err)
104+
return true
105+
}
106+
if sockAddr == nil {
107+
// Unspecified addresses should not be bound to any interface.
108+
return false
109+
}
110+
111+
// Ask Windows to find the best interface for this address by consulting
112+
// the routing table.
113+
//
114+
// On macOS this value gets cached, but on Windows we don't need to
115+
// because this API is very fast and doesn't require opening an AF_ROUTE
116+
// socket.
117+
var idx uint32
118+
err = getBestInterface(sockAddr, &idx)
119+
if err != nil {
120+
logf("[unexpected] netns: Coder soft isolation: error getting best interface, binding to default: %v", err)
121+
return true
122+
}
123+
124+
if isInterfaceCoderInterface(int(idx)) {
125+
logf("[unexpected] netns: Coder soft isolation: detected socket destined for Coder interface, binding to default")
126+
return true
127+
}
128+
129+
// It doesn't look like our own interface, so we don't need to bind the
130+
// socket to the default interface.
131+
return false
132+
}
133+
134+
// The default isolation behavior is to always bind to the default
135+
// interface.
136+
return true
137+
}
138+
77139
// sockoptBoundInterface is the value of IP_UNICAST_IF and IPV6_UNICAST_IF.
78140
//
79141
// See https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options
@@ -124,3 +186,48 @@ func nativeToBigEndian(i uint32) uint32 {
124186
}
125187
return bits.ReverseBytes32(i)
126188
}
189+
190+
// getSockAddr returns the Windows sockaddr for the given address, or nil if
191+
// the address is not specified.
192+
func getSockAddr(address string) (windows.Sockaddr, error) {
193+
host, port, err := net.SplitHostPort(address)
194+
if err != nil {
195+
return nil, fmt.Errorf("invalid address %q: %w", address, err)
196+
}
197+
if host == "" {
198+
// netip.ParseAddr("") will fail
199+
return nil, nil
200+
}
201+
202+
addr, err := netip.ParseAddr(host)
203+
if err != nil {
204+
return nil, fmt.Errorf("invalid address %q: %w", address, err)
205+
}
206+
if addr.Zone() != "" {
207+
// Addresses with zones *can* be represented as a Sockaddr with extra
208+
// effort, but we don't use or support them currently.
209+
return nil, fmt.Errorf("invalid address %q, has zone: %w", address, err)
210+
}
211+
if addr.IsUnspecified() {
212+
// This covers the cases of 0.0.0.0 and [::].
213+
return nil, nil
214+
}
215+
216+
portInt, err := strconv.ParseUint(port, 10, 16)
217+
if err != nil {
218+
return nil, fmt.Errorf("invalid port %q: %w", port, err)
219+
}
220+
221+
if addr.Is4() {
222+
return &windows.SockaddrInet4{
223+
Port: int(portInt), // nolint:gosec // portInt is always in range
224+
Addr: addr.As4(),
225+
}, nil
226+
} else if addr.Is6() {
227+
return &windows.SockaddrInet6{
228+
Port: int(portInt), // nolint:gosec // portInt is always in range
229+
Addr: addr.As16(),
230+
}, nil
231+
}
232+
return nil, fmt.Errorf("invalid address %q, is not IPv4 or IPv6: %w", address, err)
233+
}

0 commit comments

Comments
 (0)