Skip to content

Commit bf78966

Browse files
deansheatherdeansheather
authored
chore: remove soft isolation configurability (#19069)
Undoes a lot of the changes in 5319d47 Keeps the `netns.SetCoderSoftIsolation()` call, but always sets it to `true` when using a TUN device.
1 parent 1320b8d commit bf78966

File tree

8 files changed

+127
-196
lines changed

8 files changed

+127
-196
lines changed

‎tailnet/conn.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,6 @@ type Options struct {
102102
BlockEndpoints bool
103103
Logger slog.Logger
104104
ListenPort uint16
105-
// UseSoftNetIsolation enables our homemade soft isolation feature in the
106-
// netns package. This option will only be considered if TUNDev is set.
107-
//
108-
// The Coder soft isolation mode is a workaround to allow Coder Connect to
109-
// connect to Coder servers behind corporate VPNs, and relaxes some of the
110-
// loop protections that come with Tailscale.
111-
//
112-
// When soft isolation is disabled, the netns package will function as
113-
// normal and route all traffic through the default interface (and block all
114-
// traffic to other VPN interfaces) on macOS and Windows.
115-
UseSoftNetIsolation bool
116105

117106
// CaptureHook is a callback that captures Disco packets and packets sent
118107
// into the tailnet tunnel.
@@ -169,10 +158,13 @@ func NewConn(options *Options) (conn *Conn, err error) {
169158
}
170159

171160
useNetNS := options.TUNDev != nil
172-
useSoftIsolation := useNetNS && options.UseSoftNetIsolation
173-
options.Logger.Debug(context.Background(), "network isolation configuration", slog.F("use_netns", useNetNS), slog.F("use_soft_isolation", useSoftIsolation))
161+
options.Logger.Debug(context.Background(), "network isolation configuration", slog.F("use_netns", useNetNS))
174162
netns.SetEnabled(useNetNS)
175-
netns.SetCoderSoftIsolation(useSoftIsolation)
163+
// The Coder soft isolation mode is a workaround to allow Coder Connect to
164+
// connect to Coder servers behind corporate VPNs, and relaxes some of the
165+
// loop protections that come with Tailscale.
166+
// See the comment above the netns function for more details.
167+
netns.SetCoderSoftIsolation(useNetNS)
176168

177169
var telemetryStore *TelemetryStore
178170
if options.TelemetrySink != nil {

‎vpn/client.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,13 @@ func NewClient() Client {
6969
}
7070

7171
type Options struct {
72-
Headers http.Header
73-
Logger slog.Logger
74-
UseSoftNetIsolation bool
75-
DNSConfigurator dns.OSConfigurator
76-
Router router.Router
77-
TUNDevice tun.Device
78-
WireguardMonitor *netmon.Monitor
79-
UpdateHandler tailnet.UpdatesHandler
72+
Headers http.Header
73+
Logger slog.Logger
74+
DNSConfigurator dns.OSConfigurator
75+
Router router.Router
76+
TUNDevice tun.Device
77+
WireguardMonitor *netmon.Monitor
78+
UpdateHandler tailnet.UpdatesHandler
8079
}
8180

8281
type derpMapRewriter struct {
@@ -164,7 +163,6 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string
164163
DERPForceWebSockets: connInfo.DERPForceWebSockets,
165164
Logger: options.Logger,
166165
BlockEndpoints: connInfo.DisableDirectConnections,
167-
UseSoftNetIsolation: options.UseSoftNetIsolation,
168166
DNSConfigurator: options.DNSConfigurator,
169167
Router: options.Router,
170168
TUNDev: options.TUNDevice,

‎vpn/speaker_internal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestMain(m *testing.M) {
2323
goleak.VerifyTestMain(m, testutil.GoleakOptions...)
2424
}
2525

26-
const expectedHandshake = "codervpn tunnel 1.3\n"
26+
const expectedHandshake = "codervpn tunnel 1.2\n"
2727

2828
// TestSpeaker_RawPeer tests the speaker with a peer that we simulate by directly making reads and
2929
// writes to the other end of the pipe. There should be at least one test that does this, rather

‎vpn/tunnel.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -265,14 +265,13 @@ func (t *Tunnel) start(req *StartRequest) error {
265265
svrURL,
266266
apiToken,
267267
&Options{
268-
Headers: header,
269-
Logger: t.clientLogger,
270-
UseSoftNetIsolation: req.GetTunnelUseSoftNetIsolation(),
271-
DNSConfigurator: networkingStack.DNSConfigurator,
272-
Router: networkingStack.Router,
273-
TUNDevice: networkingStack.TUNDevice,
274-
WireguardMonitor: networkingStack.WireguardMonitor,
275-
UpdateHandler: t,
268+
Headers: header,
269+
Logger: t.clientLogger,
270+
DNSConfigurator: networkingStack.DNSConfigurator,
271+
Router: networkingStack.Router,
272+
TUNDevice: networkingStack.TUNDevice,
273+
WireguardMonitor: networkingStack.WireguardMonitor,
274+
UpdateHandler: t,
276275
},
277276
)
278277
if err != nil {

‎vpn/tunnel_internal_test.go

Lines changed: 28 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@ package vpn
22

33
import (
44
"context"
5-
"encoding/json"
65
"maps"
76
"net"
8-
"net/http"
97
"net/netip"
108
"net/url"
119
"slices"
@@ -24,51 +22,32 @@ import (
2422
"github.com/coder/quartz"
2523

2624
maputil "github.com/coder/coder/v2/coderd/util/maps"
27-
"github.com/coder/coder/v2/codersdk"
2825
"github.com/coder/coder/v2/tailnet"
2926
"github.com/coder/coder/v2/tailnet/proto"
3027
"github.com/coder/coder/v2/testutil"
3128
)
3229

3330
func newFakeClient(ctx context.Context, t *testing.T) *fakeClient {
3431
return &fakeClient{
35-
t: t,
36-
ctx: ctx,
37-
connCh: make(chan *fakeConn, 1),
38-
}
39-
}
40-
41-
func newFakeClientWithOptsCh(ctx context.Context, t *testing.T) *fakeClient {
42-
return &fakeClient{
43-
t: t,
44-
ctx: ctx,
45-
connCh: make(chan *fakeConn, 1),
46-
optsCh: make(chan *Options, 1),
32+
t: t,
33+
ctx: ctx,
34+
ch: make(chan *fakeConn, 1),
4735
}
4836
}
4937

5038
type fakeClient struct {
51-
t *testing.T
52-
ctx context.Context
53-
connCh chan *fakeConn
54-
optsCh chan *Options // options will be written to this channel if it's not nil
39+
t *testing.T
40+
ctx context.Context
41+
ch chan *fakeConn
5542
}
5643

5744
var _ Client = (*fakeClient)(nil)
5845

59-
func (f *fakeClient) NewConn(_ context.Context, _ *url.URL, _ string, opts *Options) (Conn, error) {
60-
if f.optsCh != nil {
61-
select {
62-
case <-f.ctx.Done():
63-
return nil, f.ctx.Err()
64-
case f.optsCh <- opts:
65-
}
66-
}
67-
46+
func (f *fakeClient) NewConn(context.Context, *url.URL, string, *Options) (Conn, error) {
6847
select {
6948
case <-f.ctx.Done():
7049
return nil, f.ctx.Err()
71-
case conn := <-f.connCh:
50+
case conn := <-f.ch:
7251
return conn, nil
7352
}
7453
}
@@ -155,53 +134,37 @@ func TestTunnel_StartStop(t *testing.T) {
155134
t.Parallel()
156135

157136
ctx := testutil.Context(t, testutil.WaitShort)
158-
client := newFakeClientWithOptsCh(ctx, t)
137+
client := newFakeClient(ctx, t)
159138
conn := newFakeConn(tailnet.WorkspaceUpdate{}, time.Time{})
160139

161140
_, mgr := setupTunnel(t, ctx, client, quartz.NewMock(t))
162141

163142
errCh := make(chan error, 1)
164143
var resp *TunnelMessage
165144
// When: we start the tunnel
166-
telemetry := codersdk.CoderDesktopTelemetry{
167-
DeviceID: "device001",
168-
DeviceOS: "macOS",
169-
CoderDesktopVersion: "0.24.8",
170-
}
171-
telemetryJSON, err := json.Marshal(telemetry)
172-
require.NoError(t, err)
173145
go func() {
174146
r, err := mgr.unaryRPC(ctx, &ManagerMessage{
175147
Msg: &ManagerMessage_Start{
176148
Start: &StartRequest{
177149
TunnelFileDescriptor: 2,
178-
// Use default value for TunnelUseSoftNetIsolation
179-
CoderUrl: "https://coder.example.com",
180-
ApiToken: "fakeToken",
150+
CoderUrl: "https://coder.example.com",
151+
ApiToken: "fakeToken",
181152
Headers: []*StartRequest_Header{
182153
{Name: "X-Test-Header", Value: "test"},
183154
},
184-
DeviceOs: telemetry.DeviceOS,
185-
DeviceId: telemetry.DeviceID,
186-
CoderDesktopVersion: telemetry.CoderDesktopVersion,
155+
DeviceOs: "macOS",
156+
DeviceId: "device001",
157+
CoderDesktopVersion: "0.24.8",
187158
},
188159
},
189160
})
190161
resp = r
191162
errCh <- err
192163
}()
193-
194-
// Then: `NewConn` is called
195-
opts := testutil.RequireReceive(ctx, t, client.optsCh)
196-
require.Equal(t, http.Header{
197-
"X-Test-Header": {"test"},
198-
codersdk.CoderDesktopTelemetryHeader: {string(telemetryJSON)},
199-
}, opts.Headers)
200-
require.False(t, opts.UseSoftNetIsolation) // the default is false
201-
testutil.RequireSend(ctx, t, client.connCh, conn)
202-
164+
// Then: `NewConn` is called,
165+
testutil.RequireSend(ctx, t, client.ch, conn)
203166
// And: a response is received
204-
err = testutil.TryReceive(ctx, t, errCh)
167+
err := testutil.TryReceive(ctx, t, errCh)
205168
require.NoError(t, err)
206169
_, ok := resp.Msg.(*TunnelMessage_Start)
207170
require.True(t, ok)
@@ -234,7 +197,7 @@ func TestTunnel_PeerUpdate(t *testing.T) {
234197
wsID1 := uuid.UUID{1}
235198
wsID2 := uuid.UUID{2}
236199

237-
client := newFakeClientWithOptsCh(ctx, t)
200+
client := newFakeClient(ctx, t)
238201
conn := newFakeConn(tailnet.WorkspaceUpdate{
239202
UpsertedWorkspaces: []*tailnet.Workspace{
240203
{
@@ -248,28 +211,22 @@ func TestTunnel_PeerUpdate(t *testing.T) {
248211

249212
tun, mgr := setupTunnel(t, ctx, client, quartz.NewMock(t))
250213

251-
// When: we start the tunnel
252214
errCh := make(chan error, 1)
253215
var resp *TunnelMessage
254216
go func() {
255217
r, err := mgr.unaryRPC(ctx, &ManagerMessage{
256218
Msg: &ManagerMessage_Start{
257219
Start: &StartRequest{
258-
TunnelFileDescriptor: 2,
259-
TunnelUseSoftNetIsolation: true,
260-
CoderUrl: "https://coder.example.com",
261-
ApiToken: "fakeToken",
220+
TunnelFileDescriptor: 2,
221+
CoderUrl: "https://coder.example.com",
222+
ApiToken: "fakeToken",
262223
},
263224
},
264225
})
265226
resp = r
266227
errCh <- err
267228
}()
268-
269-
// Then: `NewConn` is called
270-
opts := testutil.RequireReceive(ctx, t, client.optsCh)
271-
require.True(t, opts.UseSoftNetIsolation)
272-
testutil.RequireSend(ctx, t, client.connCh, conn)
229+
testutil.RequireSend(ctx, t, client.ch, conn)
273230
err := testutil.TryReceive(ctx, t, errCh)
274231
require.NoError(t, err)
275232
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -334,7 +291,7 @@ func TestTunnel_NetworkSettings(t *testing.T) {
334291
resp = r
335292
errCh <- err
336293
}()
337-
testutil.RequireSend(ctx, t, client.connCh, conn)
294+
testutil.RequireSend(ctx, t, client.ch, conn)
338295
err := testutil.TryReceive(ctx, t, errCh)
339296
require.NoError(t, err)
340297
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -475,7 +432,7 @@ func TestTunnel_sendAgentUpdate(t *testing.T) {
475432
resp = r
476433
errCh <- err
477434
}()
478-
testutil.RequireSend(ctx, t, client.connCh, conn)
435+
testutil.RequireSend(ctx, t, client.ch, conn)
479436
err := testutil.TryReceive(ctx, t, errCh)
480437
require.NoError(t, err)
481438
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -646,7 +603,7 @@ func TestTunnel_sendAgentUpdateReconnect(t *testing.T) {
646603
resp = r
647604
errCh <- err
648605
}()
649-
testutil.RequireSend(ctx, t, client.connCh, conn)
606+
testutil.RequireSend(ctx, t, client.ch, conn)
650607
err := testutil.TryReceive(ctx, t, errCh)
651608
require.NoError(t, err)
652609
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -746,7 +703,7 @@ func TestTunnel_sendAgentUpdateWorkspaceReconnect(t *testing.T) {
746703
resp = r
747704
errCh <- err
748705
}()
749-
testutil.RequireSend(ctx, t, client.connCh, conn)
706+
testutil.RequireSend(ctx, t, client.ch, conn)
750707
err := testutil.TryReceive(ctx, t, errCh)
751708
require.NoError(t, err)
752709
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -849,7 +806,7 @@ func TestTunnel_slowPing(t *testing.T) {
849806
resp = r
850807
errCh <- err
851808
}()
852-
testutil.RequireSend(ctx, t, client.connCh, conn)
809+
testutil.RequireSend(ctx, t, client.ch, conn)
853810
err := testutil.TryReceive(ctx, t, errCh)
854811
require.NoError(t, err)
855812
_, ok := resp.Msg.(*TunnelMessage_Start)
@@ -938,7 +895,7 @@ func TestTunnel_stopMidPing(t *testing.T) {
938895
resp = r
939896
errCh <- err
940897
}()
941-
testutil.RequireSend(ctx, t, client.connCh, conn)
898+
testutil.RequireSend(ctx, t, client.ch, conn)
942899
err := testutil.TryReceive(ctx, t, errCh)
943900
require.NoError(t, err)
944901
_, ok := resp.Msg.(*TunnelMessage_Start)

‎vpn/version.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ var CurrentSupportedVersions = RPCVersionList{
2323
// - preferred_derp: The server that DERP relayed connections are
2424
// using, if they're not using P2P.
2525
// - preferred_derp_latency: The latency to the preferred DERP
26-
// 1.3 adds:
27-
// - tunnel_use_soft_net_isolation to the StartRequest
28-
{Major: 1, Minor: 3},
26+
{Major: 1, Minor: 2},
2927
},
3028
}
3129

0 commit comments

Comments
 (0)