net/tstun: block looped disco traffic

Updates #1526 (maybe fixes? time will tell)

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

disco/disco.go

@@ -57,6 +57,16 @@ func LooksLikeDiscoWrapper(p []byte) bool {
 	return string(p[:len(Magic)]) == Magic
 }
 
+// Source returns the slice of p that represents the
+// disco public key source, and whether p looks like
+// a disco message.
+func Source(p []byte) (src []byte, ok bool) {
+	if !LooksLikeDiscoWrapper(p) {
+		return nil, false
+	}
+	return p[len(Magic):][:keyLen], true
+}
+
 // Parse parses the encrypted part of the message from inside the
 // nacl secretbox.
 func Parse(p []byte) (Message, error) {

net/tstun/wrap.go

@@ -7,6 +7,7 @@
 package tstun
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io"
@@ -19,7 +20,9 @@ import (
 	"golang.zx2c4.com/wireguard/device"
 	"golang.zx2c4.com/wireguard/tun"
 	"inet.af/netaddr"
+	"tailscale.com/disco"
 	"tailscale.com/net/packet"
+	"tailscale.com/tailcfg"
 	"tailscale.com/tstime/mono"
 	"tailscale.com/types/ipproto"
 	"tailscale.com/types/logger"
@@ -76,6 +79,7 @@ type Wrapper struct {
 
 	destIPActivity atomic.Value // of map[netaddr.IP]func()
 	destMACAtomic  atomic.Value // of [6]byte
+	discoKey       atomic.Value // of tailcfg.DiscoKey
 
 	// buffer stores the oldest unconsumed packet from tdev.
 	// It is made a static buffer in order to avoid allocations.
@@ -196,6 +200,30 @@ func (t *Wrapper) SetDestIPActivityFuncs(m map[netaddr.IP]func()) {
 	t.destIPActivity.Store(m)
 }
 
+// SetDiscoKey sets the current discovery key.
+//
+// It is only used for filtering out bogus traffic when network
+// stack(s) get confused; see Issue 1526.
+func (t *Wrapper) SetDiscoKey(k tailcfg.DiscoKey) {
+	t.discoKey.Store(k)
+}
+
+// isSelfDisco reports whether packet p
+// looks like a Disco packet from ourselves.
+// See Issue 1526.
+func (t *Wrapper) isSelfDisco(p *packet.Parsed) bool {
+	if p.IPProto != ipproto.UDP {
+		return false
+	}
+	pkt := p.Payload()
+	discoSrc, ok := disco.Source(pkt)
+	if !ok {
+		return false
+	}
+	selfDiscoPub, ok := t.discoKey.Load().(tailcfg.DiscoKey)
+	return ok && bytes.Equal(selfDiscoPub[:], discoSrc)
+}
+
 func (t *Wrapper) Close() error {
 	var err error
 	t.closeOnce.Do(func() {
@@ -483,6 +511,16 @@ func (t *Wrapper) filterIn(buf []byte) filter.Response {
 		}
 	}
 
+	// Issue 1526 workaround: if we see disco packets over
+	// Tailscale from ourselves, then drop them, as that shouldn't
+	// happen unless a networking stack is confused, as it seems
+	// macOS in Network Extension mode might be.
+	if p.IPProto == ipproto.UDP && // disco is over UDP; avoid isSelfDisco call for TCP/etc
+		t.isSelfDisco(p) {
+		t.logf("[unexpected] received self disco package over tstun; dropping")
+		return filter.DropSilently
+	}
+
 	if t.PreFilterIn != nil {
 		if res := t.PreFilterIn(p, t); res.IsDrop() {
 			return res

net/tstun/wrap_test.go

@@ -15,7 +15,10 @@ import (
 
 	"golang.zx2c4.com/wireguard/tun/tuntest"
 	"inet.af/netaddr"
+	"tailscale.com/disco"
 	"tailscale.com/net/packet"
+	"tailscale.com/tailcfg"
+	"tailscale.com/tstest"
 	"tailscale.com/tstime/mono"
 	"tailscale.com/types/ipproto"
 	"tailscale.com/types/logger"
@@ -486,3 +489,32 @@ func TestPeerAPIBypass(t *testing.T) {
 		})
 	}
 }
+
+// Issue 1526: drop disco frames from ourselves.
+func TestFilterDiscoLoop(t *testing.T) {
+	var memLog tstest.MemLogger
+	discoPub := tailcfg.DiscoKey{1: 1, 2: 2}
+	tw := &Wrapper{logf: memLog.Logf}
+	tw.SetDiscoKey(discoPub)
+	uh := packet.UDP4Header{
+		IP4Header: packet.IP4Header{
+			IPProto: ipproto.UDP,
+			Src:     netaddr.IPv4(1, 2, 3, 4),
+			Dst:     netaddr.IPv4(5, 6, 7, 8),
+		},
+		SrcPort: 9,
+		DstPort: 10,
+	}
+	discoPayload := fmt.Sprintf("%s%s%s", disco.Magic, discoPub[:], [disco.NonceLen]byte{})
+	pkt := make([]byte, uh.Len()+len(discoPayload))
+	uh.Marshal(pkt)
+	copy(pkt[uh.Len():], discoPayload)
+
+	got := tw.filterIn(pkt)
+	if got != filter.DropSilently {
+		t.Errorf("got %v; want DropSilently", got)
+	}
+	if got, want := memLog.String(), "[unexpected] received self disco package over tstun; dropping\n"; got != want {
+		t.Errorf("log output mismatch\n got: %q\nwant: %q\n", got, want)
+	}
+}

wgengine/userspace.go

@@ -332,6 +332,8 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	closePool.add(e.magicConn)
 	e.magicConn.SetNetworkUp(e.linkMon.InterfaceState().AnyInterfaceUp())
 
+	tsTUNDev.SetDiscoKey(e.magicConn.DiscoPublicKey())
+
 	if conf.RespondToPing {
 		e.tundev.PostFilterIn = echoRespondToAll
 	}