On Windows, also set the global DNS suffix search path

This ensures that the search path works as intended on Windows Server
2019 (and hopefully on other windows platforms as well) where it hasn't
been enough to just set the search-path for the Telepresence VIF.

Signed-off-by: Thomas Hallgren <[email protected]>

Author: thallgren

CHANGELOG.md

@@ -9,6 +9,8 @@
 
 - Feature: There's a new --address flag to the intercept command allowing users to set the target IP of the intercept.
 
+- Bugfix: DNS on windows is more reliable and performant.
+
 - Bugfix: The kubeconfig is made self-contained before running Telepresence daemon in a Docker container.
 
 - Bugfix: The client will no longer need cluster wide permissions when connected to a namespace scoped Traffic Manager.

pkg/client/rootd/dns/server_windows.go

@@ -45,7 +45,7 @@ func (s *Server) updateRouterDNS(c context.Context, paths []string, dev vif.Devi
 	s.namespaces = namespaces
 	s.search = search
 	s.domainsLock.Unlock()
-	err := dev.SetDNS(c, s.config.RemoteIp, search)
+	err := dev.SetDNS(c, s.clusterDomain, s.config.RemoteIp, search)
 	s.flushDNS()
 	if err != nil {
 		return fmt.Errorf("failed to set DNS: %w", err)

pkg/proc/cmd.go

@@ -1,7 +1,10 @@
 package proc
 
 import (
+	"bytes"
 	"context"
+	"fmt"
+	"strings"
 
 	"github.com/datawire/dlib/dexec"
 	"github.com/datawire/dlib/dlog"
@@ -19,3 +22,24 @@ func StdCommand(ctx context.Context, exe string, args ...string) *dexec.Cmd {
 	dlog.Debugf(ctx, shellquote.ShellString(exe, args))
 	return cmd
 }
+
+// CaptureErr disables command logging, captures Stdout and Stderr to two different buffers.
+// If an error occurs, the stdout output is discarded and the stderr output is included in the
+// returned error unless the error itself already contains that output.
+// On success, any output on stderr is discarded and the stdout output is returned.
+func CaptureErr(ctx context.Context, cmd *dexec.Cmd) ([]byte, error) {
+	var stdOut, stdErr bytes.Buffer
+	cmd.DisableLogging = true
+	cmd.Stdout = &stdOut
+	cmd.Stderr = &stdErr
+	dlog.Debugf(ctx, shellquote.ShellArgsString(cmd.Args))
+	if err := cmd.Run(); err != nil {
+		if es := strings.TrimSpace(stdErr.String()); es != "" {
+			if !strings.Contains(err.Error(), es) {
+				err = fmt.Errorf("%s: %w", es, err)
+			}
+		}
+		return nil, err
+	}
+	return stdOut.Bytes(), nil
+}

pkg/vif/device.go

@@ -36,7 +36,7 @@ type Device interface {
 	Name() string
 	AddSubnet(context.Context, *net.IPNet) error
 	RemoveSubnet(context.Context, *net.IPNet) error
-	SetDNS(context.Context, net.IP, []string) (err error)
+	SetDNS(context.Context, string, net.IP, []string) (err error)
 }
 
 const defaultDevMtu = 1500
@@ -99,8 +99,8 @@ func (d *device) Name() string {
 }
 
 // SetDNS sets the DNS configuration for the device on the windows platform.
-func (d *device) SetDNS(ctx context.Context, server net.IP, domains []string) (err error) {
-	return d.dev.setDNS(ctx, server, domains)
+func (d *device) SetDNS(ctx context.Context, clusterDomain string, server net.IP, domains []string) (err error) {
+	return d.dev.setDNS(ctx, clusterDomain, server, domains)
 }
 
 func (d *device) SetMTU(mtu int) error {

pkg/vif/device_unix.go

@@ -11,7 +11,7 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-func (t *nativeDevice) setDNS(_ context.Context, server net.IP, domains []string) (err error) {
+func (t *nativeDevice) setDNS(context.Context, string, net.IP, []string) (err error) {
 	// DNS is configured by other means than through the actual device
 	return nil
 }

pkg/vif/device_windows.go

@@ -6,16 +6,19 @@ import (
 	"fmt"
 	"net"
 	"net/netip"
+	"os"
 	"strings"
+	"time"
 
 	"golang.org/x/sys/windows"
+	"golang.org/x/sys/windows/registry"
 	"golang.zx2c4.com/wireguard/tun"
 	"golang.zx2c4.com/wireguard/windows/tunnel/winipcfg"
 
+	"github.com/datawire/dlib/dcontext"
 	"github.com/datawire/dlib/derror"
 	"github.com/datawire/dlib/dlog"
 	"github.com/telepresenceio/telepresence/v2/pkg/proc"
-	"github.com/telepresenceio/telepresence/v2/pkg/shellquote"
 	"github.com/telepresenceio/telepresence/v2/pkg/vif/buffer"
 )
 
@@ -115,7 +118,17 @@ func (t *nativeDevice) removeSubnet(_ context.Context, subnet *net.IPNet) error
 	return t.getLUID().DeleteIPAddress(prefixFromIPNet(subnet))
 }
 
-func (t *nativeDevice) setDNS(ctx context.Context, server net.IP, domains []string) (err error) {
+func (t *nativeDevice) setDNS(ctx context.Context, clusterDomain string, server net.IP, searchList []string) (err error) {
+	// This function must not be interrupted by a context cancellation, so we give it a timeout instead.
+	parentCtx := ctx
+	ctx, cancel := context.WithCancel(dcontext.WithoutCancel(ctx))
+	go func() {
+		<-parentCtx.Done()
+		// Give this function some time to complete its task. Configuring DSN on windows is slow.
+		time.Sleep(10 * time.Second)
+		cancel()
+	}()
+
 	ipFamily := func(ip net.IP) winipcfg.AddressFamily {
 		f := winipcfg.AddressFamily(windows.AF_INET6)
 		if ip4 := ip.To4(); ip4 != nil {
@@ -130,46 +143,103 @@ func (t *nativeDevice) setDNS(ctx context.Context, server net.IP, domains []stri
 			_ = luid.FlushDNS(oldFamily)
 		}
 	}
-	if err = luid.SetDNS(family, []netip.Addr{addrFromIP(server)}, domains); err != nil {
+	serverStr := server.String()
+	servers16, err := windows.UTF16PtrFromString(serverStr)
+	if err != nil {
 		return err
 	}
+	searchList16, err := windows.UTF16PtrFromString(strings.Join(searchList, ","))
+	if err != nil {
+		return err
+	}
+	guid, err := luid.GUID()
+	if err != nil {
+		return err
+	}
+	dnsInterfaceSettings := &winipcfg.DnsInterfaceSettings{
+		Version:    winipcfg.DnsInterfaceSettingsVersion1,
+		Flags:      winipcfg.DnsInterfaceSettingsFlagNameserver | winipcfg.DnsInterfaceSettingsFlagSearchList,
+		NameServer: servers16,
+		SearchList: searchList16,
+	}
+	if family == windows.AF_INET6 {
+		dnsInterfaceSettings.Flags |= winipcfg.DnsInterfaceSettingsFlagIPv6
+	}
+	if err = winipcfg.SetInterfaceDnsSettings(*guid, dnsInterfaceSettings); err != nil {
+		return err
+	}
+
+	// Unless we also update the global DNS search path, the one for the device doesn't work on some platforms.
+	// This behavior is mainly observed on Windows Server editions.
 
-	// On some systems (e.g. CircleCI but not josecv's windows box), SetDNS isn't enough to allow the domains to be resolved,
-	// and the network adapter's domain has to be set explicitly.
-	// It's actually way easier to do this via powershell than any system calls that can be run from go code
-	domain := ""
-	if len(domains) > 0 {
-		// Quote the domain to prevent powershell injection
-		domain = shellquote.ShellArgsString([]string{strings.TrimSuffix(domains[0], ".")})
-	}
-	// It's apparently well known that WMI queries can hang under various conditions, so we add a timeout here to prevent hanging the daemon
-	// Fun fact: terminating the context that powershell is running in will not stop a hanging WMI call (!) perhaps because it is considered uninterruptible
-	// For more on WMI queries hanging, see:
-	//     * http://www.yusufozturk.info/windows-powershell/how-to-avoid-wmi-query-hangs-in-powershell.html
-	//     * https://theolddogscriptingblog.wordpress.com/2012/05/11/wmi-hangs-and-how-to-avoid-them/
-	//     * https://stackoverflow.com/questions/24294408/gwmi-query-hangs-powershell-script
-	//     * http://use-powershell.blogspot.com/2018/03/get-wmiobject-hangs.html
-	pshScript := fmt.Sprintf(`
-$job = Get-WmiObject Win32_NetworkAdapterConfiguration -filter "interfaceindex='%d'" -AsJob | Wait-Job -Timeout 30
-if ($job.State -ne 'Completed') {
-	throw "timed out getting network adapter after 30 seconds."
-}
-$obj = $job | Receive-Job
-$job = Invoke-WmiMethod -InputObject $obj -Name SetDNSDomain -ArgumentList "%s" -AsJob | Wait-Job -Timeout 30
-if ($job.State -ne 'Completed') {
-	throw "timed out setting network adapter DNS Domain after 30 seconds."
-}
-$job | Receive-Job
-`, t.interfaceIndex, domain)
-	cmd := proc.CommandContext(ctx, "powershell.exe", "-NoProfile", "-NonInteractive", pshScript)
-	cmd.DisableLogging = true // disable chatty logging
-	dlog.Debugf(ctx, "Calling powershell's SetDNSDomain %q", domain)
-	if err := cmd.Run(); err != nil {
-		// Log the error, but don't actually fail on it: This is all just a fallback for SetDNS, so the domains might actually be working
-		dlog.Errorf(ctx, "Failed to set NetworkAdapterConfiguration DNS Domain: %v. Will proceed, but namespace mapping might not be functional.", err)
+	// Retrieve the current global search paths so that paths that aren't related to
+	// the cluster domain (i.e. not managed by us) can be retained.
+	gss, err := getGlobalSearchList()
+	if err != nil {
+		return err
+	}
+	if oldLen := len(gss); oldLen > 0 {
+		// Windows does not use a dot suffix in the search path.
+		clusterDomain = strings.TrimSuffix(clusterDomain, ".")
+
+		// Put our new search path in front of other entries. Then include those
+		// that don't end with our cluster domain (these are entries that aren't
+		// managed by Telepresence).
+		newGss := make([]string, len(searchList), oldLen)
+		copy(newGss, searchList)
+		for _, gs := range gss {
+			if !strings.HasSuffix(gs, clusterDomain) {
+				newGss = append(newGss, gs)
+			}
+		}
+		gss = newGss
+	} else {
+		gss = searchList
 	}
 	t.dns = server
-	return nil
+	return setGlobalSearchList(ctx, gss)
+}
+
+func psList(values []string) string {
+	var sb strings.Builder
+	sb.WriteString("@(")
+	for i, gs := range values {
+		if i > 0 {
+			sb.WriteByte(',')
+		}
+		sb.WriteByte('"')
+		sb.WriteString(strings.TrimSuffix(gs, "."))
+		sb.WriteByte('"')
+	}
+	sb.WriteByte(')')
+	return sb.String()
+}
+
+func getGlobalSearchList() ([]string, error) {
+	rk, err := registry.OpenKey(registry.LOCAL_MACHINE, `System\CurrentControlSet\Services\Tcpip\Parameters`, registry.QUERY_VALUE)
+	if err != nil {
+		if os.IsNotExist(err) {
+			err = nil
+		}
+		return nil, err
+	}
+	csv, _, err := rk.GetStringValue("SearchList")
+	if err != nil {
+		if os.IsNotExist(err) {
+			err = nil
+		}
+		return nil, err
+	}
+	if csv == "" {
+		return nil, nil
+	}
+	return strings.Split(csv, ","), nil
+}
+
+func setGlobalSearchList(ctx context.Context, gss []string) error {
+	cmd := proc.CommandContext(ctx, "powershell.exe", "Set-DnsClientGlobalSetting", "-SuffixSearchList", psList(gss))
+	_, err := proc.CaptureErr(ctx, cmd)
+	return err
 }
 
 func (t *nativeDevice) setMTU(int) error {