cmd/derper: serve STUN responses out of a separate processcrawshaw/stunchild

Updates #8434

Signed-off-by: David Crawshaw <crawshaw@tailscale.com>

2 files changed, 138 insertions, 7 deletions

diff --git a/cmd/derper/derper.go b/cmd/derper/derper.go
index 8b0feb639..2382a48b5 100644
--- a/cmd/derper/derper.go
+++ b/cmd/derper/derper.go
@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"net/netip"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -146,6 +147,17 @@ func main() {
 		log.Fatalf("invalid server address: %v", err)
 	}
 
+	if *runSTUN {
+		go serveSTUN(listenHost, *stunPort)
+
+		// Keep derper in original process in dev mode.
+		// This is easier for SIGQUIT and debuggers.
+		if !*dev {
+			runNiceDERPChild()
+			return
+		}
+	}
+
 	cfg := loadConfig()
 
 	serveTLS := tsweb.IsProd443(*addr) || *certMode == "manual"
@@ -219,10 +231,6 @@ func main() {
 	}))
 	debug.Handle("traffic", "Traffic check", http.HandlerFunc(s.ServeDebugTraffic))
 
-	if *runSTUN {
-		go serveSTUN(listenHost, *stunPort)
-	}
-
 	quietLogger := log.New(logFilter{}, "", 0)
 	httpsrv := &http.Server{
 		Addr:     *addr,
@@ -241,7 +249,7 @@ func main() {
 	}
 
 	if serveTLS {
-		log.Printf("derper: serving on %s with TLS", *addr)
+		log.Printf("derper: serving on %s with TLS (pid %d)", *addr, os.Getpid())
 		var certManager certProvider
 		certManager, err = certProviderByCertMode(*certMode, *certDir, *hostname)
 		if err != nil {
@@ -317,7 +325,7 @@ func main() {
 		}
 		err = rateLimitedListenAndServeTLS(httpsrv)
 	} else {
-		log.Printf("derper: serving on %s", *addr)
+		log.Printf("derper: serving on %s (pid %d)", *addr, os.Getpid())
 		err = httpsrv.ListenAndServe()
 	}
 	if err != nil && err != http.ErrServerClosed {
@@ -325,6 +333,37 @@ func main() {
 	}
 }
 
+// runNiceDERPChild starts a child process at a slightly lower priority that
+// does everything except run the STUN server.
+//
+// This means that STUN packets get answered by a separate, higher-priority
+// process.
+func runNiceDERPChild() {
+	// Build an arg list, filtering out the --stun flag.
+	var args []string
+	flag.Visit(func(f *flag.Flag) {
+		if f.Name == "stun" {
+			return
+		}
+		args = append(args, "-"+f.Name, f.Value.String())
+	})
+	args = append(args, "-stun=false")
+	bin, err := os.Executable()
+	if err != nil {
+		log.Fatalf("cannot find self: %v", err)
+	}
+	args = append([]string{"-n", "1", bin}, args...)
+	cmd := exec.Command("nice", args...)
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	if err := cmd.Run(); err != nil {
+		if exitErr, _ := err.(*exec.ExitError); exitErr != nil {
+			os.Exit(exitErr.ExitCode())
+		}
+		os.Exit(1)
+	}
+}
+
 const (
 	noContentChallengeHeader = "X-Tailscale-Challenge"
 	noContentResponseHeader  = "X-Tailscale-Response"
@@ -366,7 +405,7 @@ func serveSTUN(host string, port int) {
 	if err != nil {
 		log.Fatalf("failed to open STUN listener: %v", err)
 	}
-	log.Printf("running STUN server on %v", pc.LocalAddr())
+	log.Printf("running STUN server on %v (pid %d)", pc.LocalAddr(), os.Getpid())
 	serverSTUNListener(context.Background(), pc.(*net.UDPConn))
 }
 
diff --git a/cmd/derper/derper_test.go b/cmd/derper/derper_test.go
index b70d18d1b..73d752f73 100644
--- a/cmd/derper/derper_test.go
+++ b/cmd/derper/derper_test.go
@@ -8,8 +8,14 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"runtime"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	"tailscale.com/net/stun"
 )
@@ -128,3 +134,89 @@ func TestNoContent(t *testing.T) {
 		})
 	}
 }
+
+func TestSTUNChild(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping subprocess test on windows")
+	}
+	tmp := t.TempDir()
+	bin := filepath.Join(tmp, "derper")
+	// TODO(crawshaw): most of this test is spent here building the binary.
+	// If we break out the derper main function into its own function
+	// (in the style of cmd/tailscale/cli) then we can call main directly
+	// from this process and save this time.
+	if err := exec.Command("go", "build", "-o", bin, "tailscale.com/cmd/derper").Run(); err != nil {
+		t.Fatalf("building cmd/derper: %v", err)
+	}
+
+	b := &iobuf{
+		runningSTUN: make(chan string, 1),
+		runningDERP: make(chan string, 1),
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, bin, "-stun", "-c", filepath.Join(tmp, "derper.cfg"), "-a", ":18421", "-stun-port", "18422")
+	cmd.Stderr = b
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	var stunPID, derpPID string
+	select {
+	case stunPID = <-b.runningSTUN:
+	case <-ctx.Done():
+		t.Fatal("timeout waiting for STUN to start")
+	}
+	select {
+	case derpPID = <-b.runningDERP:
+	case <-ctx.Done():
+		t.Fatal("timeout waiting for DERP to start")
+	}
+	if stunPID == derpPID {
+		t.Errorf("STUN and DERP running in same process: %s", stunPID)
+	}
+	cmd.Process.Kill()
+	if t.Failed() {
+		t.Logf("output: %s", b)
+	}
+	cmd.Wait()
+}
+
+type iobuf struct {
+	runningSTUN chan string // sent STUN pid
+	runningDERP chan string // sent DERP pid
+
+	mu       sync.Mutex
+	b        []byte
+	seenSTUN bool
+	seenDERP bool
+}
+
+func (b *iobuf) String() string {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return string(b.b)
+}
+
+var stunRE = regexp.MustCompile(`running STUN server on .* \(pid (\d+)\)`)
+var derpRE = regexp.MustCompile(`derper: serving on .* \(pid (\d+)\)`)
+
+func (b *iobuf) Write(p []byte) (n int, err error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	b.b = append(b.b, p...)
+	if !b.seenSTUN {
+		if m := stunRE.FindSubmatch(b.b); len(m) == 2 {
+			b.seenSTUN = true
+			b.runningSTUN <- string(m[1])
+		}
+	}
+	if !b.seenDERP {
+		if m := derpRE.FindSubmatch(b.b); len(m) == 2 {
+			b.seenDERP = true
+			b.runningDERP <- string(m[1])
+		}
+	}
+	return len(p), nil
+}