From a66f0161d0a8f02ae77357c9090b7b68b13394ee Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 10 Apr 2025 13:30:31 +0000 Subject: [PATCH] feat: VPN sets DNS match domain using Hostname Suffix --- tailnet/configmaps.go | 24 ++++++++++----- tailnet/configmaps_internal_test.go | 45 +++++++++++++++-------------- tailnet/conn.go | 12 ++++++++ tailnet/controllers.go | 2 +- tailnet/controllers_test.go | 10 ++++--- vpn/client.go | 6 +++- 6 files changed, 65 insertions(+), 34 deletions(-) diff --git a/tailnet/configmaps.go b/tailnet/configmaps.go index 605fe559bffac..26b6801130c9e 100644 --- a/tailnet/configmaps.go +++ b/tailnet/configmaps.go @@ -36,7 +36,10 @@ const lostTimeout = 15 * time.Minute // CoderDNSSuffix is the default DNS suffix that we append to Coder DNS // records. -const CoderDNSSuffix = "coder." +const ( + CoderDNSSuffix = "coder" + CoderDNSSuffixFQDN = dnsname.FQDN(CoderDNSSuffix + ".") +) // engineConfigurable is the subset of wgengine.Engine that we use for configuration. // @@ -69,20 +72,26 @@ type configMaps struct { filterDirty bool closing bool - engine engineConfigurable - static netmap.NetworkMap + engine engineConfigurable + static netmap.NetworkMap + hosts map[dnsname.FQDN][]netip.Addr peers map[uuid.UUID]*peerLifecycle addresses []netip.Prefix derpMap *tailcfg.DERPMap logger slog.Logger blockEndpoints bool + matchDomain dnsname.FQDN // for testing clock quartz.Clock } -func newConfigMaps(logger slog.Logger, engine engineConfigurable, nodeID tailcfg.NodeID, nodeKey key.NodePrivate, discoKey key.DiscoPublic) *configMaps { +func newConfigMaps( + logger slog.Logger, engine engineConfigurable, + nodeID tailcfg.NodeID, nodeKey key.NodePrivate, discoKey key.DiscoPublic, + matchDomain dnsname.FQDN, +) *configMaps { pubKey := nodeKey.Public() c := &configMaps{ phased: phased{Cond: *(sync.NewCond(&sync.Mutex{}))}, @@ -125,8 +134,9 @@ func newConfigMaps(logger slog.Logger, engine engineConfigurable, nodeID tailcfg Caps: []filter.CapMatch{}, }}, }, - peers: make(map[uuid.UUID]*peerLifecycle), - clock: quartz.NewReal(), + peers: make(map[uuid.UUID]*peerLifecycle), + matchDomain: matchDomain, + clock: quartz.NewReal(), } go c.configLoop() return c @@ -338,7 +348,7 @@ func (c *configMaps) reconfig(nm *netmap.NetworkMap, hosts map[dnsname.FQDN][]ne dnsCfg.Hosts = hosts dnsCfg.OnlyIPv6 = true dnsCfg.Routes = map[dnsname.FQDN][]*dnstype.Resolver{ - CoderDNSSuffix: nil, + c.matchDomain: nil, } } cfg, err := nmcfg.WGCfg(nm, Logger(c.logger.Named("net.wgconfig")), netmap.AllowSingleHosts, "") diff --git a/tailnet/configmaps_internal_test.go b/tailnet/configmaps_internal_test.go index 69244faf00aad..1727d4b5e27cd 100644 --- a/tailnet/configmaps_internal_test.go +++ b/tailnet/configmaps_internal_test.go @@ -34,7 +34,7 @@ func TestConfigMaps_setAddresses_different(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() addrs := []netip.Prefix{netip.MustParsePrefix("192.168.0.200/32")} @@ -93,7 +93,7 @@ func TestConfigMaps_setAddresses_same(t *testing.T) { nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() addrs := []netip.Prefix{netip.MustParsePrefix("192.168.0.200/32")} - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() // Given: addresses already set @@ -123,7 +123,7 @@ func TestConfigMaps_updatePeers_new(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() p1ID := uuid.UUID{1} @@ -193,7 +193,7 @@ func TestConfigMaps_updatePeers_new_waitForHandshake_neverConfigures(t *testing. nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) uut.clock = mClock @@ -237,7 +237,7 @@ func TestConfigMaps_updatePeers_new_waitForHandshake_outOfOrder(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) uut.clock = mClock @@ -308,7 +308,7 @@ func TestConfigMaps_updatePeers_new_waitForHandshake(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) uut.clock = mClock @@ -379,7 +379,7 @@ func TestConfigMaps_updatePeers_new_waitForHandshake_timeout(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) uut.clock = mClock @@ -437,7 +437,7 @@ func TestConfigMaps_updatePeers_same(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() // Then: we don't configure @@ -496,7 +496,7 @@ func TestConfigMaps_updatePeers_disconnect(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() p1ID := uuid.UUID{1} @@ -564,7 +564,7 @@ func TestConfigMaps_updatePeers_lost(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) start := mClock.Now() @@ -649,7 +649,7 @@ func TestConfigMaps_updatePeers_lost_and_found(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) start := mClock.Now() @@ -734,7 +734,7 @@ func TestConfigMaps_setAllPeersLost(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() mClock := quartz.NewMock(t) start := mClock.Now() @@ -820,7 +820,7 @@ func TestConfigMaps_setBlockEndpoints_different(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() p1ID := uuid.MustParse("10000000-0000-0000-0000-000000000000") @@ -864,7 +864,7 @@ func TestConfigMaps_setBlockEndpoints_same(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() p1ID := uuid.MustParse("10000000-0000-0000-0000-000000000000") @@ -907,7 +907,7 @@ func TestConfigMaps_setDERPMap_different(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() derpMap := &tailcfg.DERPMap{ @@ -948,7 +948,7 @@ func TestConfigMaps_setDERPMap_same(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() // Given: DERP Map already set @@ -1017,7 +1017,7 @@ func TestConfigMaps_fillPeerDiagnostics(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() // Given: DERP Map and peer already set @@ -1125,7 +1125,7 @@ func TestConfigMaps_updatePeers_nonexist(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), CoderDNSSuffixFQDN) defer uut.close() // Then: we don't configure @@ -1166,7 +1166,8 @@ func TestConfigMaps_addRemoveHosts(t *testing.T) { nodePrivateKey := key.NewNode() nodeID := tailcfg.NodeID(5) discoKey := key.NewDisco() - uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public()) + suffix := dnsname.FQDN("test.") + uut := newConfigMaps(logger, fEng, nodeID, nodePrivateKey, discoKey.Public(), suffix) defer uut.close() addr1 := CoderServicePrefix.AddrFromUUID(uuid.New()) @@ -1190,8 +1191,10 @@ func TestConfigMaps_addRemoveHosts(t *testing.T) { req := testutil.RequireRecvCtx(ctx, t, fEng.reconfig) require.Equal(t, req.dnsCfg, &dns.Config{ Routes: map[dnsname.FQDN][]*dnstype.Resolver{ - CoderDNSSuffix: nil, + suffix: nil, }, + // Note that host names and Routes are independent --- so we faithfully reproduce the hosts, even though + // they don't match the route. Hosts: map[dnsname.FQDN][]netip.Addr{ "agent.myws.me.coder.": { addr1, @@ -1219,7 +1222,7 @@ func TestConfigMaps_addRemoveHosts(t *testing.T) { req = testutil.RequireRecvCtx(ctx, t, fEng.reconfig) require.Equal(t, req.dnsCfg, &dns.Config{ Routes: map[dnsname.FQDN][]*dnstype.Resolver{ - CoderDNSSuffix: nil, + suffix: nil, }, Hosts: map[dnsname.FQDN][]netip.Addr{ "newagent.myws.me.coder.": { diff --git a/tailnet/conn.go b/tailnet/conn.go index 0a1ee1977e98b..c3ebd246c539f 100644 --- a/tailnet/conn.go +++ b/tailnet/conn.go @@ -120,6 +120,9 @@ type Options struct { // WireguardMonitor is optional, and is passed to the underlying wireguard // engine. WireguardMonitor *netmon.Monitor + // DNSMatchDomain is the DNS suffix to use as a match domain. Only relevant for TUN connections that configure the + // OS DNS resolver. + DNSMatchDomain string } // TelemetrySink allows tailnet.Conn to send network telemetry to the Coder @@ -267,12 +270,21 @@ func NewConn(options *Options) (conn *Conn, err error) { netStack.ProcessLocalIPs = true } + if options.DNSMatchDomain == "" { + options.DNSMatchDomain = CoderDNSSuffix + } + matchDomain, err := dnsname.ToFQDN(options.DNSMatchDomain + ".") + if err != nil { + return nil, xerrors.Errorf("convert hostname suffix (%s) to fully-qualified domain: %w", + options.DNSMatchDomain, err) + } cfgMaps := newConfigMaps( options.Logger, wireguardEngine, nodeID, nodePrivateKey, magicConn.DiscoPublicKey(), + matchDomain, ) cfgMaps.setAddresses(options.Addresses) if options.DERPMap != nil { diff --git a/tailnet/controllers.go b/tailnet/controllers.go index b5f37311a0f71..1d2a348b985f3 100644 --- a/tailnet/controllers.go +++ b/tailnet/controllers.go @@ -1309,7 +1309,7 @@ func NewTunnelAllWorkspaceUpdatesController( t := &TunnelAllWorkspaceUpdatesController{ logger: logger, coordCtrl: c, - dnsNameOptions: DNSNameOptions{"coder"}, + dnsNameOptions: DNSNameOptions{CoderDNSSuffix}, } for _, opt := range opts { opt(t) diff --git a/tailnet/controllers_test.go b/tailnet/controllers_test.go index 089d1b1e82a29..41b2479c6643c 100644 --- a/tailnet/controllers_test.go +++ b/tailnet/controllers_test.go @@ -1637,7 +1637,7 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) { fUH := newFakeUpdateHandler(ctx, t) fDNS := newFakeDNSSetter(ctx, t) coordC, updateC, updateCtrl := setupConnectedAllWorkspaceUpdatesController(ctx, t, logger, - tailnet.WithDNS(fDNS, "testy", tailnet.DNSNameOptions{Suffix: "coder"}), + tailnet.WithDNS(fDNS, "testy", tailnet.DNSNameOptions{Suffix: tailnet.CoderDNSSuffix}), tailnet.WithHandler(fUH), ) @@ -1664,7 +1664,8 @@ func TestTunnelAllWorkspaceUpdatesController_DeleteAgent(t *testing.T) { require.Equal(t, w1a1ID[:], coordCall.req.GetAddTunnel().GetId()) testutil.RequireSendCtx(ctx, t, coordCall.err, nil) - expectedCoderConnectFQDN, err := dnsname.ToFQDN(fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "coder")) + expectedCoderConnectFQDN, err := dnsname.ToFQDN( + fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, tailnet.CoderDNSSuffix)) require.NoError(t, err) // DNS for w1a1 @@ -1785,7 +1786,7 @@ func TestTunnelAllWorkspaceUpdatesController_DNSError(t *testing.T) { fConn := &fakeCoordinatee{} tsc := tailnet.NewTunnelSrcCoordController(logger, fConn) uut := tailnet.NewTunnelAllWorkspaceUpdatesController(logger, tsc, - tailnet.WithDNS(fDNS, "testy", tailnet.DNSNameOptions{Suffix: "coder"}), + tailnet.WithDNS(fDNS, "testy", tailnet.DNSNameOptions{Suffix: tailnet.CoderDNSSuffix}), ) updateC := newFakeWorkspaceUpdateClient(ctx, t) @@ -1806,7 +1807,8 @@ func TestTunnelAllWorkspaceUpdatesController_DNSError(t *testing.T) { upRecvCall := testutil.RequireRecvCtx(ctx, t, updateC.recv) testutil.RequireSendCtx(ctx, t, upRecvCall.resp, initUp) - expectedCoderConnectFQDN, err := dnsname.ToFQDN(fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "coder")) + expectedCoderConnectFQDN, err := dnsname.ToFQDN( + fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, tailnet.CoderDNSSuffix)) require.NoError(t, err) // DNS for w1a1 diff --git a/vpn/client.go b/vpn/client.go index 85e0d45c3d6f8..da066bbcd62b3 100644 --- a/vpn/client.go +++ b/vpn/client.go @@ -7,6 +7,7 @@ import ( "net/url" "golang.org/x/xerrors" + "tailscale.com/net/dns" "tailscale.com/net/netmon" "tailscale.com/wgengine/router" @@ -108,9 +109,11 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string return nil, xerrors.Errorf("get connection info: %w", err) } // default to DNS suffix of "coder" if the server hasn't set it (might be too old). - dnsNameOptions := tailnet.DNSNameOptions{Suffix: "coder"} + dnsNameOptions := tailnet.DNSNameOptions{Suffix: tailnet.CoderDNSSuffix} + dnsMatch := tailnet.CoderDNSSuffix if connInfo.HostnameSuffix != "" { dnsNameOptions.Suffix = connInfo.HostnameSuffix + dnsMatch = connInfo.HostnameSuffix } headers.Set(codersdk.SessionTokenHeader, token) @@ -134,6 +137,7 @@ func (*client) NewConn(initCtx context.Context, serverURL *url.URL, token string Router: options.Router, TUNDev: options.TUNDevice, WireguardMonitor: options.WireguardMonitor, + DNSMatchDomain: dnsMatch, }) if err != nil { return nil, xerrors.Errorf("create tailnet: %w", err)