From f6e6569dfe03743cb43df57d935d733b24c58367 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sun, 25 Jun 2023 09:31:05 -0400 Subject: [PATCH] cmd/derper: serve STUN responses out of a separate process Updates #8434 Signed-off-by: David Crawshaw --- cmd/derper/derper.go | 53 +++++++++++++++++++--- cmd/derper/derper_test.go | 92 +++++++++++++++++++++++++++++++++++++++ 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 +}