envknob: generalize Windows tailscaled-env.txt support

ipnserver previously had support for a Windows-only environment
variable mechanism that further only worked when Windows was running
as a service, not from a console.

But we want it to work from tailscaed too, and we want it to work on
macOS and Synology. So move it to envknob, now that envknob can change
values at runtime post-init.

A future change will wire this up for more platforms, and do something
more for CLI flags like --port, which the bug was originally about.

Updates #5114

Change-Id: I9fd69a9a91bb0f308fc264d4a6c33e0cbe352d71
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/tailscaled.go

@@ -131,8 +131,12 @@ var subCommands = map[string]*func([]string) error{
 
 var beCLI func() // non-nil if CLI is linked in
 
+var diskConfigErr error
+
 func main() {
 	envknob.PanicIfAnyEnvCheckedInInit()
+	diskConfigErr = envknob.ApplyDiskConfig()
+
 	printVersion := false
 	flag.IntVar(&args.verbose, "verbose", 0, "log verbosity level; 0 is default, 1 or higher are increasingly verbose")
 	flag.BoolVar(&args.cleanup, "cleanup", false, "clean up system state and exit")
@@ -309,6 +313,10 @@ func run() error {
 		pol.Shutdown(ctx)
 	}()
 
+	if diskConfigErr != nil {
+		log.Printf("Error reading environment config: %v", diskConfigErr)
+	}
+
 	if isWindowsService() {
 		// Run the IPN server from the Windows service manager.
 		log.Printf("Running service...")

cmd/tailscaled/tailscaled_windows.go

@@ -197,6 +197,9 @@ func beWindowsSubprocess() bool {
 
 	log.Printf("Program starting: v%v: %#v", version.Long, os.Args)
 	log.Printf("subproc mode: logid=%v", logid)
+	if diskConfigErr != nil {
+		log.Printf("Error reading environment config: %v", diskConfigErr)
+	}
 
 	go func() {
 		b := make([]byte, 16)

envknob/envknob.go

@@ -17,6 +17,9 @@
 package envknob
 
 import (
+	"bufio"
+	"fmt"
+	"io"
 	"log"
 	"os"
 	"runtime"
@@ -321,3 +324,50 @@ func PanicIfAnyEnvCheckedInInit() {
 		panic("envknob check of called from init function: " + string(envCheckedInInitStack))
 	}
 }
+
+var platformApplyDiskConfig func() error
+
+// ApplyDiskConfig returns a platform-specific config file of environment keys/values and
+// applies them. On Linux and Unix operating systems, it's a no-op and always returns nil.
+// If no platform-specific config file is found, it also returns nil.
+//
+// It exists primarily for Windows to make it easy to apply environment variables to
+// a running service in a way similar to modifying /etc/default/tailscaled on Linux.
+// On Windows, you use %ProgramData%\Tailscale\tailscaled-env.txt instead.
+func ApplyDiskConfig() error {
+	if f := platformApplyDiskConfig; f != nil {
+		return f()
+	}
+	return nil
+}
+
+// applyKeyValueEnv reads key=value lines r and calls Setenv for each.
+//
+// Empty lines and lines beginning with '#' are skipped.
+//
+// Values can be double quoted, in which case they're unquoted using
+// strconv.Unquote.
+func applyKeyValueEnv(r io.Reader) error {
+	bs := bufio.NewScanner(r)
+	for bs.Scan() {
+		line := strings.TrimSpace(bs.Text())
+		if line == "" || line[0] == '#' {
+			continue
+		}
+		k, v, ok := strings.Cut(line, "=")
+		k = strings.TrimSpace(k)
+		if !ok || k == "" {
+			continue
+		}
+		v = strings.TrimSpace(v)
+		if strings.HasPrefix(v, `"`) {
+			var err error
+			v, err = strconv.Unquote(v)
+			if err != nil {
+				return fmt.Errorf("invalid value in line %q: %v", line, err)
+			}
+		}
+		Setenv(k, v)
+	}
+	return bs.Err()
+}

envknob/envknob_windows.go

@@ -0,0 +1,27 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package envknob
+
+import (
+	"os"
+	"path/filepath"
+)
+
+func init() {
+	platformApplyDiskConfig = platformApplyDiskConfigWindows
+}
+
+func platformApplyDiskConfigWindows() error {
+	name := filepath.Join(os.Getenv("ProgramData"), "Tailscale", "tailscaled-env.txt")
+	f, err := os.Open(name)
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+	return applyKeyValueEnv(f)
+}

ipn/ipnserver/server.go

@@ -932,14 +932,6 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		startTime := time.Now()
 		log.Printf("exec: %#v %v", executable, args)
 		cmd := exec.Command(executable, args...)
-		if runtime.GOOS == "windows" {
-			extraEnv, err := loadExtraEnv()
-			if err != nil {
-				logf("errors loading extra env file; ignoring: %v", err)
-			} else {
-				cmd.Env = append(os.Environ(), extraEnv...)
-			}
-		}
 
 		// Create a pipe object to use as the subproc's stdin.
 		// When the writer goes away, the reader gets EOF.
@@ -1208,38 +1200,3 @@ func findQnapTaildropDir(name string) (string, error) {
 	}
 	return "", fmt.Errorf("shared folder %q not found", name)
 }
-
-func loadExtraEnv() (env []string, err error) {
-	if runtime.GOOS != "windows" {
-		return nil, nil
-	}
-	name := filepath.Join(os.Getenv("ProgramData"), "Tailscale", "tailscaled-env.txt")
-	contents, err := os.ReadFile(name)
-	if os.IsNotExist(err) {
-		return nil, nil
-	}
-	if err != nil {
-		return nil, err
-	}
-	for _, line := range strings.Split(string(contents), "\n") {
-		line = strings.TrimSpace(line)
-		if line == "" || line[0] == '#' {
-			continue
-		}
-		k, v, ok := strings.Cut(line, "=")
-		if !ok || k == "" {
-			continue
-		}
-		if strings.HasPrefix(v, `"`) {
-			var err error
-			v, err = strconv.Unquote(v)
-			if err != nil {
-				return nil, fmt.Errorf("invalid value in line %q: %v", line, err)
-			}
-			env = append(env, k+"="+v)
-		} else {
-			env = append(env, line)
-		}
-	}
-	return env, nil
-}