ipn: allow b to be nil in NewBackendServer

A couple of code paths in ipnserver use a NewBackendServer with a nil
backend just to call the callback with an encapsulated error message.
This covers a panic case seen in logs.

For #1920

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

cmd/tailscale/tailscale.go

@@ -4,7 +4,7 @@
 
 // The tailscale command is the Tailscale command-line client. It interacts
 // with the tailscaled node agent.
-package main // import "tailscale.com/cmd/tailscaled"
+package main // import "tailscale.com/cmd/tailscale"
 
 import (
 	"fmt"
@@ -12,10 +12,10 @@ import (
 	"path/filepath"
 	"strings"
 
-	"tailscale.com/cmd/tailscaled/cli"
+	"tailscale.com/cmd/tailscale/cli"
 )
 
-func tailscale_main() {
+func main() {
 	args := os.Args[1:]
 	if name, _ := os.Executable(); strings.HasSuffix(filepath.Base(name), ".cgi") {
 		args = []string{"web", "-cgi"}

cmd/tailscaled/depaware.txt

@@ -69,6 +69,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         inet.af/netstack/tcpip/transport/udp                         from inet.af/netstack/tcpip/adapters/gonet+
         inet.af/netstack/waiter                                      from inet.af/netstack/tcpip+
         inet.af/peercred                                             from tailscale.com/ipn/ipnserver
+   W 💣 inet.af/wf                                                   from tailscale.com/wf
         rsc.io/goversion/version                                     from tailscale.com/version
         tailscale.com/atomicfile                                     from tailscale.com/ipn+
         tailscale.com/client/tailscale/apitype                       from tailscale.com/ipn/ipnlocal+
@@ -115,7 +116,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/smallzstd                                      from tailscale.com/ipn/ipnserver+
         tailscale.com/syncs                                          from tailscale.com/net/interfaces+
         tailscale.com/tailcfg                                        from tailscale.com/control/controlclient+
-   W 💣 tailscale.com/tempfork/wireguard-windows/firewall            from tailscale.com/cmd/tailscaled
    W    tailscale.com/tsconst                                        from tailscale.com/net/interfaces
         tailscale.com/tstime                                         from tailscale.com/wgengine/magicsock
         tailscale.com/types/empty                                    from tailscale.com/control/controlclient+
@@ -143,6 +143,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/winutil                                   from tailscale.com/logpolicy+
         tailscale.com/version                                        from tailscale.com/cmd/tailscaled+
         tailscale.com/version/distro                                 from tailscale.com/control/controlclient+
+   W    tailscale.com/wf                                             from tailscale.com/cmd/tailscaled
         tailscale.com/wgengine                                       from tailscale.com/cmd/tailscaled+
         tailscale.com/wgengine/filter                                from tailscale.com/control/controlclient+
         tailscale.com/wgengine/magicsock                             from tailscale.com/wgengine+

cmd/tailscaled/main.go

@@ -1,16 +0,0 @@
-package main
-
-import (
-	"os"
-	"strings"
-)
-
-func main() {
-	if strings.HasSuffix(os.Args[0], "tailscaled") {
-		tailscaled_main()
-	} else if strings.HasSuffix(os.Args[0], "tailscale") {
-		tailscale_main()
-	} else {
-		panic(os.Args[0])
-	}
-}

cmd/tailscaled/tailscaled.go

@@ -101,7 +101,7 @@ var subCommands = map[string]*func([]string) error{
 	"debug":                   &debugModeFunc,
 }
 
-func tailscaled_main() {
+func main() {
 	// We aren't very performance sensitive, and the parts that are
 	// performance sensitive (wireguard) try hard not to do any memory
 	// allocations. So let's be aggressive about garbage collection,

cmd/tailscaled/tailscaled_windows.go

@@ -21,7 +21,6 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"net"
 	"os"
 	"time"
 
@@ -32,9 +31,9 @@ import (
 	"tailscale.com/logpolicy"
 	"tailscale.com/net/dns"
 	"tailscale.com/net/tstun"
-	"tailscale.com/tempfork/wireguard-windows/firewall"
 	"tailscale.com/types/logger"
 	"tailscale.com/version"
+	"tailscale.com/wf"
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/netstack"
 	"tailscale.com/wgengine/router"
@@ -144,13 +143,13 @@ func beFirewallKillswitch() bool {
 
 	luid, err := winipcfg.LUIDFromGUID(&guid)
 	if err != nil {
-		log.Fatalf("no interface with GUID %q", guid)
+		log.Fatalf("no interface with GUID %q: %v", guid, err)
 	}
 
-	noProtection := false
-	var dnsIPs []net.IP // unused in called code.
 	start := time.Now()
-	firewall.EnableFirewall(uint64(luid), noProtection, dnsIPs)
+	if _, err := wf.New(uint64(luid)); err != nil {
+		log.Fatalf("filewall creation failed: %v", err)
+	}
 	log.Printf("killswitch enabled, took %s", time.Since(start))
 
 	// Block until the monitor goroutine shuts us down.

go.mod

@@ -26,7 +26,7 @@ require (
 	github.com/peterbourgon/ff/v2 v2.0.0
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/tailscale/depaware v0.0.0-20201214215404-77d1e9757027
-	github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d
+	github.com/tailscale/wireguard-go v0.0.0-20210511223652-bbf7d6cfb6ac
 	github.com/tcnksm/go-httpstat v0.2.0
 	github.com/toqueteos/webbrowser v1.2.0
 	go4.org/mem v0.0.0-20201119185036-c04c5a6ff174

go.sum

@@ -131,6 +131,8 @@ github.com/tailscale/wireguard-go v0.0.0-20210510175647-030c638da3df h1:ekBw6cxm
 github.com/tailscale/wireguard-go v0.0.0-20210510175647-030c638da3df/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
 github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d h1:qJSz1zlpuPLmfACtnj+tAH4g3iasJMBW8dpeFm5f4wg=
 github.com/tailscale/wireguard-go v0.0.0-20210510192616-d1aa5623121d/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
+github.com/tailscale/wireguard-go v0.0.0-20210511223652-bbf7d6cfb6ac h1:QHlYXuUunb1hGlosIDNu8JV2oyjCuLTnFROrRDo5LzM=
+github.com/tailscale/wireguard-go v0.0.0-20210511223652-bbf7d6cfb6ac/go.mod h1:ys4yUmhKncXy1jWP34qUHKipRjl322VVhxoh1Rkfo7c=
 github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=
 github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8=
 github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=

ipn/message.go

@@ -104,7 +104,9 @@ func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(Notify)) *
 		b:             b,
 		sendNotifyMsg: sendNotifyMsg,
 	}
-	if sendNotifyMsg != nil {
+	// b may be nil if the BackendServer is being created just to
+	// encapsulate and send an error message.
+	if sendNotifyMsg != nil && b != nil {
 		b.SetNotifyCallback(bs.send)
 	}
 	return bs

ipn/message_test.go

@@ -187,3 +187,17 @@ func TestClientServer(t *testing.T) {
 	})
 	flushUntil(Running)
 }
+
+func TestNilBackend(t *testing.T) {
+	var called *Notify
+	bs := NewBackendServer(t.Logf, nil, func(n Notify) {
+		called = &n
+	})
+	bs.SendErrorMessage("Danger, Will Robinson!")
+	if called == nil {
+		t.Errorf("expect callback to be called, wasn't")
+	}
+	if called.ErrMessage == nil || *called.ErrMessage != "Danger, Will Robinson!" {
+		t.Errorf("callback got wrong error: %v", called.ErrMessage)
+	}
+}